Merge pull request #48500 from radcortez/runtime-config-warning

Warn if Runtime configuration is used in BuildSteps and without a RuntimeValue in Recorders

Author: gsmet

core/deployment/src/main/java/io/quarkus/deployment/ExtensionLoader.java

@@ -1,5 +1,6 @@
 package io.quarkus.deployment;
 
+import static io.quarkus.deployment.ExtensionLoaderConfig.ReportRuntimeConfigAtDeployment.warn;
 import static io.quarkus.deployment.util.ReflectUtil.isBuildProducerOf;
 import static io.quarkus.deployment.util.ReflectUtil.isConsumerOf;
 import static io.quarkus.deployment.util.ReflectUtil.isListOf;
@@ -113,7 +114,6 @@ private ExtensionLoader() {
     }
 
     private static final Logger loadLog = Logger.getLogger("io.quarkus.deployment");
-    private static final Logger cfgLog = Logger.getLogger("io.quarkus.configuration");
     @SuppressWarnings("unchecked")
     private static final Class<? extends BooleanSupplier>[] EMPTY_BOOLEAN_SUPPLIER_CLASS_ARRAY = new Class[0];
 
@@ -207,6 +207,7 @@ public boolean canHandleObject(final Object obj, final boolean staticInit) {
                     }
                 };
 
