Skip to content

Auto-bootstrap: add verbose logging #1376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,6 @@ public synchronized Map<String, PrincipalSecretsResult> bootstrapRealms(
PrincipalSecretsResult secretsResult =
bootstrapServiceAndCreatePolarisPrincipalForRealm(
realmContext, metaStoreManagerMap.get(realmContext.getRealmIdentifier()));

if (rootCredentialsSet.credentials().containsKey(realm)) {
LOGGER.info("Bootstrapped realm {} using preset credentials.", realm);
}

results.put(realmContext.getRealmIdentifier(), secretsResult);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import jakarta.ws.rs.container.ContainerRequestContext;
import jakarta.ws.rs.core.Context;
import java.time.Clock;
import java.util.stream.Collectors;
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.PolarisDefaultDiagServiceImpl;
import org.apache.polaris.core.PolarisDiagnostics;
Expand Down Expand Up @@ -68,8 +69,11 @@
import org.eclipse.microprofile.config.inject.ConfigProperty;
import org.eclipse.microprofile.context.ManagedExecutor;
import org.eclipse.microprofile.context.ThreadContext;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class QuarkusProducers {
private static final Logger LOGGER = LoggerFactory.getLogger(QuarkusProducers.class);

@Produces
@ApplicationScoped // cannot be singleton because it is mocked in tests
Expand Down Expand Up @@ -159,9 +163,68 @@ public void maybeBootstrap(
MetaStoreManagerFactory factory,
QuarkusPersistenceConfiguration config,
RealmContextConfiguration realmContextConfiguration) {
var rootCredentialsSet = RootCredentialsSet.fromEnvironment();
var rootCredentials = rootCredentialsSet.credentials();
if (config.isAutoBootstrap()) {
RootCredentialsSet rootCredentialsSet = RootCredentialsSet.fromEnvironment();
factory.bootstrapRealms(realmContextConfiguration.realms(), rootCredentialsSet);
var realmIds = realmContextConfiguration.realms();

LOGGER.info(
"Bootstrapping realm(s) {}, if necessary, from root credentials set provided via the environment variable {} or Java system property {} ...",
realmIds.stream().map(r -> "'" + r + "'").collect(Collectors.joining(", ")),
RootCredentialsSet.ENVIRONMENT_VARIABLE,
RootCredentialsSet.SYSTEM_PROPERTY);

var result = factory.bootstrapRealms(realmIds, rootCredentialsSet);

result.forEach(
(realm, secrets) -> {
var principalSecrets = secrets.getPrincipalSecrets();

var log =
LOGGER
.atInfo()
.addArgument(realm)
.addArgument(RootCredentialsSet.ENVIRONMENT_VARIABLE)
.addArgument(RootCredentialsSet.SYSTEM_PROPERTY);
if (rootCredentials.containsKey(realm)) {
log.log(
"Realm '{}' automatically bootstrapped, credentials taken from root credentials set provided via the environment variable {} or Java system property {}, not printed to stdout.");
} else {
log.log(
"Realm '{}' automatically bootstrapped, credentials were not present in root credentials set provided via the environment variable {} or Java system property {}, see separate message printed to stdout.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the admin tool here we gate this behind a command-line argument, do we need to do something similar here or should we always print?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the case when the realm's to be automatically bootstrapped, but the credentials are generated (not provided).
The above case (creds provided) never prints the credentials.

String msg =
String.format(
"realm: %1s root principal credentials: %2s:%3s",
realm,
principalSecrets.getPrincipalClientId(),
principalSecrets.getMainSecret());
System.out.println(msg);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH, I do not think it's worth printing credentials to STDOUT at all. What's the use case for that that cannot be covered by user-provided credentials?

Cf. #1428 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to change the behavior in this PR.
But I agree that generating and printing credentials isn't that great.

}
});

var unusedRealmSecrets =
realmIds.stream()
.filter(rootCredentials::containsKey)
.filter(r -> !result.containsKey(r))
.map(r -> "'" + r + "'")
.collect(Collectors.joining(", "));
if (!unusedRealmSecrets.isEmpty()) {
// This is intentionally an error to highlight the importance of the situation.
LOGGER.error(
"The realms {} are already fully bootstrapped but the secrets are still available via the environment variable {} or Java system property {}. "
+ "Remove this security sensitive information from the environment / Java system properties!",
unusedRealmSecrets,
RootCredentialsSet.ENVIRONMENT_VARIABLE,
RootCredentialsSet.SYSTEM_PROPERTY);
}
} else if (!rootCredentials.isEmpty()) {
// This is intentionally an error to highlight the importance of the situation.
LOGGER.error(
"Secrets for the realms {} are available via the environment variable {} or Java system property {}. "
+ "Remove this security sensitive information from the environment / Java system properties!",
rootCredentials.keySet(),
RootCredentialsSet.ENVIRONMENT_VARIABLE,
RootCredentialsSet.SYSTEM_PROPERTY);
}
}

Expand Down