-
Notifications
You must be signed in to change notification settings - Fork 250
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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 | ||
|
@@ -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."); | ||
String msg = | ||
String.format( | ||
"realm: %1s root principal credentials: %2s:%3s", | ||
realm, | ||
principalSecrets.getPrincipalClientId(), | ||
principalSecrets.getMainSecret()); | ||
System.out.println(msg); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't want to change the behavior in this PR. |
||
} | ||
}); | ||
|
||
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); | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.