Make admin tool bootstrap idempotent for already-bootstrapped realms by sepandr · Pull Request #4996 · apache/polaris

sepandr · 2026-07-07T17:47:42Z

Running the admin tool's bootstrap against a realm that is already bootstrapped currently throws IllegalArgumentException ("It appears this metastore manager has already been bootstrapped…") and exits with the generic EXIT_CODE_BOOTSTRAP_ERROR (3) — the same code as every real failure. That makes automated deployments (Kubernetes Jobs, ArgoCD hook Jobs that run bootstrap on every deploy) fail on any redeploy, with no reliable way to distinguish the benign case short of matching the error text, and Kubernetes offers no "treat exit N as success" escape hatch. See #4995 for full context.

This change makes re-bootstrap a graceful no-op:

AuthBootstrapUtil.createPolarisPrincipalForRealm returns a PrincipalSecretsResult with ReturnStatus.ENTITY_ALREADY_EXISTS instead of throwing when the root principal already exists (same for the duplicated check in LocalPolarisMetaStoreManagerFactory).
PolarisMetaStoreManager.bootstrapPolarisService propagates that result (javadoc updated).
BootstrapCommand prints Realm '<realm>' is already bootstrapped; skipping. for such realms and keeps exit code 0; all other failures still exit 3. --print-credentials never prints anything for skipped realms — existing credentials are never returned or altered, and realms in the same invocation that are not yet bootstrapped are still bootstrapped normally.

Testing: new testBootstrapAgainIsNoOp in BasePolarisMetaStoreManagerTest (runs for both TreeMap metastore variants) asserts the second bootstrap reports alreadyExists() and the realm stays fully intact; existing bootstrap/purge CLI tests pass unchanged (:polaris-core:test, :polaris-admin:test in-memory variants run locally; an admin-CLI-level double-bootstrap test isn't possible yet because consecutive launches don't reuse the dev-services container — see the existing TODO in RelationalJdbcBootstrapCommandTest). Also verified behaviorally against a live 1.5.0 deployment (relational-jdbc on PostgreSQL 16) where re-running bootstrap on an existing realm currently fails.

AI assistance disclosure: this PR was developed with the assistance of Claude (Anthropic's coding assistant), per the Guidelines for AI-assisted Contributions. I have reviewed, tested, and understand all of the changes, and assume full authorship of and responsibility for the contribution.

Checklist

🛡️ Don't disclose security issues! (contact security@apache.org)
🔗 Clearly explained why the changes are needed, or linked related issues: Fixes Admin tool bootstrap command is not idempotent: fails on already-bootstrapped realms #4995
🧪 Added/updated tests with good coverage, or manually tested (and explained how)
💡 Added comments for complex logic
🧾 Updated CHANGELOG.md (if needed)
📚 Updated documentation in site/content/in-dev/unreleased (if needed)

dimas-b

Thanks for your contribution, @sepandr ! Very reasonable change.

Given that is affects bootstrap behaviour, let's keep it in review for a few more days for other people to have a chance to comment.

dimas-b · 2026-07-07T22:41:53Z

FYI: @obelix74 , @tokoko , @cccs-cat001

sepandr · 2026-07-07T23:37:49Z

Thanks for your contribution, @sepandr ! Very reasonable change.

Given that is affects bootstrap behaviour, let's keep it in review for a few more days for other people to have a chance to comment.

@dimas-b thanks for your review, sounds good!

cccs-cat001

I've been wanting this for months now! Great change :)

vigneshio

ServiceProducers assumes all bootstrap results have secrets (getPrincipalSecrets()), but with this change some results will be alreadyExists. Worth handling here or quick follow-up? WDYT ?? @dimas-b

dimas-b · 2026-07-08T14:15:03Z

ServiceProducers assumes all bootstrap results have secrets (getPrincipalSecrets()), but with this change some results will be alreadyExists. Worth handling here or quick follow-up?

I'd say the code should not assume that 😅 Only the first bootstrap should emit auto-generated credentials. If that's what you mean by follow-up then, let's do that :)

My personal preference is to avoid generating user-owned secrets in Polaris completely. Right now the admin use has both options: 1) provide a secret or 2) let Polaris generate it.

Ideally, the schema setup should be idempotent. The "root" user set up should be separate (e.g. via Terraform). With external IdP the root user does not even have to exist, but that requires more work to follow #4356

All in all, +1 to follow-up improvements, but I'm afraid it's a long road 😅

dimas-b · 2026-07-09T23:38:25Z

@sepandr : Some of the commits mention Claude as a formal co-author. It is fine to use AI to make contributions, but a human has to assume authorship, I think.

Cf. #4106 (comment)

Cf. https://polaris.apache.org/community/contributing-guidelines/#guidelines-for-ai-assisted-contributions

