collado-mike commented on code in PR #461:
URL: https://github.com/apache/polaris/pull/461#discussion_r1858924693
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java:
##########
@@ -95,10 +97,26 @@ public synchronized Map<String, PrincipalSecretsResult>
bootstrapRealms(List<Str
RealmContext realmContext = () -> realm;
if
(!metaStoreManagerMap.containsKey(realmContext.getRealmIdentifier())) {
initializeForRealm(realmContext);
+ // While bootstrapping we need to act as a fake privileged context
since the real
+ // CallContext hasn't even been resolved yet.
+ PolarisCallContext polarisContext =
+ new PolarisCallContext(
+
sessionSupplierMap.get(realmContext.getRealmIdentifier()).get(), diagServices);
PrincipalSecretsResult secretsResult =
bootstrapServiceAndCreatePolarisPrincipalForRealm(
- realmContext,
metaStoreManagerMap.get(realmContext.getRealmIdentifier()));
+ realmContext,
+ metaStoreManagerMap.get(realmContext.getRealmIdentifier()),
+ polarisContext);
results.put(realmContext.getRealmIdentifier(), secretsResult);
+ if (this.printCredentials(polarisContext)) {
+ String msg =
+ String.format(
+ "realm: %1s root principal credentials: %2s:%3s",
+ realmContext.getRealmIdentifier(),
+ secretsResult.getPrincipalSecrets().getPrincipalClientId(),
+ secretsResult.getPrincipalSecrets().getMainSecret());
+ System.out.println(msg);
+ }
Review Comment:
I think this logic belongs to the secrets generator. The MetaStoreManager
doesn't need to know anything about whether the secrets generated are provided
by the user or if they've been generated randomly. So why would it be concerned
with printing the credentials? The secrets generator knows if the secrets were
provided explicitly or if they were randomly generated.
I think the `bootstrap` command should take a `print-credentials` config
flag and the constructed secrets generator can react accordingly.
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java:
##########
@@ -196,6 +211,20 @@ private PrincipalSecretsResult
bootstrapServiceAndCreatePolarisPrincipalForRealm
throw new IllegalArgumentException(overrideMessage);
}
+ boolean environmentVariableCredentials =
+ PrincipalSecretsGenerator.hasCredentialVariables(
+ realmContext.getRealmIdentifier(),
PolarisEntityConstants.getRootPrincipalName());
+ if (!this.printCredentials(polarisContext) &&
!environmentVariableCredentials) {
+ String failureMessage =
+ String.format(
+ "It appears that environment variables were not provided for
root credentials, and that printing "
+ + "the root credentials is disabled via %s. If bootstrapping
were to proceed, there would be no way "
+ + "to recover the root credentials",
+ PolarisConfiguration.BOOTSTRAP_PRINT_CREDENTIALS.key);
+ LOGGER.error("\n\n {} \n\n", failureMessage);
Review Comment:
Similar here - why is the metastore aware of whether the secrets were
provided by environment variables? What if there are other impls of secrets
generators that don't rely on env variables? E.g., we could have one that calls
AWS SecretsManager to dynamically generate and store the secrets without any
env variables. Should this code throw an exception?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
