adds a default domain on openshift if one is not specified (#23324)

Closes #21741

docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc

@@ -72,3 +72,9 @@ New code example:
     </button>
 </div>
 ----
+
+= Default Keycloak CR Hostname
+
+When running on OpenShift, with ingress enabled, and with the spec.ingress.classname set to openshift-default, you may leave the spec.hostname.hostname unpopulated in the Keycloak CR.
+The operator will assign a default hostname to the stored version of the CR similar to what would be created by an OpenShift Route without an explicit host - that is ingress-namespace.appsDomain
+If the appsDomain changes, or should you need a different hostname for any reason, then update the Keycloak CR.

docs/guides/operator/basic-deployment.adoc

@@ -89,6 +89,10 @@ See <@links.server id="hostname"/> for the available configurations.
 
 For development purposes, this {section} will use `test.keycloak.org`.
 
+When running on OpenShift, with ingress enabled, and with the spec.ingress.classname set to openshift-default, you may leave the spec.hostname.hostname unpopulated in the Keycloak CR.
+The operator will assign a default hostname to the stored version of the CR similar to what would be created by an OpenShift Route without an explicit host - that is ingress-namespace.appsDomain
+If the appsDomain changes, or should you need a different hostname for any reason, then update the Keycloak CR.
+
 ==== TLS Certificate and key
 
 See your Certification Authority to obtain the certificate and the key.

operator/pom.xml

@@ -142,6 +142,11 @@
             <artifactId>rest-assured</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

operator/src/main/java/org/keycloak/operator/controllers/KeycloakController.java

@@ -19,6 +19,7 @@ package org.keycloak.operator.controllers;
 import io.fabric8.kubernetes.api.model.Service;
 import io.fabric8.kubernetes.api.model.apps.StatefulSet;
 import io.fabric8.kubernetes.client.KubernetesClient;
+import io.fabric8.kubernetes.client.utils.KubernetesResourceUtil;
 import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
@@ -39,9 +40,12 @@ import org.keycloak.operator.Constants;
 import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
 import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatus;
 import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusAggregator;
+import org.keycloak.operator.crds.v2alpha1.deployment.spec.HostnameSpec;
+import org.keycloak.operator.crds.v2alpha1.deployment.spec.HostnameSpecBuilder;
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 
 import jakarta.inject.Inject;
@@ -55,6 +59,8 @@ import jakarta.inject.Inject;
     })
 public class KeycloakController implements Reconciler<Keycloak>, EventSourceInitializer<Keycloak>, ErrorStatusHandler<Keycloak> {
 
+    public static final String OPENSHIFT_DEFAULT = "openshift-default";
+
     @Inject
     KubernetesClient client;
 
@@ -101,16 +107,31 @@ public class KeycloakController implements Reconciler<Keycloak>, EventSourceInit
 
         Log.infof("--- Reconciling Keycloak: %s in namespace: %s", kcName, namespace);
 
+        boolean modifiedSpec = false;
         if (kc.getSpec().getInstances() == null) {
             // explicitly set defaults - and let another reconciliation happen
             // this avoids ensuring unintentional modifications have not been made to the cr
             kc.getSpec().setInstances(1);
+            modifiedSpec = true;
+        }
+        if (kc.getSpec().getIngressSpec() != null && kc.getSpec().getIngressSpec().isIngressEnabled()
+                && OPENSHIFT_DEFAULT.equals(kc.getSpec().getIngressSpec().getIngressClassName())
+                && Optional.ofNullable(kc.getSpec().getHostnameSpec()).map(HostnameSpec::getHostname).isEmpty()) {
+            var optionalHostname = generateOpenshiftHostname(kc, context);
+            if (optionalHostname.isPresent()) {
+                kc.getSpec().setHostnameSpec(new HostnameSpecBuilder(kc.getSpec().getHostnameSpec())
+                        .withHostname(optionalHostname.get()).build());
+                modifiedSpec = true;
+            }
+        }
+
+        if (modifiedSpec) {
             return UpdateControl.updateResource(kc);
         }
 
         var statusAggregator = new KeycloakStatusAggregator(kc.getStatus(), kc.getMetadata().getGeneration());
 
-        var kcDeployment = new KeycloakDeployment(client, config, kc, context.getSecondaryResource(StatefulSet.class).orElse(null), KeycloakAdminSecretDependentResource.getName(kc));
+        var kcDeployment = new KeycloakDeployment(context.getClient(), config, kc, context.getSecondaryResource(StatefulSet.class).orElse(null), KeycloakAdminSecretDependentResource.getName(kc));
         kcDeployment.setWatchedSecrets(watchedSecrets);
         kcDeployment.createOrUpdateReconciled();
         kcDeployment.updateStatus(statusAggregator);
@@ -148,4 +169,16 @@ public class KeycloakController implements Reconciler<Keycloak>, EventSourceInit
 
         return ErrorStatusUpdateControl.updateStatus(kc);
     }
