diff --git a/docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc b/docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc index 0294a5fbe66..172f9e2f3d3 100644 --- a/docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc +++ b/docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc @@ -72,3 +72,9 @@ New code example: ---- + += 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. diff --git a/docs/guides/operator/basic-deployment.adoc b/docs/guides/operator/basic-deployment.adoc index fc17d981af0..205f18d6eea 100644 --- a/docs/guides/operator/basic-deployment.adoc +++ b/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. diff --git a/operator/pom.xml b/operator/pom.xml index 8bcac51b386..6be165360c9 100644 --- a/operator/pom.xml +++ b/operator/pom.xml @@ -142,6 +142,11 @@ rest-assured test + + org.mockito + mockito-core + test + diff --git a/operator/src/main/java/org/keycloak/operator/controllers/KeycloakController.java b/operator/src/main/java/org/keycloak/operator/controllers/KeycloakController.java index 04dc358596a..6f57a5c66b7 100644 --- a/operator/src/main/java/org/keycloak/operator/controllers/KeycloakController.java +++ b/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, EventSourceInitializer, ErrorStatusHandler { + public static final String OPENSHIFT_DEFAULT = "openshift-default"; + @Inject KubernetesClient client; @@ -101,16 +107,31 @@ public class KeycloakController implements Reconciler, 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, EventSourceInit return ErrorStatusUpdateControl.updateStatus(kc); } + + public static Optional generateOpenshiftHostname(Keycloak keycloak, Context context) { + return getAppsDomain(context).map(s -> KubernetesResourceUtil.sanitizeName(String.format("%s-%s", + KeycloakIngressDependentResource.getName(keycloak), keycloak.getMetadata().getNamespace())) + "." + s); + } + + public static Optional getAppsDomain(Context 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())); + } } diff --git a/operator/src/main/java/org/keycloak/operator/controllers/WatchedSecretsController.java b/operator/src/main/java/org/keycloak/operator/controllers/WatchedSecretsController.java index c46c301a60f..3f56c78668e 100644 --- a/operator/src/main/java/org/keycloak/operator/controllers/WatchedSecretsController.java +++ b/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, EventSourceInitializer, WatchedSecrets { - @Inject - KubernetesClient client; + private volatile KubernetesClient client; private final SimpleInboundEventSource eventSource = new SimpleInboundEventSource(); @@ -65,6 +63,7 @@ public class WatchedSecretsController implements Reconciler, EventSource @Override public Map prepareEventSources(EventSourceContext context) { this.secrets = context.getPrimaryCache(); + this.client = context.getClient(); return Map.of(); } diff --git a/operator/src/main/kubernetes/kubernetes.yml b/operator/src/main/kubernetes/kubernetes.yml index bc111585273..c1d22592341 100644 --- a/operator/src/main/kubernetes/kubernetes.yml +++ b/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 diff --git a/operator/src/test/java/org/keycloak/operator/testsuite/integration/BaseOperatorTest.java b/operator/src/test/java/org/keycloak/operator/testsuite/integration/BaseOperatorTest.java index 92cf5872af2..795fb71bfbb 100644 --- a/operator/src/test/java/org/keycloak/operator/testsuite/integration/BaseOperatorTest.java +++ b/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 - .apps() - .statefulSets() - .inNamespace(namespace) - .withName("postgresql-db") - .get()).isNull(); - }); + Log.infof("Waiting for postgres to be deleted"); + k8sclient.apps().statefulSets().inNamespace(namespace).withName(POSTGRESQL_NAME).withTimeout(2, TimeUnit.MINUTES).delete(); } // 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(); } diff --git a/operator/src/test/java/org/keycloak/operator/testsuite/integration/KeycloakIngressTest.java b/operator/src/test/java/org/keycloak/operator/testsuite/integration/KeycloakIngressTest.java index 9e1575254fc..94d35904e01 100644 --- a/operator/src/test/java/org/keycloak/operator/testsuite/integration/KeycloakIngressTest.java +++ b/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; - // 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); + kc.getSpec().setIngressSpec(new IngressSpecBuilder().withIngressClassName(KeycloakController.OPENSHIFT_DEFAULT).build()); + // 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; - hostnameSpecBuilder.withHostname(testHostname); - } - else { - testHostname = kubernetesIp; + kc.getSpec().setIngressSpec(new IngressSpecBuilder().withIngressClassName(KeycloakController.OPENSHIFT_DEFAULT).build()); } 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"); } diff --git a/operator/src/test/java/org/keycloak/operator/testsuite/unit/KeycloakControllerTest.java b/operator/src/test/java/org/keycloak/operator/testsuite/unit/KeycloakControllerTest.java new file mode 100644 index 00000000000..3fb51fa43fe --- /dev/null +++ b/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 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 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()); + } + +}