Skip to content

Commit cfcd59d

Browse files
authored
Make admin tool bootstrap idempotent for already-bootstrapped realms (apache#4996)
* Make admin tool bootstrap idempotent for already-bootstrapped realms 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 * Address review feedback: exact status assertion, distinct skip-completion 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
1 parent 07c10da commit cfcd59d

6 files changed

Lines changed: 53 additions & 15 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,12 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
6262
- Added optional `sessionPolicy` field to `SigV4AuthenticationParameters` for catalog federation. When set, the IAM session policy JSON is attached to the STS AssumeRole request, allowing administrators to restrict vended credentials to only the required AWS services and actions (Principle of Least Privilege).
6363

6464
### Changes
65+
- The admin tool's `bootstrap` command is now idempotent: bootstrapping a realm that is already
66+
bootstrapped is reported as "Realm '<realm>' is already bootstrapped; skipping." and no longer
67+
fails the command, making automated bootstrap jobs safe to re-run. Correspondingly,
68+
`PolarisMetaStoreManager.bootstrapPolarisService` now returns a result with
69+
`ReturnStatus.ENTITY_ALREADY_EXISTS` instead of throwing `IllegalArgumentException` when the
70+
root principal already exists. Existing credentials are never returned or altered.
6571
- Added REPL support to Polaris CLI.
6672
- The `nosql maintenance-run` admin command now rejects a new run when the latest recorded maintenance run is still unfinished, unless the operator explicitly passes `--supersede-run=<run-id>`.
6773
- Added version option to Polaris CLI.

polaris-core/src/main/java/org/apache/polaris/core/auth/AuthBootstrapUtil.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.apache.polaris.core.entity.PrincipalEntity;
3333
import org.apache.polaris.core.entity.PrincipalRoleEntity;
3434
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
35+
import org.apache.polaris.core.persistence.dao.entity.BaseResult;
3536
import org.apache.polaris.core.persistence.dao.entity.CreatePrincipalResult;
3637
import org.apache.polaris.core.persistence.dao.entity.GenerateEntityIdResult;
3738
import org.apache.polaris.core.persistence.dao.entity.PrincipalSecretsResult;
@@ -54,11 +55,9 @@ public static PrincipalSecretsResult createPolarisPrincipalForRealm(
5455

5556
Optional<PrincipalEntity> preliminaryRootPrincipal = metaStoreManager.findRootPrincipal(ctx);
5657
if (preliminaryRootPrincipal.isPresent()) {
57-
String overrideMessage =
58-
"It appears this metastore manager has already been bootstrapped. "
59-
+ "To continue bootstrapping, please first purge the metastore with the `purge` command.";
60-
LOGGER.error("\n\n {} \n\n", overrideMessage);
61-
throw new IllegalArgumentException(overrideMessage);
58+
LOGGER.info("Realm is already bootstrapped (root principal exists); nothing to do.");
59+
return new PrincipalSecretsResult(
60+
BaseResult.ReturnStatus.ENTITY_ALREADY_EXISTS, "realm is already bootstrapped");
6261
}
6362

6463
// Create a root container entity that can represent the securable for any top-level grants.

polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -195,11 +195,11 @@ private PrincipalSecretsResult bootstrapServiceAndCreatePolarisPrincipalForRealm
195195
Optional<PrincipalEntity> preliminaryRootPrincipal =
196196
metaStoreManager.findRootPrincipal(polarisContext);
197197
if (preliminaryRootPrincipal.isPresent()) {
198-
String overrideMessage =
199-
"It appears this metastore manager has already been bootstrapped. "
200-
+ "To continue bootstrapping, please first purge the metastore with the `purge` command.";
201-
LOGGER.error("\n\n {} \n\n", overrideMessage);
202-
throw new IllegalArgumentException(overrideMessage);
198+
LOGGER.info(
199+
"Realm {} is already bootstrapped (root principal exists); nothing to do.",
200+
realmContext.getRealmIdentifier());
201+
return new PrincipalSecretsResult(
202+
BaseResult.ReturnStatus.ENTITY_ALREADY_EXISTS, "realm is already bootstrapped");
203203
}
204204

205205
metaStoreManager.bootstrapPolarisService(polarisContext);

polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,16 @@ public interface PolarisMetaStoreManager
6666

6767
/**
6868
* Bootstrap the Polaris service, creating the root catalog, root principal, and associated
69-
* service admin role. Will fail if the service has already been bootstrapped.
69+
* service admin role. If the service has already been bootstrapped, this is a no-op that returns
70+
* a result with {@link BaseResult.ReturnStatus#ENTITY_ALREADY_EXISTS}.
7071
*
7172
* @param callCtx call context
72-
* @return the result of the bootstrap attempt
73+
* @return the result of the bootstrap attempt; {@code alreadyExists()} is true if the service was
74+
* already bootstrapped
7375
*/
7476
@NonNull
7577
default BaseResult bootstrapPolarisService(@NonNull PolarisCallContext callCtx) {
76-
AuthBootstrapUtil.createPolarisPrincipalForRealm(this, callCtx);
77-
return new BaseResult(BaseResult.ReturnStatus.SUCCESS);
78+
return AuthBootstrapUtil.createPolarisPrincipalForRealm(this, callCtx);
7879
}
7980

8081
/**

polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import org.apache.polaris.core.entity.PrincipalEntity;
4545
import org.apache.polaris.core.entity.TaskEntity;
4646
import org.apache.polaris.core.exceptions.AlreadyExistsException;
47+
import org.apache.polaris.core.persistence.dao.entity.BaseResult;
4748
import org.apache.polaris.core.persistence.pagination.PageToken;
4849
import org.assertj.core.api.Assertions;
4950
import org.assertj.core.api.InstanceOfAssertFactories;
@@ -83,6 +84,19 @@ protected void validateBootstrap() {
8384
polarisTestMetaStoreManager.validateBootstrap();
8485
}
8586

87+
/** re-bootstrapping an already-bootstrapped service must be a graceful no-op */
88+
@Test
89+
protected void testBootstrapAgainIsNoOp() {
90+
PolarisMetaStoreManager metaStoreManager = polarisTestMetaStoreManager.polarisMetaStoreManager;
91+
// the test driver bootstrapped the service already; bootstrap a second time
92+
BaseResult secondBootstrap =
93+
metaStoreManager.bootstrapPolarisService(polarisTestMetaStoreManager.polarisCallContext);
94+
Assertions.assertThat(secondBootstrap.getReturnStatus())
95+
.isEqualTo(BaseResult.ReturnStatus.ENTITY_ALREADY_EXISTS);
96+
// the existing realm must be left fully intact
97+
polarisTestMetaStoreManager.validateBootstrap();
98+
}
99+
86100
@Test
87101
protected void testCreateTestCatalog() {
88102
// allocate test driver

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ public Integer call() {
155155

156156
// Log any errors:
157157
boolean success = true;
158+
int alreadyBootstrappedCount = 0;
158159
for (Map.Entry<String, PrincipalSecretsResult> result : results.entrySet()) {
159160
if (result.getValue().isSuccess()) {
160161
String realm = result.getKey();
@@ -169,6 +170,15 @@ public Integer call() {
169170
result.getValue().getPrincipalSecrets().getMainSecret());
170171
spec.commandLine().getOut().println(msg);
171172
}
173+
} else if (result.getValue().alreadyExists()) {
174+
// Re-running bootstrap on an existing realm is a no-op, not an error; this
175+
// keeps automated bootstrap jobs idempotent. Existing credentials are never
176+
// returned or altered.
177+
alreadyBootstrappedCount++;
178+
String realm = result.getKey();
179+
spec.commandLine()
180+
.getOut()
181+
.printf("Realm '%s' is already bootstrapped; skipping.%n", realm);
172182
} else {
173183
String realm = result.getKey();
174184
spec.commandLine()
@@ -181,7 +191,15 @@ public Integer call() {
181191
}
182192

183193
if (success) {
184-
spec.commandLine().getOut().println("Bootstrap completed successfully.");
194+
if (alreadyBootstrappedCount > 0) {
195+
spec.commandLine()
196+
.getOut()
197+
.printf(
198+
"Bootstrap completed (%d realm(s) already bootstrapped).%n",
199+
alreadyBootstrappedCount);
200+
} else {
201+
spec.commandLine().getOut().println("Bootstrap completed successfully.");
202+
}
185203
return 0;
186204
} else {
187205
spec.commandLine().getErr().println("Bootstrap encountered errors during operation.");

0 commit comments

Comments
 (0)