From 24acc4c7d1baf8a6dd9bc70757a35a7406f132b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Barto=C5=A1?= Date: Wed, 19 Oct 2022 12:10:04 +0200 Subject: [PATCH] Add hostname options to Keycloak CR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #14395 Co-authored-by: Václav Muzikář --- .../controllers/KeycloakDistConfigurator.java | 31 +-- .../operator/controllers/KeycloakIngress.java | 5 +- .../v2alpha1/deployment/KeycloakSpec.java | 32 +-- .../deployment/spec/HostnameSpec.java | 82 +++++++ .../src/main/resources/example-keycloak.yaml | 5 +- .../integration/KeycloakDeploymentTest.java | 30 ++- .../integration/KeycloakIngressTest.java | 18 +- .../integration/WatchedSecretsTest.java | 7 +- .../testsuite/unit/CRSerializationTest.java | 30 ++- .../unit/KeycloakDistConfiguratorTest.java | 222 ++++++++++-------- .../testsuite/unit/PodTemplateTest.java | 19 +- .../correct-podtemplate-keycloak.yml | 3 +- .../resources/empty-podtemplate-keycloak.yml | 3 +- ...ialization-keycloak-cr-with-empty-list.yml | 1 - .../test-serialization-keycloak-cr.yml | 7 +- 15 files changed, 321 insertions(+), 174 deletions(-) create mode 100644 operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/spec/HostnameSpec.java diff --git a/operator/src/main/java/org/keycloak/operator/controllers/KeycloakDistConfigurator.java b/operator/src/main/java/org/keycloak/operator/controllers/KeycloakDistConfigurator.java index bfbfefe4ae..b309182146 100644 --- a/operator/src/main/java/org/keycloak/operator/controllers/KeycloakDistConfigurator.java +++ b/operator/src/main/java/org/keycloak/operator/controllers/KeycloakDistConfigurator.java @@ -33,6 +33,7 @@ import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusBuilder; import org.keycloak.operator.crds.v2alpha1.deployment.ValueOrSecret; import org.keycloak.operator.crds.v2alpha1.deployment.spec.DatabaseSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.FeatureSpec; +import org.keycloak.operator.crds.v2alpha1.deployment.spec.HostnameSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.HttpSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.TransactionsSpec; @@ -91,30 +92,12 @@ public class KeycloakDistConfigurator { /* ---------- Configuration of first-class citizen fields ---------- */ public void configureHostname() { - var kcContainer = deployment.getSpec().getTemplate().getSpec().getContainers().get(0); - var hostname = keycloakCR.getSpec().getHostname(); - var envVars = kcContainer.getEnv(); - if (keycloakCR.getSpec().isHostnameDisabled()) { - var disableStrictHostname = List.of( - new EnvVarBuilder() - .withName("KC_HOSTNAME_STRICT") - .withValue("false") - .build(), - new EnvVarBuilder() - .withName("KC_HOSTNAME_STRICT_BACKCHANNEL") - .withValue("false") - .build()); - - envVars.addAll(disableStrictHostname); - } else { - var enabledStrictHostname = List.of( - new EnvVarBuilder() - .withName("KC_HOSTNAME") - .withValue(hostname) - .build()); - - envVars.addAll(enabledStrictHostname); - } + optionMapper(keycloakCR.getSpec().getHostnameSpec()) + .mapOption("hostname", HostnameSpec::getHostname) + .mapOption("hostname-admin", HostnameSpec::getAdmin) + .mapOption("hostname-admin-url", HostnameSpec::getAdminUrl) + .mapOption("hostname-strict", HostnameSpec::isStrict) + .mapOption("hostname-strict-backchannel", HostnameSpec::isStrictBackchannel); } public void configureFeatures() { diff --git a/operator/src/main/java/org/keycloak/operator/controllers/KeycloakIngress.java b/operator/src/main/java/org/keycloak/operator/controllers/KeycloakIngress.java index 0b7eb48d44..4f0807d374 100644 --- a/operator/src/main/java/org/keycloak/operator/controllers/KeycloakIngress.java +++ b/operator/src/main/java/org/keycloak/operator/controllers/KeycloakIngress.java @@ -101,8 +101,9 @@ public class KeycloakIngress extends OperatorManagedResource implements StatusUp .endSpec() .build(); - if (!keycloak.getSpec().isHostnameDisabled()) { - ingress.getSpec().getRules().get(0).setHost(keycloak.getSpec().getHostname()); + final var hostnameSpec = keycloak.getSpec().getHostnameSpec(); + if (hostnameSpec != null && hostnameSpec.getHostname() != null) { + ingress.getSpec().getRules().get(0).setHost(hostnameSpec.getHostname()); } return ingress; diff --git a/operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/KeycloakSpec.java b/operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/KeycloakSpec.java index 7556b595d9..3c1e85009e 100644 --- a/operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/KeycloakSpec.java +++ b/operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/KeycloakSpec.java @@ -16,20 +16,18 @@ */ package org.keycloak.operator.crds.v2alpha1.deployment; -import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonPropertyDescription; import io.fabric8.kubernetes.api.model.LocalObjectReference; -import org.keycloak.operator.Constants; import org.keycloak.operator.crds.v2alpha1.deployment.spec.DatabaseSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.FeatureSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.HttpSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.UnsupportedSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.IngressSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.TransactionsSpec; +import org.keycloak.operator.crds.v2alpha1.deployment.spec.HostnameSpec; -import javax.validation.constraints.NotNull; import java.util.ArrayList; import java.util.List; @@ -48,11 +46,6 @@ public class KeycloakSpec { "expressed as a keys (reference: https://www.keycloak.org/server/all-config) and values that can be either direct values or references to secrets.") private List serverConfiguration; // can't use Set due to a bug in Sundrio https://github.com/sundrio/sundrio/issues/316 - @NotNull - @JsonPropertyDescription("Hostname for the Keycloak server.\n" + - "The special value `" + Constants.INSECURE_DISABLE + "` disables the hostname strict resolution.") - private String hostname; - @JsonProperty("http") @JsonPropertyDescription("In this section you can configure Keycloak features related to HTTP and HTTPS") private HttpSpec httpSpec; @@ -79,18 +72,9 @@ public class KeycloakSpec { @JsonPropertyDescription("In this section you can find all properties related to connect to a database.") private DatabaseSpec databaseSpec; - public String getHostname() { - return hostname; - } - - public void setHostname(String hostname) { - this.hostname = hostname; - } - - @JsonIgnore - public boolean isHostnameDisabled() { - return this.hostname.equals(Constants.INSECURE_DISABLE); - } + @JsonProperty("hostname") + @JsonPropertyDescription("In this section you can configure Keycloak hostname and related properties.") + private HostnameSpec hostnameSpec; public HttpSpec getHttpSpec() { return httpSpec; @@ -140,6 +124,14 @@ public class KeycloakSpec { this.databaseSpec = databaseSpec; } + public HostnameSpec getHostnameSpec() { + return hostnameSpec; + } + + public void setHostnameSpec(HostnameSpec hostnameSpec) { + this.hostnameSpec = hostnameSpec; + } + public int getInstances() { return instances; } diff --git a/operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/spec/HostnameSpec.java b/operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/spec/HostnameSpec.java new file mode 100644 index 0000000000..21aa6be856 --- /dev/null +++ b/operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/spec/HostnameSpec.java @@ -0,0 +1,82 @@ +/* + * 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.crds.v2alpha1.deployment.spec; + +import com.fasterxml.jackson.annotation.JsonPropertyDescription; +import io.sundr.builder.annotations.Buildable; + +import java.io.Serializable; + +@Buildable(editableEnabled = false, builderPackage = "io.fabric8.kubernetes.api.builder") +public class HostnameSpec implements Serializable { + + @JsonPropertyDescription("Hostname for the Keycloak server.") + private String hostname; + + @JsonPropertyDescription("The hostname for accessing the administration console.") + private String admin; + + @JsonPropertyDescription("Set the base URL for accessing the administration console, including scheme, host, port and path") + private String adminUrl; + + @JsonPropertyDescription("Disables dynamically resolving the hostname from request headers.") + private Boolean strict; + + @JsonPropertyDescription("By default backchannel URLs are dynamically resolved from request headers to allow internal and external applications.") + private Boolean strictBackchannel; + + public String getHostname() { + return hostname; + } + + public void setHostname(String hostname) { + this.hostname = hostname; + } + + public String getAdmin() { + return admin; + } + + public void setAdmin(String admin) { + this.admin = admin; + } + + public String getAdminUrl() { + return adminUrl; + } + + public void setAdminUrl(String adminUrl) { + this.adminUrl = adminUrl; + } + + public Boolean isStrict() { + return strict; + } + + public void setStrict(Boolean strict) { + this.strict = strict; + } + + public Boolean isStrictBackchannel() { + return strictBackchannel; + } + + public void setStrictBackchannel(Boolean strictBackchannel) { + this.strictBackchannel = strictBackchannel; + } +} \ No newline at end of file diff --git a/operator/src/main/resources/example-keycloak.yaml b/operator/src/main/resources/example-keycloak.yaml index feb610339e..c79b821a16 100644 --- a/operator/src/main/resources/example-keycloak.yaml +++ b/operator/src/main/resources/example-keycloak.yaml @@ -13,6 +13,7 @@ spec: passwordSecret: name: keycloak-db-secret key: password - hostname: example.com http: - tlsSecret: example-tls-secret \ No newline at end of file + tlsSecret: example-tls-secret + hostname: + hostname: example.com \ No newline at end of file diff --git a/operator/src/test/java/org/keycloak/operator/testsuite/integration/KeycloakDeploymentTest.java b/operator/src/test/java/org/keycloak/operator/testsuite/integration/KeycloakDeploymentTest.java index a0bf8c003d..0221d08139 100644 --- a/operator/src/test/java/org/keycloak/operator/testsuite/integration/KeycloakDeploymentTest.java +++ b/operator/src/test/java/org/keycloak/operator/testsuite/integration/KeycloakDeploymentTest.java @@ -30,13 +30,14 @@ import org.awaitility.Awaitility; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.EnabledIfSystemProperty; import org.keycloak.operator.Constants; -import org.keycloak.operator.controllers.KeycloakDistConfigurator; -import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusCondition; -import org.keycloak.operator.testsuite.utils.K8sUtils; import org.keycloak.operator.controllers.KeycloakAdminSecret; +import org.keycloak.operator.controllers.KeycloakDistConfigurator; import org.keycloak.operator.controllers.KeycloakService; import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak; +import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusCondition; import org.keycloak.operator.crds.v2alpha1.deployment.ValueOrSecret; +import org.keycloak.operator.crds.v2alpha1.deployment.spec.HostnameSpecBuilder; +import org.keycloak.operator.testsuite.utils.K8sUtils; import java.nio.charset.StandardCharsets; import java.time.Duration; @@ -257,7 +258,12 @@ public class KeycloakDeploymentTest extends BaseOperatorTest { public void testHostnameStrictDisabled() { try { var kc = getDefaultKeycloakDeployment(); - kc.getSpec().setHostname(Constants.INSECURE_DISABLE); + var hostnameSpec = new HostnameSpecBuilder() + .withStrict(false) + .withStrictBackchannel(false) + .build(); + kc.getSpec().setHostnameSpec(hostnameSpec); + deployKeycloak(k8sclient, kc, true); var service = new KeycloakService(k8sclient, kc); @@ -284,9 +290,15 @@ public class KeycloakDeploymentTest extends BaseOperatorTest { final int httpsPort = 8543; final int httpPort = 8180; var kc = getDefaultKeycloakDeployment(); - kc.getSpec().setHostname(Constants.INSECURE_DISABLE); kc.getSpec().getHttpSpec().setHttpsPort(httpsPort); kc.getSpec().getHttpSpec().setHttpPort(httpPort); + + var hostnameSpec = new HostnameSpecBuilder() + .withStrict(false) + .withStrictBackchannel(false) + .build(); + kc.getSpec().setHostnameSpec(hostnameSpec); + deployKeycloak(k8sclient, kc, true); assertKeycloakAccessibleViaService(kc, true, httpsPort); @@ -302,11 +314,17 @@ public class KeycloakDeploymentTest extends BaseOperatorTest { final int httpsPort = 8543; final int httpPort = 8180; var kc = getDefaultKeycloakDeployment(); - kc.getSpec().setHostname(Constants.INSECURE_DISABLE); kc.getSpec().getHttpSpec().setHttpsPort(httpsPort); kc.getSpec().getHttpSpec().setHttpPort(httpPort); kc.getSpec().getHttpSpec().setTlsSecret(null); kc.getSpec().getHttpSpec().setHttpEnabled(true); + + var hostnameSpec = new HostnameSpecBuilder() + .withStrict(false) + .withStrictBackchannel(false) + .build(); + kc.getSpec().setHostnameSpec(hostnameSpec); + deployKeycloak(k8sclient, kc, true); assertKeycloakAccessibleViaService(kc, false, httpPort); 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 58a0dec232..328e922566 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 @@ -25,6 +25,7 @@ import org.awaitility.Awaitility; import org.junit.jupiter.api.Test; import org.keycloak.operator.Constants; import org.keycloak.operator.crds.v2alpha1.deployment.spec.IngressSpec; +import org.keycloak.operator.crds.v2alpha1.deployment.spec.HostnameSpecBuilder; import org.keycloak.operator.testsuite.utils.K8sUtils; import org.keycloak.operator.controllers.KeycloakIngress; @@ -39,9 +40,13 @@ public class KeycloakIngressTest extends BaseOperatorTest { @Test public void testIngressOnHTTP() { var kc = K8sUtils.getDefaultKeycloakDeployment(); - kc.getSpec().setHostname(Constants.INSECURE_DISABLE); kc.getSpec().getHttpSpec().setTlsSecret(null); kc.getSpec().getHttpSpec().setHttpEnabled(true); + var hostnameSpec = new HostnameSpecBuilder() + .withStrict(false) + .withStrictBackchannel(false) + .build(); + kc.getSpec().setHostnameSpec(hostnameSpec); K8sUtils.deployKeycloak(k8sclient, kc, true); Awaitility.await() @@ -70,7 +75,12 @@ public class KeycloakIngressTest extends BaseOperatorTest { @Test public void testIngressOnHTTPS() { var kc = K8sUtils.getDefaultKeycloakDeployment(); - kc.getSpec().setHostname(Constants.INSECURE_DISABLE); + var hostnameSpec = new HostnameSpecBuilder() + .withStrict(false) + .withStrictBackchannel(false) + .build(); + kc.getSpec().setHostnameSpec(hostnameSpec); + K8sUtils.deployKeycloak(k8sclient, kc, true); Awaitility.await() @@ -101,7 +111,9 @@ public class KeycloakIngressTest extends BaseOperatorTest { @Test public void testIngressHostname() { var kc = K8sUtils.getDefaultKeycloakDeployment(); - kc.getSpec().setHostname("foo.bar"); + var hostnameSpec = new HostnameSpecBuilder().withHostname("foo.bar").build(); + kc.getSpec().setHostnameSpec(hostnameSpec); + K8sUtils.deployKeycloak(k8sclient, kc, true); var ingress = new KeycloakIngress(k8sclient, kc); diff --git a/operator/src/test/java/org/keycloak/operator/testsuite/integration/WatchedSecretsTest.java b/operator/src/test/java/org/keycloak/operator/testsuite/integration/WatchedSecretsTest.java index e0349b8326..8e9ec2196d 100644 --- a/operator/src/test/java/org/keycloak/operator/testsuite/integration/WatchedSecretsTest.java +++ b/operator/src/test/java/org/keycloak/operator/testsuite/integration/WatchedSecretsTest.java @@ -28,6 +28,7 @@ import org.keycloak.operator.controllers.WatchedSecretsStore; import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak; import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusCondition; import org.keycloak.operator.crds.v2alpha1.deployment.ValueOrSecret; +import org.keycloak.operator.crds.v2alpha1.deployment.spec.HostnameSpecBuilder; import java.util.Base64; import java.util.HashSet; @@ -161,12 +162,14 @@ public class WatchedSecretsTest extends BaseOperatorTest { public void testSingleSecretMultipleKeycloaks() { try { var kc1 = getDefaultKeycloakDeployment(); + var kc1Hostname = new HostnameSpecBuilder().withHostname("kc1.local").build(); kc1.getMetadata().setName(kc1.getMetadata().getName() + "-1"); - kc1.getSpec().setHostname("kc1.local"); + kc1.getSpec().setHostnameSpec(kc1Hostname); var kc2 = getDefaultKeycloakDeployment(); + var kc2Hostname = new HostnameSpecBuilder().withHostname("kc2.local").build(); kc2.getMetadata().setName(kc2.getMetadata().getName() + "-2"); - kc2.getSpec().setHostname("kc2.local"); // to prevent Ingress conflicts + kc2.getSpec().setHostnameSpec(kc2Hostname); // to prevent Ingress conflicts deployKeycloak(k8sclient, kc1, true); deployKeycloak(k8sclient, kc2, true); diff --git a/operator/src/test/java/org/keycloak/operator/testsuite/unit/CRSerializationTest.java b/operator/src/test/java/org/keycloak/operator/testsuite/unit/CRSerializationTest.java index 02f44878ef..8e1905bcec 100644 --- a/operator/src/test/java/org/keycloak/operator/testsuite/unit/CRSerializationTest.java +++ b/operator/src/test/java/org/keycloak/operator/testsuite/unit/CRSerializationTest.java @@ -19,13 +19,13 @@ package org.keycloak.operator.testsuite.unit; import io.fabric8.kubernetes.client.utils.Serialization; import org.hamcrest.CoreMatchers; -import org.hamcrest.Matchers; import org.junit.jupiter.api.Test; import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak; import org.keycloak.operator.crds.v2alpha1.deployment.ValueOrSecret; import org.keycloak.operator.crds.v2alpha1.deployment.spec.DatabaseSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.FeatureSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.TransactionsSpec; +import org.keycloak.operator.crds.v2alpha1.deployment.spec.HostnameSpec; import java.util.List; @@ -34,6 +34,7 @@ import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasProperty; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -44,7 +45,7 @@ public class CRSerializationTest { public void testDeserialization() { Keycloak keycloak = Serialization.unmarshal(this.getClass().getResourceAsStream("/test-serialization-keycloak-cr.yml"), Keycloak.class); - assertEquals("my-hostname", keycloak.getSpec().getHostname()); + assertEquals("my-hostname", keycloak.getSpec().getHostnameSpec().getHostname()); assertEquals("my-image", keycloak.getSpec().getImage()); assertEquals("my-tls-secret", keycloak.getSpec().getHttpSpec().getTlsSecret()); assertFalse(keycloak.getSpec().getIngressSpec().isIngressEnabled()); @@ -78,7 +79,7 @@ public class CRSerializationTest { } @Test - public void featureSpecificationDeserialization(){ + public void featureSpecification() { Keycloak keycloak = Serialization.unmarshal(this.getClass().getResourceAsStream("/test-serialization-keycloak-cr.yml"), Keycloak.class); final FeatureSpec featureSpec = keycloak.getSpec().getFeatureSpec(); @@ -95,4 +96,27 @@ public class CRSerializationTest { assertThat(disabledFeatures.get(1), CoreMatchers.is("step-up-authentication")); } + @Test + public void hostnameSpecification() { + Keycloak keycloak = Serialization.unmarshal(this.getClass().getResourceAsStream("/test-serialization-keycloak-cr.yml"), Keycloak.class); + + HostnameSpec hostnameSpec = keycloak.getSpec().getHostnameSpec(); + assertThat(hostnameSpec, notNullValue()); + + assertThat(hostnameSpec.getHostname(), is("my-hostname")); + assertThat(hostnameSpec.getAdmin(), is("my-admin-hostname")); + assertThat(hostnameSpec.getAdminUrl(), is("https://www.my-admin-hostname.org:8448/something")); + assertThat(hostnameSpec.isStrict(), is(true)); + assertThat(hostnameSpec.isStrictBackchannel(), is(true)); + + keycloak = Serialization.unmarshal(this.getClass().getResourceAsStream("/empty-podtemplate-keycloak.yml"), Keycloak.class); + + hostnameSpec = keycloak.getSpec().getHostnameSpec(); + assertThat(hostnameSpec, notNullValue()); + assertThat(hostnameSpec.getHostname(), is("example.com")); + assertThat(hostnameSpec.getAdmin(), nullValue()); + assertThat(hostnameSpec.getAdminUrl(), nullValue()); + assertThat(hostnameSpec.isStrict(), nullValue()); + assertThat(hostnameSpec.isStrictBackchannel(), nullValue()); + } } \ No newline at end of file diff --git a/operator/src/test/java/org/keycloak/operator/testsuite/unit/KeycloakDistConfiguratorTest.java b/operator/src/test/java/org/keycloak/operator/testsuite/unit/KeycloakDistConfiguratorTest.java index cc4d493374..cc9e7a3192 100644 --- a/operator/src/test/java/org/keycloak/operator/testsuite/unit/KeycloakDistConfiguratorTest.java +++ b/operator/src/test/java/org/keycloak/operator/testsuite/unit/KeycloakDistConfiguratorTest.java @@ -24,7 +24,6 @@ import io.fabric8.kubernetes.api.model.apps.StatefulSetBuilder; import io.quarkus.test.junit.QuarkusTest; import org.junit.jupiter.api.Test; import org.keycloak.common.util.CollectionUtil; -import org.keycloak.common.util.ObjectUtil; import org.keycloak.operator.Constants; import org.keycloak.operator.controllers.KeycloakDistConfigurator; import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak; @@ -34,12 +33,17 @@ import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusCondition; import org.keycloak.operator.crds.v2alpha1.deployment.ValueOrSecret; import org.keycloak.operator.testsuite.utils.K8sUtils; -import java.util.Collections; +import java.util.Collection; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.Optional; import java.util.function.Consumer; -import java.util.function.Function; +import java.util.stream.Collectors; import static org.assertj.core.api.Assertions.assertThat; +import static org.keycloak.common.util.ObjectUtil.isBlank; +import static org.keycloak.operator.controllers.KeycloakDistConfigurator.getKeycloakOptionEnvVarName; import static org.keycloak.operator.testsuite.utils.CRAssert.assertKeycloakStatusCondition; import static org.keycloak.operator.testsuite.utils.CRAssert.assertKeycloakStatusDoesNotContainMessage; @@ -48,50 +52,34 @@ public class KeycloakDistConfiguratorTest { @Test public void enabledFeatures() { - testFirstClassCitizen("KC_FEATURES", "features", - KeycloakDistConfigurator::configureFeatures, "docker", "authorization"); + testFirstClassCitizen(Map.of("features", "docker,authorization"), KeycloakDistConfigurator::configureFeatures); } @Test public void disabledFeatures() { - testFirstClassCitizen("KC_FEATURES_DISABLED", "features-disabled", - KeycloakDistConfigurator::configureFeatures, "admin", "step-up-authentication"); + testFirstClassCitizen(Map.of("features-disabled", "admin,step-up-authentication"), KeycloakDistConfigurator::configureFeatures); } @Test public void transactions() { - testFirstClassCitizen("KC_TRANSACTION_XA_ENABLED", "transaction-xa-enabled", - KeycloakDistConfigurator::configureTransactions, "false"); + testFirstClassCitizen(Map.of("transaction-xa-enabled", "false"), KeycloakDistConfigurator::configureTransactions); } @Test - public void httpEnabled() { - testFirstClassCitizen("KC_HTTP_ENABLED", "http-enabled", - KeycloakDistConfigurator::configureHttp, "true"); + public void http() { + final Map expectedValues = Map.of( + "http-enabled", "true", + "http-port", "123", + "https-port", "456", + "https-certificate-file", Constants.CERTIFICATES_FOLDER + "/tls.crt", + "https-certificate-key-file", Constants.CERTIFICATES_FOLDER + "/tls.key" + ); + + testFirstClassCitizen(expectedValues, KeycloakDistConfigurator::configureHttp); } @Test - public void httpPort() { - testFirstClassCitizen("KC_HTTP_PORT", "http-port", - KeycloakDistConfigurator::configureHttp, "123"); - } - - @Test - public void httpsPort() { - testFirstClassCitizen("KC_HTTPS_PORT", "https-port", - KeycloakDistConfigurator::configureHttp, "456"); - } - - @Test - public void tlsSecret() { - testFirstClassCitizen("KC_HTTPS_CERTIFICATE_FILE", "https-certificate-file", - KeycloakDistConfigurator::configureHttp, Constants.CERTIFICATES_FOLDER + "/tls.crt"); - testFirstClassCitizen("KC_HTTPS_CERTIFICATE_KEY_FILE", "https-certificate-key-file", - KeycloakDistConfigurator::configureHttp, Constants.CERTIFICATES_FOLDER + "/tls.key"); - } - - @Test - public void testEmptyLists() { + public void featuresEmptyLists() { final Keycloak keycloak = K8sUtils.getResourceFromFile("test-serialization-keycloak-cr-with-empty-list.yml", Keycloak.class); final StatefulSet deployment = getBasicKcDeployment(); final KeycloakDistConfigurator distConfig = new KeycloakDistConfigurator(keycloak, deployment, null); @@ -103,33 +91,61 @@ public class KeycloakDistConfiguratorTest { } @Test - public void testDatabaseSettings() { - testFirstClassCitizen("KC_DB", "db", - KeycloakDistConfigurator::configureDatabase, "vendor"); - testFirstClassCitizen("KC_DB_USERNAME", "db-username", - KeycloakDistConfigurator::configureDatabase, "usernameSecret"); - testFirstClassCitizen("KC_DB_PASSWORD", "db-password", - KeycloakDistConfigurator::configureDatabase, "passwordSecret"); - testFirstClassCitizen("KC_DB_SCHEMA", "db-schema", - KeycloakDistConfigurator::configureDatabase, "schema"); - testFirstClassCitizen("KC_DB_URL_HOST", "db-url-host", - KeycloakDistConfigurator::configureDatabase, "host"); - testFirstClassCitizen("KC_DB_URL_PORT", "db-url-port", - KeycloakDistConfigurator::configureDatabase, "123"); - testFirstClassCitizen("KC_DB_POOL_INITIAL_SIZE", "db-pool-initial-size", - KeycloakDistConfigurator::configureDatabase, "1"); - testFirstClassCitizen("KC_DB_POOL_MIN_SIZE", "db-pool-min-size", - KeycloakDistConfigurator::configureDatabase, "2"); - testFirstClassCitizen("KC_DB_POOL_MAX_SIZE", "db-pool-max-size", - KeycloakDistConfigurator::configureDatabase, "3"); + public void db() { + final Map expectedValues = new HashMap<>(Map.of( + "db", "vendor", + "db-username", "usernameSecret", + "db-password", "passwordSecret", + "db-url-database", "database", + "db-schema", "schema", + "db-url-host", "host", + "db-url-port", "123", + "db-pool-initial-size", "1", + "db-pool-min-size", "2", + "db-pool-max-size", "3" + )); + expectedValues.put("db-url", "url"); + + testFirstClassCitizen(expectedValues, KeycloakDistConfigurator::configureDatabase); + } + + @Test + public void hostname() { + final Map expectedValues = Map.of( + "hostname", "my-hostname", + "hostname-admin-url", "https://www.my-admin-hostname.org:8448/something", + "hostname-strict", "true", + "hostname-strict-backchannel", "true", + "hostname-admin", "my-admin-hostname" + ); + + testFirstClassCitizen(expectedValues, KeycloakDistConfigurator::configureHostname); + } + + @Test + public void missingHostname() { + final Keycloak keycloak = K8sUtils.getResourceFromFile("test-serialization-keycloak-cr-with-empty-list.yml", Keycloak.class); + final StatefulSet deployment = getBasicKcDeployment(); + final KeycloakDistConfigurator distConfig = new KeycloakDistConfigurator(keycloak, deployment, null); + + final List envVars = deployment.getSpec().getTemplate().getSpec().getContainers().get(0).getEnv(); + + distConfig.configureHostname(); + + assertEnvVarNotPresent(envVars, "KC_HOSTNAME"); + assertEnvVarNotPresent(envVars, "KC_HOSTNAME_ADMIN"); + assertEnvVarNotPresent(envVars, "KC_HOSTNAME_ADMIN_URL"); + assertEnvVarNotPresent(envVars, "KC_HOSTNAME_STRICT"); + assertEnvVarNotPresent(envVars, "KC_HOSTNAME_STRICT-BACKCHANNEL"); } /* UTILS */ - private void testFirstClassCitizen(String envVarName, String optionName, Consumer config, String... expectedValues) { - testFirstClassCitizen("/test-serialization-keycloak-cr.yml", envVarName, optionName, config, expectedValues); + + private void testFirstClassCitizen(Map expectedValues, Consumer config) { + testFirstClassCitizen("/test-serialization-keycloak-cr.yml", expectedValues, config); } - private void testFirstClassCitizen(String crName, String envVarName, String optionName, Consumer config, String... expectedValues) { + private void testFirstClassCitizen(String crName, Map expectedValues, Consumer config) { final Keycloak keycloak = K8sUtils.getResourceFromFile(crName, Keycloak.class); final StatefulSet deployment = getBasicKcDeployment(); final KeycloakDistConfigurator distConfig = new KeycloakDistConfigurator(keycloak, deployment, null); @@ -137,54 +153,89 @@ public class KeycloakDistConfiguratorTest { final Container container = deployment.getSpec().getTemplate().getSpec().getContainers().get(0); assertThat(container).isNotNull(); - assertEnvVarNotPresent(container.getEnv(), envVarName); - assertWarningStatus(distConfig, optionName, false); + final List serverConfig = expectedValues.keySet() + .stream() + .map(f -> new ValueOrSecret(f, "foo")) + .collect(Collectors.toUnmodifiableList()); + + keycloak.getSpec().setServerConfiguration(serverConfig); + + final var expectedFields = expectedValues.keySet(); + + assertWarningStatusFirstClassFields(distConfig, false, expectedFields); + expectedValues.forEach((k, v) -> assertEnvVarNotPresent(container.getEnv(), getKeycloakOptionEnvVarName(k))); config.accept(distConfig); - assertContainerEnvVar(container.getEnv(), envVarName, expectedValues); - - keycloak.getSpec().setServerConfiguration(List.of(new ValueOrSecret(optionName, "foo"))); - assertWarningStatus(distConfig, optionName, true); + assertWarningStatusFirstClassFields(distConfig, true, expectedFields); + expectedValues.forEach((k, v) -> assertContainerEnvVar(container.getEnv(), getKeycloakOptionEnvVarName(k), v)); } /** * assertContainerEnvVar(container.getEnv(), "KC_FEATURES", "admin,authorization"); * assertContainerEnvVar(container.getEnv(), "KC_HOSTNAME", "someHostname"); */ - private void assertContainerEnvVar(List envVars, String varName, String... expectedValue) { + private void assertContainerEnvVar(List envVars, String varName, String expectedValue) { assertThat(envVars).isNotNull(); assertEnvVarPresent(envVars, varName); - final List foundValues = getValuesFromEnvVar(envVars, varName); - assertThat(CollectionUtil.isNotEmpty(foundValues)).isTrue(); - for (String val : expectedValue) { - assertThat(foundValues.contains(val)).isTrue(); - } + final String foundValue = envVars.stream().filter(f -> varName.equals(f.getName())) + .findFirst() + .map(envVar -> { + if (envVar.getValue() != null) { + return envVar.getValue(); + } + + if (envVar.getValueFrom() != null && envVar.getValueFrom().getSecretKeyRef() != null) { + return envVar.getValueFrom().getSecretKeyRef().getName(); + } + + return null; + }) + .orElse(null); + + assertThat(foundValue).isNotNull(); + assertThat(foundValue).isEqualTo(expectedValue); } private void assertEnvVarPresent(List envVars, String varName) { assertThat(containsEnvironmentVariable(envVars, varName)).isTrue(); - } private void assertEnvVarNotPresent(List envVars, String varName) { assertThat(containsEnvironmentVariable(envVars, varName)).isFalse(); } - private void assertWarningStatus(KeycloakDistConfigurator distConfig, String optionName, boolean expectWarning) { - final String message = "warning: You need to specify these fields as the first-class citizen of the CR: " + optionName; + private void assertWarningStatusFirstClassFields(KeycloakDistConfigurator distConfig, boolean expectWarning, Collection firstClassFields) { + final String message = "warning: You need to specify these fields as the first-class citizen of the CR: "; final KeycloakStatusBuilder statusBuilder = new KeycloakStatusBuilder(); distConfig.validateOptions(statusBuilder); final KeycloakStatus status = statusBuilder.build(); if (expectWarning) { assertKeycloakStatusCondition(status, KeycloakStatusCondition.HAS_ERRORS, false, message); + + var fullMessage = getFullMessageFromStatus(status, message); + assertThat(fullMessage).isPresent(); + + var foundFields = fullMessage.get().substring(message.length()); + assertThat(isBlank(foundFields)).isFalse(); + assertThat(foundFields.split(",")).containsAll(firstClassFields); } else { assertKeycloakStatusDoesNotContainMessage(status, message); } } + private Optional getFullMessageFromStatus(KeycloakStatus status, String containedMessage) { + if (isBlank(containedMessage)) return Optional.empty(); + + return status.getConditions() + .stream() + .filter(f -> f.getMessage().contains(containedMessage)) + .findAny() + .map(KeycloakStatusCondition::getMessage); + } + private StatefulSet getBasicKcDeployment() { return new StatefulSetBuilder() .withNewSpec() @@ -201,34 +252,7 @@ public class KeycloakDistConfiguratorTest { } private boolean containsEnvironmentVariable(List envVars, String varName) { - if (CollectionUtil.isEmpty(envVars) || ObjectUtil.isBlank(varName)) return false; + if (CollectionUtil.isEmpty(envVars) || isBlank(varName)) return false; return envVars.stream().anyMatch(f -> varName.equals(f.getName())); } - - /** - * Returns values of environment variable separated by comma (f.e KC_FEATURES=admin2,ciba) - */ - private List getValuesFromEnvVar(List envVars, String varName) { - if (CollectionUtil.isEmpty(envVars) || ObjectUtil.isBlank(varName)) return Collections.emptyList(); - - return envVars.stream().filter(f -> varName.equals(f.getName())) - .findFirst() - .map(new Function() { - @Override - public String apply(EnvVar envVar) { - if (envVar.getValue() != null) { - return envVar.getValue(); - } - - if (envVar.getValueFrom() != null && envVar.getValueFrom().getSecretKeyRef() != null) { - return envVar.getValueFrom().getSecretKeyRef().getName(); - } - - return null; - } - }) - .map(f -> f.split(",")) - .map(List::of) - .orElseGet(Collections::emptyList); - } -} +} \ No newline at end of file diff --git a/operator/src/test/java/org/keycloak/operator/testsuite/unit/PodTemplateTest.java b/operator/src/test/java/org/keycloak/operator/testsuite/unit/PodTemplateTest.java index ae2a1783fc..f436c6d978 100644 --- a/operator/src/test/java/org/keycloak/operator/testsuite/unit/PodTemplateTest.java +++ b/operator/src/test/java/org/keycloak/operator/testsuite/unit/PodTemplateTest.java @@ -28,8 +28,9 @@ import org.junit.jupiter.api.Test; import org.keycloak.operator.Config; import org.keycloak.operator.controllers.KeycloakDeployment; import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak; -import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakSpec; -import org.keycloak.operator.crds.v2alpha1.deployment.spec.HttpSpec; +import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakSpecBuilder; +import org.keycloak.operator.crds.v2alpha1.deployment.spec.HostnameSpecBuilder; +import org.keycloak.operator.crds.v2alpha1.deployment.spec.HttpSpecBuilder; import org.keycloak.operator.crds.v2alpha1.deployment.spec.UnsupportedSpec; import static org.assertj.core.api.Assertions.assertThat; @@ -56,15 +57,15 @@ public class PodTemplateTest { } }; var kc = new Keycloak(); - var spec = new KeycloakSpec(); - spec.setUnsupported(new UnsupportedSpec(podTemplate)); - spec.setHostname("example.com"); - var httpSpec = new HttpSpec(); - httpSpec.setTlsSecret("example-tls-secret"); - spec.setHttpSpec(httpSpec); + var httpSpec = new HttpSpecBuilder().withTlsSecret("example-tls-secret").build(); + var hostnameSpec = new HostnameSpecBuilder().withHostname("example.com").build(); + + kc.setSpec(new KeycloakSpecBuilder().withUnsupported(new UnsupportedSpec(podTemplate)) + .withHttpSpec(httpSpec) + .withHostnameSpec(hostnameSpec) + .build()); - kc.setSpec(spec); var deployment = new KeycloakDeployment(null, config, kc, existingDeployment, "dummy-admin"); return (StatefulSet) deployment.getReconciledResource().get(); } diff --git a/operator/src/test/resources/correct-podtemplate-keycloak.yml b/operator/src/test/resources/correct-podtemplate-keycloak.yml index a7d0a13722..4117b4dbb4 100644 --- a/operator/src/test/resources/correct-podtemplate-keycloak.yml +++ b/operator/src/test/resources/correct-podtemplate-keycloak.yml @@ -13,7 +13,8 @@ spec: value: postgres - name: db-password value: testpassword - hostname: example.com + hostname: + hostname: example.com unsupported: podTemplate: metadata: diff --git a/operator/src/test/resources/empty-podtemplate-keycloak.yml b/operator/src/test/resources/empty-podtemplate-keycloak.yml index bd97bd91dd..4a89e90d01 100644 --- a/operator/src/test/resources/empty-podtemplate-keycloak.yml +++ b/operator/src/test/resources/empty-podtemplate-keycloak.yml @@ -13,6 +13,7 @@ spec: value: postgres - name: db-password value: testpassword - hostname: example.com + hostname: + hostname: example.com unsupported: podTemplate: diff --git a/operator/src/test/resources/test-serialization-keycloak-cr-with-empty-list.yml b/operator/src/test/resources/test-serialization-keycloak-cr-with-empty-list.yml index 73932680e2..4f75839ed4 100644 --- a/operator/src/test/resources/test-serialization-keycloak-cr-with-empty-list.yml +++ b/operator/src/test/resources/test-serialization-keycloak-cr-with-empty-list.yml @@ -6,6 +6,5 @@ spec: features: enabled: - - hostname: my-hostname http: tlsSecret: my-tls-secret \ No newline at end of file diff --git a/operator/src/test/resources/test-serialization-keycloak-cr.yml b/operator/src/test/resources/test-serialization-keycloak-cr.yml index 3eaf493d1f..5c472f771f 100644 --- a/operator/src/test/resources/test-serialization-keycloak-cr.yml +++ b/operator/src/test/resources/test-serialization-keycloak-cr.yml @@ -10,7 +10,6 @@ spec: value: value1 - name: features value: docker - hostname: my-hostname db: vendor: vendor usernameSecret: @@ -34,6 +33,12 @@ spec: httpPort: 123 httpsPort: 456 tlsSecret: my-tls-secret + hostname: + hostname: my-hostname + admin: my-admin-hostname + adminUrl: https://www.my-admin-hostname.org:8448/something + strict: true + strictBackchannel: true features: enabled: - docker