feat(openbao-migrations): support optional JWT issuer override#236
Merged
Merged
Conversation
Add an optional JWT_ISSUER_OVERRIDE input that, when set, overrides the
derived in-cluster issuer ("http://<service>.<namespace>.svc.cluster.local")
stamped into the generated JWT secret roles. Leaving it empty preserves the
existing per-service default, so current deployments are unaffected.
Also build the JWT secret role JSON with jq (--arg/--argjson) instead of a
heredoc, so every string value is safely escaped (quotes, backslashes, and
control characters) rather than interpolated directly.
📝 WalkthroughWalkthroughSIS JWT secret-role generation now accepts an optional ChangesJWT issuer override
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@migrations/openbao/migrations/utils/functions.sh`:
- Line 483: Update the quoted_scopes construction in the scopes-processing
function to stop hand-building JSON with sed. Use jq to split the raw scopes
string, preserve valid escaping and quoting, and explicitly convert an empty
SCOPES value to an empty JSON array rather than an array containing an empty
string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a71b4c11-bce6-4e87-989f-4f798c1a380f
📒 Files selected for processing (7)
migrations/openbao/addons/nvcf-ui/setup_nvcf-ui.shmigrations/openbao/job.yamlmigrations/openbao/migrations/09_setup_nvcf-api.shmigrations/openbao/migrations/14_setup_nvca.shmigrations/openbao/migrations/15_setup_nvca-operator.shmigrations/openbao/migrations/20_setup_nvct.shmigrations/openbao/migrations/utils/functions.sh
gsharma-nvidia
approved these changes
Jul 17, 2026
Contributor
|
🎉 This PR is included in version nvcf-openbao-migrations-v0.17.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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.
Closes #246
Summary
Adds an optional
JWT_ISSUER_OVERRIDEto the OpenBao migrations so the JWTissclaim stamped into the generated secret roles can be overridden, and hardens the
role-JSON construction by building it with
jq.What changed
migrations/utils/functions.sh—generate_jwt_secret_roleaccepts an optionalissuer-override argument; when non-empty it replaces the derived
http://<service>.<namespace>.svc.cluster.localissuer. The role JSON is now builtwith
jq -n --arg/--argjsoninstead of a heredoc, so every string value is escapedcorrectly (quotes, backslashes, control characters).
09_setup_nvcf-api.sh,14_setup_nvca.sh,15_setup_nvca-operator.sh,20_setup_nvct.sh,addons/nvcf-ui/setup_nvcf-ui.sh— pass through${JWT_ISSUER_OVERRIDE:-}for the relevant signer role.job.yaml— exposeJWT_ISSUER_OVERRIDE(empty by default) as an example env var.Why
Lets the migrations run with a custom issuer where the default in-cluster issuer isn't
appropriate, without forking the scripts. The
jq-based construction also removes aclass of JSON-escaping bugs from shell interpolation.
Backward compatibility
JWT_ISSUER_OVERRIDEdefaults to empty; when unset, the derived per-service issuer andthe exact role-JSON shape are unchanged, so existing deployments are unaffected.
Testing
bash -non all modified scripts.jq-built role JSON is byte-equivalent to the previous heredoc output forthe default (no-override) case, and that a set override is reflected in the minted
token's
issclaim.