fix: refining activation condition error handling (#43197)

closes: #43096

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
This commit is contained in:
Steven Hawkins
2025-10-15 07:44:39 -04:00
committed by GitHub
parent 02dfb4bd8a
commit 43ee41e8a8
10 changed files with 153 additions and 46 deletions

View File

@@ -206,6 +206,7 @@ jobs:
run: |
kubectl apply -f src/test/resources/service-monitor-crds.yml
./scripts/check-crd-installed.sh servicemonitors
kubectl delete pod -l name=keycloak-operator
- name: Deploy an example Keycloak with ServiceMonitor
working-directory: operator/scripts

View File

@@ -125,6 +125,8 @@ execute the following commands:
kubectl create namespace custom-namespace
kubectl -n custom-namespace apply -f https://raw.githubusercontent.com/keycloak/keycloak-k8s-resources/{version}/kubernetes/kubernetes.yml
kubectl patch clusterrolebinding keycloak-operator-clusterrole-binding --type='json' -p='[{"op": "replace", "path": "/subjects/0/namespace", "value":"custom-namespace"}]'
# if you have existing keycloaks, restart the operator after patching the clusterrolebinding
kubectl rollout restart -n custom-namespace Deployment/keycloak-operator
----
</@profile.ifCommunity>

View File

@@ -34,7 +34,6 @@ import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
import io.javaoperatorsdk.operator.api.reconciler.Workflow;
import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
import io.javaoperatorsdk.operator.processing.dependent.workflow.CRDPresentActivationCondition;
import io.javaoperatorsdk.operator.processing.event.source.EventSource;
import io.quarkus.logging.Log;
import jakarta.inject.Inject;
@@ -67,8 +66,7 @@ import java.util.concurrent.TimeUnit;
@Dependent(type = KeycloakNetworkPolicyDependentResource.class, reconcilePrecondition = KeycloakNetworkPolicyDependentResource.EnabledCondition.class),
@Dependent(
type = KeycloakServiceMonitorDependentResource.class,
activationCondition = CRDPresentActivationCondition.class,
reconcilePrecondition = KeycloakServiceMonitorDependentResource.ReconcilePrecondition.class
activationCondition = KeycloakServiceMonitorDependentResource.ActivationCondition.class
),
})
public class KeycloakController implements Reconciler<Keycloak> {
@@ -230,6 +228,10 @@ public class KeycloakController implements Reconciler<Keycloak> {
}
distConfigurator.validateOptions(keycloakCR, status);
context.managedWorkflowAndDependentResourceContext()
.get(KeycloakServiceMonitorDependentResource.SERVICE_MONITOR_WARNING, String.class)
.ifPresent(status::addWarningMessage);
}
public static boolean isRolling(StatefulSet existingDeployment) {

View File

@@ -75,6 +75,7 @@ import java.util.stream.Stream;
import static org.keycloak.operator.Utils.addResources;
import static org.keycloak.operator.controllers.KeycloakDistConfigurator.getKeycloakOptionEnvVarName;
import static org.keycloak.operator.crds.v2alpha1.CRDUtils.LEGACY_MANAGEMENT_ENABLED;
import static org.keycloak.operator.crds.v2alpha1.CRDUtils.isTlsConfigured;
import static org.keycloak.operator.crds.v2alpha1.deployment.spec.TracingSpec.convertTracingAttributesToString;
@@ -83,7 +84,6 @@ import static org.keycloak.operator.crds.v2alpha1.deployment.spec.TracingSpec.co
)
public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependentResource<StatefulSet, Keycloak> {
public static final String HTTP_MANAGEMENT_HEALTH_ENABLED = "http-management-health-enabled";
public static final String HTTP_MANAGEMENT_SCHEME = "http-management-scheme";
public static final String POD_IP = "POD_IP";
@@ -356,8 +356,7 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
// Set bind address as this is required for JGroups to form a cluster in IPv6 environments
containerBuilder.addToArgs(0, "-Djgroups.bind.address=$(%s)".formatted(POD_IP));
var healthEnabled = readConfigurationValue(HTTP_MANAGEMENT_HEALTH_ENABLED, keycloakCR, context).map(Boolean::valueOf).orElse(true);
ManagementEndpoint endpoint = managementEndpoint(keycloakCR, context, healthEnabled);
ManagementEndpoint endpoint = managementEndpoint(keycloakCR, context, true);
// probes
var readinessOptionalSpec = Optional.ofNullable(keycloakCR.getSpec().getReadinessProbeSpec());
@@ -592,7 +591,7 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
return keycloak.getMetadata().getName();
}
private static Optional<String> readConfigurationValue(String key, Keycloak keycloakCR, Context<Keycloak> context) {
static Optional<String> readConfigurationValue(String key, Keycloak keycloakCR, Context<Keycloak> context) {
return Optional.ofNullable(keycloakCR.getSpec()).map(KeycloakSpec::getAdditionalOptions)
.flatMap(l -> l.stream().filter(sc -> sc.getName().equals(key)).findFirst().map(serverConfigValue -> {
if (serverConfigValue.getValue() != null) {
@@ -644,20 +643,27 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
toUpdate.getMetadata().getAnnotations().put(Constants.KEYCLOAK_UPDATE_REVISION_ANNOTATION, revision);
}
record ManagementEndpoint(String relativePath, String protocol, int port) {}
record ManagementEndpoint(String relativePath, String protocol, int port, String portName) {}
static ManagementEndpoint managementEndpoint(Keycloak keycloakCR, Context<Keycloak> context, boolean useMgmtProtocolPort) {
static ManagementEndpoint managementEndpoint(Keycloak keycloakCR, Context<Keycloak> context, boolean health) {
boolean tls = isTlsConfigured(keycloakCR);
String protocol = tls ? "HTTPS" : "HTTP";
int port;
String portName;
if (useMgmtProtocolPort) {
var legacy = readConfigurationValue(LEGACY_MANAGEMENT_ENABLED, keycloakCR, context).map(Boolean::valueOf).orElse(false);
var healthManagementEnabled = readConfigurationValue(CRDUtils.HTTP_MANAGEMENT_HEALTH_ENABLED, keycloakCR, context).map(Boolean::valueOf).orElse(true);
if (!legacy && (!health || healthManagementEnabled)) {
port = HttpManagementSpec.managementPort(keycloakCR);
portName = Constants.KEYCLOAK_MANAGEMENT_PORT_NAME;
if (readConfigurationValue(HTTP_MANAGEMENT_SCHEME, keycloakCR, context).filter("http"::equals).isPresent()) {
protocol = "HTTP";
}
} else {
port = tls ? HttpSpec.httpsPort(keycloakCR) : HttpSpec.httpPort(keycloakCR);
portName = tls ? Constants.KEYCLOAK_HTTPS_PORT_NAME : Constants.KEYCLOAK_HTTP_PORT_NAME;
}
var relativePath = readConfigurationValue(Constants.KEYCLOAK_HTTP_MANAGEMENT_RELATIVE_PATH_KEY, keycloakCR, context)
@@ -665,6 +671,6 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
.map(path -> !path.endsWith("/") ? path + "/" : path)
.orElse("/");
return new ManagementEndpoint(relativePath, protocol, port);
return new ManagementEndpoint(relativePath, protocol, port, portName);
}
}

View File

@@ -1,16 +1,19 @@
package org.keycloak.operator.controllers;
import static org.keycloak.operator.controllers.KeycloakDeploymentDependentResource.managementEndpoint;
import static org.keycloak.operator.crds.v2alpha1.CRDUtils.LEGACY_MANAGEMENT_ENABLED;
import static org.keycloak.operator.crds.v2alpha1.CRDUtils.METRICS_ENABLED;
import static org.keycloak.operator.crds.v2alpha1.CRDUtils.configuredOptions;
import java.net.HttpURLConnection;
import org.keycloak.operator.Constants;
import org.keycloak.operator.Utils;
import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
import org.keycloak.operator.crds.v2alpha1.deployment.spec.ServiceMonitorSpec;
import io.fabric8.kubernetes.api.model.networking.v1.Ingress;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.kubernetes.client.Watcher;
import io.fabric8.kubernetes.client.WatcherException;
import io.fabric8.openshift.api.model.monitoring.v1.ServiceMonitor;
import io.fabric8.openshift.api.model.monitoring.v1.ServiceMonitorBuilder;
import io.javaoperatorsdk.operator.api.config.informer.Informer;
@@ -19,28 +22,75 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent;
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
import io.quarkus.logging.Log;
@KubernetesDependent(
informer = @Informer(labelSelector = Constants.DEFAULT_LABELS_AS_STRING)
)
public class KeycloakServiceMonitorDependentResource extends CRUDKubernetesDependentResource<ServiceMonitor, Keycloak> {
public static class ReconcilePrecondition implements Condition<Ingress, Keycloak> {
public static final String WARN_METRICS_NOT_ENABLED = "A ServiceMonitor will not be created because `metrics-enabled` is not true.";
public static final String WARN_CRD_NOT_INSTALLED = "A ServiceMonitor will not be created because the ServiceMonitor CRD is not installed.";
static String SERVICE_MONITOR_WARNING = "ServiceMonitorWarning";
volatile Boolean crdInstalled;
public static class ActivationCondition implements Condition<ServiceMonitor, Keycloak> {
@Override
public boolean isMet(DependentResource<Ingress, Keycloak> dependentResource, Keycloak primary,
Context<Keycloak> context) {
var opts = configuredOptions(primary);
if (Boolean.parseBoolean(opts.get(LEGACY_MANAGEMENT_ENABLED)) || !Boolean.parseBoolean(opts.getOrDefault(METRICS_ENABLED, "false"))) {
public boolean isMet(DependentResource<ServiceMonitor, Keycloak> dependentResource, Keycloak primary, Context<Keycloak> context) {
if (!ServiceMonitorSpec.get(primary).isEnabled()) {
return false;
}
return ServiceMonitorSpec.get(primary).isEnabled();
var opts = configuredOptions(primary);
if (!Boolean.parseBoolean(opts.getOrDefault(METRICS_ENABLED, "false"))) {
context.managedWorkflowAndDependentResourceContext().put(SERVICE_MONITOR_WARNING, WARN_METRICS_NOT_ENABLED);
return false;
}
if (!isCRDInstalled(dependentResource, context, (KeycloakServiceMonitorDependentResource)dependentResource)) {
context.managedWorkflowAndDependentResourceContext().put(SERVICE_MONITOR_WARNING, WARN_CRD_NOT_INSTALLED);
return false;
}
return true;
}
private boolean isCRDInstalled(DependentResource<ServiceMonitor, Keycloak> dependentResource,
Context<Keycloak> context, KeycloakServiceMonitorDependentResource serviceMonitorDependentResource) {
if (serviceMonitorDependentResource.crdInstalled != null) {
return serviceMonitorDependentResource.crdInstalled;
}
Watcher<ServiceMonitor> dummyWatcher = new Watcher<ServiceMonitor>() {
@Override
public void eventReceived(Action action, ServiceMonitor resource) {
}
@Override
public void onClose(WatcherException cause) {
}
};
try (var watch = context.getClient().resources(dependentResource.resourceType()).watch(dummyWatcher)) {
serviceMonitorDependentResource.crdInstalled = true;
return true;
} catch (KubernetesClientException e) {
if (e.getCode() == HttpURLConnection.HTTP_NOT_FOUND) {
Log.warn("ServiceMonitors will not be managed by the operator as the CRD for ServiceMonitors not installed. If the CRD is installed later, the operator will need to be restarted.");
serviceMonitorDependentResource.crdInstalled = false;
return false;
}
throw e;
}
}
}
@Override
protected ServiceMonitor desired(Keycloak primary, Context<Keycloak> context) {
var endpoint = managementEndpoint(primary, context, true);
var endpoint = managementEndpoint(primary, context, false);
var meta = primary.getMetadata();
var spec = ServiceMonitorSpec.get(primary);
return new ServiceMonitorBuilder()
@@ -60,7 +110,7 @@ public class KeycloakServiceMonitorDependentResource extends CRUDKubernetesDepen
.addNewEndpoint()
.withInterval(spec.getInterval())
.withPath(endpoint.relativePath() + "metrics")
.withPort(Constants.KEYCLOAK_MANAGEMENT_PORT_NAME)
.withPort(endpoint.portName())
.withScheme(endpoint.protocol().toLowerCase())
.withScrapeTimeout(spec.getScrapeTimeout())
.withNewTlsConfig()

View File

@@ -21,10 +21,8 @@ import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Predicate;
import java.util.stream.Stream;
import org.keycloak.operator.Constants;
import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
@@ -47,6 +45,7 @@ import io.javaoperatorsdk.operator.api.reconciler.Context;
*/
public final class CRDUtils {
private static final String HEALTH_ENABLED = "health-enabled";
public static final String HTTP_MANAGEMENT_HEALTH_ENABLED = "http-management-health-enabled";
public static final String METRICS_ENABLED = "metrics-enabled";
public static final String LEGACY_MANAGEMENT_ENABLED = "legacy-observability-interface";
@@ -71,11 +70,8 @@ public final class CRDUtils {
return false;
}
// Only metrics and health use the management endpoint.
return Stream.of(METRICS_ENABLED, HEALTH_ENABLED)
.map(options::get)
.filter(Objects::nonNull)
.anyMatch(Boolean::parseBoolean);
return Boolean.parseBoolean(options.get(METRICS_ENABLED)) || (Boolean.parseBoolean(options.get(HEALTH_ENABLED))
&& Boolean.parseBoolean(options.getOrDefault(HTTP_MANAGEMENT_HEALTH_ENABLED, Boolean.toString(true))));
}
public static Map<String, String> configuredOptions(Keycloak keycloak) {

View File

@@ -93,12 +93,6 @@ metadata:
labels:
app.kubernetes.io/name: keycloak-operator
rules:
- apiGroups:
- apiextensions.k8s.io
resources:
- customresourcedefinitions
verbs:
- get
- apiGroups:
- config.openshift.io
resources:

View File

@@ -8,9 +8,12 @@ import org.awaitility.Awaitility;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.keycloak.operator.controllers.KeycloakServiceMonitorDependentResource;
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.ServiceMonitorSpecBuilder;
import org.keycloak.operator.testsuite.utils.CRAssert;
import org.keycloak.operator.testsuite.utils.K8sUtils;
import io.fabric8.kubernetes.client.KubernetesClient;
@@ -24,18 +27,23 @@ public class ServiceMonitorTest extends BaseOperatorTest {
@Test
public void testServiceMonitorDisabledNoMetrics() {
Assumptions.assumeTrue(isServiceMonitorAvailable(k8sclient));
var kc = getTestKeycloakDeployment(true, false);;
var kc = getTestKeycloakDeployment(true, false);
kc.getSpec().setAdditionalOptions(List.of(new ValueOrSecret("metrics-enabled", "false")));
K8sUtils.deployKeycloak(k8sclient, kc, true);
ServiceMonitor sm = getServiceMonitor(kc);
assertThat(sm).isNull();
CRAssert.assertKeycloakStatusCondition(
k8sclient.resources(Keycloak.class).withName(kc.getMetadata().getName()).get(),
KeycloakStatusCondition.HAS_ERRORS, false,
KeycloakServiceMonitorDependentResource.WARN_METRICS_NOT_ENABLED);
}
@Test
public void testServiceMonitorCreatedWithMetricsEnabled() {
Assumptions.assumeTrue(isServiceMonitorAvailable(k8sclient));
var kc = getTestKeycloakDeployment(true, false);;
var kc = getTestKeycloakDeployment(true, false);
K8sUtils.deployKeycloak(k8sclient, kc, true);
Awaitility.await().untilAsserted(() -> {
@@ -48,7 +56,7 @@ public class ServiceMonitorTest extends BaseOperatorTest {
@Test
public void testServiceMonitorDisabledExplicitly() {
Assumptions.assumeTrue(isServiceMonitorAvailable(k8sclient));
var kc = getTestKeycloakDeployment(true, false);;
var kc = getTestKeycloakDeployment(true, false);
kc.getSpec().setServiceMonitorSpec(
new ServiceMonitorSpecBuilder()
.withEnabled(false)
@@ -61,20 +69,24 @@ public class ServiceMonitorTest extends BaseOperatorTest {
}
@Test
public void testServiceMonitorDisabledLegacyManagement() {
public void testServiceMonitorLegacyManagement() {
Assumptions.assumeTrue(isServiceMonitorAvailable(k8sclient));
var kc = getTestKeycloakDeployment(true, false);;
kc.getSpec().setAdditionalOptions(List.of(new ValueOrSecret("legacy-observability-interface", "true")));
var kc = getTestKeycloakDeployment(true, false);
kc.getSpec().getAdditionalOptions().add(new ValueOrSecret("legacy-observability-interface", "true"));
K8sUtils.deployKeycloak(k8sclient, kc, true);
ServiceMonitor sm = getServiceMonitor(kc);
assertThat(sm).isNull();
Awaitility.await().untilAsserted(() -> {
var sm = getServiceMonitor(kc);
assertThat(sm).isNotNull();
assertThat(sm.getSpec().getEndpoints()).hasSize(1);
assertThat(sm.getSpec().getEndpoints().get(0).getPort()).isEqualTo("https");
});
}
@Test
public void testServiceMonitorConfigProperties() {
Assumptions.assumeTrue(isServiceMonitorAvailable(k8sclient));
var kc = getTestKeycloakDeployment(true, false);;
var kc = getTestKeycloakDeployment(true, false);
kc.getSpec().setServiceMonitorSpec(
new ServiceMonitorSpecBuilder()
.withInterval("1s")
@@ -92,14 +104,14 @@ public class ServiceMonitorTest extends BaseOperatorTest {
});
}
private ServiceMonitor getServiceMonitor(Keycloak kc) {
static ServiceMonitor getServiceMonitor(Keycloak kc) {
return k8sclient.resources(ServiceMonitor.class)
.inNamespace(kc.getMetadata().getNamespace())
.withName(kc.getMetadata().getName())
.get();
}
private boolean isServiceMonitorAvailable(KubernetesClient client) {
static boolean isServiceMonitorAvailable(KubernetesClient client) {
return client
.apiextensions()
.v1()

View File

@@ -0,0 +1,43 @@
package org.keycloak.operator.testsuite.integration;
import static org.assertj.core.api.Assertions.assertThat;
import java.util.concurrent.TimeUnit;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.keycloak.operator.controllers.KeycloakServiceMonitorDependentResource;
import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusCondition;
import org.keycloak.operator.testsuite.utils.CRAssert;
import org.keycloak.operator.testsuite.utils.K8sUtils;
import io.fabric8.openshift.api.model.monitoring.v1.ServiceMonitor;
import io.quarkus.test.junit.QuarkusTest;
@QuarkusTest
public class ServiceMonitorUninstalledTest extends BaseOperatorTest {
@BeforeAll
public static void beforeAll() {
k8sclient.apiextensions().v1().customResourceDefinitions().withName(new ServiceMonitor().getFullResourceName())
.withTimeout(10, TimeUnit.SECONDS).delete();
}
@Test
public void testServiceMonitorNoCRD() {
Assumptions.assumeFalse(ServiceMonitorTest.isServiceMonitorAvailable(k8sclient));
var kc = getTestKeycloakDeployment(true, false);
K8sUtils.deployKeycloak(k8sclient, kc, true);
ServiceMonitor sm = ServiceMonitorTest.getServiceMonitor(kc);
assertThat(sm).isNull();
CRAssert.assertKeycloakStatusCondition(
k8sclient.resources(Keycloak.class).withName(kc.getMetadata().getName()).get(),
KeycloakStatusCondition.HAS_ERRORS, false,
KeycloakServiceMonitorDependentResource.WARN_CRD_NOT_INSTALLED);
}
}

View File

@@ -57,6 +57,7 @@ import org.keycloak.operator.controllers.KeycloakDistConfigurator;
import org.keycloak.operator.controllers.KeycloakRealmImportJobDependentResource;
import org.keycloak.operator.controllers.KeycloakUpdateJobDependentResource;
import org.keycloak.operator.controllers.WatchedResources;
import org.keycloak.operator.crds.v2alpha1.CRDUtils;
import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakBuilder;
import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakSpecBuilder;
@@ -416,7 +417,7 @@ public class PodTemplateTest {
@Test
public void testHealthOnMain() {
var result = getDeployment(null, new StatefulSet(),
spec -> spec.withAdditionalOptions(new ValueOrSecret(KeycloakDeploymentDependentResource.HTTP_MANAGEMENT_HEALTH_ENABLED, "false")))
spec -> spec.withAdditionalOptions(new ValueOrSecret(CRDUtils.HTTP_MANAGEMENT_HEALTH_ENABLED, "false")))
.getSpec()
.getTemplate()
.getSpec()