eric-maynard commented on code in PR #1799:
URL: https://github.com/apache/polaris/pull/1799#discussion_r2125452297
##########
quarkus/admin/src/main/java/org/apache/polaris/admintool/BootstrapCommand.java:
##########
@@ -90,45 +90,41 @@ public Integer call() {
|| inputOptions.stdinOptions.credentials.isEmpty()
? RootCredentialsSet.EMPTY
:
RootCredentialsSet.fromList(inputOptions.stdinOptions.credentials);
- if (inputOptions.stdinOptions.credentials == null
- || inputOptions.stdinOptions.credentials.isEmpty()) {
- if (!inputOptions.stdinOptions.printCredentials) {
- spec.commandLine()
- .getErr()
- .println(
- "Specify either `--credentials` or `--print-credentials`
to ensure"
- + " the root user is accessible after bootstrapping.");
- return EXIT_CODE_BOOTSTRAP_ERROR;
- }
+ if (rootCredentialsSet.credentials().isEmpty()
+ && !inputOptions.stdinOptions.printCredentials) {
+ spec.commandLine()
+ .getErr()
+ .println(
+ "Specify either `--credentials` or `--print-credentials` to
ensure"
+ + " the root user is accessible after bootstrapping.");
+ return EXIT_CODE_BOOTSTRAP_ERROR;
}
}
- // Execute the bootstrap
Map<String, PrincipalSecretsResult> results =
metaStoreManagerFactory.bootstrapRealms(realms, rootCredentialsSet);
- // Log any errors:
boolean success = true;
for (Map.Entry<String, PrincipalSecretsResult> result :
results.entrySet()) {
- if (result.getValue().isSuccess()) {
- String realm = result.getKey();
+ String realm = result.getKey();
+ PrincipalSecretsResult secretsResult = result.getValue();
+ if (secretsResult.isSuccess()) {
spec.commandLine().getOut().printf("Realm '%s' successfully
bootstrapped.%n", realm);
- if (inputOptions.stdinOptions != null &&
inputOptions.stdinOptions.printCredentials) {
- String msg =
- String.format(
- "realm: %1s root principal credentials: %2s:%3s",
- result.getKey(),
-
result.getValue().getPrincipalSecrets().getPrincipalClientId(),
- result.getValue().getPrincipalSecrets().getMainSecret());
- spec.commandLine().getOut().println(msg);
+ if (inputOptions.stdinOptions != null
+ && inputOptions.stdinOptions.printCredentials
+ && !rootCredentialsSet.credentials().containsKey(realm)) {
Review Comment:
`--generate-credentials` seems redundant with the supplies credentials. That
is, there are 4 modes and 2 of them would be invalid:
| Credentials Provided | --generate-credentials | Valid |
| ------------- | ------------- | --- |
| false | false | true |
| false | true | false |
| true | false | false |
| true | true | true |
Besides that, we'd lose what appears to me a valid behavior -- echoing back
non-generated credentials
##########
quarkus/admin/src/main/java/org/apache/polaris/admintool/BootstrapCommand.java:
##########
@@ -90,45 +90,41 @@ public Integer call() {
|| inputOptions.stdinOptions.credentials.isEmpty()
? RootCredentialsSet.EMPTY
:
RootCredentialsSet.fromList(inputOptions.stdinOptions.credentials);
- if (inputOptions.stdinOptions.credentials == null
- || inputOptions.stdinOptions.credentials.isEmpty()) {
- if (!inputOptions.stdinOptions.printCredentials) {
- spec.commandLine()
- .getErr()
- .println(
- "Specify either `--credentials` or `--print-credentials`
to ensure"
- + " the root user is accessible after bootstrapping.");
- return EXIT_CODE_BOOTSTRAP_ERROR;
- }
+ if (rootCredentialsSet.credentials().isEmpty()
+ && !inputOptions.stdinOptions.printCredentials) {
+ spec.commandLine()
+ .getErr()
+ .println(
+ "Specify either `--credentials` or `--print-credentials` to
ensure"
+ + " the root user is accessible after bootstrapping.");
+ return EXIT_CODE_BOOTSTRAP_ERROR;
}
}
- // Execute the bootstrap
Map<String, PrincipalSecretsResult> results =
metaStoreManagerFactory.bootstrapRealms(realms, rootCredentialsSet);
- // Log any errors:
boolean success = true;
for (Map.Entry<String, PrincipalSecretsResult> result :
results.entrySet()) {
- if (result.getValue().isSuccess()) {
- String realm = result.getKey();
+ String realm = result.getKey();
+ PrincipalSecretsResult secretsResult = result.getValue();
+ if (secretsResult.isSuccess()) {
spec.commandLine().getOut().printf("Realm '%s' successfully
bootstrapped.%n", realm);
- if (inputOptions.stdinOptions != null &&
inputOptions.stdinOptions.printCredentials) {
- String msg =
- String.format(
- "realm: %1s root principal credentials: %2s:%3s",
- result.getKey(),
-
result.getValue().getPrincipalSecrets().getPrincipalClientId(),
- result.getValue().getPrincipalSecrets().getMainSecret());
- spec.commandLine().getOut().println(msg);
+ if (inputOptions.stdinOptions != null
+ && inputOptions.stdinOptions.printCredentials
+ && !rootCredentialsSet.credentials().containsKey(realm)) {
Review Comment:
`--generate-credentials` seems redundant with the supplied credentials. That
is, there are 4 modes and 2 of them would be invalid:
| Credentials Provided | --generate-credentials | Valid |
| ------------- | ------------- | --- |
| false | false | true |
| false | true | false |
| true | false | false |
| true | true | true |
Besides that, we'd lose what appears to me a valid behavior -- echoing back
non-generated credentials
--
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]
