Make Keycloak fail with an error when the persisted build options differs from those provided (#33241)

* PropertyException is now thrown instead of a warning
* Operator guides clarification around health and metrics options

Closes: #32717

Signed-off-by: Peter Zaoral <pzaoral@redhat.com>
This commit is contained in:
Peter Zaoral 2024-09-30 19:28:23 +02:00 committed by GitHub
parent 8bbae59b60
commit d5d6390b1c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 36 additions and 12 deletions

View file

@ -302,7 +302,7 @@ The following event types are now deprecated and will be removed in a future ver
= `--import-realm` option can import the master realm
When running a `start` or `start-dev` command with the `--import-realm` option before the master realm exists, it will be imported if it exists in the import material. The previous behavior was that the master realm was created first, then its import skipped.
When running a `start` or `start-dev` command with the `--import-realm` option before the master realm exists, it will be imported if it exists in the import material. The previous behavior was that the master realm was created first, then its import skipped.
= BouncyCastle FIPS updated
@ -325,3 +325,7 @@ Keycloak JS now utilizes the Web Crypto API to calculate the SHA-256 digests nee
- `createRegisterUrl()`
Make sure to update your code to `await` these methods.
= Stricter startup behavior for build-time options
When the provided build-time options differ at startup from the values persisted in the server image during the last optimized {project_name} build, {project_name} will now fail to start. Previously, a warning message was displayed in such cases.

View file

@ -22,6 +22,8 @@ To avoid this delay, you can provide a custom image with the augmentation built-
With a custom image, you can also specify the Keycloak _build-time_ configurations and extensions during the build of the container.
WARNING: When using optimized custom image, `health-enabled` and `metrics-enabled` options need to be explicitly set in the Containerfile.
For instructions on how to build such an image, see <@links.server id="containers"/>.
=== Providing a custom {project_name} image

View file

@ -375,7 +375,7 @@ public class Picocli {
if (options.includeRuntime) {
disabledMappers.addAll(PropertyMappers.getDisabledRuntimeMappers().values());
}
checkSpiOptions(options, ignoredBuildTime, ignoredRunTime);
for (OptionCategory category : abstractCommand.getOptionCategories()) {
@ -430,7 +430,7 @@ public class Picocli {
Logger logger = Logger.getLogger(Picocli.class); // logger can't be instantiated in a class field
if (!ignoredBuildTime.isEmpty()) {
logger.warn(format("The following build time options have values that differ from what is persisted - the new values will NOT be used until another build is run: %s\n",
throw new PropertyException(format("The following build time options have values that differ from what is persisted - the new values will NOT be used until another build is run: %s\n",
String.join(", ", ignoredBuildTime)));
} else if (!ignoredRunTime.isEmpty()) {
logger.warn(format("The following run time options were found, but will be ignored during build time: %s\n",
@ -460,7 +460,7 @@ public class Picocli {
continue;
}
boolean buildTimeOption = key.endsWith("-provider") || key.endsWith("-provider-default") || key.endsWith("-enabled");
ConfigValue configValue = Configuration.getConfigValue(key);
String configValueStr = configValue.getValue();
@ -468,7 +468,7 @@ public class Picocli {
if (configValueStr == null || configValue.getConfigSourceOrdinal() < 300) {
continue;
}
if (!options.includeBuildTime) {
if (buildTimeOption) {
String currentValue = getRawPersistedProperty(key).orElse(null);

View file

@ -69,12 +69,12 @@ public class StartCommandDistTest {
@Test
@RawDistOnly(reason = "Containers are immutable")
void warnSpiBuildtimeAtRuntime(KeycloakDistribution dist) {
void errorSpiBuildtimeAtRuntime(KeycloakDistribution dist) {
CLIResult result = dist.run("build");
result.assertBuild();
result = dist.run("start", "--optimized", "--http-enabled=true", "--hostname-strict=false", "--spi-events-listener-jboss-logging-enabled=false");
result.assertMessage("The following build time options have values that differ from what is persisted - the new values will NOT be used until another build is run: kc.spi-events-listener-jboss-logging-enabled");
result.assertError("The following build time options have values that differ from what is persisted - the new values will NOT be used until another build is run: kc.spi-events-listener-jboss-logging-enabled");
}
@Test
@ -211,12 +211,12 @@ public class StartCommandDistTest {
@Test
@RawDistOnly(reason = "Containers are immutable")
void testWarningWhenOverridingNonCliBuildOptionsDuringStart(KeycloakDistribution dist) {
void testErrorWhenOverridingNonCliBuildOptionsDuringStart(KeycloakDistribution dist) {
CLIResult cliResult = dist.run("build", "--features=preview");
cliResult.assertBuild();
dist.setEnvVar("KC_DB", "postgres");
cliResult = dist.run("start", "--optimized", "--hostname=localhost", "--http-enabled=true");
cliResult.assertMessage("The following build time options have values that differ from what is persisted - the new values will NOT be used until another build is run: kc.db");
cliResult.assertError("The following build time options have values that differ from what is persisted - the new values will NOT be used until another build is run: kc.db");
}
@Test

View file

@ -51,7 +51,22 @@ public class KeycloakQuarkusServerDeployableContainer extends AbstractQuarkusDep
logProcessor = new LogProcessor(new BufferedReader(new InputStreamReader(container.getInputStream())));
stdoutForwarderThread = new Thread(logProcessor);
stdoutForwarderThread.start();
waitForReadiness();
try {
waitForReadiness();
} catch (Exception e) {
if (logProcessor.containsBuildTimeOptionsError()) {
log.warn("The build time options have values that differ from what is persisted. Restarting container...");
container.destroy();
container = startContainer();
logProcessor = new LogProcessor(new BufferedReader(new InputStreamReader(container.getInputStream())));
stdoutForwarderThread = new Thread(logProcessor);
stdoutForwarderThread.start();
waitForReadiness();
} else {
throw e;
}
}
} catch (Exception e) {
throw new RuntimeException(e);
}
@ -285,8 +300,7 @@ public class KeycloakQuarkusServerDeployableContainer extends AbstractQuarkusDep
loggedLines.remove(0);
}
}
}
catch (IOException e) {
} catch (IOException e) {
throw new RuntimeException(e);
}
}
@ -294,5 +308,9 @@ public class KeycloakQuarkusServerDeployableContainer extends AbstractQuarkusDep
public String getBufferedLog() {
return String.join("\n", loggedLines);
}
public boolean containsBuildTimeOptionsError() {
return loggedLines.stream().anyMatch(line -> line.contains("The following build time options have values that differ from what is persisted"));
}
}
}