Repository navigation
Conversation
shiju-nv
requested review from
a team,
derekwaynecarr,
mrunalp and
sjenning
as code owners
October 12, 2026 15:42
|
Label |
shiju-nv
force-pushed
the
feat/3319-typed-failure-reporting/shiju-nv
branch
from
October 12, 2026 18:07
e7d1a41 to
b36ab6c
Compare
Signed-off-by: Shiju <shiju@nvidia.com>
Signed-off-by: Shiju <shiju@nvidia.com>
Signed-off-by: Shiju <shiju@nvidia.com>
Signed-off-by: Shiju <shiju@nvidia.com>
Remove duplication and dead paths from the token-grant failure reporting without changing classification, the workload-visible 502 response, or the wire format. - Build the gRPC failure envelope with tonic-types, store the typed failure in EndpointFailure, and share one request-ID rule and one endpoint-status authority check. - Merge the gateway exchange client wrapper, drop forwarding validators, and name the request field when a reported cause is rejected. - Inject fallback credentials only from live provider state and delete the unread per-connection credential copy. Keep one 502 writer per path and restore the base token-cache handling. - Check the token endpoint after the cache lookup, still before any credential acquisition. - Document 429 and 5xx classification as implemented, and drop tests that repeat retained assertions through the same code path. Signed-off-by: Shiju <shiju@nvidia.com>
Cut the token-grant failure tests to one test per distinct behavior across core, gateway, supervisor, CLI and Go, and replace the 794-line closure E2E with status assertions in the existing route-recovery E2E: the refused grant appears in sandbox status with the response's request ID, code, stage and upstream status, and the matching grant's later success clears it. - Pin the live provider snapshot with one helper used by inspected and fallback forwarding, and inject selector-matched grants through inject_for_selectors, which borrows the pinned context. Move the test-only wrapper into test_support. - Drop the 35-second wrappers around the gateway grant RPC. The shared channel bounds the connection attempt and the gateway bounds its token-service calls. - Log the token service's OAuth error and error_description, without control characters and limited to 256 characters each, with the request ID in the gateway or supervisor log. Status and the workload response still carry only the bounded cause. - Keep one check that an untrusted token-service certificate cannot assert a cause. Signed-off-by: Shiju <shiju@nvidia.com>
The injection tests named EndpointObservationSender through the parent module, which no longer imports it. The features suite lockfile kept the tonic TLS dependency that the removed E2E fixture added. Signed-off-by: Shiju <shiju@nvidia.com>
Pass the response-size width as a named argument; an implicitly captured constant panics at runtime. Name the mutation type in the validation table for Clippy. Signed-off-by: Shiju <shiju@nvidia.com>
MAX_TOKEN_RESPONSE_BYTES exceeds u16::MAX, the largest format width, so the padded fixture panicked. Pad with repeat_n as before. Signed-off-by: Shiju <shiju@nvidia.com>
shiju-nv
force-pushed
the
feat/3319-typed-failure-reporting/shiju-nv
branch
from
October 13, 2026 18:04
b019445 to
488ead9
Compare
Main added the ListProvidersRequest label selector. The merged public schema hash comes from the Branch Checks storage_proto test output. Signed-off-by: Shiju <shiju@nvidia.com>
2 of 14 tasks
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A refused dynamic token grant now gives authorized callers a bounded cause and recovery action through sandbox reads, while the workload receives a generic HTTP 502 and an opaque request ID. Reporting covers inspected HTTP protocols and selector-based forwarding, including failures before credential selection.
Related Issue
Closes #3319
Changes
Classify each refused grant into one closed set of codes, each with a fixed recovery action from the existing provider recovery vocabulary. No cause retains remote descriptions, tokens or error chains.
provider_authorization_requiredFAILED_PRECONDITIONworkload_identity_rejectedPERMISSION_DENIEDconfiguration_errorFAILED_PRECONDITIONtimeoutDEADLINE_EXCEEDEDunavailableUNAVAILABLEunclassifiedUNKNOWNRecognize a token service's
provider_authorization_requiredonly with the exactopenshell.token-grant.v1contract over verified HTTPS; a bare 401 staysunclassified. The gateway can also establish that cause from missing or expired stored subject credentials. HTTP 429 and 5xx without a recognized extension code areunavailable, as are connect failures, including TLS certificate or hostname verification failures. A failure after the connection is established isunclassified, and any timeout, including a slow response body, istimeout.The gateway returns a refused grant to the supervisor as a gRPC status with a single
ErrorInfodetail (reasonTOKEN_GRANT_FAILURE). The supervisor decodes it strictly. Any other status, such as details over 1024 bytes, more than one detail, or unknown metadata keys, codes or stages, becomesunclassifiedat thegateway_rpcstage.Carry one supervisor-minted request ID through both token-service hops, the gateway, refusal logs, status and workload response (
x-openshell-request-id).Retain one
SandboxStatus.last_token_grant_failurewith a bounded destination and failure cause. Reuse authenticated status reports and sandbox reads, with no new public RPC or request history. Matching grant success and configuration/session resets clear it; unrelated success retains it. MCP endpoint status also retains its configured-endpoint cause.Proto additions, all on existing messages or new messages they reference:
EndpointFailureandEndpointFailureDomain(domainTOKEN_GRANT), carrying code, recovery action, provider, credential key, stage, optional upstream HTTP status and request ID.TokenGrantStatusandTokenGrantFailureObservation(destination host and port plus anEndpointFailure).ReportEndpointStatusRequest.token_grant_status = 8. Omitting it keeps the previous behavior; a present message with nolast_failureclears the stored cause.EndpointObservation.failure = 3,EndpointStatus.last_failure = 7andSandboxStatus.last_token_grant_failure = 17.In Rust, the supervisor builds the proto
EndpointFailurefrom a typedTokenGrantFailurethroughopenshell_core::endpoint_failure::EndpointFailure. The gateway validates reported causes against the same closed sets and names the rejected field and reason (EndpointFailureValidationError) without echoing rejected values.Select the live credential snapshot on fallback forwarding so an idle socket cannot report an old grant under a new provider revision, and drop the per-connection credential copy that no request reads anymore. Expose the result through CLI JSON/YAML/text, the Go SDK and documentation.
Add a real REST workload test with native CA verification at both grant hops, authenticated persisted reads, exact correlation, redaction, denied anonymous and supervisor-credential readers, and recovery.
Testing
Focused checks pass on the final head: core 586, server 747 (provider, policy, sandbox, session and schema modules), network 1120 (token grant, L7 and proxy modules) and CLI rendering tests; Go converter tests; strict Clippy on all targets of the five affected production packages; E2E target compilation; formatting and Markdown lint. The repository pre-commit hook (workspace formatting and Clippy on all targets) passed.
Final-head Branch Checks passed all 24 jobs on the first attempt. E2E passed with 59 successful jobs and six optional skipped jobs. The verified-HTTPS REST case passed in bundled Podman, and its target completed all three tests. Optional Windows, GPU and Helm contexts are not runtime proof.
The feature-enabled E2E target retains 13 preexisting Clippy diagnostics. Its library and a diagnostic run allowing only those known categories pass; no lint configuration was changed. The hosted default-feature strict Clippy gate passed.
Checklist