fix: ensuring update job handles secrets when none exist in the cr

closes #39939

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
Signed-off-by: Steven Hawkins <shawkins@redhat.com>
Co-authored-by: Václav Muzikář <vaclav@muzikari.cz>
This commit is contained in:
Steven Hawkins 2025-06-04 12:29:35 -04:00 committed by GitHub
parent 706390addd
commit 72d3063a54
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 61 additions and 9 deletions

View File

@ -20,11 +20,13 @@ package org.keycloak.operator.controllers;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import io.fabric8.kubernetes.api.model.HasMetadata;
import org.keycloak.operator.Constants;
import org.keycloak.operator.ContextUtils;
import org.keycloak.operator.Utils;
@ -84,7 +86,7 @@ public class KeycloakUpdateJobDependentResource extends CRUDKubernetesDependentR
}
@Override
protected Job desired(Keycloak primary, Context<Keycloak> context) {
public Job desired(Keycloak primary, Context<Keycloak> context) {
var builder = new JobBuilder();
builder.withMetadata(createMetadata(jobName(primary), primary));
var specBuilder = builder.withNewSpec();
@ -142,11 +144,11 @@ public class KeycloakUpdateJobDependentResource extends CRUDKubernetesDependentR
// if there is some corner case we are not considering that would leave the upgrade job unscheduable, we'll address that later
// mix in the existing state
var desiredPullSecrets = builder.buildImagePullSecrets();
var desiredPullSecrets = Optional.ofNullable(builder.buildImagePullSecrets()).orElse(List.of());
current.getSpec().getTemplate().getSpec().getImagePullSecrets().stream().filter(s -> !desiredPullSecrets.contains(s)).forEach(builder::addToImagePullSecrets);
// TODO: if the name is the same, but the volume has changed this merging behavior will be inconsistent / incorrect. For example is someone changes which
// configmap the cache config is using
var desiredVolumes = builder.buildVolumes().stream().map(Volume::getName).collect(Collectors.toSet());
var desiredVolumes = Optional.ofNullable(builder.buildVolumes()).orElse(List.of()).stream().map(Volume::getName).collect(Collectors.toSet());
current.getSpec().getTemplate().getSpec().getVolumes().stream().filter(v -> !desiredVolumes.contains(v.getName())).forEach(builder::addToVolumes);
// TODO: what else should get merged - there could be additional stuff from the unsupported PodTemplate

View File

@ -60,11 +60,11 @@ public class KeycloakSpec {
private Boolean startOptimized;
@JsonPropertyDescription("Secret(s) that might be used when pulling an image from a private container image registry or repository.")
private List<LocalObjectReference> imagePullSecrets;
private List<LocalObjectReference> imagePullSecrets = new ArrayList<LocalObjectReference>();
@JsonPropertyDescription("Configuration of the Keycloak server.\n" +
"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<ValueOrSecret> additionalOptions; // can't use Set due to a bug in Sundrio https://github.com/sundrio/sundrio/issues/316
private List<ValueOrSecret> additionalOptions = new ArrayList<ValueOrSecret>(); // can't use Set due to a bug in Sundrio https://github.com/sundrio/sundrio/issues/316
@JsonProperty("http")
@JsonPropertyDescription("In this section you can configure Keycloak features related to HTTP and HTTPS")

View File

@ -22,6 +22,7 @@ import io.fabric8.kubernetes.api.model.AffinityBuilder;
import io.fabric8.kubernetes.api.model.Container;
import io.fabric8.kubernetes.api.model.EnvVar;
import io.fabric8.kubernetes.api.model.IntOrString;
import io.fabric8.kubernetes.api.model.LocalObjectReference;
import io.fabric8.kubernetes.api.model.PodTemplateSpec;
import io.fabric8.kubernetes.api.model.PodTemplateSpecBuilder;
import io.fabric8.kubernetes.api.model.ProbeBuilder;
@ -33,6 +34,7 @@ import io.fabric8.kubernetes.api.model.VolumeMount;
import io.fabric8.kubernetes.api.model.apps.StatefulSet;
import io.fabric8.kubernetes.api.model.apps.StatefulSetBuilder;
import io.fabric8.kubernetes.api.model.apps.StatefulSetSpec;
import io.fabric8.kubernetes.api.model.batch.v1.Job;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.utils.Serialization;
import io.javaoperatorsdk.operator.api.reconciler.Context;
@ -48,6 +50,7 @@ import org.keycloak.operator.Constants;
import org.keycloak.operator.Utils;
import org.keycloak.operator.controllers.KeycloakDeploymentDependentResource;
import org.keycloak.operator.controllers.KeycloakDistConfigurator;
import org.keycloak.operator.controllers.KeycloakUpdateJobDependentResource;
import org.keycloak.operator.controllers.WatchedResources;
import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakBuilder;
@ -59,6 +62,7 @@ import org.keycloak.operator.crds.v2alpha1.deployment.spec.UnsupportedSpec;
import org.mockito.Mockito;
import java.util.List;
import java.util.Optional;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.stream.Collectors;
@ -68,6 +72,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.keycloak.operator.ContextUtils.DIST_CONFIGURATOR_KEY;
import static org.keycloak.operator.ContextUtils.NEW_DEPLOYMENT_KEY;
import static org.keycloak.operator.ContextUtils.OLD_DEPLOYMENT_KEY;
import static org.keycloak.operator.ContextUtils.OPERATOR_CONFIG_KEY;
import static org.keycloak.operator.ContextUtils.WATCHED_RESOURCES_KEY;
@ -86,17 +91,28 @@ public class PodTemplateTest {
KeycloakDeploymentDependentResource deployment;
@Inject
KeycloakUpdateJobDependentResource jobResource;
@BeforeEach
protected void setup() {
this.deployment = new KeycloakDeploymentDependentResource();
}
private StatefulSet getDeployment(PodTemplateSpec podTemplate, StatefulSet existingDeployment, Consumer<KeycloakSpecBuilder> additionalSpec) {
var kc = new KeycloakBuilder().withNewMetadata().withName("instance").withNamespace("keycloak-ns").endMetadata().build();
existingDeployment = new StatefulSetBuilder(existingDeployment).editOrNewSpec().editOrNewSelector()
var kc = createKeycloak(podTemplate, additionalSpec);
existingDeployment = new StatefulSetBuilder(existingDeployment).editOrNewMetadata().endMetadata().editOrNewSpec().editOrNewSelector()
.withMatchLabels(Utils.allInstanceLabels(kc))
.endSelector().endSpec().build();
//noinspection unchecked
Context<Keycloak> context = mockContext(existingDeployment);
return deployment.desired(kc, context);
}
private Keycloak createKeycloak(PodTemplateSpec podTemplate, Consumer<KeycloakSpecBuilder> additionalSpec) {
var kc = new KeycloakBuilder().withNewMetadata().withName("instance").withNamespace("keycloak-ns").endMetadata().build();
var httpSpec = new HttpSpecBuilder().withTlsSecret("example-tls-secret").build();
var hostnameSpec = new HostnameSpecBuilder().withHostname("example.com").build();
@ -110,8 +126,10 @@ public class PodTemplateTest {
}
kc.setSpec(keycloakSpecBuilder.build());
return kc;
}
//noinspection unchecked
private Context<Keycloak> mockContext(StatefulSet existingDeployment) {
Context<Keycloak> context = Mockito.mock(Context.class);
ManagedWorkflowAndDependentResourceContext managedWorkflowAndDependentResourceContext = Mockito.mock(ManagedWorkflowAndDependentResourceContext.class);
Mockito.when(context.managedWorkflowAndDependentResourceContext()).thenReturn(managedWorkflowAndDependentResourceContext);
@ -120,7 +138,7 @@ public class PodTemplateTest {
Mockito.when(managedWorkflowAndDependentResourceContext.getMandatory(WATCHED_RESOURCES_KEY, WatchedResources.class)).thenReturn(watchedResources);
Mockito.when(managedWorkflowAndDependentResourceContext.getMandatory(DIST_CONFIGURATOR_KEY, KeycloakDistConfigurator.class)).thenReturn(distConfigurator);
Mockito.when(context.getClient()).thenReturn(Mockito.mock(KubernetesClient.class));
return deployment.desired(kc, context);
return context;
}
private StatefulSet getDeployment(PodTemplateSpec podTemplate, StatefulSet existingDeployment) {
@ -642,4 +660,36 @@ public class PodTemplateTest {
assertThat(podTemplate.getSpec().getAffinity()).isNotEqualTo(affinity);
}
private Job getUpdateJob(Consumer<KeycloakSpecBuilder> newSpec, Consumer<KeycloakSpecBuilder> oldSpec, Consumer<StatefulSetBuilder> existingModifier) {
// create an existing from the old spec and modifier
StatefulSetBuilder existingBuilder = getDeployment(null, null, oldSpec).toBuilder();
existingModifier.accept(existingBuilder);
StatefulSet existingStatefulSet = existingBuilder.build();
// determine the desired statefulset state
StatefulSetBuilder desired = getDeployment(null, existingStatefulSet, newSpec).toBuilder();
// setup the mock context
Context<Keycloak> context = mockContext(existingStatefulSet);
var managedWorkflowAndDependentResourceContext = context.managedWorkflowAndDependentResourceContext();
Mockito.when(managedWorkflowAndDependentResourceContext.get(OLD_DEPLOYMENT_KEY, StatefulSet.class)).thenReturn(Optional.of(existingStatefulSet));
Mockito.when(managedWorkflowAndDependentResourceContext.getMandatory(NEW_DEPLOYMENT_KEY, StatefulSet.class)).thenReturn(desired.build());
return jobResource.desired(createKeycloak(null, newSpec), context);
}
@Test
public void testUpdateJobSecretHandling() {
Job job = getUpdateJob(builder -> {}, builder -> {}, builder -> {});
assertEquals(List.of(), job.getSpec().getTemplate().getSpec().getImagePullSecrets());
LocalObjectReference secret = new LocalObjectReference("secret");
Consumer<StatefulSetBuilder> addSecret = builder -> builder.editSpec().editTemplate().editSpec().addToImagePullSecrets(secret).endSpec().endTemplate().endSpec();
job = getUpdateJob(builder -> {}, builder -> {}, addSecret);
assertEquals(List.of(new LocalObjectReference("secret")), job.getSpec().getTemplate().getSpec().getImagePullSecrets());
job = getUpdateJob(builder -> builder.addToImagePullSecrets(new LocalObjectReference("new-secret")), builder -> {}, addSecret);
assertEquals(List.of(new LocalObjectReference("new-secret"), new LocalObjectReference("secret")), job.getSpec().getTemplate().getSpec().getImagePullSecrets());
}
}