feat(operator): Support for JAVA_OPTS_APPEND when creating app deployment

operator/controller/src/main/java/io/apicurio/registry/operator/EnvironmentVariables.java

@@ -1,6 +1,7 @@
 package io.apicurio.registry.operator;
 
 public class EnvironmentVariables {
+    public static final String JAVA_OPTS_APPEND = "JAVA_OPTS_APPEND";
 
     public static final String QUARKUS_PROFILE = "QUARKUS_PROFILE";
     public static final String QUARKUS_HTTP_ACCESS_LOG_ENABLED = "QUARKUS_HTTP_ACCESS_LOG_ENABLED";

operator/controller/src/main/java/io/apicurio/registry/operator/resource/app/AppDeploymentResource.java

@@ -12,6 +12,7 @@
 import io.apicurio.registry.operator.feat.KafkaSql;
 import io.apicurio.registry.operator.feat.PostgresSql;
 import io.apicurio.registry.operator.feat.security.Auth;
+import io.apicurio.registry.operator.utils.JavaOptsAppend;
 import io.apicurio.registry.operator.status.ReadyConditionManager;
 import io.apicurio.registry.operator.status.StatusManager;
 import io.fabric8.kubernetes.api.model.Container;
@@ -56,6 +57,14 @@ protected Deployment desired(ApicurioRegistry3 primary, Context<ApicurioRegistry
         ofNullable(primary.getSpec()).map(ApicurioRegistry3Spec::getApp).map(AppSpec::getEnv)
                 .ifPresent(env -> env.forEach(e -> envVars.put(e.getName(), e)));
 
+        // Handling of JAVA_OPTS_APPEND env var - we will merge whatever we find in the CR
+        // with whatever we might set ourselves in the logic of the operator.
+        var javaOptsAppend = new JavaOptsAppend();
+        if (envVars.containsKey(EnvironmentVariables.JAVA_OPTS_APPEND)) {
+            javaOptsAppend.setOptsFromEnvVar(envVars.get(EnvironmentVariables.JAVA_OPTS_APPEND).getValue());
+            envVars.remove(EnvironmentVariables.JAVA_OPTS_APPEND);
+        }
+
         addEnvVar(envVars, new EnvVarBuilder().withName(EnvironmentVariables.QUARKUS_PROFILE).withValue("prod").build());
         addEnvVar(envVars, new EnvVarBuilder().withName(EnvironmentVariables.QUARKUS_HTTP_ACCESS_LOG_ENABLED).withValue("true").build());
 
@@ -106,6 +115,11 @@ protected Deployment desired(ApicurioRegistry3 primary, Context<ApicurioRegistry
                     }
                 });
 
+        // Set the JAVA_OPTS_APPEND env var that may have been built up
+        if (!javaOptsAppend.isEmpty()) {
+            envVars.put(EnvironmentVariables.JAVA_OPTS_APPEND, javaOptsAppend.toEnvVar());
+        }
+
         // Set the ENV VARs on the deployment's container spec.
         var container = getContainerFromDeployment(deployment, REGISTRY_APP_CONTAINER_NAME);
         container.setEnv(envVars.values().stream().toList());

operator/controller/src/main/java/io/apicurio/registry/operator/utils/JavaOptsAppend.java

