fix: removes default values from cache stack option (#28310)

* fix: removes default values from cache stack option

also adding a way to update expected files

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
This commit is contained in:
Steven Hawkins 2024-04-04 11:08:48 -04:00 committed by GitHub
parent 7cbe609571
commit 54af571f1e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 53 additions and 30 deletions

View file

@ -111,3 +111,11 @@ To use a specific database container image, use the option `-Dkc.db.postgresql.c
Example:
../mvnw clean install -Dkc.test.storage.database=true -Dtest=PostgreSQLDistTest -Dkc.db.postgresql.container.image=postgres:alpine
### Updating Expectations
Changing to the help output will cause HelpCommandDistTest to fail. You may use:
KEYCLOAK_REPLACE_EXPECTED=true ../mvnw clean install -Dtest=HelpCommandDistTest
to replace the expected output, then use a diff to ensure the changes look good.

View file

@ -1,6 +1,9 @@
package org.keycloak.config;
import java.io.File;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;
public class CachingOptions {
@ -47,8 +50,9 @@ public class CachingOptions {
public static final Option<Stack> CACHE_STACK = new OptionBuilder<>("cache-stack", Stack.class)
.category(OptionCategory.CACHE)
.expectedValues(List.of())
.description("Define the default stack to use for cluster communication and node discovery. This option only takes effect "
+ "if 'cache' is set to 'ispn'. Default: udp.")
+ "if 'cache' is set to 'ispn'. Default: udp. Built-in values include: " + Stream.of(Stack.values()).map(Stack::name).collect(Collectors.joining(", ")))
.buildTime(true)
.build();
@ -92,14 +96,14 @@ public class CachingOptions {
.category(OptionCategory.CACHE)
.description(String.format("The hostname of the remote server for the remote store configuration. "
+ "It replaces the 'host' attribute of 'remote-server' tag of the configuration specified via XML file (see '%s' option.). "
+ "If the option is specified, '%s' and '%s' are required as well and the related configuration in XML file should not be present.",
+ "If the option is specified, '%s' and '%s' are required as well and the related configuration in XML file should not be present.",
CACHE_CONFIG_FILE_PROPERTY, CACHE_REMOTE_USERNAME_PROPERTY, CACHE_REMOTE_PASSWORD_PROPERTY))
.build();
public static final Option<Integer> CACHE_REMOTE_PORT = new OptionBuilder<>(CACHE_REMOTE_PORT_PROPERTY, Integer.class)
.category(OptionCategory.CACHE)
.description(String.format("The port of the remote server for the remote store configuration. "
+ "It replaces the 'port' attribute of 'remote-server' tag of the configuration specified via XML file (see '%s' option.).",
+ "It replaces the 'port' attribute of 'remote-server' tag of the configuration specified via XML file (see '%s' option.).",
CACHE_CONFIG_FILE_PROPERTY))
.defaultValue(11222)
.build();
@ -108,7 +112,7 @@ public class CachingOptions {
.category(OptionCategory.CACHE)
.description(String.format("The username for the authentication to the remote server for the remote store. "
+ "It replaces the 'username' attribute of 'digest' tag of the configuration specified via XML file (see '%s' option.). "
+ "If the option is specified, '%s' and '%s' are required as well and the related configuration in XML file should not be present.",
+ "If the option is specified, '%s' and '%s' are required as well and the related configuration in XML file should not be present.",
CACHE_CONFIG_FILE_PROPERTY, CACHE_REMOTE_HOST_PROPERTY, CACHE_REMOTE_PASSWORD_PROPERTY))
.build();
@ -116,7 +120,7 @@ public class CachingOptions {
.category(OptionCategory.CACHE)
.description(String.format("The password for the authentication to the remote server for the remote store. "
+ "It replaces the 'password' attribute of 'digest' tag of the configuration specified via XML file (see '%s' option.). "
+ "If the option is specified, '%s' and '%s' are required as well and the related configuration in XML file should not be present.",
+ "If the option is specified, '%s' and '%s' are required as well and the related configuration in XML file should not be present.",
CACHE_CONFIG_FILE_PROPERTY, CACHE_REMOTE_HOST_PROPERTY, CACHE_REMOTE_USERNAME_PROPERTY))
.build();

View file

@ -62,9 +62,9 @@ public class ClusterConfigDistTest {
}
@Test
@Launch({ "build", "--cache-stack=invalid" })
@Launch({ "start-dev", "--cache=ispn", "--cache-stack=invalid" })
void failInvalidClusterStack(LaunchResult result) {
assertTrue(result.getErrorOutput().contains("Invalid value for option '--cache-stack': invalid. Expected values are: tcp, udp, kubernetes, ec2, azure, google"));
assertTrue(result.getOutput().contains("No such JGroups stack 'invalid'"));
}
@Test

View file

@ -18,23 +18,23 @@
package org.keycloak.it.cli.dist;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.keycloak.it.cli.dist.GelfRemovedTest.INCLUDE_GELF_PROPERTY;
import static org.keycloak.quarkus.runtime.cli.command.AbstractStartCommand.OPTIMIZED_BUILD_OPTION_LONG;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.Locale;
import java.util.regex.Pattern;
import org.apache.commons.io.FileUtils;
import org.approvaltests.Approvals;
import org.approvaltests.namer.NamedEnvironment;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.OS;
import org.keycloak.it.approvaltests.KcNamerFactory;
import org.keycloak.it.junit5.extension.CLIResult;
import org.keycloak.it.junit5.extension.DistributionTest;
import org.keycloak.it.junit5.extension.RawDistOnly;
@ -52,6 +52,8 @@ import io.quarkus.test.junit.main.LaunchResult;
@RawDistOnly(reason = "Verifying the help message output doesn't need long spin-up of docker dist tests.")
public class HelpCommandDistTest {
public static final String REPLACE_EXPECTED = "KEYCLOAK_REPLACE_EXPECTED";
@BeforeAll
public static void assumeGelfEnabled() {
Assumptions.assumeTrue(Boolean.getBoolean(INCLUDE_GELF_PROPERTY), "Assume GELF support is given in order to simplify these test cases");
@ -176,23 +178,32 @@ public class HelpCommandDistTest {
}
private void assertHelp(CLIResult result) {
// normalize the output to prevent changes around the feature toggles to mark the output to differ
String output = result.getOutput().replaceAll("((Disables|Enables) a set of one or more features. Possible values are: )[^.]{30,}", "$1<...>");
String osName = System.getProperty("os.name");
if(osName.toLowerCase(Locale.ROOT).contains("windows")) {
// On Windows, all output should have at least one "kc.bat" in it.
MatcherAssert.assertThat(output, Matchers.containsString("kc.bat"));
output = output.replaceAll("kc.bat", "kc.sh");
output = output.replaceAll(Pattern.quote("data\\log\\"), "data/log/");
// line wrap which looks differently due to ".bat" vs. ".sh"
output = output.replaceAll("including\nbuild ", "including build\n");
}
try {
// normalize the output to prevent changes around the feature toggles to mark the output to differ
String output = result.getOutput().replaceAll("((Disables|Enables) a set of one or more features. Possible values are: )[^.]{30,}", "$1<...>");
String osName = System.getProperty("os.name");
if(osName.toLowerCase(Locale.ROOT).contains("windows")) {
// On Windows, all output should have at least one "kc.bat" in it.
MatcherAssert.assertThat(output, Matchers.containsString("kc.bat"));
output = output.replaceAll("kc.bat", "kc.sh");
output = output.replaceAll(Pattern.quote("data\\log\\"), "data/log/");
// line wrap which looks differently due to ".bat" vs. ".sh"
output = output.replaceAll("including\nbuild ", "including build\n");
}
Approvals.verify(output);
} catch (Exception cause) {
throw new RuntimeException("Failed to assert help", cause);
} catch (AssertionError cause) {
if ("true".equals(System.getenv(REPLACE_EXPECTED))) {
try {
FileUtils.write(Approvals.createApprovalNamer().getApprovedFile(".txt"), output,
StandardCharsets.UTF_8);
} catch (IOException e) {
throw new RuntimeException("Failed to assert help, and could not replace expected", cause);
}
} else {
throw cause;
}
}
}
}

View file

@ -30,7 +30,7 @@ Cache:
--cache-stack <stack>
Define the default stack to use for cluster communication and node discovery.
This option only takes effect if 'cache' is set to 'ispn'. Default: udp.
Possible values are: tcp, udp, kubernetes, ec2, azure, google.
Built-in values include: tcp, udp, kubernetes, ec2, azure, google
Database:

View file

@ -64,7 +64,7 @@ Cache:
--cache-stack <stack>
Define the default stack to use for cluster communication and node discovery.
This option only takes effect if 'cache' is set to 'ispn'. Default: udp.
Possible values are: tcp, udp, kubernetes, ec2, azure, google.
Built-in values include: tcp, udp, kubernetes, ec2, azure, google
Config:

View file

@ -67,7 +67,7 @@ Cache:
--cache-stack <stack>
Define the default stack to use for cluster communication and node discovery.
This option only takes effect if 'cache' is set to 'ispn'. Default: udp.
Possible values are: tcp, udp, kubernetes, ec2, azure, google.
Built-in values include: tcp, udp, kubernetes, ec2, azure, google
Config:

View file

@ -65,7 +65,7 @@ Cache:
--cache-stack <stack>
Define the default stack to use for cluster communication and node discovery.
This option only takes effect if 'cache' is set to 'ispn'. Default: udp.
Possible values are: tcp, udp, kubernetes, ec2, azure, google.
Built-in values include: tcp, udp, kubernetes, ec2, azure, google
Config:

View file

@ -68,7 +68,7 @@ Cache:
--cache-stack <stack>
Define the default stack to use for cluster communication and node discovery.
This option only takes effect if 'cache' is set to 'ispn'. Default: udp.
Possible values are: tcp, udp, kubernetes, ec2, azure, google.
Built-in values include: tcp, udp, kubernetes, ec2, azure, google
Config: