From 0d3ca438edeea658ecbaae3854c04ca52f92523e Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Mon, 1 Aug 2022 14:20:18 -0700 Subject: [PATCH] Aligning kc.bat with latest changes to kc.sh Closes #11185 Closes #13472 --- quarkus/dist/src/main/content/bin/kc.bat | 42 ++++++--- .../it/utils/RawKeycloakDistribution.java | 92 ++++++++----------- .../it/cli/dist/HelpCommandDistTest.java | 9 +- .../it/cli/dist/ImportAtStartupDistTest.java | 3 + .../it/cli/dist/StartCommandDistTest.java | 7 ++ ...andTest.testBuildHelp.windows.approved.txt | 1 - 6 files changed, 87 insertions(+), 67 deletions(-) diff --git a/quarkus/dist/src/main/content/bin/kc.bat b/quarkus/dist/src/main/content/bin/kc.bat index 72ce7463d8..9eb3a18c41 100644 --- a/quarkus/dist/src/main/content/bin/kc.bat +++ b/quarkus/dist/src/main/content/bin/kc.bat @@ -57,14 +57,15 @@ if not "%KEY:~0,2%"=="--" if "%KEY:~0,2%"=="-D" ( if not "%KEY:~0,2%"=="--" if not "%KEY:~0,1%"=="-" ( set "CONFIG_ARGS=%CONFIG_ARGS% %KEY%" ) -if "%KEY:~0,2%"=="--" if not "%KEY:~0,2%"=="-D" if "%KEY:~0,1%"=="-" ( - if "%~2"=="" ( - set "CONFIG_ARGS=%CONFIG_ARGS% %KEY%" - ) else ( - set "CONFIG_ARGS=%CONFIG_ARGS% %KEY% %~2%" +if not "%KEY:~0,2%"=="-D" ( + if "%KEY:~0,1%"=="-" ( + if "%~2"=="" ( + set "CONFIG_ARGS=%CONFIG_ARGS% %KEY%" + ) else ( + set "CONFIG_ARGS=%CONFIG_ARGS% %KEY% %~2%" + ) + shift ) - - shift ) shift goto READ-ARGS @@ -129,14 +130,33 @@ set "JAVA_RUN_OPTS=%JAVA_OPTS% -Dkc.home.dir="%DIRNAME%.." -Djboss.server.config SetLocal EnableDelayedExpansion -set "ONLY_BUILD_OPTION= build" -set "NO_AUTO_BUILD_OPTION=optimized" +set "OPTIMIZED_OPTION=--optimized" +set "HELP_LONG_OPTION=--help" +set "BUILD_OPTION= build" +set IS_HELP_SHORT=false + +echo "%CONFIG_ARGS%" | findstr /r "\<-h\>" > nul + +if not errorlevel == 1 ( + set IS_HELP_SHORT=true +) + +set START_SERVER=true + +if "!CONFIG_ARGS:%OPTIMIZED_OPTION%=!"=="!CONFIG_ARGS!" if "!CONFIG_ARGS:%BUILD_OPTION%=!"=="!CONFIG_ARGS!" if "!CONFIG_ARGS:%HELP_LONG_OPTION%=!"=="!CONFIG_ARGS!" if "%IS_HELP_SHORT%" == "false" ( + setlocal enabledelayedexpansion -if "!CONFIG_ARGS:%NO_AUTO_BUILD_OPTION%=!"=="!CONFIG_ARGS!" if "!CONFIG_ARGS:%ONLY_BUILD_OPTION%=!"=="!CONFIG_ARGS!" ( "%JAVA%" -Dkc.config.build-and-exit=true %JAVA_RUN_OPTS% + + if not !errorlevel! == 0 ( + set START_SERVER=false + ) + set "JAVA_RUN_OPTS=-Dkc.config.built=true %JAVA_RUN_OPTS%" ) -"%JAVA%" %JAVA_RUN_OPTS% +if "%START_SERVER%" == "true" ( + "%JAVA%" %JAVA_RUN_OPTS% +) :END diff --git a/quarkus/tests/integration/src/main/java/org/keycloak/it/utils/RawKeycloakDistribution.java b/quarkus/tests/integration/src/main/java/org/keycloak/it/utils/RawKeycloakDistribution.java index 98d26ca897..e56851b148 100644 --- a/quarkus/tests/integration/src/main/java/org/keycloak/it/utils/RawKeycloakDistribution.java +++ b/quarkus/tests/integration/src/main/java/org/keycloak/it/utils/RawKeycloakDistribution.java @@ -35,7 +35,6 @@ import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.security.cert.X509Certificate; import java.util.ArrayList; -import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -43,7 +42,6 @@ import java.util.Properties; import java.util.concurrent.*; import java.util.function.Consumer; import java.util.stream.Collectors; -import java.util.stream.Stream; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLContext; @@ -52,20 +50,22 @@ import javax.net.ssl.SSLSocketFactory; import javax.net.ssl.TrustManager; import javax.net.ssl.X509TrustManager; +import io.quarkus.deployment.util.FileUtil; import io.quarkus.fs.util.ZipUtils; -import org.apache.commons.io.FileUtils; import org.keycloak.common.Version; import org.keycloak.it.junit5.extension.CLIResult; -import org.keycloak.quarkus.runtime.Environment; import org.keycloak.quarkus.runtime.cli.command.Build; import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper; import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers; import static org.keycloak.quarkus.runtime.Environment.LAUNCH_MODE; +import static org.keycloak.quarkus.runtime.Environment.isWindows; public final class RawKeycloakDistribution implements KeycloakDistribution { + private static final int DEFAULT_SHUTDOWN_TIMEOUT_SECONDS = 10; + private Process keycloak; private int exitCode = -1; private final Path distPath; @@ -128,24 +128,14 @@ public final class RawKeycloakDistribution implements KeycloakDistribution { public void stop() { if (isRunning()) { try { - if (Environment.isWindows()) { - // On Windows, we're executing kc.bat in a runtime as "keycloak", - // so tha java process is an actual child process - // we have to kill first. - killChildProcessesOnWindows(false); - } + // On Windows, we need to make sure sub-processes are terminated first + destroyDescendantsOnWindows(keycloak, false); keycloak.destroy(); - keycloak.waitFor(10, TimeUnit.SECONDS); + keycloak.waitFor(DEFAULT_SHUTDOWN_TIMEOUT_SECONDS, TimeUnit.SECONDS); exitCode = keycloak.exitValue(); } catch (Exception cause) { - if (Environment.isWindows()) { - try { - killChildProcessesOnWindows(true); - } catch (Exception e) { - throw new RuntimeException("Failed to stop the server", e); - } - } + destroyDescendantsOnWindows(keycloak, true); keycloak.destroyForcibly(); throw new RuntimeException("Failed to stop the server", cause); } @@ -154,19 +144,36 @@ public final class RawKeycloakDistribution implements KeycloakDistribution { shutdownOutputExecutor(); } - private void killChildProcessesOnWindows(boolean isForced) { - for (ProcessHandle childProcessHandle : keycloak.children().collect(Collectors.toList())) { - CompletableFuture onExit = childProcessHandle.onExit(); - if (isForced) { - childProcessHandle.destroyForcibly(); + private void destroyDescendantsOnWindows(Process parent, boolean force) { + if (!isWindows()) { + return; + } + + CompletableFuture allProcesses = CompletableFuture.completedFuture(null); + + for (ProcessHandle process : parent.descendants().collect(Collectors.toList())) { + if (force) { + process.destroyForcibly(); } else { - childProcessHandle.destroy(); + process.destroy(); } - //for whatever reason windows doesnt wait for the termination, - // and parent process returns immediately with exitCode 1 but is not exited, leading to - // "failed to start the distribution" bc files that should be deleted - // are used by another process, so we need this here. - onExit.join(); + + allProcesses = CompletableFuture.allOf(allProcesses, process.onExit()); + } + + try { + allProcesses.get(DEFAULT_SHUTDOWN_TIMEOUT_SECONDS, TimeUnit.SECONDS); + } catch (Exception cause) { + throw new RuntimeException("Failed to terminate descendants processes", cause); + } + + try { + // TODO: remove this. do not ask why, but on Windows we are here even though the process was previously terminated + // without this pause, tests re-installing dist before tests should fail + // looks like pausing the current thread let windows to cleanup processes? + // more likely it is env dependent + Thread.sleep(500); + } catch (InterruptedException e) { } } @@ -195,7 +202,7 @@ public final class RawKeycloakDistribution implements KeycloakDistribution { public String[] getCliArgs(List arguments) { List allArgs = new ArrayList<>(); - if (Environment.isWindows()) { + if (isWindows()) { allArgs.add(distPath.resolve("bin") + File.separator + SCRIPT_CMD_INVOKABLE); } else { allArgs.add(SCRIPT_CMD_INVOKABLE); @@ -340,13 +347,7 @@ public final class RawKeycloakDistribution implements KeycloakDistribution { Path dPath = distRootPath.resolve(distDirName.substring(0, distDirName.lastIndexOf('.'))); if (!inited || (reCreate || !dPath.toFile().exists())) { - - if (!Environment.isWindows()) { - FileUtils.deleteDirectory(dPath.toFile()); - } else { - deleteTempFilesOnWindows(dPath); - } - + FileUtil.deleteDirectory(dPath); ZipUtils.unzip(distFile.toPath(), distRootPath); } @@ -363,23 +364,6 @@ public final class RawKeycloakDistribution implements KeycloakDistribution { } } - private void deleteTempFilesOnWindows(Path dPath) { - if (Files.exists(dPath)) { - try (Stream walk = Files.walk(dPath)) { - walk.sorted(Comparator.reverseOrder()) - .forEach(s -> { - try { - Files.delete(s); - } catch (IOException e) { - throw new RuntimeException("Could not delete temp directory for distribution", e); - } - }); - } catch (IOException e) { - throw new RuntimeException("Could not traverse temp directory for distribution to delete files", e); - } - } - } - private void readOutput() { try ( BufferedReader outStream = new BufferedReader(new InputStreamReader(keycloak.getInputStream())); diff --git a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/HelpCommandDistTest.java b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/HelpCommandDistTest.java index 21b3795e8c..7b390efe55 100644 --- a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/HelpCommandDistTest.java +++ b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/HelpCommandDistTest.java @@ -21,6 +21,7 @@ import static org.junit.Assert.assertEquals; import java.util.List; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.OS; import org.keycloak.it.cli.HelpCommandTest; import org.keycloak.it.junit5.extension.CLIResult; import org.keycloak.it.junit5.extension.DistributionTest; @@ -35,7 +36,13 @@ public class HelpCommandDistTest extends HelpCommandTest { public void testHelpDoesNotStartReAugJvm(KeycloakDistribution dist) { for (String helpCmd : List.of("-h", "--help", "--help-all")) { for (String cmd : List.of("", "start", "start-dev", "build")) { - CLIResult run = dist.run("--debug", cmd, helpCmd); + String debugOption = "--debug"; + + if (OS.WINDOWS.isCurrentOs()) { + debugOption = "--debug=8787"; + } + + CLIResult run = dist.run(debugOption, cmd, helpCmd); assertSingleJvmStarted(run); } } diff --git a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/ImportAtStartupDistTest.java b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/ImportAtStartupDistTest.java index c3121fa249..2a8bffec34 100644 --- a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/ImportAtStartupDistTest.java +++ b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/ImportAtStartupDistTest.java @@ -20,6 +20,8 @@ package org.keycloak.it.cli.dist; import java.nio.file.Path; import java.util.function.Consumer; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledOnOs; +import org.junit.jupiter.api.condition.OS; import org.keycloak.it.junit5.extension.BeforeStartDistribution; import org.keycloak.it.junit5.extension.CLIResult; import org.keycloak.it.junit5.extension.DistributionTest; @@ -60,6 +62,7 @@ public class ImportAtStartupDistTest { } @Test + @EnabledOnOs(value = { OS.LINUX, OS.MAC }, disabledReason = "different shell escaping behaviour on Windows.") @BeforeStartDistribution(CreateRealmConfigurationFile.class) @Launch({"start-dev", "--import-realm=some-file"}) void failSetValueToImportRealmOption(LaunchResult result) { diff --git a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/StartCommandDistTest.java b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/StartCommandDistTest.java index 369d1bf778..25012d949a 100644 --- a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/StartCommandDistTest.java +++ b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/StartCommandDistTest.java @@ -65,4 +65,11 @@ public class StartCommandDistTest extends StartCommandTest { cliResult.assertStarted(); } + @Test + @Launch({ "start", "--optimized", "--http-enabled=true", "--hostname-strict=false", "--cache=local" }) + void testStartUsingOptimizedDoesNotAllowBuildOptions(LaunchResult result) { + CLIResult cliResult = (CLIResult) result; + cliResult.assertError("Unknown option: '--cache'"); + } + } diff --git a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/approvals/cli/help/HelpCommandTest.testBuildHelp.windows.approved.txt b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/approvals/cli/help/HelpCommandTest.testBuildHelp.windows.approved.txt index a77d37262e..bb35dabfba 100644 --- a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/approvals/cli/help/HelpCommandTest.testBuildHelp.windows.approved.txt +++ b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/approvals/cli/help/HelpCommandTest.testBuildHelp.windows.approved.txt @@ -93,4 +93,3 @@ Examples: Change the relative path: $ kc.bat build --http-relative-path=/auth - \ No newline at end of file