Fix bootstrap silently creating realms with an unreachable root principal - #5526
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Correct the invalid --credentials diagnostic and address the new multi-realm CLI documentation requirement.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR prevents bootstrapping realms without reachable root credentials by validating credential coverage for every requested realm.
Changes:
- Rejects incomplete or misspelled realm credentials.
- Adds tests for incomplete credential input.
File summaries
| File | Summary |
|---|---|
runtime/admin/src/test/java/org/apache/polaris/admintool/BootstrapCommandTestBase.java |
Adds validation-failure test coverage. |
runtime/admin/src/main/java/org/apache/polaris/admintool/BootstrapCommand.java |
Validates credential coverage and reports uncovered realms. The diagnostic should use --credential, and the new multi-realm requirement should be documented. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ayushtkn
force-pushed
the
bootstrapUnreachableRealm
branch
2 times, most recently
from
September 16, 2026 07:15
6fa69fb to
3bd8670
Compare
github-project-automation
Bot
moved this from PRs In Progress
to Ready to merge
in Basic Kanban Board
flyingImer
approved these changes
Sep 16, 2026
nandorKollar
previously approved these changes
Sep 17, 2026
…ipal bootstrap required --credential or --print-credentials only when no credentials at all were supplied. A non-empty but incomplete list passed that check, so any realm not named in it was bootstrapped with a generated secret that is never printed. PrincipalSecretsGenerator falls back to RANDOM_SECRETS for a realm absent from the credentials map, and the command still reported the realm successfully bootstrapped. Nothing validates that the realm named in a --credential triplet is one of the requested realms either, so a misspelled realm name produced the same result. Such a realm cannot be used or recovered: its root principal is the only principal, no other credential can be minted without it, and re-running bootstrap is a no-op for an existing realm. The guard now covers every requested realm and names the ones that are missing credentials.
ayushtkn
force-pushed
the
bootstrapUnreachableRealm
branch
from
September 18, 2026 14:38
3bd8670 to
63601bf
Compare
Member
Author
|
Had to rebase, to resolve conflicts. Only CHANGELOG had conflicts, rest stays as is |
dimas-b
approved these changes
Sep 18, 2026
dimas-b
left a comment
dimas-b
left a comment
Contributor
There was a problem hiding this comment.
Merging based on previous approvals.
github-project-automation
Bot
moved this from Ready to merge
to Done
in Basic Kanban Board
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.
bootstrap required credentials only when none at all were supplied:
A non-empty but incomplete list passes that check, so this succeeds and reports both realms bootstrapped:
realm2gets a random secret, becausePrincipalSecretsGenerator.bootstrapfalls back toRANDOM_SECRETSfor any realm absent from the credentials map, and without--print-credentialsthat secret is never shown. Nothing validates the realm named in a--credentialeither, so a misspelled one (-r realm1 -c realm2,...) produces the same result.The realm is then unusable: root is its only principal, no credential can be minted without it, and re-running bootstrap skips an already bootstrapped realm. Recovery requires purge and a second bootstrap.
The guard now checks that every requested realm is covered, and names the ones that are not.
--print-credentialsshort-circuits as before. A partially covered invocation without it now exits non-zero instead of bootstrapping the uncovered realms unusably.Checklist
CHANGELOG.md(if needed)site/content/in-dev/unreleased(if needed)