Skip to content

Backport operator secret sequencing fix #23705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Jamstah wants to merge 2 commits into from
Closed

Backport operator secret sequencing fix #23705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
73de629
Also replaces md5 usage with sha-256 (#21162)
shawkins Jul 17, 2023
14bc5c8
allows normal reconciliation to continue even if secrets are not pres…
shawkins Sep 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions operator/src/main/java/org/keycloak/operator/Constants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import java.util.stream.Collectors;
import java.util.stream.Stream;

public final class Constants {
public static final String CRDS_GROUP = "k8s.keycloak.org";
Expand All @@ -34,13 +36,15 @@ public final class Constants {
public static final String MANAGED_BY_VALUE = "keycloak-operator";
public static final String COMPONENT_LABEL = "app.kubernetes.io/component";
public static final String KEYCLOAK_COMPONENT_LABEL = "operator.keycloak.org/component";
public static final String KEYCLOAK_WATCHED_SECRET_HASH_ANNOTATION = "operator.keycloak.org/watched-secret-hash";
public static final String KEYCLOAK_WATCHING_ANNOTATION = "operator.keycloak.org/watching-secrets";
public static final String KEYCLOAK_MISSING_SECRETS_ANNOTATION = "operator.keycloak.org/missing-secrets";

public static final Map<String, String> DEFAULT_LABELS = Collections.unmodifiableMap(new TreeMap<>(Map.of(
"app", NAME,
MANAGED_BY_LABEL, MANAGED_BY_VALUE
)));
public static final String DEFAULT_LABELS_AS_STRING = "app=keycloak,app.kubernetes.io/managed-by=keycloak-operator";

public static final String DEFAULT_LABELS_AS_STRING = Utils.toSelectorString(DEFAULT_LABELS);
public static final Map<String, String> DEFAULT_LABELS = Collections
.unmodifiableMap(Stream.of(DEFAULT_LABELS_AS_STRING.split(",")).map(s -> s.split("="))
.collect(Collectors.toMap(e -> e[0], e -> e[1], (u1, u2) -> u1, TreeMap::new)));

public static final List<ValueOrSecret> DEFAULT_DIST_CONFIG_LIST = List.of(
new ValueOrSecret("health-enabled", "true"),
Expand Down
18 changes: 8 additions & 10 deletions operator/src/main/java/org/keycloak/operator/controllers/KeycloakController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ public class KeycloakController implements Reconciler<Keycloak>, EventSourceInit
@Inject
Config config;

@Inject
WatchedSecrets watchedSecrets;

@Override
public Map<String, EventSource> prepareEventSources(EventSourceContext<Keycloak> context) {
String namespace = context.getControllerConfiguration().getConfigurationService().getKubernetesClient().getNamespace();
Expand Down Expand Up @@ -89,9 +92,7 @@ public Map<String, EventSource> prepareEventSources(EventSourceContext<Keycloak>

return EventSourceInitializer.nameEventSources(statefulSetEvent,
servicesEvent,
ingressesEvent,
WatchedSecretsStore.getStoreEventSource(client, namespace),
WatchedSecretsStore.getWatchedSecretsEventSource(client, namespace));
ingressesEvent, watchedSecrets.getWatchedSecretsEventSource());
}

@Override
Expand All @@ -107,14 +108,9 @@ public UpdateControl<Keycloak> reconcile(Keycloak kc, Context<Keycloak> context)
kcAdminSecret.createOrUpdateReconciled();

var kcDeployment = new KeycloakDeployment(client, config, kc, context.getSecondaryResource(StatefulSet.class).orElse(null), kcAdminSecret.getName());
var watchedSecrets = new WatchedSecretsStore(kcDeployment.getConfigSecretsNames(), client, kc);
kcDeployment.setWatchedSecrets(watchedSecrets);
kcDeployment.createOrUpdateReconciled();
if (watchedSecrets.changesDetected()) {
Log.info("Config Secrets modified, restarting deployment");
kcDeployment.rollingRestart();
}
kcDeployment.updateStatus(statusAggregator);
watchedSecrets.createOrUpdateReconciled();

var kcService = new KeycloakService(client, kc);
kcService.updateStatus(statusAggregator);
Expand All @@ -141,7 +137,9 @@ public UpdateControl<Keycloak> reconcile(Keycloak kc, Context<Keycloak> context)
}

if (status.findCondition(KeycloakStatusCondition.READY)
.filter(c -> !Boolean.TRUE.equals(c.getStatus())).isPresent()) {
.filter(c -> !Boolean.TRUE.equals(c.getStatus())).isPresent() || context.getSecondaryResource(StatefulSet.class)
.map(s -> s.getMetadata().getAnnotations().get(Constants.KEYCLOAK_MISSING_SECRETS_ANNOTATION))
.filter(Boolean::valueOf).isPresent()) {
updateControl.rescheduleAfter(10, TimeUnit.SECONDS);
}

Expand Down
41 changes: 28 additions & 13 deletions operator/src/main/java/org/keycloak/operator/controllers/KeycloakDeployment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import io.fabric8.kubernetes.api.model.EnvVar;
import io.fabric8.kubernetes.api.model.EnvVarBuilder;
import io.fabric8.kubernetes.api.model.EnvVarSourceBuilder;
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.PodSpec;
import io.fabric8.kubernetes.api.model.PodSpecFluent.ContainersNested;
import io.fabric8.kubernetes.api.model.PodStatus;
Expand Down Expand Up @@ -54,13 +53,14 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.TreeSet;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.keycloak.operator.crds.v2alpha1.CRDUtils.isTlsConfigured;

public class KeycloakDeployment extends OperatorManagedResource implements StatusUpdater<KeycloakStatusAggregator> {
public class KeycloakDeployment extends OperatorManagedResource<StatefulSet> implements StatusUpdater<KeycloakStatusAggregator> {

private final Config operatorConfig;
private final KeycloakDistConfigurator distConfigurator;
Expand All @@ -71,6 +71,7 @@ public class KeycloakDeployment extends OperatorManagedResource implements Statu
private final String adminSecretName;

private Set<String> serverConfigSecretsNames;
private WatchedSecrets watchedSecrets;

private boolean migrationInProgress;

Expand All @@ -87,8 +88,13 @@ public KeycloakDeployment(KubernetesClient client, Config config, Keycloak keycl
addRemainingEnvVars();
}

public void setWatchedSecrets(WatchedSecrets watchedSecrets) {
this.watchedSecrets = watchedSecrets;
}

@Override
public Optional<HasMetadata> getReconciledResource() {
public Optional<StatefulSet> getReconciledResource() {
StatefulSet baseDeployment = new StatefulSetBuilder(this.baseDeployment).build(); // clone not to change the base template
if (existingDeployment == null) {
Log.info("No existing Deployment found, using the default");
}
Expand All @@ -102,13 +108,29 @@ public Optional<HasMetadata> getReconciledResource() {

migrateDeployment(existingDeployment, baseDeployment);
}

var configSecretsNames = getConfigSecretsNames();
if (!configSecretsNames.isEmpty() && watchedSecrets != null) {
watchedSecrets.annotateDeployment(configSecretsNames, keycloakCR, baseDeployment);
}

return Optional.of(baseDeployment);
}

private boolean hasExpectedMatchLabels(StatefulSet statefulSet) {
return Optional.ofNullable(statefulSet).map(s -> getInstanceLabels().equals(s.getSpec().getSelector().getMatchLabels())).orElse(true);
}

@Override
public Optional<StatefulSet> createOrUpdateReconciled() {
var ret = super.createOrUpdateReconciled();
// after the change to the statefulset has been "committed", start watching
if (watchedSecrets != null) {
ret.map(StatefulSet.class::cast).ifPresent(watchedSecrets::addLabelsToWatchedSecrets);
}
return ret;
}

public void validatePodTemplate(KeycloakStatusAggregator status) {
var spec = getPodTemplateSpec();
if (spec.isEmpty()) {
Expand Down Expand Up @@ -391,24 +413,17 @@ private void checkForPodErrors(KeycloakStatusAggregator status) {
});
}

public Set<String> getConfigSecretsNames() {
Set<String> ret = new HashSet<>(serverConfigSecretsNames);
public List<String> getConfigSecretsNames() {
TreeSet<String> ret = new TreeSet<>(serverConfigSecretsNames);
ret.addAll(distConfigurator.getSecretNames());
return ret;
return new ArrayList<>(ret);
}

@Override
public String getName() {
return keycloakCR.getMetadata().getName();
}

public void rollingRestart() {
client.apps().statefulSets()
.inNamespace(getNamespace())
.withName(getName())
.rolling().restart();
}

public void migrateDeployment(StatefulSet previousDeployment, StatefulSet reconciledDeployment) {
if (previousDeployment == null
|| previousDeployment.getSpec() == null
Expand Down
14 changes: 7 additions & 7 deletions operator/src/main/java/org/keycloak/operator/controllers/OperatorManagedResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.OwnerReference;
import io.fabric8.kubernetes.api.model.OwnerReferenceBuilder;
import io.fabric8.kubernetes.client.CustomResource;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.kubernetes.client.dsl.base.PatchContext;
Expand All @@ -39,20 +38,20 @@
*
* @author Vaclav Muzikar <vmuzikar@redhat.com>
*/
public abstract class OperatorManagedResource {
public abstract class OperatorManagedResource<T extends HasMetadata> {
private static final String KEYCLOAK_OPERATOR_FIELD_MANAGER = "keycloak-operator";
protected KubernetesClient client;
protected CustomResource<?, ?> cr;
protected HasMetadata cr;

public OperatorManagedResource(KubernetesClient client, CustomResource<?, ?> cr) {
public OperatorManagedResource(KubernetesClient client, HasMetadata cr) {
this.client = client;
this.cr = cr;
}

protected abstract Optional<HasMetadata> getReconciledResource();
protected abstract Optional<T> getReconciledResource();

public void createOrUpdateReconciled() {
getReconciledResource().ifPresent(resource -> {
public Optional<T> createOrUpdateReconciled() {
return getReconciledResource().map(resource -> {
try {
setInstanceLabels(resource);
setOwnerReferences(resource);
Expand Down Expand Up @@ -80,6 +79,7 @@ public void createOrUpdateReconciled() {
}
Log.debugf("Successfully created or updated resource: %s %s/%s", resource.getKind(), resource.getMetadata().getNamespace(),
resource.getMetadata().getName());
return resource;
} catch (Exception e) {
Log.errorf("Failed to create or update resource %s %s/%s", resource.getKind(), resource.getMetadata().getNamespace(),
resource.getMetadata().getName());
Expand Down
44 changes: 44 additions & 0 deletions operator/src/main/java/org/keycloak/operator/controllers/WatchedSecrets.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* 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.controllers;

import io.fabric8.kubernetes.api.model.apps.StatefulSet;
import io.javaoperatorsdk.operator.processing.event.source.EventSource;

import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;

import java.util.List;

/**
* Provides a mechanism to track secrets
*
* @author Vaclav Muzikar <vmuzikar@redhat.com>
*/
public interface WatchedSecrets {
public static final String WATCHED_SECRETS_LABEL_VALUE = "watched-secret";

/**
* @param deployment mutable resource being reconciled, it will be updated with annotations
*/
void annotateDeployment(List<String> desiredWatchedSecretsNames, Keycloak keycloakCR, StatefulSet deployment);

EventSource getWatchedSecretsEventSource();

void addLabelsToWatchedSecrets(StatefulSet deployment);

}
Loading