@@ -0,0 +1,133 @@
+package io.apicurio.registry.operator.utils;
+
+import io.apicurio.registry.operator.EnvironmentVariables;
+import io.fabric8.kubernetes.api.model.EnvVar;
+import io.fabric8.kubernetes.api.model.EnvVarBuilder;
+
+import java.util.Set;
+import java.util.TreeSet;
+
+/**
+ * Models the value of the JAVA_OPTS_APPEND environment variable. This is a convenient way to merge values and
+ * handle conflicts.
+ */
+public class JavaOptsAppend {
+
+    private final Set<String> opts = new TreeSet<>();
+
+    /**
+     * Called to set the initial value of the ENV var using an existing value, typically configured in the
+     * "env" section of the CR. This will be a value set by the app deployer, with runtime options they want
+     * to have enabled.
+     */
+    public void setOptsFromEnvVar(String value) {
+        opts.clear();
+        if (value != null && !value.trim().isEmpty()) {
+            opts.addAll(Set.of(value.split(" ")));
+        }
+    }
+
+    /**
+     * Converts this object into an {@link EnvVar} instance.
+     */
+    public EnvVar toEnvVar() {
+        EnvVarBuilder builder = new EnvVarBuilder();
+        return builder.withName(EnvironmentVariables.JAVA_OPTS_APPEND).withValue(String.join(" ", opts))
+                .build();
+    }
+
+    /**
+     * Add another option. If the option already exists, do nothing. Whether the option already exists may
+     * depend on the option. Custom logic may be needed to properly support certain options.
+     */
+    public void addOpt(String optValue) {
+        if (!containsOpt(optValue)) {
+            opts.add(optValue);
+        }
+    }
+
+    /**
+     * Returns 'true' if there are no options set.
+     */
+    public boolean isEmpty() {
+        return opts.isEmpty();
+    }
+
+    /**
+     * Returns 'true' if the option already exists. The logic for whether an option already exists depends on
+     * the option.
+     */
+    public boolean containsOpt(String optValue) {
+        if (optValue == null || optValue.trim().isEmpty()) {
+            return false;
+        }
+        if (optValue.startsWith("-D") || optValue.startsWith("-XX:")) {
+            return containsValuedParameter(optValue);
+        }
+        if (optValue.startsWith("-Xms")) {
+            return containsXms();
+        }
+        if (optValue.startsWith("-Xmx")) {
+            return containsXmx();
+        }
+        if (optValue.startsWith("-javaagent:")) {
+            return containsJavaAgent();
+        }
+        if (optValue.startsWith("-agentlib:")) {
+            return containsAgentLib();
+        }
+        return opts.contains(optValue);
+    }
+
+    /**
+     * Returns 'true' if an option with the given prefix already exists.
+     */
+    public boolean containsOptByPrefix(String prefix) {
+        return this.opts.stream().anyMatch(s -> s.startsWith(prefix));
+    }
+
+    /**
+     * Checks if an option of one of the following form exists already:
+     * <ul>
+     * <li>-Dmy.property.name=foo</li>
+     * <li>-XX:OptionName=bar</li>
+     * </ul>
+     * Looks for another option with the same name but potentially different value.
+     */
+    private boolean containsValuedParameter(String optValue) {
+        if (optValue.contains("=")) {
+            String prefix = optValue.substring(0, optValue.indexOf("="));
+            return containsOptByPrefix(prefix);
+        }
+        return opts.contains(optValue);
+    }
+
+    /**
+     * Checks for an option of the form "-Xms512m".
+     */
+    private boolean containsXms() {
+        return containsOptByPrefix("-Xms");
+    }
+
+    /**
+     * Checks for an option of the form "-Xmx1024m".
+     */
+    private boolean containsXmx() {
+        return containsOptByPrefix("-Xmx");
+    }
+
+    /**
+     * Checks for an option of the form "-javaagent:/path/to/agent.jar"
+     */
+    private boolean containsJavaAgent() {
+        return containsOptByPrefix("-javaagent:");
+    }
+
+    /**
+     * Checks for an option of the form "-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:5005"
+     */
+    private boolean containsAgentLib() {
+        return containsOptByPrefix("-agentlib:");
+    }
+
+}

operator/controller/src/test/java/io/apicurio/registry/operator/it/ITBase.java

