fix (jkube-kit/enricher) : ServiceAccountEnricher handles `serviceAcc…

…ount` configuration option (eclipse-jkube#2187) ServiceAccountEnricher used to only handle `serviceAccounts` resource configuration option. `serviceAccount` used to be used by PodTemplateHandler while generating opinionated controllers. Add a case for handling `serviceAccount` in ServiceAccountEnricher Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia · Jun 1, 2023 · 4c8e49b · 4c8e49b
1 parent d1efdb0
commit 4c8e49b
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -28,6 +28,7 @@ Usage:
 * Fix #2166: Potential command line injection in SpringBootWatcher
 * Fix #2170: `internal-microservice` profile prevents Service exposure
 * Fix #2174: Profile merge constructor accounts for parentProfile field
+* Fix #2187: `serviceAccount` configuration option has stopped working
 
 ### 1.12.0 (2023-04-03)
 * Fix #1179: Move storageClass related functionality out of VolumePermissionEnricher to PersistentVolumeClaimStorageClassEnricher

diff --git a/...src/test/java/org/eclipse/jkube/gradle/plugin/tests/ServiceAccountNameViaGroovyDSLIT.java b/...src/test/java/org/eclipse/jkube/gradle/plugin/tests/ServiceAccountNameViaGroovyDSLIT.java
@@ -36,8 +36,8 @@ class ServiceAccountNameViaGroovyDSLIT {
 
   static Stream<Arguments> testInput() {
     return Stream.of(
-//        arguments("no-fragment", new String[] {}),
-//        arguments("unrelated-fragment", new String[] {"-Pjkube.resourceDir=./regularFragmentDir"}),
+        arguments("no-fragment", new String[] {}),
+        arguments("unrelated-fragment", new String[] {"-Pjkube.resourceDir=./regularFragmentDir"}),
         arguments("fragment-overriding-serviceaccount", new String[] {"-Pjkube.resourceDir=./overridingServiceAccountNameFragmentDir"})
     );
   }

diff --git a/...cher/generic/src/main/java/org/eclipse/jkube/enricher/generic/ServiceAccountEnricher.java b/...cher/generic/src/main/java/org/eclipse/jkube/enricher/generic/ServiceAccountEnricher.java
@@ -16,12 +16,15 @@
 import io.fabric8.kubernetes.api.builder.TypedVisitor;
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.api.model.KubernetesListBuilder;
+import io.fabric8.kubernetes.api.model.PodSpec;
 import io.fabric8.kubernetes.api.model.ServiceAccount;
 import io.fabric8.kubernetes.api.model.ServiceAccountBuilder;
 import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder;
 import lombok.AllArgsConstructor;
 import lombok.Getter;
+import org.apache.commons.lang3.StringUtils;
 import org.eclipse.jkube.kit.common.Configs;
+import org.eclipse.jkube.kit.common.util.JKubeProjectUtil;
 import org.eclipse.jkube.kit.config.resource.PlatformMode;
 import org.eclipse.jkube.kit.config.resource.ResourceConfig;
 import org.eclipse.jkube.kit.config.resource.ServiceAccountConfig;
@@ -61,6 +64,9 @@ public void create(PlatformMode mode, KubernetesListBuilder builder) {
                 .filter(sa -> sa.getDeploymentRef() != null)
                 .collect(Collectors.toMap(ServiceAccountConfig::getDeploymentRef, ServiceAccountConfig::getName)));
         }
+        if (resourceConfig != null && StringUtils.isNotBlank(resourceConfig.getServiceAccount())) {
+            deploymentToSaPair.put(JKubeProjectUtil.createDefaultResourceName(getContext().getGav().getSanitizedArtifactId()), resourceConfig.getServiceAccount());
+        }
         builder.addAllToServiceAccountItems(createServiceAccountFromResourceConfig(resourceConfig));
         builder.addAllToServiceAccountItems(createServiceAccountsReferencedInDeployment(builder, deploymentToSaPair));
     }
