8000 fix: remove erroneous spi warnings by shawkins · Pull Request #33648 · keycloak/keycloak · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: remove erroneous spi warnings #33648

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 21, 2024
Merged

fix: remove erroneous spi warnings #33648

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/KeycloakMain.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ public static void main(String[] args) {

try {
PropertyMappers.sanitizeDisabledMappers();
Picocli.validateConfig(cliArgs, new Start());
PrintWriter outStream = new PrintWriter(System.out, true);
Picocli.validateConfig(cliArgs, new Start(), outStream);
} catch (PropertyException | ProfileException e) {
handleUsageError(e.getMessage(), e.getCause());
return;
Expand Down
58 changes: 44 additions & 14 deletions quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/Picocli.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
import java.util.stream.Collectors;

import org.eclipse.microprofile.config.spi.ConfigSource;
import org.jboss.logging.Logger;
import org.keycloak.common.profile.ProfileException;
import org.keycloak.config.DeprecatedMetadata;
import org.keycloak.config.Option;
Expand All @@ -81,14 +80,19 @@
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers;
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper;
import org.keycloak.quarkus.runtime.Environment;
import org.keycloak.quarkus.runtime.KeycloakMain;

import io.quarkus.bootstrap.runner.QuarkusEntryPoint;
import io.smallrye.config.ConfigValue;

import picocli.CommandLine;
import picocli.CommandLine.ParameterException;
import picocli.CommandLine.ParseResult;
import picocli.CommandLine.DuplicateOptionAnnotationsException;
import picocli.CommandLine.Help.Ansi;
import picocli.CommandLine.Help.Ansi.Style;
import picocli.CommandLine.Help.ColorScheme;
import picocli.CommandLine.IFactory;
import picocli.CommandLine.Model.CommandSpec;
import picocli.CommandLine.Model.ISetter;
import picocli.CommandLine.Model.OptionSpec;
Expand Down Expand Up @@ -349,8 +353,9 @@ private static boolean hasProviderChanges() {
*
* @param cliArgs
* @param abstractCommand
* @param outWriter
*/
public static void validateConfig(List<String> cliArgs, AbstractCommand abstractCommand) {
public static void validateConfig(List<String> cliArgs, AbstractCommand abstractCommand, PrintWriter outWriter) {
IncludeOptions options = getIncludeOptions(cliArgs, abstractCommand, abstractCommand.getName());

if (!options.includeBuildTime && !options.includeRuntime) {
Expand Down Expand Up @@ -427,24 +432,22 @@ public static void validateConfig(List<String> cliArgs, AbstractCommand abstract
}
}

Logger logger = Logger.getLogger(Picocli.class); // logger can't be instantiated in a class field

if (!ignoredBuildTime.isEmpty()) {
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",
String.join(", ", ignoredRunTime)));
warn(format("The following run time options were found, but will be ignored during build time: %s\n",
String.join(", ", ignoredRunTime)), outWriter);
}

if (!disabledBuildTime.isEmpty()) {
outputDisabledProperties(disabledBuildTime, true, logger);
outputDisabledProperties(disabledBuildTime, true, outWriter);
} else if (!disabledRunTime.isEmpty()) {
outputDisabledProperties(disabledRunTime, false, logger);
outputDisabledProperties(disabledRunTime, false, outWriter);
}

if (!deprecatedInUse.isEmpty()) {
logger.warn("The following used options or option values are DEPRECATED and will be removed or their behaviour changed in a future release:\n" + String.join("\n", deprecatedInUse) + "\nConsult the Release Notes for details.");
warn("The following used options or option values are DEPRECATED and will be removed or their behaviour changed in a future release:\n" + String.join("\n", deprecatedInUse) + "\nConsult the Release Notes for details.", outWriter);
}
} finally {
DisabledMappersInterceptor.enable(disabledMappersInterceptorEnabled);
Expand Down Expand Up @@ -543,11 +546,16 @@ private static void handleDisabled(Set<String> disabledInUse, PropertyMapper<?>
}
disabledInUse.add(sb.toString());
}

private static void warn(String text, PrintWriter outwriter) {
ColorScheme defaultColorScheme = picocli.CommandLine.Help.defaultColorScheme(Help.Ansi.AUTO);
outwriter.println(defaultColorScheme.apply("WARNING: ", Arrays.asList(Style.fg_yellow, Style.bold)) + text);
}

private static void outputDisabledProperties(Set<String> properties, boolean build, Logger logger) {
logger.warn(format("The following used %s time options are UNAVAILABLE and will be ignored during %s time:\n %s",
private static void outputDisabledProperties(Set<String> properties, boolean build, PrintWriter outWriter) {
warn(format("The following used %s time options are UNAVAILABLE and will be ignored during %s time:\n %s",
build ? "build" : "run", build ? "run" : "build",
String.join("\n", properties)));
String.join("\n", properties)), outWriter);
}

private static boolean hasConfigChanges(CommandLine cmdCommand) {
Expand Down Expand Up @@ -671,7 +679,16 @@ private static boolean isProviderKey(String key) {
}

public CommandLine createCommandLine(Consumer<CommandSpec> consumer) {
CommandSpec spec = CommandSpec.forAnnotatedObject(new Main()).name(Environment.getCommand());
CommandSpec spec = CommandSpec.forAnnotatedObject(new Main(), new IFactory() {
@Override
public <K> K create(Class<K> cls) throws Exception {
K result = CommandLine.defaultFactory().create(cls);
if (result instanceof AbstractCommand ac) {
ac.setPicocli(Picocli.this);
}
return result;
}
}).name(Environment.getCommand());
consumer.accept(spec);

CommandLine cmd = new CommandLine(spec);
Expand All @@ -681,13 +698,17 @@ public CommandLine createCommandLine(Consumer<CommandSpec> consumer) {
cmd.setHelpFactory(new HelpFactory());
cmd.getHelpSectionMap().put(SECTION_KEY_COMMAND_LIST, new SubCommandListRenderer());
cmd.setErr(getErrWriter());

cmd.setOut(getOutWriter());
return cmd;
}

protected PrintWriter getErrWriter() {
return new PrintWriter(System.err, true);
}

protected PrintWriter getOutWriter() {
return new PrintWriter(System.out, true);
}

private static void addHelp(CommandSpec currentSpec) {
try {
Expand Down Expand Up @@ -961,4 +982,13 @@ private static void checkChangesInBuildOptionsDuringAutoBuild() {
}
}
}

public void start(CommandLine cmd) {
KeycloakMain.start((ExecutionExceptionHandler) cmd.getExecutionExceptionHandler(), cmd.getErr(), cmd.getParseResult().originalArgs().toArray(new String[0]));
}

public void build() throws Throwable {
QuarkusEntryPoint.main();
}

}
7 changes: 6 additions & 1 deletion quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/AbstractCommand.java
View file
Open in desktop
5D40
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public abstract class AbstractCommand {

@Spec
protected CommandSpec spec;
protected Picocli picocli;

protected void executionError(CommandLine cmd, String message) {
executionError(cmd, message, null);
Expand Down Expand Up @@ -65,13 +66,17 @@ public List<OptionCategory> getOptionCategories() {
}

protected void validateConfig() {
Picocli.validateConfig(ConfigArgsConfigSource.getAllCliArgs(), this);
Picocli.validateConfig(ConfigArgsConfigSource.getAllCliArgs(), this, spec.commandLine().getOut());
}

public abstract String getName();

public CommandLine getCommandLine() {
return spec.commandLine();
}

public void setPicocli(Picocli picocli) {
this.picocli = picocli;
}

}
13 changes: 2 additions & 11 deletions .../runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/AbstractStartCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource;
import org.keycloak.quarkus.runtime.configuration.mappers.HostnameV2PropertyMappers;
import org.keycloak.quarkus.runtime.configuration.mappers.HttpPropertyMappers;
import org.keycloak.url.HostnameV2ProviderFactory;

import java.util.EnumSet;
import java.util.List;
Expand All @@ -38,8 +37,6 @@
public abstract class AbstractStartCommand extends AbstractCommand implements Runnable {
public static final String OPTIMIZED_BUILD_OPTION_LONG = "--optimized";

private boolean skipStart;

@Override
public void run() {
Environment.setParsedCommand(this);
Expand All @@ -52,10 +49,8 @@ public void run() {
if (ConfigArgsConfigSource.getAllCliArgs().contains(OPTIMIZED_BUILD_OPTION_LONG) && !wasBuildEverRun()) {
executionError(spec.commandLine(), Messages.optimizedUsedForFirstStartup());
}

if (!skipStart) {
KeycloakMain.start((ExecutionExceptionHandler) cmd.getExecutionExceptionHandler(), cmd.getErr(), cmd.getParseResult().originalArgs().toArray(new String[0]));
}

picocli.start(cmd);
}

protected void doBeforeRun() {
Expand All @@ -76,8 +71,4 @@ protected EnumSet<OptionCategory> excludedCategories() {
return EnumSet.of(OptionCategory.IMPORT, OptionCategory.EXPORT);
}

public void setSkipStart(boolean skipStart) {
this.skipStart = skipStart;
}

}
4 changes: 2 additions & 2 deletions quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/Build.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public void run() {
configureBuildClassLoader();

beforeReaugmentationOnWindows();
QuarkusEntryPoint.main();
picocli.build();

if (!isDevProfile()) {
println(spec.commandLine(), "Server configuration updated and persisted. Run the following command to review the configuration:\n");
Expand Down Expand Up @@ -133,7 +133,7 @@ private void beforeReaugmentationOnWindows() {
private void cleanTempResources() {
if (!LaunchMode.current().isDevOrTest()) {
// only needed for dev/testing purposes
getHomePath().resolve("quarkus-artifact.properties").toFile().delete();
Optional.ofNullable(getHomePath()).ifPresent(path -> path.resolve("quarkus-artifact.properties").toFile().delete());
}
}

Expand Down
71 changes: 47 additions & 24 deletions quarkus/runtime/src/test/java/or/keycloak/quarkus/runtime/cli/PicocliTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,16 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.Arrays;
import java.util.List;

import org.junit.Test;
import org.keycloak.quarkus.runtime.cli.Picocli;
import org.keycloak.quarkus.runtime.cli.command.AbstractStartCommand;
import org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource;
import org.keycloak.quarkus.runtime.configuration.test.AbstractConfigurationTest;

Expand All @@ -43,20 +42,34 @@ public class PicocliTest extends AbstractConfigurationTest {
private class NonRunningPicocli extends Picocli {

final StringWriter err = new StringWriter();
final StringWriter out = new StringWriter();
SmallRyeConfig config;
int exitCode = Integer.MAX_VALUE;

String getErrString() {
// normalize line endings - TODO: could also normalize non-printable chars
// but for now those are part of the expected output
return System.lineSeparator().equals("\n") ? err.toString()
: err.toString().replace(System.lineSeparator(), "\n");
return normalize(err);
}

// normalize line endings - TODO: could also normalize non-printable chars
// but for now those are part of the expected output
String normalize(StringWriter writer) {
return System.lineSeparator().equals("\n") ? writer.toString()
: writer.toString().replace(System.lineSeparator(), "\n");
}

String getOutString() {
return normalize(out);
}

@Override
protected PrintWriter getErrWriter() {
return new PrintWriter(err, true);
}

@Override
protected PrintWriter getOutWriter() {
return new PrintWriter(out, true);
}

@Override
protected void exitOnFailure(int exitCode, CommandLine cmd) {
Expand All @@ -67,33 +80,29 @@ protected int runReAugmentationIfNeeded(List<String> cliArgs, CommandLine cmd, C
throw new AssertionError("Should not reaugment");
};

@Override
protected int run(CommandLine cmd, String[] argArray) {
skipStart(cmd);
return super.run(cmd, argArray);
}

private void skipStart(CommandLine cmd) {
for (CommandLine sub : cmd.getSubcommands().values()) {
if (sub.getCommand() instanceof AbstractStartCommand) {
((AbstractStartCommand) (sub.getCommand())).setSkipStart(true);
}
skipStart(sub);
}
}

@Override
public void parseAndRun(List<String> cliArgs) {
ConfigArgsConfigSource.setCliArgs(cliArgs.toArray(String[]::new));
config = createConfig();
super.parseAndRun(cliArgs);
}

@Override
public void start(CommandLine cmd) {
// skip
}

@Override
public void build() throws Throwable {
// skip
}

};

NonRunningPicocli pseudoLaunch(String... args) {
NonRunningPicocli nonRunningPicocli = new NonRunningPicocli();
nonRunningPicocli.parseAndRun(Arrays.asList(args));
ConfigArgsConfigSource.setCliArgs(args);
var cliArgs = Picocli.parseArgs(args);
nonRunningPicocli.parseAndRun(cliArgs);
return nonRunningPicocli;
}

Expand Down Expand Up @@ -205,5 +214,19 @@ public void failSingleParamWithSpace() {
assertThat(nonRunningPicocli.getErrString(), containsString(
"Option: '--db postgres' is not expected to contain whitespace, please remove any unnecessary quoting/escaping"));
}


@Test
public void spiRuntimeAllowedWithStart() {
NonRunningPicocli nonRunningPicocli = pseudoLaunch("start", "--http-enabled=true", "--spi-something-pass=changeme");
assertEquals(CommandLine.ExitCode.OK, nonRunningPicocli.exitCode);
assertThat(nonRunningPicocli.getOutString(), not(containsString("kc.spi-something-pass")));
}

@Test
public void spiRuntimeWarnWithBuild() {
NonRunningPicocli nonRunningPicocli = pseudoLaunch("build", "--spi-something-pass=changeme");
assertEquals(CommandLine.ExitCode.OK, nonRunningPicocli.exitCode);
assertThat(nonRunningPicocli.getOutString(), containsString("The following run time options were found, but will be ignored during build time: kc.spi-something-pass"));
}

}
0