fix: redoing the watching logic to provide a better status (#43817)

closes: #43777

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
This commit is contained in:
Steven Hawkins 2025-11-11 03:34:58 -05:00 committed by GitHub
parent ce05241c7f
commit 0064e060fc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 110 additions and 85 deletions

View File

@ -16,11 +16,13 @@
*/
package org.keycloak.operator.controllers;
import io.fabric8.kubernetes.api.model.ConfigMap;
import io.fabric8.kubernetes.api.model.ContainerState;
import io.fabric8.kubernetes.api.model.ContainerStateWaiting;
import io.fabric8.kubernetes.api.model.ContainerStatus;
import io.fabric8.kubernetes.api.model.PodSpec;
import io.fabric8.kubernetes.api.model.PodStatus;
import io.fabric8.kubernetes.api.model.Secret;
import io.fabric8.kubernetes.api.model.apps.StatefulSet;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.kubernetes.client.readiness.Readiness;
@ -227,6 +229,11 @@ public class KeycloakController implements Reconciler<Keycloak> {
status.addRollingUpdateMessage("Rolling out deployment update");
}
watchedResources.getMissing(existingDeployment, ConfigMap.class)
.ifPresent(m -> status.addWarningMessage("The following ConfigMaps are missing: " + m));
watchedResources.getMissing(existingDeployment, Secret.class)
.ifPresent(m -> status.addWarningMessage("The following Secrets are missing: " + m));
distConfigurator.validateOptions(keycloakCR, status);
context.managedWorkflowAndDependentResourceContext()

View File

@ -133,8 +133,8 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
WatchedResources watchedResources = ContextUtils.getWatchedResources(context);
StatefulSet baseDeployment = createBaseDeployment(primary, context, operatorConfig);
TreeSet<String> allSecrets = new TreeSet<>();
TreeSet<String> allConfigMaps = new TreeSet<>();
WatchedResources.Watched allSecrets = new WatchedResources.Watched();
WatchedResources.Watched allConfigMaps = new WatchedResources.Watched();
if (isTlsConfigured(primary)) {
configureTLS(primary, baseDeployment, allSecrets);
}
@ -145,13 +145,8 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
Optional.ofNullable(primary.getSpec().getCacheSpec())
.ifPresent(c -> configureCache(baseDeployment, kcContainer, c, allConfigMaps));
if (!allSecrets.isEmpty()) {
watchedResources.annotateDeployment(new ArrayList<>(allSecrets), Secret.class, baseDeployment, context.getClient());
}
if (!allConfigMaps.isEmpty()) {
watchedResources.annotateDeployment(new ArrayList<>(allConfigMaps), ConfigMap.class, baseDeployment, context.getClient());
}
watchedResources.annotateDeployment(allSecrets, Secret.class, baseDeployment, context);
watchedResources.annotateDeployment(allConfigMaps, ConfigMap.class, baseDeployment, context);
// default to the new revision - will be overriden to the old one if needed
UpdateSpec.getRevision(primary).ifPresent(rev -> addUpdateRevisionAnnotation(rev, baseDeployment));
@ -190,7 +185,7 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
};
}
private void configureCache(StatefulSet deployment, Container kcContainer, CacheSpec spec, TreeSet<String> allConfigMaps) {
private void configureCache(StatefulSet deployment, Container kcContainer, CacheSpec spec, WatchedResources.Watched allConfigMaps) {
Optional.ofNullable(spec.getConfigMapFile()).ifPresent(configFile -> {
if (configFile.getName() == null || configFile.getKey() == null) {
throw new IllegalStateException("Cache file ConfigMap requires both a name and a key");
@ -211,11 +206,11 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
deployment.getSpec().getTemplate().getSpec().getVolumes().add(0, volume);
kcContainer.getVolumeMounts().add(0, volumeMount);
allConfigMaps.add(configFile.getName());
allConfigMaps.add(configFile.getName(), configFile.getOptional());
});
}
private void addTruststores(Keycloak keycloakCR, StatefulSet deployment, Container kcContainer, TreeSet<String> allSecrets, TreeSet<String> allConfigMaps) {
private void addTruststores(Keycloak keycloakCR, StatefulSet deployment, Container kcContainer, WatchedResources.Watched allSecrets, WatchedResources.Watched allConfigMaps) {
for (Truststore truststore : keycloakCR.getSpec().getTruststores().values()) {
// for now we'll assume only secrets, later we can support configmaps
TruststoreSource source = truststore.getSecret();
@ -236,7 +231,7 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
deployment.getSpec().getTemplate().getSpec().getVolumes().add(0, volume);
kcContainer.getVolumeMounts().add(0, volumeMount);
allSecrets.add(secretName);
allSecrets.add(secretName, source.getOptional());
} else {
source = truststore.getConfigMap();
if (source != null) {
@ -256,13 +251,13 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
deployment.getSpec().getTemplate().getSpec().getVolumes().add(0, volume);
kcContainer.getVolumeMounts().add(0, volumeMount);
allConfigMaps.add(name);
allConfigMaps.add(name, source.getOptional());
}
}
}
}
void configureTLS(Keycloak keycloakCR, StatefulSet deployment, TreeSet<String> allSecrets) {
void configureTLS(Keycloak keycloakCR, StatefulSet deployment, WatchedResources.Watched allSecrets) {
var kcContainer = deployment.getSpec().getTemplate().getSpec().getContainers().get(0);
var volume = new VolumeBuilder()
@ -280,7 +275,7 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
deployment.getSpec().getTemplate().getSpec().getVolumes().add(0, volume);
kcContainer.getVolumeMounts().add(0, volumeMount);
allSecrets.add(keycloakCR.getSpec().getHttpSpec().getTlsSecret());
allSecrets.add(keycloakCR.getSpec().getHttpSpec().getTlsSecret(), null);
}
private boolean hasExpectedMatchLabels(StatefulSet statefulSet, Keycloak keycloak) {
@ -454,7 +449,7 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
}
}
private void addEnvVars(StatefulSet baseDeployment, Keycloak keycloakCR, TreeSet<String> allSecrets, Context<Keycloak> context) {
private void addEnvVars(StatefulSet baseDeployment, Keycloak keycloakCR, WatchedResources.Watched allSecrets, Context<Keycloak> context) {
var distConfigurator = ContextUtils.getDistConfigurator(context);
var firstClasssEnvVars = distConfigurator.configureDistOptions(keycloakCR);
@ -484,11 +479,9 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
// watch the secrets used by secret key - we don't currently expect configmaps or watch the initial-admin
TreeSet<String> serverConfigSecretsNames = envVars.stream().map(EnvVar::getValueFrom).filter(Objects::nonNull)
.map(EnvVarSource::getSecretKeyRef).filter(Objects::nonNull).map(SecretKeySelector::getName).collect(Collectors.toCollection(TreeSet::new));
.map(EnvVarSource::getSecretKeyRef).filter(Objects::nonNull).peek(s -> allSecrets.add(s.getName(), s.getOptional())).map(SecretKeySelector::getName).collect(Collectors.toCollection(TreeSet::new));
Log.debugf("Found config secrets names: %s", serverConfigSecretsNames);
allSecrets.addAll(serverConfigSecretsNames);
}
private static void setTracingEnvVars(Keycloak keycloakCR, Map<String, EnvVar> varMap) {

View File

@ -17,20 +17,55 @@
package org.keycloak.operator.controllers;
import java.util.Arrays;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.keycloak.operator.Utils;
import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.apps.StatefulSet;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import jakarta.enterprise.context.ApplicationScoped;
import org.keycloak.operator.Utils;
@ApplicationScoped
public class WatchedResources {
public static class Watched {
public static Watched of(String... values) {
Watched result = new Watched();
Stream.of(values).forEach(v -> result.add(v, null));
return result;
}
Map<String, Boolean> map = new LinkedHashMap<String, Boolean>();
public void add(String name, Boolean optional) {
map.merge(name, optional != null && optional, (b1, b2) -> b1 && b2);
}
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (getClass() != obj.getClass()) {
return false;
}
Watched other = (Watched) obj;
return Objects.equals(map, other.map);
}
}
public static final String KEYCLOAK_WATCHED_HASH_ANNOTATION_PREFIX = "operator.keycloak.org/watched-";
public static final String KEYCLOAK_WATCHING_ANNOTATION_PREFIX = "operator.keycloak.org/watching-";
public static final String KEYCLOAK_MISSING_ANNOTATION_PREFIX = "operator.keycloak.org/missing-";
@ -39,22 +74,37 @@ public class WatchedResources {
* @param deployment mutable resource being reconciled, it will be updated with
* annotations
*/
public <T extends HasMetadata> void annotateDeployment(List<String> names, Class<T> type, StatefulSet deployment,
KubernetesClient client) {
List<T> current = fetch(names, type, deployment.getMetadata().getNamespace(), client);
public <T extends HasMetadata> void annotateDeployment(Watched watched, Class<T> type, StatefulSet deployment, Context<Keycloak> context) {
if (watched.map.isEmpty()) {
return;
}
List<T> current = new ArrayList<>();
List<String> missing = new ArrayList<>();
watched.map.entrySet().stream().forEach(e -> {
var resource = context.getClient().resources(type)
.inNamespace(deployment.getMetadata().getNamespace()).withName(e.getKey()).get();
if (resource == null) {
if (!e.getValue()) {
missing.add(e.getKey());
}
} else {
current.add(resource);
}
});
String plural = HasMetadata.getPlural(type);
deployment.getMetadata().getAnnotations().put(WatchedResources.KEYCLOAK_MISSING_ANNOTATION_PREFIX + plural,
Boolean.valueOf(current.size() < names.size()).toString());
deployment.getMetadata().getAnnotations().put(WatchedResources.KEYCLOAK_WATCHING_ANNOTATION_PREFIX + plural,
String.join(";", names));
if (!missing.isEmpty()) {
deployment.getMetadata().getAnnotations().put(WatchedResources.KEYCLOAK_MISSING_ANNOTATION_PREFIX + plural, String.join(", ", missing));
}
deployment.getMetadata().getAnnotations().put(WatchedResources.KEYCLOAK_WATCHING_ANNOTATION_PREFIX + plural, Boolean.TRUE.toString());
deployment.getSpec().getTemplate().getMetadata().getAnnotations()
.put(WatchedResources.KEYCLOAK_WATCHED_HASH_ANNOTATION_PREFIX + HasMetadata.getKind(type).toLowerCase() + "-hash", getHash(current));
.put(WatchedResources.KEYCLOAK_WATCHED_HASH_ANNOTATION_PREFIX + HasMetadata.getKind(type).toLowerCase() + "-hash", Utils.hash(current));
}
public boolean hasMissing(StatefulSet deployment) {
return deployment.getMetadata().getAnnotations().entrySet().stream()
.anyMatch(e -> e.getKey().startsWith(WatchedResources.KEYCLOAK_MISSING_ANNOTATION_PREFIX)
&& Boolean.parseBoolean(e.getValue()));
public Optional<String> getMissing(StatefulSet deployment, Class<?> type) {
String plural = HasMetadata.getPlural(type);
return Optional.ofNullable(deployment.getMetadata().getAnnotations().get(WatchedResources.KEYCLOAK_MISSING_ANNOTATION_PREFIX + plural));
}
public boolean isWatching(StatefulSet deployment) {
@ -63,21 +113,4 @@ public class WatchedResources {
&& e.getValue() != null && !e.getValue().isEmpty());
}
public <T extends HasMetadata> String getHash(List<T> current) {
return Utils.hash(current);
}
private <T extends HasMetadata> List<T> fetch(List<String> names, Class<T> type, String namespace,
KubernetesClient client) {
return names.stream().map(n -> client.resources(type).inNamespace(namespace).withName(n).get())
.filter(Objects::nonNull).collect(Collectors.toList());
}
public List<String> getNames(StatefulSet deployment, Class<? extends HasMetadata> type) {
return Optional
.ofNullable(deployment.getMetadata().getAnnotations().get(WatchedResources.KEYCLOAK_WATCHING_ANNOTATION_PREFIX + HasMetadata.getPlural(type)))
.filter(watching -> !watching.isEmpty())
.map(watching -> watching.split(";")).map(Arrays::asList).orElse(List.of());
}
}

View File

@ -119,6 +119,7 @@ public class KeycloakDeploymentTest extends BaseOperatorTest {
// CR
var kc = getTestKeycloakDeployment(true);
var deploymentName = kc.getMetadata().getName();
k8sclient.resource(K8sUtils.getDefaultTlsSecret()).withTimeout(30, SECONDS).delete();
deployKeycloak(k8sclient, kc, false, false);
// Check Operator has deployed Keycloak and the statefulset exists, this allows for the watched secret to be picked up
@ -129,7 +130,7 @@ public class KeycloakDeploymentTest extends BaseOperatorTest {
Awaitility.await().ignoreExceptions().untilAsserted(() -> {
assertThat(stsResource.get()).isNotNull();
Keycloak keycloak = keycloakResource.get();
CRAssert.assertKeycloakStatusCondition(keycloak, KeycloakStatusCondition.HAS_ERRORS, false);
CRAssert.assertKeycloakStatusCondition(keycloak, KeycloakStatusCondition.HAS_ERRORS, false, "example-tls-secret");
CRAssert.assertKeycloakStatusCondition(keycloak, KeycloakStatusCondition.READY, false);
});
}

View File

@ -30,6 +30,7 @@ import org.keycloak.operator.testsuite.unit.WatchedResourcesTest;
import org.keycloak.operator.testsuite.utils.K8sUtils;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.keycloak.operator.testsuite.utils.K8sUtils.deployKeycloak;
import static org.keycloak.operator.testsuite.utils.K8sUtils.getResourceFromFile;
@ -48,12 +49,12 @@ public class KeycloakTruststoresTests extends BaseOperatorTest {
Awaitility.await().ignoreExceptions().untilAsserted(() -> {
StatefulSet statefulSet = stsResource.get();
assertEquals("true",
statefulSet.getMetadata().getAnnotations().get(WatchedResourcesTest.KEYCLOAK_MISSING_SECRETS_ANNOTATION));
assertTrue(statefulSet.getMetadata().getAnnotations().get(WatchedResourcesTest.KEYCLOAK_WATCHING_ANNOTATION)
statefulSet.getMetadata().getAnnotations().get(WatchedResourcesTest.KEYCLOAK_WATCHING_ANNOTATION));
assertTrue(statefulSet.getMetadata().getAnnotations().get(WatchedResourcesTest.KEYCLOAK_MISSING_SECRETS_ANNOTATION)
.contains("xyz"));
assertEquals("true",
statefulSet.getMetadata().getAnnotations().get(WatchedResourcesTest.KEYCLOAK_MISSING_CONFIGMAPS_ANNOTATION));
assertTrue(statefulSet.getMetadata().getAnnotations().get(WatchedResourcesTest.KEYCLOAK_WATCHING_CONFIGMAPS_ANNOTATION)
statefulSet.getMetadata().getAnnotations().get(WatchedResourcesTest.KEYCLOAK_WATCHING_CONFIGMAPS_ANNOTATION));
assertTrue(statefulSet.getMetadata().getAnnotations().get(WatchedResourcesTest.KEYCLOAK_MISSING_CONFIGMAPS_ANNOTATION)
.contains("abc"));
});
}
@ -72,13 +73,11 @@ public class KeycloakTruststoresTests extends BaseOperatorTest {
deployKeycloak(k8sclient, kc, true);
Resource<StatefulSet> stsResource = k8sclient.resources(StatefulSet.class).withName(deploymentName);
StatefulSet statefulSet = stsResource.get();
assertEquals("false",
statefulSet.getMetadata().getAnnotations().get(WatchedResourcesTest.KEYCLOAK_MISSING_SECRETS_ANNOTATION));
assertNull(statefulSet.getMetadata().getAnnotations().get(WatchedResourcesTest.KEYCLOAK_MISSING_SECRETS_ANNOTATION));
assertTrue(statefulSet.getSpec().getTemplate().getSpec().getContainers().get(0).getVolumeMounts().stream()
.anyMatch(v -> v.getMountPath()
.equals("/opt/keycloak/conf/truststores/secret-example-truststore-secret")));
assertEquals("false",
statefulSet.getMetadata().getAnnotations().get(WatchedResourcesTest.KEYCLOAK_MISSING_CONFIGMAPS_ANNOTATION));
assertNull(statefulSet.getMetadata().getAnnotations().get(WatchedResourcesTest.KEYCLOAK_MISSING_CONFIGMAPS_ANNOTATION));
assertTrue(statefulSet.getSpec().getTemplate().getSpec().getContainers().get(0).getVolumeMounts().stream()
.anyMatch(v -> v.getMountPath()
.equals("/opt/keycloak/conf/truststores/configmap-abc")));

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.controllers.WatchedResources.Watched;
import org.keycloak.operator.crds.v2alpha1.CRDUtils;
import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakBuilder;
@ -620,7 +621,7 @@ public class PodTemplateTest {
.findFirst().orElseThrow();
assertThat(volume.getConfigMap().getName()).isEqualTo("cm");
Mockito.verify(this.watchedResources).annotateDeployment(Mockito.eq(List.of("cm")), Mockito.eq(ConfigMap.class), Mockito.any(), Mockito.any());
Mockito.verify(this.watchedResources).annotateDeployment(Mockito.eq(Watched.of("cm")), Mockito.eq(ConfigMap.class), Mockito.any(), Mockito.any());
}
@Test
@ -895,7 +896,7 @@ public class PodTemplateTest {
envVar = env.get("SECRET");
assertEquals("key", envVar.getValueFrom().getSecretKeyRef().getKey());
Mockito.verify(this.watchedResources).annotateDeployment(Mockito.eq(List.of("example-tls-secret", "instance-initial-admin", "my-secret")), Mockito.eq(Secret.class), Mockito.any(), Mockito.any());
Mockito.verify(this.watchedResources).annotateDeployment(Mockito.eq(Watched.of("example-tls-secret", "instance-initial-admin", "my-secret")), Mockito.eq(Secret.class), Mockito.any(), Mockito.any());
}
}

View File

@ -17,23 +17,21 @@
package org.keycloak.operator.testsuite.unit;
import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
import io.fabric8.kubernetes.api.model.Secret;
import io.fabric8.kubernetes.api.model.SecretBuilder;
import io.fabric8.kubernetes.api.model.apps.StatefulSetBuilder;
import io.quarkus.test.junit.QuarkusTest;
import static org.junit.jupiter.api.Assertions.assertEquals;
import org.junit.jupiter.api.Test;
import org.keycloak.operator.controllers.WatchedResources;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.UUID;
import jakarta.inject.Inject;
import org.junit.jupiter.api.Test;
import org.keycloak.operator.Utils;
import org.keycloak.operator.controllers.WatchedResources;
import static org.junit.jupiter.api.Assertions.assertEquals;
import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
import io.fabric8.kubernetes.api.model.Secret;
import io.fabric8.kubernetes.api.model.SecretBuilder;
import io.quarkus.test.junit.QuarkusTest;
import jakarta.inject.Inject;
@QuarkusTest
public class WatchedResourcesTest {
@ -49,21 +47,14 @@ public class WatchedResourcesTest {
@Test
public void testHashing() {
assertEquals("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", watchedResources.getHash(List.of()));
assertEquals("b5655bfe4d4e130f5023a76a5de0906cf84eb5895bda5d44642673f9eb4024bf", watchedResources.getHash(List.of(newSecret(Map.of("a", "b")), newSecret(Map.of("c", "d")))));
assertEquals("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Utils.hash(List.of()));
assertEquals("b5655bfe4d4e130f5023a76a5de0906cf84eb5895bda5d44642673f9eb4024bf", Utils.hash(List.of(newSecret(Map.of("a", "b")), newSecret(Map.of("c", "d")))));
assertEquals("d526224334e65c71095be909b2d14c52f1589abb84a3c76fbe79dd75d7132fbb",
watchedResources.getHash(List.of(new ConfigMapBuilder().withNewMetadata().withName("x")
Utils.hash(List.of(new ConfigMapBuilder().withNewMetadata().withName("x")
.withAnnotations(Map.of(UUID.randomUUID().toString(), UUID.randomUUID().toString()))
.endMetadata().withData(Map.of("a", "b")).build())));
}
@Test
public void testGetSecretNames() {
assertEquals(List.of(), watchedResources.getNames(new StatefulSetBuilder().withNewMetadata().addToAnnotations(WatchedResourcesTest.KEYCLOAK_WATCHING_ANNOTATION, "").endMetadata().build(), Secret.class));
assertEquals(Arrays.asList("something"), watchedResources.getNames(new StatefulSetBuilder().withNewMetadata().addToAnnotations(WatchedResourcesTest.KEYCLOAK_WATCHING_ANNOTATION, "something").endMetadata().build(), Secret.class));
assertEquals(Arrays.asList("x", "y"), watchedResources.getNames(new StatefulSetBuilder().withNewMetadata().addToAnnotations(WatchedResourcesTest.KEYCLOAK_WATCHING_ANNOTATION, "x;y").endMetadata().build(), Secret.class));
}
private Secret newSecret(Map<String, String> data) {
return new SecretBuilder().withNewMetadata().withName(UUID.randomUUID().toString())
.withLabels(Map.of(UUID.randomUUID().toString(), UUID.randomUUID().toString())).endMetadata()