@@ -88,13 +94,16 @@ public void visit(DeploymentBuilder deploymentBuilder) {
                     serviceAccounts.add(createServiceAccount(serviceAccountName));
                 }
                 if(deploymentToSaPair.containsKey(deploymentBuilder.buildMetadata().getName())) {
-                    deploymentBuilder.editSpec()
-                        .editTemplate()
-                        .editSpec()
-                        .withServiceAccountName(deploymentToSaPair.get(deploymentBuilder.buildMetadata().getName()))
-                        .endSpec()
-                        .endTemplate()
-                        .endSpec();
+                    PodSpec podSpec = deploymentBuilder.buildSpec().getTemplate().getSpec();
+                    if (StringUtils.isBlank(podSpec.getServiceAccount()) && StringUtils.isBlank(podSpec.getServiceAccountName())) {
+                        deploymentBuilder.editSpec()
+                            .editTemplate()
+                            .editSpec()
+                            .withServiceAccountName(deploymentToSaPair.get(deploymentBuilder.buildMetadata().getName()))
+                            .endSpec()
+                            .endTemplate()
+                            .endSpec();
+                    }
                 }
             }
         });

diff --git a/.../generic/src/test/java/org/eclipse/jkube/enricher/generic/ServiceAccountEnricherTest.java b/.../generic/src/test/java/org/eclipse/jkube/enricher/generic/ServiceAccountEnricherTest.java
@@ -18,6 +18,9 @@
 import io.fabric8.kubernetes.api.model.ServiceAccountBuilder;
 import io.fabric8.kubernetes.api.model.apps.Deployment;
 import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder;
+import io.fabric8.kubernetes.api.model.apps.DeploymentSpec;
+import io.fabric8.kubernetes.api.model.PodSpec;
+import io.fabric8.kubernetes.api.model.PodTemplateSpec;
 import org.assertj.core.api.InstanceOfAssertFactories;
 import org.eclipse.jkube.kit.common.JavaProject;
 import org.eclipse.jkube.kit.config.resource.PlatformMode;
@@ -128,6 +131,41 @@ void create_withSkipCreateEnabledAndFragment_shouldNotCreateServiceAccount() {
             .hasOnlyElementsOfType(Deployment.class);
     }
 
+    @Test
+    void create_whenResourceConfigServiceAccountSet_thenUseResourceConfigServiceAccount() {
+        // Given
+        context.getProperties().put("jkube.enricher.jkube-serviceaccount.skipCreate", "true");
+        context = context.toBuilder()
+            .resources(ResourceConfig.builder().serviceAccount("sa-from-config").build())
+            .project(JavaProject.builder().groupId("org.example").artifactId("cheese").version("0.0.1").build())
+            .build();
+        final KubernetesListBuilder builder = new KubernetesListBuilder();
+        builder.addToItems(createNewDeploymentFragment());
+
+        // When
+        new ServiceAccountEnricher(context).create(PlatformMode.kubernetes, builder);
+
+        // Then
+        assertControllerHasServiceAccountNameSet(builder, "sa-from-config");
+    }
+
+    @Test
+    void create_whenBothFragmentAndResourceConfigHaveServiceAccountSet_thenUseFragmentServiceAccount() {
+        // Given
+        context.getProperties().put("jkube.enricher.jkube-serviceaccount.skipCreate", "true");
+        context = context.toBuilder()
+            .resources(ResourceConfig.builder().serviceAccount("sa-from-config").build())
+            .build();
+        final KubernetesListBuilder builder = new KubernetesListBuilder();
+        builder.addToItems(createNewDeploymentFragmentWithServiceAccountNameConfigured("sa-from-fragment"));
+
+        // When
+        new ServiceAccountEnricher(context).create(PlatformMode.kubernetes, builder);
+
+        // Then
+        assertControllerHasServiceAccountNameSet(builder, "sa-from-fragment");
+    }
+
     private void enrichAndAssert(KubernetesListBuilder builder, String expectedServiceAccountName) {
         final ServiceAccountEnricher saEnricher = new ServiceAccountEnricher(context);
         saEnricher.create(PlatformMode.kubernetes, builder);
@@ -176,4 +214,16 @@ private DeploymentBuilder createNewDeploymentFragmentWithServiceAccountNameConfi
             .endTemplate()
             .endSpec();
     }
+
+    private void assertControllerHasServiceAccountNameSet(KubernetesListBuilder builder, String serviceAccountName) {
+        assertThat(builder.buildItems())
+            .hasSize(1)
+            .asList()
+            .singleElement(InstanceOfAssertFactories.type(Deployment.class))
+            .extracting(Deployment::getSpec)
+            .extracting(DeploymentSpec::getTemplate)
+            .extracting(PodTemplateSpec::getSpec)
+            .extracting(PodSpec::getServiceAccountName)
+            .isEqualTo(serviceAccountName);
+    }
 }