fix(grpc-proxy): render ratelimiterToken for the rate-limit client - #905
Merged
Conversation
The rate-limit client reads a fixed bearer token under the ratelimiterToken key of the vault-agent secrets file, but the chart never rendered that key, so it fell back to OAuth2 with the placeholder no-id/no-secret credentials. Every check was rejected Unauthenticated and IsRateLimited fails open, so gRPC functions were never throttled on self-hosted installs. OpenBao already provisions the signer role and sign policy for services/ratelimiter-api/jwt/sign/grpc-proxy-proxy, so only the template block was missing. Mirrors what the invocation-service chart does for the HTTP path. Closes #902 Signed-off-by: Max Xing <mxing@nvidia.com>
|
Caution Review failedAn error occurred during the review process. Please try again later. Comment |
FamousDirector
approved these changes
Aug 17, 2026
along-2017
approved these changes
Aug 17, 2026
Contributor
|
🎉 This PR is included in version helm-nvcf-grpc-proxy-v1.7.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
4 tasks
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.
TL;DR
gRPC functions were never rate limited on self-hosted installs. The grpc-proxy chart never rendered the
ratelimiterTokenkey its rate-limit client reads, so the client fell back to OAuth2 with placeholder credentials and every check failedUnauthenticated. This adds the missing vault-agent template block.Additional Details
NewRateLimitClientinsrc/invocation-plane-services/grpc-proxy/proxy/ratelimit/rate_limit.gotakes the fixed-bearer path only ifratelimiterTokenexists in the vault-agent secrets file; otherwise it uses OAuth2 with theid/secretfrom that same file, which the chart hardcodes tono-id/no-secret. Both paths were therefore dead, andIsRateLimitedreturns false (not limited) on any check error, so enforcement was silently off for the whole gRPC path with only a WARN per call.OpenBao already creates the
grpc-proxy-proxyJWT signer role underservices/ratelimiter-api/jwtwith theratelimit:check_invocationscope and attaches the sign policy to the auth role (migrations/openbao/migrations/12_setup_grpc-proxy.sh). Only the chart block was missing, so no OpenBao change is needed.deploy/helm/http-invocation/nvcf-invocation-service/vault-agent-templates/secrets.json.tmplalready renders its ratelimiter token, which is why the HTTP path enforces correctly on the same cluster. This change mirrors it.Not addressed here: a permanent auth failure on the rate-limit client still fails open and is indistinguishable from healthy operation except in logs. Tracked in the issue for separate discussion.
For the Reviewer
Single template block. Worth confirming the signer path and role name match
12_setup_grpc-proxy.sh, and that no hosted deployment relies on the current behavior. Hosted grpc-proxy renders real OAuth2 client credentials rather than placeholders, so it takes the OAuth2 path and is not expected to change.For QA
Verified end to end on a local self-hosted k3d cluster (control plane plus compute plane, ratelimiter enabled, gRPC echo function with
rateLimit: 5-MandsyncCheck: true), by A/B/A: patched chart, stock chart, patched chart again.Patched chart:
/vault/secrets/secrets.jsoncontainsratelimiterToken, claimsaud: [grpc-proxy-proxy, s:ratelimiter],scopes: [ratelimit:check_invocation],sub: grpc-proxy-proxyUsing fixed bearer token authentication for rate limit clientRESOURCE_EXHAUSTED "exceeded rate limit"external rate limit check failedWARN linesStock chart 1.6.7, same function and same limit:
ratelimiterToken;id/secretareno-id/no-secretUsing OAuth2 authentication for rate limit clientexternal rate limit check failed ... code = Unauthenticated desc = Request unauthenticated with bearerRe-applying the patched chart returned to 5 OK / 3 throttled with zero WARNs.
Streaming gRPC and
syncCheck: falsewere not exercised; both go through the sameIsRateLimitedcall. QA on an amd64 cluster is still worth doing since local coverage was arm64.Notes
The chart has no pod annotation tied to the vault-agent template contents, so an in-place upgrade updates the ConfigMap without restarting grpc-proxy, and the new key only appears after a rollout restart. Fresh installs are unaffected. Left as is here; worth a follow-up if operators are expected to pick this up without a restart.
Issues
Closes #902
Checklist