fix: preventing args to service commands (#45050)

also outputting all commands for help files

closes #44975

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
This commit is contained in:
Steven Hawkins 2026-01-05 14:31:14 -05:00 committed by GitHub
parent 5939864b76
commit 241ca57157
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 93 additions and 51 deletions

View File

@ -131,21 +131,8 @@ Example:
### Updating Expectations
Changing the help output will cause HelpCommandDistTest to fail. This test uses [ApprovalTests](https://github.com/approvals/ApprovalTests.Java) which creates `.received.txt` files containing the actual output when tests fail. To update the expected output (see [Approving The Result](https://github.com/approvals/ApprovalTests.Java/blob/master/approvaltests/docs/tutorials/GettingStarted.md#approving-the-result)):
Changing to the help output will cause HelpCommandDistTest to fail. This test uses [ApprovalTests](https://github.com/approvals/ApprovalTests.Java) which creates `.received.txt` files containing the actual output when tests fail. To update the expected output (see [Approving The Result](https://github.com/approvals/ApprovalTests.Java/blob/master/approvaltests/docs/tutorials/GettingStarted.md#approving-the-result)) run:
1. Run the failing test:
```
../mvnw clean install -Dtest=HelpCommandDistTest
```
KEYCLOAK_REPLACE_EXPECTED=true ../mvnw clean install -Dtest=HelpCommandDistTest
2. Review the generated `.received.txt` files in the test directory and compare them with the `.approved.txt` files.
3. If the changes look correct, rename the `.received.txt` files to `.approved.txt` to approve the new output:
```
# Example for a specific test
mv HelpCommandDistTest.testHelp.received.txt HelpCommandDistTest.testHelp.approved.txt
```
Note: If the files match, the received file will be deleted automatically. You must include the `.approved.` files in source control.
Alternatively, you can configure an [approval reporter](https://github.com/approvals/ApprovalTests.Java/blob/master/approvaltests/docs/reference/Reporters.md) to use a diff tool for easier comparison.
then use a diff to ensure the changes look good.

View File

@ -45,6 +45,7 @@ import org.keycloak.quarkus.runtime.cli.command.AbstractCommand;
import org.keycloak.quarkus.runtime.cli.command.AbstractNonServerCommand;
import org.keycloak.quarkus.runtime.cli.command.Build;
import org.keycloak.quarkus.runtime.cli.command.Main;
import org.keycloak.quarkus.runtime.cli.command.ShowConfig;
import org.keycloak.quarkus.runtime.cli.command.Tools;
import org.keycloak.quarkus.runtime.cli.command.WindowsService;
import org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource;
@ -93,7 +94,7 @@ public class Picocli {
public static final String ARG_SHORT_PREFIX = "-";
public static final String NO_PARAM_LABEL = "none";
private record IncludeOptions(boolean includeRuntime, boolean includeBuildTime) {
private record IncludeOptions(boolean includeRuntime, boolean includeBuildTime, boolean allowUnrecognized) {
}
private final ExecutionExceptionHandler errorHandler = new ExecutionExceptionHandler();
@ -101,6 +102,7 @@ public class Picocli {
private boolean warnedTimestampChanged;
private Ansi colorMode = hasColorSupport() ? Ansi.ON : Ansi.OFF;
private IncludeOptions options;
public static boolean hasColorSupport() {
return QuarkusConsole.hasColorSupport();
@ -139,7 +141,7 @@ public class Picocli {
}
initConfig(currentCommand);
if (!unrecognizedArgs.isEmpty()) {
if (!unrecognizedArgs.isEmpty() && options.allowUnrecognized) {
// TODO: further refactor this as these args should be the source for ConfigArgsConfigSource
unrecognizedArgs.removeIf(arg -> {
boolean hasArg = false;
@ -157,10 +159,11 @@ public class Picocli {
}
return false;
});
if (!unrecognizedArgs.isEmpty()) {
addCommandOptions(cl, currentCommand);
throw new KcUnmatchedArgumentException(cl, unrecognizedArgs);
}
}
if (!unrecognizedArgs.isEmpty()) {
addCommandOptions(cl, currentCommand);
throw new KcUnmatchedArgumentException(cl, unrecognizedArgs);
}
if (isHelpRequested(result)) {
@ -222,8 +225,6 @@ public class Picocli {
}
warnOnDuplicatedOptionsInCli();
IncludeOptions options = getIncludeOptions(abstractCommand);
if (!options.includeBuildTime && !options.includeRuntime) {
return;
}
@ -639,7 +640,7 @@ public class Picocli {
* Removes platform-specific commands on non-applicable platforms
*/
private void removePlatformSpecificCommands(CommandLine cmd) {
if (!Environment.isWindows()) {
if (getCommandMode() == CommandMode.UNIX) {
CommandLine toolsCmd = cmd.getSubcommands().get(Tools.NAME);
if (toolsCmd != null) {
CommandLine windowsServiceCmd = toolsCmd.getSubcommands().get(WindowsService.NAME);
@ -650,6 +651,18 @@ public class Picocli {
}
}
enum CommandMode {
ALL,
WIN,
UNIX
}
protected CommandMode getCommandMode() {
// not an official option, just a way for integration tests to produce the same output regardless of OS
return Optional.ofNullable(System.getenv("KEYCLOAK_COMMAND_MODE")).map(CommandMode::valueOf)
.orElse(Environment.isWindows() ? CommandMode.WIN : CommandMode.UNIX);
}
public PrintWriter getErrWriter() {
return new PrintWriter(System.err, true);
}
@ -660,16 +673,14 @@ public class Picocli {
private IncludeOptions getIncludeOptions(AbstractCommand abstractCommand) {
if (abstractCommand == null) {
return new IncludeOptions(false, false);
return new IncludeOptions(false, false, false);
}
boolean autoBuild = abstractCommand instanceof AbstractAutoBuildCommand;
boolean includeBuildTime = abstractCommand instanceof Build || (autoBuild && !abstractCommand.isOptimized());
return new IncludeOptions(autoBuild, includeBuildTime);
return new IncludeOptions(autoBuild, includeBuildTime, autoBuild || includeBuildTime || abstractCommand instanceof ShowConfig);
}
private void addCommandOptions(CommandLine command, AbstractCommand ac) {
IncludeOptions options = getIncludeOptions(ac);
if (!options.includeBuildTime && !options.includeRuntime) {
return;
}
@ -910,6 +921,7 @@ public class Picocli {
throw new IllegalStateException("Config should not be initialized until profile is determined");
}
this.parsedCommand = Optional.ofNullable(command);
options = getIncludeOptions(command);
if (!Environment.isRebuilt() && command instanceof AbstractAutoBuildCommand
&& !command.isOptimized()) {

View File

@ -1707,4 +1707,39 @@ public class PicocliTest extends AbstractConfigurationTest {
"quarkus.otel.logs.enabled", "true"
));
}
@Test
public void testWindowsServiceOnWin() {
NonRunningPicocli nonRunningPicocli = new NonRunningPicocli() {
@Override
protected CommandMode getCommandMode() {
return CommandMode.WIN;
}
};
KeycloakMain.main(new String[] {"tools", "windows-service", "--help"}, nonRunningPicocli);
assertEquals(CommandLine.ExitCode.OK, nonRunningPicocli.exitCode);
assertTrue(nonRunningPicocli.getOutString().contains("install"));
onAfter();
KeycloakMain.main(new String[] {"tools", "windows-service"}, nonRunningPicocli);
assertEquals(CommandLine.ExitCode.USAGE, nonRunningPicocli.exitCode);
assertTrue(nonRunningPicocli.getErrString().contains("Missing required subcommand"));
}
@Test
public void testWindowsServiceOnUnix() {
NonRunningPicocli nonRunningPicocli = new NonRunningPicocli() {
@Override
protected CommandMode getCommandMode() {
return CommandMode.UNIX;
}
};
KeycloakMain.main(new String[] {"tools", "windows-service", "--help"}, nonRunningPicocli);
assertEquals(CommandLine.ExitCode.OK, nonRunningPicocli.exitCode);
assertFalse(nonRunningPicocli.getOutString().contains("install"));
onAfter();
KeycloakMain.main(new String[] {"tools", "windows-service"}, nonRunningPicocli);
assertEquals(CommandLine.ExitCode.USAGE, nonRunningPicocli.exitCode);
assertTrue(nonRunningPicocli.getErrString().contains("Unknown option"));
}
}

View File

@ -17,11 +17,14 @@
package org.keycloak.it.cli.dist;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.List;
import org.keycloak.it.junit5.extension.CLIResult;
import org.keycloak.it.junit5.extension.DistributionTest;
import org.keycloak.it.junit5.extension.RawDistOnly;
import org.keycloak.it.junit5.extension.WithEnvVars;
import org.keycloak.it.utils.KeycloakDistribution;
import org.keycloak.quarkus.runtime.Environment;
import org.keycloak.quarkus.runtime.cli.command.BootstrapAdmin;
@ -36,21 +39,22 @@ import org.keycloak.quarkus.runtime.cli.command.UpdateCompatibility;
import org.keycloak.quarkus.runtime.cli.command.UpdateCompatibilityCheck;
import org.keycloak.quarkus.runtime.cli.command.UpdateCompatibilityMetadata;
import com.spun.util.io.FileUtils;
import io.quarkus.test.junit.main.Launch;
import org.apache.commons.io.FileUtils;
import org.approvaltests.Approvals;
import org.approvaltests.core.Options;
import org.approvaltests.core.VerifyResult;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.Test;
import static org.keycloak.quarkus.runtime.cli.command.AbstractAutoBuildCommand.OPTIMIZED_BUILD_OPTION_LONG;
@WithEnvVars({"KEYCLOAK_COMMAND_MODE", "ALL"})
@DistributionTest
@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";
@Test
@Launch({})
void testDefaultToHelp(CLIResult cliResult) {
@ -214,27 +218,22 @@ public class HelpCommandDistTest {
output = output
.replace("kc.bat", "kc.sh")
.replace("data\\log\\", "data/log/")
.replace("including\nbuild ", "including build\n");
.replace("\r\n", "\n");
}
// Custom comparator that strips Windows-specific lines from the approved file on non-Windows platforms
Options options = new Options().withComparator((receivedFile, approvedFile) -> {
String received = FileUtils.readFile(receivedFile);
String approved = FileUtils.readFile(approvedFile);
if (!Environment.isWindows()) {
approved = stripWindowsServiceLines(approved);
try {
Approvals.verify(output);
} catch (Error 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;
}
return VerifyResult.from(approved.equals(received));
});
Approvals.verify(output, options);
}
private String stripWindowsServiceLines(String text) {
return text
.replaceAll("(?m)^ {4}windows-service\\s+Manage Keycloak as a Windows service\\.\\R", "")
.replaceAll("(?m)^ {6}install\\s+Install Keycloak as a Windows service\\.\\R", "")
.replaceAll("(?m)^ {6}uninstall\\s+Uninstall Keycloak Windows service\\.\\R", "");
}
}
}

View File

@ -22,6 +22,8 @@ import org.keycloak.it.junit5.extension.DistributionTest;
import io.quarkus.test.junit.main.Launch;
import io.quarkus.test.junit.main.LaunchResult;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledOnOs;
import org.junit.jupiter.api.condition.OS;
import static org.junit.jupiter.api.Assertions.assertTrue;
@ -35,4 +37,11 @@ public class ToolsCommandDistTest {
() -> "The Output:\n" + result.getOutput() + "doesn't contains the expected string.");
}
@DisabledOnOs(value = OS.WINDOWS, disabledReason = "Tests for non-windows failure")
@Test
@Launch({ "tools", "windows-service" })
void windowsSericeNotAvailable(LaunchResult result) {
assertTrue(result.getErrorOutput().contains("Unknown option: 'windows-service'"));
}
}