8000 fix: prevent service account name from being set in multi-namespace mode by shawkins · Pull Request #49036 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

fix: prevent service account name from being set in multi-namespace mode #49036

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

Merged
vmuzikar merged 1 commit into from
May 22, 2026
Merged

fix: prevent service account name from being set in multi-namespace mode #49036

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 17 additions & 7 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 @@ -215,7 +215,7 @@ public static Optional<String> getAppsDomain(Context<Keycloak> context) {

public void updateStatus(Keycloak keycloakCR, StatefulSet existingDeployment, KeycloakStatusAggregator status, Context<Keycloak> context) {
status.apply(b -> b.withSelector(Utils.toSelectorString(Utils.allInstanceLabels(keycloakCR))));
validatePodTemplate(keycloakCR, status);
validatePodTemplate(keycloakCR, status, context);
if (existingDeployment == null) {
status.addNotReadyMessage("No existing StatefulSet found, waiting for creating a new one");
return;
Expand Down Expand Up @@ -251,14 +251,19 @@ public void updateStatus(Keycloak keycloakCR, StatefulSet existingDeployment, Ke
.ifPresent(status::addWarningMessage);
}

static boolean isMultiNamespace(Context<?> context) {
var config = context.getControllerConfiguration().getInformerConfig();
return config.watchAllNamespaces() || config.getNamespaces().size() > 1;
}

public static boolean isRolling(StatefulSet existingDeployment) {
return existingDeployment.getStatus() != null
&& existingDeployment.getStatus().getCurrentRevision() != null
&& existingDeployment.getStatus().getUpdateRevision() != null
&& !existingDeployment.getStatus().getCurrentRevision().equals(existingDeployment.getStatus().getUpdateRevision());
}

public void validatePodTemplate(Keycloak keycloakCR, KeycloakStatusAggregator status) {
public void validatePodTemplate(Keycloak keycloakCR, KeycloakStatusAggregator status, Context<Keycloak> context) {
var spec = KeycloakDeploymentDependentResource.getPodTemplateSpec(keycloakCR);
if (spec.isEmpty()) {
return;
Expand All @@ -274,7 +279,8 @@ public void validatePodTemplate(Keycloak keycloakCR, KeycloakStatusAggregator st
}
}

Optional.ofNullable(overlayTemplate.getSpec()).map(PodSpec::getContainers).flatMap(l -> l.stream().findFirst())
Optional<PodSpec> templateSpec = Optional.ofNullable(overlayTemplate.getSpec());
templateSpec.map(PodSpec::getContainers).flatMap(l -> l.stream().findFirst())
.ifPresent(container -> {
if (container.getName() != null) {
status.addWarningMessage("The name of the keycloak container cannot be modified");
Expand All @@ -288,10 +294,14 @@ public void validatePodTemplate(Keycloak keycloakCR, KeycloakStatusAggregator st
}
});

if (overlayTemplate.getSpec() != null &&
CollectionUtil.isNotEmpty(overlayTemplate.getSpec().getImagePullSecrets())) {
status.addWarningMessage("The imagePullSecrets of the keycloak container cannot be modified using podTemplate");
}
templateSpec.ifPresent(ts -> {
if (CollectionUtil.isNotEmpty(ts.getImagePullSecrets())) {
status.addWarningMessage("The imagePullSecrets of the keycloak container cannot be modified using podTemplate");
}
if (isMultiNamespace(context) && Optional.ofNullable(ts.getServiceAccount()).orElse(ts.getServiceAccountName()) != null) {
status.addWarningMessage("The serviceAccountName cannot be set in a multi-namespace install mode");
}
});
}

private void checkForPodErrors(KeycloakStatusAggregator status, Keycloak keycloak, StatefulSet existingDeployment, Context<Keycloak> context) {
Expand Down
5 changes: 5 additions & 0 deletions .../src/main/java/org/keycloak/operator/controllers/KeycloakDeploymentDependentResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,11 @@ private StatefulSet createBaseDeployment(Keycloak keycloakCR, Context<Keycloak>
.endTemplate()
.withReplicas(keycloakCR.getSpec().getInstances())
.endSpec();

if (KeycloakController.isMultiNamespace(context)) {
baseDeploymentBuilder = baseDeploymentBuilder.editSpec().editTemplate().editSpec().withServiceAccount(null)
.withServiceAccountName(null).endSpec().endTemplate().endSpec();
}

var specBuilder = baseDeploymentBuilder.editSpec().editTemplate().editOrNewSpec();

Expand Down
23 changes: 23 additions & 0 deletions operator/src/test/java/org/keycloak/operator/testsuite/unit/KeycloakControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@

import org.keycloak.operator.controllers.KeycloakController;
import org.keycloak.operator.crds.v2beta1.deployment.Keycloak;
import org.keycloak.operator.crds.v2beta1.deployment.KeycloakBuilder;
import org.keycloak.operator.crds.v2beta1.deployment.KeycloakStatusAggregator;
import org.keycloak.operator.crds.v2beta1.deployment.spec.IngressSpecBuilder;
import org.keycloak.operator.testsuite.utils.CRAssert;
import org.keycloak.operator.testsuite.utils.K8sUtils;

import io.fabric8.kubernetes.client.dsl.Resource;
Expand Down Expand Up @@ -67,5 +70,25 @@ void testCRDefaults() {
assertEquals(1, update.getResource().orElseThrow().getSpec().getInstances());
assertNull(update.getResource().orElseThrow().getSpec().getHostnameSpec().getHostname());
}

@Test
void testUpdateStatus() {
KeycloakController controller = new KeycloakController();
Keycloak kc = K8sUtils.getDefaultKeycloakDeployment();
kc = new KeycloakBuilder(kc).editSpec().withNewUnsupported().withNewPodTemplate().withNewSpec()
.withServiceAccountName("foo").endSpec().endPodTemplate().endUnsupported().endSpec().build();

Context<Keycloak> mockContext = Mockito.mock(Context.class, Mockito.RETURNS_DEEP_STUBS);

KeycloakStatusAggregator agg = new KeycloakStatusAggregator(null, 1L);
controller.updateStatus(kc, null, agg, mockContext);
CRAssert.assertKeycloakStatusCondition(agg.build(), "HasErrors", false, null, 1L).extracting("message").isEqualTo("");

Mockito.when(mockContext.getControllerConfiguration().getInformerConfig().watchAllNamespaces()).thenReturn(true);

agg = new KeycloakStatusAggregator(null, 1L);
controller.updateStatus(kc, null, agg, mockContext);
CRAssert.assertKeycloakStatusCondition(agg.build(), "HasErrors", false, "The serviceAccountName cannot be set in a multi-namespace install mode");
}

}
65 changes: 57 additions & 8 deletions operator/src/test/java/org/keycloak/operator/testsuite/unit/PodTemplateTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -71,6 +72,7 @@
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.config.informer.InformerConfiguration;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedWorkflowAndDependentResourceContext;
import io.quarkus.test.InjectMock;
Expand Down Expand Up @@ -111,10 +113,15 @@ public class PodTemplateTest {

@Inject
KeycloakRealmImportJobDependentResource importJobResource;

Context context;

@BeforeEach
protected void setup() {
this.deployment = new KeycloakDeploymentDependentResource();
context = Mockito.mock(Context.class, Mockito.RETURNS_DEEP_STUBS);
Mockito.when(context.getClient()).thenReturn(Mockito.mock(KubernetesClient.class));
Mockito.when(context.getControllerConfiguration().getInformerConfig()).thenReturn(Mockito.mock(InformerConfiguration.class));
}

private StatefulSet getDeployment(PodTemplateSpec podTemplate, StatefulSet existingDeployment, Consumer<KeycloakSpecBuilder> additionalSpec) {
Expand All @@ -125,7 +132,7 @@ private StatefulSet getDeployment(PodTemplateSpec podTemplate, StatefulSet exist
.endSelector().endSpec().build();

//noinspection unchecked
Context context = mockContext(null);
mockContext(null);
return deployment.initialDesired(kc, context);
}

Expand All @@ -147,16 +154,13 @@ private Keycloak createKeycloak(PodTemplateSpec podTemplate, Consumer<KeycloakSp
return kc;
}

private Context<Keycloak> mockContext(StatefulSet existingDeployment) {
Context<Keycloak> context = Mockito.mock(Context.class);
private void mockContext(StatefulSet existingDeployment) {
ManagedWorkflowAndDependentResourceContext managedWorkflowAndDependentResourceContext = Mockito.mock(ManagedWorkflowAndDependentResourceContext.class);
Mockito.when(context.managedWorkflowAndDependentResourceContext()).thenReturn(managedWorkflowAndDependentResourceContext);
Mockito.when(managedWorkflowAndDependentResourceContext.get(OLD_DEPLOYMENT_KEY, StatefulSet.class)).thenReturn(Optional.ofNullable(existingDeployment));
Mockito.when(managedWorkflowAndDependentResourceContext.getMandatory(OPERATOR_CONFIG_KEY, Config.class)).thenReturn(operatorConfig);
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 context;
}

private StatefulSet getDeployment(PodTemplateSpec podTemplate, StatefulSet existingDeployment) {
Expand Down Expand Up @@ -404,6 +408,51 @@ public void testAnnotationsAreNotMerged() {
// Assert
assertThat(podTemplate.getMetadata().getAnnotations()).containsEntry("two", "2");
}

@Test
public void testServiceAccountName() {
// in a single namespace, we'll still allow setting via the template

// Arrange
var additionalPodTemplate = new PodTemplateSpecBuilder()
.withNewSpec()
.withServiceAccount("foo")
.endSpec()
.build();

// Act
var podTemplate = getDeployment(additionalPodTemplate).getSpec().getTemplate();

// Assert
assertThat(podTemplate.getSpec().getServiceAccount()).isEqualTo("foo");

// in multinamespace we won't

Mockito.when(context.getControllerConfiguration().getInformerConfig().watchAllNamespaces()).thenReturn(true);

// Act
podTemplate = getDeployment(additionalPodTemplate).getSpec().getTemplate();

// Assert
assertThat(podTemplate.getSpec().getServiceAccount()).isNull();

// test again with serviceAccountName

additionalPodTemplate = new PodTemplateSpecBuilder()
.withNewSpec()
.withServiceAccountName("bar")
.endSpec()
.build();

Mockito.when(context.getControllerConfiguration().getInformerConfig().watchAllNamespaces()).thenReturn(false);
Mockito.when(context.getControllerConfiguration().getInformerConfig().getNamespaces()).thenReturn(Set.of("one", "two"));

// Act
podTemplate = getDeployment(additionalPodTemplate).getSpec().getTemplate();

// Assert
assertThat(podTemplate.getSpec().getServiceAccountName()).isNull();
}

@Test
public void testHttpManagment() {
Expand Down Expand Up @@ -789,7 +838,7 @@ private Job getUpdateJob(Consumer<KeycloakSpecBuilder> newSpec, Consumer<Keycloa
StatefulSetBuilder desired = getDeployment(null, existingStatefulSet, newSpec).toBuilder();

// setup the mock context
Context<Keycloak> context = mockContext(null);
mockContext(null);
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());
Expand All @@ -802,7 +851,7 @@ private Job getImportJob(Consumer<KeycloakSpecBuilder> keycloakSpec, Consumer<Ke
existingModifier.accept(existingBuilder);
StatefulSet existingStatefulSet = existingBuilder.build();

Context context = mockContext(existingStatefulSet);
mockContext(existingStatefulSet);
var kc = createKeycloak(null, keycloakSpec);
Mockito.when(context.managedWorkflowAndDependentResourceContext().getMandatory(ContextUtils.KEYCLOAK, Keycloak.class)).thenReturn(kc);

Expand Down Expand Up @@ -893,7 +942,7 @@ public void testFieldRemovalForInitContainer() {
assertNull(job.getSpec().getTemplate().getSpec().getInitContainers().get(0).getLifecycle());
assertNull(job.getSpec().getTemplate().getSpec().getInitContainers().get(0).getRestartPolicy());
}

@Test
public void testEnvVars() {
var statefulSet = getDeployment(null, null, builder -> builder.addNewEnv("JAVA_OPTS", "my opts")
Expand Down
Loading
0