+                // Load @ConfigMapping in recorded deployment code from Recorder
                 ObjectLoader mappingLoader = new ObjectLoader() {
                     @Override
                     public ResultHandle load(final BytecodeCreator body, final Object obj, final boolean staticInit) {
@@ -268,6 +269,8 @@ private static Consumer<BuildChainBuilder> loadStepsFromClass(Class<?> clazz,
             throw reportError(clazz, "Build step classes must have exactly one constructor");
         }
 
+        ExtensionLoaderConfig extensionLoaderConfig = (ExtensionLoaderConfig) readResult.getObjectsByClass()
+                .get(ExtensionLoaderConfig.class);
         EnumSet<ConfigPhase> consumingConfigPhases = EnumSet.noneOf(ConfigPhase.class);
 
         final Constructor<?> constructor = constructors[0];
@@ -591,12 +594,24 @@ private static Consumer<BuildChainBuilder> loadStepsFromClass(Class<?> clazz,
                             }
                         } else if (phase.isReadAtMain()) {
                             if (isRecorder) {
-                                methodParamFns.add((bc, bri) -> {
-                                    final RunTimeConfigurationProxyBuildItem proxies = bc
-                                            .consume(RunTimeConfigurationProxyBuildItem.class);
-                                    return proxies.getProxyObjectFor(parameterClass);
-                                });
-                                runTimeProxies.computeIfAbsent(parameterClass, ConfigMappingUtils::newInstance);
+                                if (extensionLoaderConfig.reportRuntimeConfigAtDeployment().equals(warn)) {
+                                    methodParamFns.add((bc, bri) -> {
+                                        RunTimeConfigurationProxyBuildItem proxies = bc
+                                                .consume(RunTimeConfigurationProxyBuildItem.class);
+                                        return proxies.getProxyObjectFor(parameterClass);
+                                    });
+                                    loadLog.warn(reportError(parameter,
+                                            phase + " configuration should not be consumed in Build Steps, use RuntimeValue<"
+                                                    + parameter.getType().getTypeName()
+                                                    + "> in a @Recorder constructor instead")
+                                            .getMessage());
+                                    runTimeProxies.computeIfAbsent(parameterClass, ConfigMappingUtils::newInstance);
+                                } else {
+                                    throw reportError(parameter,
+                                            phase + " configuration cannot be consumed in Build Steps, use RuntimeValue<"
+                                                    + parameter.getType().getTypeName()
+                                                    + "> in a @Recorder constructor instead");
+                                }
                             } else {
                                 throw reportError(parameter,
                                         phase + " configuration cannot be consumed here unless the method is a @Recorder");
@@ -619,11 +634,13 @@ private static Consumer<BuildChainBuilder> loadStepsFromClass(Class<?> clazz,
                         for (var ctor : ctors) {
                             if (ctors.length == 1 || ctor.isAnnotationPresent(Inject.class)) {
                                 for (var type : ctor.getGenericParameterTypes()) {
-                                    Class<?> theType = null;
+                                    Class<?> theType;
+                                    boolean isRuntimeValue = false;
                                     if (type instanceof ParameterizedType) {
                                         ParameterizedType pt = (ParameterizedType) type;
                                         if (pt.getRawType().equals(RuntimeValue.class)) {
                                             theType = (Class<?>) pt.getActualTypeArguments()[0];
+                                            isRuntimeValue = true;
                                         } else {
                                             throw new RuntimeException("Unknown recorder constructor parameter: " + type
                                                     + " in recorder " + parameter.getType());
@@ -634,11 +651,26 @@ private static Consumer<BuildChainBuilder> loadStepsFromClass(Class<?> clazz,
                                     ConfigRoot annotation = theType.getAnnotation(ConfigRoot.class);
                                     if (annotation != null) {
                                         if (recordAnnotation.value() == ExecutionTime.STATIC_INIT) {
+                                            // TODO - Check for runtime config is done in another place, we may want to make things more consistent. Rewrite once we disallow the injection of runtime objects in build steps
                                             methodConsumingConfigPhases.add(ConfigPhase.BUILD_AND_RUN_TIME_FIXED);
                                         } else {
                                             methodConsumingConfigPhases.add(annotation.phase());
+                                            if (annotation.phase().isReadAtMain() && !isRuntimeValue) {
+                                                if (extensionLoaderConfig.reportRuntimeConfigAtDeployment().equals(warn)) {
+                                                    loadLog.warn(reportError(parameter, annotation.phase() + " configuration "
+                                                            + type.getTypeName()
+                                                            + " should be injected in a @Recorder constructor as a RuntimeValue<"
+                                                            + type.getTypeName() + ">").getMessage());
+                                                } else {
+                                                    throw reportError(parameter, annotation.phase() + " configuration "
+                                                            + type.getTypeName()
+                                                            + " can only be injected in a @Recorder constructor as a RuntimeValue<"
+                                                            + type.getTypeName() + ">");
+                                                }
+                                            }
                                         }
                                         if (annotation.phase().isReadAtMain()) {
+                                            // TODO - Remove once we disallow the injection of runtime objects in build steps
                                             runTimeProxies.computeIfAbsent(theType, ConfigMappingUtils::newInstance);
                                         } else {
                                             runTimeProxies.computeIfAbsent(theType, readResult::requireObjectForClass);
@@ -835,6 +867,7 @@ public void execute(final BuildContext bc) {
                                                         }
                                                         return runTimeProxies.get(s);
                                                     }
+                                                    // TODO - Remove once we disallow the injection of runtime objects in build steps
                                                     if (s instanceof ParameterizedType) {
                                                         ParameterizedType p = (ParameterizedType) s;
                                                         if (p.getRawType() == RuntimeValue.class) {

core/deployment/src/main/java/io/quarkus/deployment/ExtensionLoaderConfig.java

@@ -0,0 +1,20 @@
+package io.quarkus.deployment;
+
+import io.quarkus.runtime.annotations.ConfigRoot;
+import io.smallrye.config.ConfigMapping;
+import io.smallrye.config.WithDefault;
+
+@ConfigMapping(prefix = "quarkus.extension-loader")
+@ConfigRoot
+public interface ExtensionLoaderConfig {
+    /**
+     * Report runtime Config objects used during deployment time.
+     */
+    @WithDefault("warn")
+    ReportRuntimeConfigAtDeployment reportRuntimeConfigAtDeployment();
+
+    enum ReportRuntimeConfigAtDeployment {
+        warn,
+        fail
+    }
+}

core/deployment/src/main/java/io/quarkus/deployment/builditem/RunTimeConfigurationProxyBuildItem.java

@@ -7,6 +7,7 @@
 /**
  * A build item that carries all the "fake" run time config objects for use by recorders.
  */
+@Deprecated(forRemoval = true, since = "3.25")
 public final class RunTimeConfigurationProxyBuildItem extends SimpleBuildItem {
     private final Map<Class<?>, Object> objects;
 

core/deployment/src/main/java/io/quarkus/deployment/configuration/ConfigMappingUtils.java

@@ -191,9 +191,9 @@ private static void registerImplicitConverter(
         }
     }
 
+    @Deprecated(forRemoval = true, since = "3.25")
     public static Object newInstance(Class<?> configClass) {
         if (configClass.isAnnotationPresent(ConfigMapping.class)) {
-            // TODO - radcortez - mapping classes cannot be initialized like this.
             return ReflectUtil.newInstance(ConfigMappingLoader.ensureLoaded(configClass).implementation());
         } else {
             return ReflectUtil.newInstance(configClass);

core/deployment/src/main/java/io/quarkus/deployment/logging/LoggingResourceProcessor.java

@@ -240,8 +240,6 @@ void miscSetup(
     LoggingSetupBuildItem setupLoggingRuntimeInit(
             final RecorderContext context,
             final LoggingSetupRecorder recorder,
-            final LogRuntimeConfig logRuntimeConfig,
-            final LogBuildTimeConfig logBuildTimeConfig,
             final CombinedIndexBuildItem combinedIndexBuildItem,
             final LogCategoryMinLevelDefaultsBuildItem categoryMinLevelDefaults,
             final Optional<StreamingLogHandlerBuildItem> streamingLogStreamHandlerBuildItem,
@@ -308,7 +306,7 @@ LoggingSetupBuildItem setupLoggingRuntimeInit(
             }
 
             shutdownListenerBuildItemBuildProducer.produce(new ShutdownListenerBuildItem(
-                    recorder.initializeLogging(logRuntimeConfig, logBuildTimeConfig, discoveredLogComponents,
+                    recorder.initializeLogging(discoveredLogComponents,
                             categoryMinLevelDefaults.content, alwaysEnableLogStream,
                             streamingDevUiLogHandler, handlers, namedHandlers,
                             possibleConsoleFormatters, possibleFileFormatters, possibleSyslogFormatters,
@@ -326,6 +324,7 @@ LoggingSetupBuildItem setupLoggingRuntimeInit(
             }
 
             SmallRyeConfig config = ConfigProvider.getConfig().unwrap(SmallRyeConfig.class);
+            LogBuildTimeConfig logBuildTimeConfig = config.getConfigMapping(LogBuildTimeConfig.class);
             LogRuntimeConfig logRuntimeConfigInBuild = config.getConfigMapping(LogRuntimeConfig.class);
             ConsoleRuntimeConfig consoleRuntimeConfig = config.getConfigMapping(ConsoleRuntimeConfig.class);
 

core/runtime/src/main/java/io/quarkus/runtime/BannerRecorder.java

@@ -7,7 +7,7 @@
 
 @Recorder
 public class BannerRecorder {
-    final RuntimeValue<BannerRuntimeConfig> bannerRuntimeConfig;
+    private final RuntimeValue<BannerRuntimeConfig> bannerRuntimeConfig;
 
     public BannerRecorder(RuntimeValue<BannerRuntimeConfig> bannerRuntimeConfig) {
         this.bannerRuntimeConfig = bannerRuntimeConfig;

core/runtime/src/main/java/io/quarkus/runtime/ExecutorRecorder.java

@@ -24,20 +24,19 @@
  */
 @Recorder
 public class ExecutorRecorder {
-
     private static final Logger log = Logger.getLogger("io.quarkus.thread-pool");
 
     private static volatile Executor current;
 
-    final ThreadPoolConfig threadPoolConfig;
+    private final RuntimeValue<ThreadPoolConfig> threadPoolConfig;
 
-    public ExecutorRecorder(ThreadPoolConfig threadPoolConfig) {
+    public ExecutorRecorder(RuntimeValue<ThreadPoolConfig> threadPoolConfig) {
         this.threadPoolConfig = threadPoolConfig;
     }
 
     public ScheduledExecutorService setupRunTime(ShutdownContext shutdownContext,
             LaunchMode launchMode, ThreadFactory threadFactory, ContextHandler<Object> contextHandler) {
-        final EnhancedQueueExecutor underlying = createExecutor(threadPoolConfig, threadFactory, contextHandler);
+        final EnhancedQueueExecutor underlying = createExecutor(threadPoolConfig.getValue(), threadFactory, contextHandler);
         if (launchMode == LaunchMode.DEVELOPMENT) {
             shutdownContext.addLastShutdownTask(new Runnable() {
                 @Override
@@ -52,10 +51,10 @@ public void run() {
                 }
             });
         } else {
-            Runnable shutdownTask = createShutdownTask(threadPoolConfig, underlying);
+            Runnable shutdownTask = createShutdownTask(threadPoolConfig.getValue(), underlying);
             shutdownContext.addLastShutdownTask(shutdownTask);
         }
-        if (threadPoolConfig.prefill()) {
+        if (threadPoolConfig.getValue().prefill()) {
             underlying.prestartAllCoreThreads();
         }
         ScheduledExecutorService managed = underlying;

core/runtime/src/main/java/io/quarkus/runtime/init/InitializationTaskRecorder.java

@@ -6,21 +6,22 @@
 
 import io.quarkus.runtime.PreventFurtherStepsException;
 import io.quarkus.runtime.Quarkus;
+import io.quarkus.runtime.RuntimeValue;
 import io.quarkus.runtime.annotations.Recorder;
 
 /**
  * A {@link Recorder} that is used to check if the application should exit once all initialization tasks are completed.
  */
 @Recorder
 public class InitializationTaskRecorder {
-    private final InitRuntimeConfig initRuntimeConfig;
+    private final RuntimeValue<InitRuntimeConfig> initRuntimeConfig;
 
-    public InitializationTaskRecorder(InitRuntimeConfig initRuntimeConfig) {
+    public InitializationTaskRecorder(RuntimeValue<InitRuntimeConfig> initRuntimeConfig) {
         this.initRuntimeConfig = initRuntimeConfig;
     }
 
     public void exitIfNeeded() {
-        if (initRuntimeConfig.initAndExit()) {
+        if (initRuntimeConfig.getValue().initAndExit()) {
             preventFurtherRecorderSteps(5, "Error attempting to gracefully shutdown after initialization",
                     () -> new PreventFurtherStepsException("Gracefully exiting after initialization.", 0));
         }

core/runtime/src/main/java/io/quarkus/runtime/logging/LoggingSetupRecorder.java

@@ -73,12 +73,18 @@
 
 @Recorder
 public class LoggingSetupRecorder {
-
     private static final org.jboss.logging.Logger log = org.jboss.logging.Logger.getLogger(LoggingSetupRecorder.class);
 
-    final RuntimeValue<ConsoleRuntimeConfig> consoleRuntimeConfig;
+    private final LogBuildTimeConfig logBuildTimeConfig;
+    private final RuntimeValue<LogRuntimeConfig> logRuntimeConfig;
+    private final RuntimeValue<ConsoleRuntimeConfig> consoleRuntimeConfig;
 
-    public LoggingSetupRecorder(RuntimeValue<ConsoleRuntimeConfig> consoleRuntimeConfig) {
+    public LoggingSetupRecorder(
+            final LogBuildTimeConfig logBuildTimeConfig,
+            final RuntimeValue<LogRuntimeConfig> logRuntimeConfig,
+            final RuntimeValue<ConsoleRuntimeConfig> consoleRuntimeConfig) {
+        this.logBuildTimeConfig = logBuildTimeConfig;
+        this.logRuntimeConfig = logRuntimeConfig;
         this.consoleRuntimeConfig = consoleRuntimeConfig;
     }
 
@@ -114,18 +120,17 @@ public String getName() {
                         return "Logging Config";
                     }
                 }).build();
-        LogRuntimeConfig logRuntimeConfig = loggingConfig.getConfigMapping(LogRuntimeConfig.class);
         LogBuildTimeConfig logBuildTimeConfig = loggingConfig.getConfigMapping(LogBuildTimeConfig.class);
+        LogRuntimeConfig logRuntimeConfig = loggingConfig.getConfigMapping(LogRuntimeConfig.class);
         ConsoleRuntimeConfig consoleRuntimeConfig = loggingConfig.getConfigMapping(ConsoleRuntimeConfig.class);
-        new LoggingSetupRecorder(new RuntimeValue<>(consoleRuntimeConfig)).initializeLogging(logRuntimeConfig,
-                logBuildTimeConfig,
-                DiscoveredLogComponents.ofEmpty(), emptyMap(), false, null, emptyList(), emptyList(), emptyList(), emptyList(),
-                emptyList(), emptyList(), banner, LaunchMode.DEVELOPMENT, false);
+        new LoggingSetupRecorder(logBuildTimeConfig, new RuntimeValue<>(logRuntimeConfig),
+                new RuntimeValue<>(consoleRuntimeConfig)).initializeLogging(
+                        DiscoveredLogComponents.ofEmpty(), emptyMap(), false, null, emptyList(), emptyList(), emptyList(),
+                        emptyList(),
+                        emptyList(), emptyList(), banner, LaunchMode.DEVELOPMENT, false);
     }
 
     public ShutdownListener initializeLogging(
-            final LogRuntimeConfig config,
-            final LogBuildTimeConfig buildConfig,
             final DiscoveredLogComponents discoveredLogComponents,
             final Map<String, InheritableLevel> categoryDefaultMinLevels,
             final boolean enableWebStream,
@@ -140,6 +145,9 @@ public ShutdownListener initializeLogging(
             final LaunchMode launchMode,
             final boolean includeFilters) {
 
+        LogBuildTimeConfig buildConfig = logBuildTimeConfig;
+        LogRuntimeConfig config = logRuntimeConfig.getValue();
+
         ShutdownNotifier shutdownNotifier = new ShutdownNotifier();
         Map<String, CategoryConfig> categories = config.categories();
         LogContext logContext = LogContext.getLogContext();

core/runtime/src/main/java/io/quarkus/runtime/shutdown/ShutdownRecorder.java

@@ -7,6 +7,7 @@
 
 import org.jboss.logging.Logger;
 
+import io.quarkus.runtime.RuntimeValue;
 import io.quarkus.runtime.annotations.Recorder;
 
 @Recorder
@@ -15,10 +16,13 @@ public class ShutdownRecorder {
     private static final Logger log = Logger.getLogger(ShutdownRecorder.class);
 
     private static volatile List<ShutdownListener> shutdownListeners;
-    private static volatile ShutdownConfig shutdownConfig;
+    private static volatile RuntimeValue<ShutdownConfig> shutdownConfig;
     private static volatile boolean delayEnabled;
 
-    public ShutdownRecorder(ShutdownConfig shutdownConfig) {
+    /**
+     * ShutdownRecorder is only called in <code>RUNTIME_INIT</code> build steps, so it is safe to set ShutdownConfig.
+     */
+    public ShutdownRecorder(RuntimeValue<ShutdownConfig> shutdownConfig) {
         ShutdownRecorder.shutdownConfig = shutdownConfig;
     }
 
@@ -50,9 +54,9 @@ private static void executePreShutdown() throws InterruptedException {
     }
 
     private static void waitForDelay() {
-        if (delayEnabled && shutdownConfig.isDelayEnabled()) {
+        if (delayEnabled && shutdownConfig.getValue().isDelayEnabled()) {
             try {
-                Thread.sleep(shutdownConfig.delay().get().toMillis());
+                Thread.sleep(shutdownConfig.getValue().delay().get().toMillis());
             } catch (InterruptedException e) {
                 log.error("Interrupted while waiting for delay, continuing to shutdown immediately");
             }
@@ -64,8 +68,8 @@ private static void executeShutdown() throws InterruptedException {
         for (ShutdownListener i : shutdownListeners) {
             i.shutdown(new LatchShutdownNotification(shutdown));
         }
-        if (shutdownConfig.isTimeoutEnabled()
-                && !shutdown.await(shutdownConfig.timeout().get().toMillis(), TimeUnit.MILLISECONDS)) {
+        if (shutdownConfig.getValue().isTimeoutEnabled()
+                && !shutdown.await(shutdownConfig.getValue().timeout().get().toMillis(), TimeUnit.MILLISECONDS)) {
             log.error("Timed out waiting for graceful shutdown, shutting down anyway.");
         }
     }