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

closes: #48382

Signed-off-by: Steve Hawkins <shawkins@redhat.com>

operator/src/main/java/org/keycloak/operator/controllers/KeycloakController.java

@@ -215,7 +215,7 @@ public class KeycloakController implements Reconciler<Keycloak> {
 
     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;
@@ -251,6 +251,11 @@ public class KeycloakController implements Reconciler<Keycloak> {
                 .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
@@ -258,7 +263,7 @@ public class KeycloakController implements Reconciler<Keycloak> {
                 && !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;
@@ -274,7 +279,8 @@ public class KeycloakController implements Reconciler<Keycloak> {
             }
         }
 
-        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");
@@ -288,10 +294,14 @@ public class KeycloakController implements Reconciler<Keycloak> {
                     }
                 });
 
-        if (overlayTemplate.getSpec() != null &&
+        templateSpec.ifPresent(ts -> {
-            CollectionUtil.isNotEmpty(overlayTemplate.getSpec().getImagePullSecrets())) {
+            if (CollectionUtil.isNotEmpty(ts.getImagePullSecrets())) {
-            status.addWarningMessage("The imagePullSecrets of the keycloak container cannot be modified using podTemplate");
+                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) {

operator/src/main/java/org/keycloak/operator/controllers/KeycloakDeploymentDependentResource.java

@@ -313,6 +313,11 @@ public class KeycloakDeploymentDependentResource extends VersionTolerantCRUDKube
                     .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();
 

operator/src/test/java/org/keycloak/operator/testsuite/unit/KeycloakControllerTest.java

@@ -19,7 +19,10 @@ package org.keycloak.operator.testsuite.unit;
 
 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;
@@ -67,5 +70,25 @@ class KeycloakControllerTest {
         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");
+    }
 
 }

operator/src/test/java/org/keycloak/operator/testsuite/unit/PodTemplateTest.java

@@ -20,6 +20,7 @@ package org.keycloak.operator.testsuite.unit;
 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;
@@ -71,6 +72,7 @@ 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.config.informer.InformerConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedWorkflowAndDependentResourceContext;
 import io.quarkus.test.InjectMock;
@@ -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) {
@@ -125,7 +132,7 @@ public class PodTemplateTest {
                 .endSelector().endSpec().build();
 
         //noinspection unchecked
-        Context context = mockContext(null);
+        mockContext(null);
         return deployment.initialDesired(kc, context);
     }
 
@@ -147,16 +154,13 @@ public class PodTemplateTest {
         return kc;
     }
 
-    private Context<Keycloak> mockContext(StatefulSet existingDeployment) {
+    private void mockContext(StatefulSet existingDeployment) {
-        Context<Keycloak> context = Mockito.mock(Context.class);
         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) {
@@ -404,6 +408,51 @@ public class PodTemplateTest {
         // 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() {
@@ -789,7 +838,7 @@ public class PodTemplateTest {
         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());
@@ -802,7 +851,7 @@ public class PodTemplateTest {
         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);
 
@@ -893,7 +942,7 @@ public class PodTemplateTest {
         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")