+
+    public static Optional<String> generateOpenshiftHostname(Keycloak keycloak, Context<Keycloak> context) {
+        return getAppsDomain(context).map(s -> KubernetesResourceUtil.sanitizeName(String.format("%s-%s",
+                KeycloakIngressDependentResource.getName(keycloak), keycloak.getMetadata().getNamespace())) + "." + s);
+    }
+
+    public static Optional<String> getAppsDomain(Context<Keycloak> context) {
+        return Optional
+                .ofNullable(context.getClient().resources(io.fabric8.openshift.api.model.config.v1.Ingress.class)
+                        .withName("cluster").get())
+                .map(i -> Optional.ofNullable(i.getSpec().getAppsDomain()).orElse(i.getSpec().getDomain()));
+    }
 }

operator/src/main/java/org/keycloak/operator/controllers/WatchedSecretsController.java

@@ -49,14 +49,12 @@ import java.util.Optional;
 import java.util.stream.Collectors;
 
 import jakarta.enterprise.context.ApplicationScoped;
-import jakarta.inject.Inject;
 
 @ApplicationScoped
 @ControllerConfiguration(labelSelector = Constants.KEYCLOAK_COMPONENT_LABEL + "=" + WatchedSecrets.WATCHED_SECRETS_LABEL_VALUE)
 public class WatchedSecretsController implements Reconciler<Secret>, EventSourceInitializer<Secret>, WatchedSecrets {
 
-    @Inject
+    private volatile KubernetesClient client;
-    KubernetesClient client;
 
     private final SimpleInboundEventSource eventSource = new SimpleInboundEventSource();
 
@@ -65,6 +63,7 @@ public class WatchedSecretsController implements Reconciler<Secret>, EventSource
     @Override
     public Map<String, EventSource> prepareEventSources(EventSourceContext<Secret> context) {
         this.secrets = context.getPrimaryCache();
+        this.client = context.getClient();
         return Map.of();
     }
 

operator/src/main/kubernetes/kubernetes.yml

@@ -5,9 +5,6 @@ metadata:
 rules:
   - apiGroups:
       - apps
-      # Extensions enabled for backward compatibility:
-      # https://github.com/fabric8io/kubernetes-client/issues/3996
-      - extensions
     resources:
       - statefulsets
     verbs:
@@ -61,6 +58,12 @@ rules:
       - delete
       - patch
       - update
+  - apiGroups:
+      - config.openshift.io
+    resources:
+      - ingresses
+    verbs:
+      - get
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding

operator/src/test/java/org/keycloak/operator/testsuite/integration/BaseOperatorTest.java

@@ -85,6 +85,7 @@ public class BaseOperatorTest implements QuarkusTestAfterEachCallback {
   public static final String TARGET_KUBERNETES_GENERATED_YML_FOLDER = "target/kubernetes/";
   public static final String OPERATOR_KUBERNETES_IP = "test.operator.kubernetes.ip";
   public static final String OPERATOR_CUSTOM_IMAGE = "test.operator.custom.image";
+  public static final String POSTGRESQL_NAME = "postgresql-db";
 
   public static final String TEST_RESULTS_DIR = "target/operator-test-results/";
   public static final String POD_LOGS_DIR = TEST_RESULTS_DIR + "pod-logs/";
@@ -205,7 +206,7 @@ public class BaseOperatorTest implements QuarkusTestAfterEachCallback {
     // Check DB has deployed and ready
     Log.info("Checking Postgres is running");
     Awaitility.await()
-            .untilAsserted(() -> assertThat(k8sclient.apps().statefulSets().inNamespace(namespace).withName("postgresql-db").get().getStatus().getReadyReplicas()).isEqualTo(1));
+            .untilAsserted(() -> assertThat(k8sclient.apps().statefulSets().inNamespace(namespace).withName(POSTGRESQL_NAME).get().getStatus().getReadyReplicas()).isEqualTo(1));
   }
 
   protected static void deployDBSecret() {
@@ -214,18 +215,8 @@ public class BaseOperatorTest implements QuarkusTestAfterEachCallback {
 
   protected static void deleteDB() {
     // Delete the Postgres StatefulSet
-    k8sclient.apps().statefulSets().inNamespace(namespace).withName("postgresql-db").delete();
-    Awaitility.await()
-            .ignoreExceptions()
-            .untilAsserted(() -> {
     Log.infof("Waiting for postgres to be deleted");
-              assertThat(k8sclient
+    k8sclient.apps().statefulSets().inNamespace(namespace).withName(POSTGRESQL_NAME).withTimeout(2, TimeUnit.MINUTES).delete();
-                      .apps()
-                      .statefulSets()
-                      .inNamespace(namespace)
-                      .withName("postgresql-db")
-                      .get()).isNull();
-            });
   }
 
   // TODO improve this (preferably move to JOSDK)
@@ -284,7 +275,7 @@ public class BaseOperatorTest implements QuarkusTestAfterEachCallback {
           if (operatorDeployment == OperatorDeployment.remote) {
               logFailed(k8sclient.apps().deployments().withName("keycloak-operator"), Deployment::getStatus);
           }
-          logFailed(k8sclient.apps().statefulSets().withName("example-kc"), StatefulSet::getStatus);
+          logFailed(k8sclient.apps().statefulSets().withName(POSTGRESQL_NAME), StatefulSet::getStatus);
       } finally {
           cleanup();
       }
@@ -334,7 +325,7 @@ public class BaseOperatorTest implements QuarkusTestAfterEachCallback {
     }
 
     Log.info("Deleting namespace : " + namespace);
-    assertThat(k8sclient.namespaces().withName(namespace).delete()).isNotNull();
+    assertThat(k8sclient.namespaces().withName(namespace).delete()).isNotEmpty();
     k8sclient.close();
   }
 

operator/src/test/java/org/keycloak/operator/testsuite/integration/KeycloakIngressTest.java

@@ -26,9 +26,9 @@ import io.quarkus.test.junit.QuarkusTest;
 import io.restassured.RestAssured;
 
 import org.awaitility.Awaitility;
-import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.keycloak.operator.Constants;
+import org.keycloak.operator.controllers.KeycloakController;
 import org.keycloak.operator.controllers.KeycloakIngressDependentResource;
 import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
 import org.keycloak.operator.crds.v2alpha1.deployment.spec.HostnameSpecBuilder;
@@ -46,22 +46,6 @@ import static org.junit.jupiter.api.Assertions.assertFalse;
 
 @QuarkusTest
 public class KeycloakIngressTest extends BaseOperatorTest {
-    private static String baseDomain;
-
-    @BeforeAll
-    public static void beforeKeycloakIngressTest() {
-        if (isOpenShift) {
-            Log.info("OpenShift detected, using real domain");
-            // see https://docs.openshift.com/container-platform/4.12/networking/ingress-operator.html#configuring-ingress
-            baseDomain = k8sclient.genericKubernetesResources("config.openshift.io/v1", "Ingress")
-                    .withName("cluster")
-                    .get()
-                    .get("spec", "domain");
-            if (baseDomain == null || baseDomain.isBlank()) {
-                throw new IllegalStateException("Couldn't fetch the base Ingress domain");
-            }
-        }
-    }
 
     @Test
     public void testIngressOnHTTP() {
@@ -71,13 +55,9 @@ public class KeycloakIngressTest extends BaseOperatorTest {
         var hostnameSpecBuilder = new HostnameSpecBuilder()
                 .withStrict(false)
                 .withStrictBackchannel(false);
-        String testHostname;
-        String baseUrl;
         if (isOpenShift) {
-            testHostname = "kc-http-" + namespace + "." + baseDomain;
+            kc.getSpec().setIngressSpec(new IngressSpecBuilder().withIngressClassName(KeycloakController.OPENSHIFT_DEFAULT).build());
-            // on OpenShift, when Keycloak is configured for HTTP only, we use edge TLS termination, i.e. Route still uses TLS
+
-            baseUrl = "https://" + testHostname + ":443";
-            hostnameSpecBuilder.withHostname(testHostname);
             // see https://github.com/keycloak/keycloak/issues/14400#issuecomment-1659900081
             kc.getSpec().setUnsupported(new UnsupportedSpecBuilder()
                     .withNewPodTemplate()
@@ -92,13 +72,21 @@ public class KeycloakIngressTest extends BaseOperatorTest {
                     .endPodTemplate()
                     .build());
         }
-        else {
+
-            baseUrl = "http://" + kubernetesIp + ":80";
-        }
         kc.getSpec().setHostnameSpec(hostnameSpecBuilder.build());
 
         K8sUtils.deployKeycloak(k8sclient, kc, true);
 
+        String baseUrl;
+
+        if (isOpenShift) {
+            String testHostname = k8sclient.resource(kc).get().getSpec().getHostnameSpec().getHostname();
+            // on OpenShift, when Keycloak is configured for HTTP only, we use edge TLS termination, i.e. Route still uses TLS
+            baseUrl = "https://" + testHostname + ":443";
+        } else {
+            baseUrl = "http://" + kubernetesIp + ":80";
+        }
+
         testIngressURLs(baseUrl);
     }
 
@@ -108,18 +96,20 @@ public class KeycloakIngressTest extends BaseOperatorTest {
         var hostnameSpecBuilder = new HostnameSpecBuilder()
                 .withStrict(false)
                 .withStrictBackchannel(false);
-        String testHostname;
         if (isOpenShift) {
-            testHostname = "kc-https-" + namespace + "." + baseDomain;
+            kc.getSpec().setIngressSpec(new IngressSpecBuilder().withIngressClassName(KeycloakController.OPENSHIFT_DEFAULT).build());
-            hostnameSpecBuilder.withHostname(testHostname);
-        }
-        else {
-            testHostname = kubernetesIp;
         }
         kc.getSpec().setHostnameSpec(hostnameSpecBuilder.build());
 
         K8sUtils.deployKeycloak(k8sclient, kc, true);
 
+        String testHostname;
+        if (isOpenShift) {
+            testHostname = k8sclient.resource(kc).get().getSpec().getHostnameSpec().getHostname();
+        } else {
+            testHostname = kubernetesIp;
+        }
+
         testIngressURLs("https://" + testHostname + ":443");
     }
 

operator/src/test/java/org/keycloak/operator/testsuite/unit/KeycloakControllerTest.java

@@ -0,0 +1,71 @@
+/*
+ * Copyright 2022 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.operator.testsuite.unit;
+
+import io.fabric8.kubernetes.client.dsl.Resource;
+import io.fabric8.openshift.api.model.config.v1.Ingress;
+import io.fabric8.openshift.api.model.config.v1.IngressBuilder;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
+
+import org.junit.jupiter.api.Test;
+import org.keycloak.operator.controllers.KeycloakController;
+import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
+import org.keycloak.operator.crds.v2alpha1.deployment.spec.IngressSpecBuilder;
+import org.keycloak.operator.testsuite.utils.K8sUtils;
+import org.mockito.Mockito;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class KeycloakControllerTest {
+
+    @Test
+    void testCRDefaults() {
+        KeycloakController controller = new KeycloakController();
+        Keycloak kc = K8sUtils.getDefaultKeycloakDeployment();
+        kc.getSpec().setInstances(null);
+        kc.getSpec().getHostnameSpec().setHostname(null);
+        kc.getSpec().setIngressSpec(new IngressSpecBuilder().withIngressClassName(KeycloakController.OPENSHIFT_DEFAULT).build());
+        kc.getMetadata().setNamespace("ns");
+        Context<Keycloak> mockContext = Mockito.mock(Context.class, Mockito.RETURNS_DEEP_STUBS);
+        var ingressConfig = new IngressBuilder().withNewSpec().withDomain("openshift.com").endSpec().build();
+        var mockResource = Mockito.mock(Resource.class);
+        Mockito.when(mockResource.get()).thenReturn(ingressConfig);
+        Mockito.when(mockContext.getClient().resources(Ingress.class).withName("cluster")).thenReturn(mockResource);
+
+        // both the instances and hostname should be updated
+        UpdateControl<Keycloak> update = controller.reconcile(kc, mockContext);
+
+        assertTrue(update.isUpdateResource());
+        assertEquals(1, update.getResource().getSpec().getInstances());
+        assertEquals("example-kc-ingress-ns.openshift.com", update.getResource().getSpec().getHostnameSpec().getHostname());
+
+        // just the instances should be updated if not openshift-default
+        kc = K8sUtils.getDefaultKeycloakDeployment();
+        kc.getSpec().setIngressSpec(null);
+        kc.getSpec().setInstances(null);
+        kc.getSpec().getHostnameSpec().setHostname(null);
+        update = controller.reconcile(kc, mockContext);
+        assertTrue(update.isUpdateResource());
+        assertEquals(1, update.getResource().getSpec().getInstances());
+        assertNull(update.getResource().getSpec().getHostnameSpec().getHostname());
+    }
+
+}