Bootstrapping a realm that already has a root principal used to throw IllegalArgumentException, which BootstrapCommand mapped to the generic EXIT_CODE_BOOTSTRAP_ERROR. Automated deployments (e.g. Kubernetes Jobs or ArgoCD hooks) that run bootstrap on every deploy therefore failed on any redeploy, and callers could not distinguish the benign case from real failures except by matching the error-message text. AuthBootstrapUtil.createPolarisPrincipalForRealm now returns a PrincipalSecretsResult with ENTITY_ALREADY_EXISTS instead of throwing, PolarisMetaStoreManager.bootstrapPolarisService propagates that result, and BootstrapCommand reports "Realm 'x' is already bootstrapped; skipping." while keeping exit code 0. Existing credentials are never returned or altered; realms that are not yet bootstrapped are still bootstrapped normally. Fixes apache#4995

…tion message - testBootstrapAgainIsNoOp now asserts ReturnStatus.ENTITY_ALREADY_EXISTS directly instead of isSuccess() + alreadyExists() - BootstrapCommand reports 'Bootstrap completed (N realm(s) already bootstrapped).' when realms were skipped, keeping 'Bootstrap completed successfully.' for full bootstraps

sepandr · 2026-07-09T23:47:05Z

Thanks for the pointer @dimas-b — you're right. I've rewritten the branch to drop the Co-Authored-By trailer (no AI tool references remain in the commit messages; the code is unchanged) and added an AI-assistance disclosure to the PR description instead, per the contributing guidelines.

dimas-b

Thanks, @sepandr !

Merging as soon as CI is green.

flyrain · 2026-07-10T00:04:10Z

We may accidentally make this function not suitable for any callers other than bootstrapping. I understand bootstrapping is the only use case now, should we change the method name to make it explicitly only for bootstrapping?

The class name is specific to bootstrapping. Renaming to avoid confusion is also fine. @flyrain : Would you like to propose a PR for that?

flyrain · 2026-07-10T00:25:52Z

Can we wait until all comments are resolved?

dimas-b · 2026-07-10T00:30:34Z

Can we wait until all comments are resolved?

@flyrain : I enabled auto-merge (as noted above) based on the previous approvals and the 2 day review period. I believe you could have disabled the auto-merge.

flyrain · 2026-07-10T00:35:50Z

Can we wait until all comments are resolved?

@flyrain : I enabled auto-merge (as noted above) based on the previous approvals and the 2 day review period. I believe you could have disabled the auto-merge.

Got it. I guess auto merge will naturally ignore new comments. I'd suggest using it with caution.

github-project-automation Bot added this to Basic Kanban Board Jul 7, 2026

github-project-automation Bot moved this to PRs In Progress in Basic Kanban Board Jul 7, 2026

dimas-b previously approved these changes Jul 7, 2026

View reviewed changes

github-project-automation Bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Jul 7, 2026

dimas-b requested review from MonkeyCanCode, dennishuo, flyrain and nandorKollar July 7, 2026 22:41

ayushtkn approved these changes Jul 8, 2026

View reviewed changes

nandorKollar previously approved these changes Jul 8, 2026

View reviewed changes

cccs-cat001 approved these changes Jul 8, 2026

View reviewed changes

vigneshio approved these changes Jul 8, 2026

View reviewed changes

Comment thread ...c/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java

Comment thread runtime/admin/src/main/java/org/apache/polaris/admintool/BootstrapCommand.java

sepandr dismissed stale reviews from nandorKollar and dimas-b via d316f18 July 8, 2026 13:43

sepandr requested review from dimas-b, nandorKollar and vigneshio July 8, 2026 13:58

dimas-b approved these changes Jul 8, 2026

View reviewed changes

vigneshio approved these changes Jul 8, 2026

View reviewed changes

nandorKollar approved these changes Jul 8, 2026

View reviewed changes

dimas-b mentioned this pull request Jul 9, 2026

bootstrap: skip existing realms instead of failing #5013

Closed

sepandr added 2 commits July 9, 2026 16:46

sepandr force-pushed the bootstrap-already-bootstrapped-idempotent branch from d316f18 to c60a606 Compare July 9, 2026 23:46

dimas-b approved these changes Jul 9, 2026

View reviewed changes

dimas-b enabled auto-merge (squash) July 9, 2026 23:54

flyrain reviewed Jul 10, 2026

View reviewed changes

dimas-b merged commit cfcd59d into apache:main Jul 10, 2026
25 checks passed

github-project-automation Bot moved this from Ready to merge to Done in Basic Kanban Board Jul 10, 2026

Uh oh!

Conversation

sepandr commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

dimas-b left a comment

Choose a reason for hiding this comment

Uh oh!

dimas-b commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sepandr commented Jul 7, 2026

Uh oh!

cccs-cat001 left a comment

Choose a reason for hiding this comment

Uh oh!

vigneshio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dimas-b commented Jul 8, 2026

Uh oh!

dimas-b commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sepandr commented Jul 9, 2026

Uh oh!

dimas-b left a comment

Choose a reason for hiding this comment

Uh oh!

flyrain Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimas-b Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

flyrain commented Jul 10, 2026

Uh oh!

dimas-b commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flyrain commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

sepandr commented Jul 7, 2026 •

edited

Loading

dimas-b commented Jul 7, 2026 •

edited

Loading

dimas-b commented Jul 9, 2026 •

edited

Loading

flyrain Jul 10, 2026 •

edited

Loading

dimas-b commented Jul 10, 2026 •

edited

Loading