@@ -118,46 +118,46 @@ public void beforeEach(TestInfo testInfo) {
     }
 
     protected static void checkDeploymentExists(ApicurioRegistry3 primary, String component, int replicas) {
-        await().ignoreExceptions().untilAsserted(() -> {
+        await().atMost(Duration.ofSeconds(30)).ignoreExceptions().untilAsserted(() -> {
             assertThat(client.apps().deployments()
                     .withName(primary.getMetadata().getName() + "-" + component + "-deployment").get()
                     .getStatus().getReadyReplicas()).isEqualTo(replicas);
         });
     }
 
     protected static void checkDeploymentDoesNotExist(ApicurioRegistry3 primary, String component) {
-        await().ignoreExceptions().untilAsserted(() -> {
+        await().atMost(Duration.ofSeconds(30)).ignoreExceptions().untilAsserted(() -> {
             assertThat(client.apps().deployments()
                     .withName(primary.getMetadata().getName() + "-" + component + "-deployment").get())
                     .isNull();
         });
     }
 
     protected static void checkServiceExists(ApicurioRegistry3 primary, String component) {
-        await().ignoreExceptions().untilAsserted(() -> {
+        await().atMost(Duration.ofSeconds(30)).ignoreExceptions().untilAsserted(() -> {
             assertThat(client.services()
                     .withName(primary.getMetadata().getName() + "-" + component + "-service").get())
                     .isNotNull();
         });
     }
 
     protected static void checkServiceDoesNotExist(ApicurioRegistry3 primary, String component) {
-        await().ignoreExceptions().untilAsserted(() -> {
+        await().atMost(Duration.ofSeconds(30)).ignoreExceptions().untilAsserted(() -> {
             assertThat(client.services()
                     .withName(primary.getMetadata().getName() + "-" + component + "-service").get()).isNull();
         });
     }
 
     protected static void checkIngressExists(ApicurioRegistry3 primary, String component) {
-        await().ignoreExceptions().untilAsserted(() -> {
+        await().atMost(Duration.ofSeconds(30)).ignoreExceptions().untilAsserted(() -> {
             assertThat(client.network().v1().ingresses()
                     .withName(primary.getMetadata().getName() + "-" + component + "-ingress").get())
                     .isNotNull();
         });
     }
 
     protected static void checkIngressDoesNotExist(ApicurioRegistry3 primary, String component) {
-        await().ignoreExceptions().untilAsserted(() -> {
+        await().atMost(Duration.ofSeconds(30)).ignoreExceptions().untilAsserted(() -> {
             assertThat(client.network().v1().ingresses()
                     .withName(primary.getMetadata().getName() + "-" + component + "-ingress").get()).isNull();
         });
@@ -167,7 +167,7 @@ protected static PodDisruptionBudget checkPodDisruptionBudgetExists(ApicurioRegi
             String component) {
         final ValueOrNull<PodDisruptionBudget> rval = new ValueOrNull<>();
 
-        await().ignoreExceptions().untilAsserted(() -> {
+        await().atMost(Duration.ofSeconds(30)).ignoreExceptions().untilAsserted(() -> {
             PodDisruptionBudget pdb = client.policy().v1().podDisruptionBudget()
                     .withName(primary.getMetadata().getName() + "-" + component + "-poddisruptionbudget")
                     .get();
@@ -181,7 +181,7 @@ protected static PodDisruptionBudget checkPodDisruptionBudgetExists(ApicurioRegi
     protected static NetworkPolicy checkNetworkPolicyExists(ApicurioRegistry3 primary, String component) {
         final ValueOrNull<NetworkPolicy> rval = new ValueOrNull<>();
 
-        await().ignoreExceptions().untilAsserted(() -> {
+        await().atMost(Duration.ofSeconds(30)).ignoreExceptions().untilAsserted(() -> {
             NetworkPolicy networkPolicy = client.network().v1().networkPolicies()
                     .withName(primary.getMetadata().getName() + "-" + component + "-networkpolicy").get();
             assertThat(networkPolicy).isNotNull();
@@ -220,7 +220,7 @@ private static void createTestResources() throws Exception {
 
     private static void startOperatorLogs() {
         List<Pod> operatorPods = new ArrayList<>();
-        await().ignoreExceptions().untilAsserted(() -> {
+        await().atMost(Duration.ofSeconds(30)).ignoreExceptions().untilAsserted(() -> {
             operatorPods.clear();
             operatorPods.addAll(client.pods()
                     .withLabels(Map.of(

operator/controller/src/test/java/io/apicurio/registry/operator/unit/JavaOptsAppendTest.java

@@ -0,0 +1,107 @@
+package io.apicurio.registry.operator.unit;
+
+import io.apicurio.registry.operator.EnvironmentVariables;
+import io.apicurio.registry.operator.utils.JavaOptsAppend;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class JavaOptsAppendTest {
+
+    @Test
+    public void testJavaOpts_Empty() throws Exception {
+        JavaOptsAppend optsAppend = new JavaOptsAppend();
+        Assertions.assertThat(optsAppend.isEmpty()).isTrue();
+        Assertions.assertThat(optsAppend.toEnvVar()).isNotNull();
+        Assertions.assertThat(optsAppend.toEnvVar().getName())
+                .isEqualTo(EnvironmentVariables.JAVA_OPTS_APPEND);
+        Assertions.assertThat(optsAppend.toEnvVar().getValue()).isEqualTo("");
+    }
+
+    @Test
+    public void testJavaOpts_EmptyEnvVar() throws Exception {
+        JavaOptsAppend optsAppend = new JavaOptsAppend();
+        optsAppend.setOptsFromEnvVar("");
+        Assertions.assertThat(optsAppend.isEmpty()).isTrue();
+        Assertions.assertThat(optsAppend.toEnvVar()).isNotNull();
+        Assertions.assertThat(optsAppend.toEnvVar().getName())
+                .isEqualTo(EnvironmentVariables.JAVA_OPTS_APPEND);
+        Assertions.assertThat(optsAppend.toEnvVar().getValue()).isEqualTo("");
+    }
+
+    @Test
+    public void testJavaOpts_EnvVar() throws Exception {
+        JavaOptsAppend optsAppend = new JavaOptsAppend();
+        optsAppend.setOptsFromEnvVar("-Xms512m -Xmx1g -XX:+UseG1GC -Dspring.profiles.active=prod");
+        Assertions.assertThat(optsAppend.isEmpty()).isFalse();
+        Assertions.assertThat(optsAppend.toEnvVar()).isNotNull();
+        Assertions.assertThat(optsAppend.toEnvVar().getName())
+                .isEqualTo(EnvironmentVariables.JAVA_OPTS_APPEND);
+        Assertions.assertThat(optsAppend.toEnvVar().getValue())
+                .isEqualTo("-Dspring.profiles.active=prod -XX:+UseG1GC -Xms512m -Xmx1g");
+    }
+
+    @Test
+    public void testJavaOpts_EnvVarWithAdds() throws Exception {
+        JavaOptsAppend optsAppend = new JavaOptsAppend();
+        optsAppend.setOptsFromEnvVar("-Xms512m -Xmx1g");
+        optsAppend.addOpt("-XX:+UseG1GC");
+        optsAppend.addOpt("-Dspring.profiles.active=prod");
+        Assertions.assertThat(optsAppend.isEmpty()).isFalse();
+        Assertions.assertThat(optsAppend.toEnvVar()).isNotNull();
+        Assertions.assertThat(optsAppend.toEnvVar().getName())
+                .isEqualTo(EnvironmentVariables.JAVA_OPTS_APPEND);
+        Assertions.assertThat(optsAppend.toEnvVar().getValue())
+                .isEqualTo("-Dspring.profiles.active=prod -XX:+UseG1GC -Xms512m -Xmx1g");
+    }
+
+    @Test
+    public void testJavaOpts_ParamConflict() throws Exception {
+        JavaOptsAppend optsAppend = new JavaOptsAppend();
+        optsAppend.addOpt("-Dspring.profiles.active=prod");
+        optsAppend.addOpt("-Dspring.profiles.active=dev");
+        Assertions.assertThat(optsAppend.toEnvVar().getValue()).isEqualTo("-Dspring.profiles.active=prod");
+
+        optsAppend = new JavaOptsAppend();
+        optsAppend.addOpt("-XX:ReservedCodeCacheSize=128m");
+        optsAppend.addOpt("-XX:ReservedCodeCacheSize=256m");
+        Assertions.assertThat(optsAppend.toEnvVar().getValue()).isEqualTo("-XX:ReservedCodeCacheSize=128m");
+
+        optsAppend = new JavaOptsAppend();
+        optsAppend.addOpt("-XX:ReservedCodeCacheSize=128m");
+        optsAppend.addOpt("-XX:ReservedCodeCacheSize=256m");
+        optsAppend.addOpt("-XX:+HeapDumpOnOutOfMemoryError");
+        optsAppend.addOpt("-XX:+HeapDumpOnOutOfMemoryError");
+        optsAppend.addOpt("-XX:HeapDumpPath=/path/to/dumps/");
+        optsAppend.addOpt("-XX:HeapDumpPath=/path/to/other-dumps/");
+        Assertions.assertThat(optsAppend.toEnvVar().getValue()).isEqualTo(
+                "-XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=/path/to/dumps/ -XX:ReservedCodeCacheSize=128m");
+
+        optsAppend = new JavaOptsAppend();
+        optsAppend.setOptsFromEnvVar("-XX:ReservedCodeCacheSize=128m -XX:+HeapDumpOnOutOfMemoryError");
+        optsAppend.addOpt("-XX:ReservedCodeCacheSize=256m");
+        optsAppend.addOpt("-XX:+HeapDumpOnOutOfMemoryError");
+        optsAppend.addOpt("-XX:HeapDumpPath=/path/to/dumps/");
+        optsAppend.addOpt("-XX:HeapDumpPath=/path/to/other-dumps/");
+        Assertions.assertThat(optsAppend.toEnvVar().getValue()).isEqualTo(
+                "-XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=/path/to/dumps/ -XX:ReservedCodeCacheSize=128m");
+    }
+
+    @Test
+    public void testJavaOpts_Conflicts() throws Exception {
+        JavaOptsAppend optsAppend = new JavaOptsAppend();
+        optsAppend.setOptsFromEnvVar(
+                "-Xms512m -Xmx1g -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:5005");
+        optsAppend.addOpt("-Xms256m");
+        optsAppend.addOpt("-Xmx2g");
+        optsAppend.addOpt("-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:12345");
+        optsAppend.addOpt("-javaagent:/path/to/agent.jar");
+        optsAppend.addOpt("-javaagent:/path/to/alt_agent.jar");
+        Assertions.assertThat(optsAppend.isEmpty()).isFalse();
+        Assertions.assertThat(optsAppend.toEnvVar()).isNotNull();
+        Assertions.assertThat(optsAppend.toEnvVar().getName())
+                .isEqualTo(EnvironmentVariables.JAVA_OPTS_APPEND);
+        Assertions.assertThat(optsAppend.toEnvVar().getValue()).isEqualTo(
+                "-Xms512m -Xmx1g -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:5005 -javaagent:/path/to/agent.jar");
+    }
+
+}