Merge pull request #25750 from Sgitario/fix_wrong_service_account

Kubernetes Client: Use the correct service account name
quarkusio · Jun 27, 2022 · 31932db · 31932db
2 parents 66036cd + 0e6b2b3
commit 31932db
Show file tree

Hide file tree

Showing 5 changed files with 164 additions and 4 deletions.
diff --git a/...yment/src/main/java/io/quarkus/kubernetes/deployment/AddRoleBindingResourceDecorator.java b/...yment/src/main/java/io/quarkus/kubernetes/deployment/AddRoleBindingResourceDecorator.java
@@ -0,0 +1,62 @@
+package io.quarkus.kubernetes.deployment;
+
+import java.util.Collections;
+
+import io.dekorate.kubernetes.decorator.ResourceProvidingDecorator;
+import io.dekorate.utils.Strings;
+import io.fabric8.kubernetes.api.model.KubernetesListBuilder;
+import io.fabric8.kubernetes.api.model.ObjectMeta;
+import io.fabric8.kubernetes.api.model.rbac.RoleBindingBuilder;
+
+/**
+ * Workaround for: https://github.com/dekorateio/dekorate/issues/987
+ * Once the issue is fixed in upstream, and we bump the Dekorate version, we should delete this decorator.
+ */
+public class AddRoleBindingResourceDecorator extends ResourceProvidingDecorator<KubernetesListBuilder> {
+
+    private static final String DEFAULT_RBAC_API_GROUP = "rbac.authorization.k8s.io";
+
+    public static enum RoleKind {
+        Role,
+        ClusterRole
+    }
+
+    private final String serviceAccount;
+    private final String name;
+    private final String role;
+    private final RoleKind kind;
+
+    public AddRoleBindingResourceDecorator(String name, String serviceAccount, String role, RoleKind kind) {
+        this.name = name;
+        this.serviceAccount = serviceAccount;
+        this.role = role;
+        this.kind = kind;
+    }
+
+    public void visit(KubernetesListBuilder list) {
+        // If name is null, it will get the first deployment resource.
+        ObjectMeta meta = getMandatoryDeploymentMetadata(list, name);
+        String roleBindingName = meta.getName() + "-" + this.role;
+        String serviceAccount = Strings.isNotNullOrEmpty(this.serviceAccount) ? this.serviceAccount : meta.getName();
+
+        if (contains(list, "rbac.authorization.k8s.io/v1", "RoleBinding", roleBindingName)) {
+            return;
+        }
+
+        list.addToItems(new RoleBindingBuilder()
+                .withNewMetadata()
+                .withName(roleBindingName)
+                .withLabels(Strings.isNotNullOrEmpty(name) ? getMandatoryDeploymentMetadata(list, name).getLabels()
+                        : Collections.emptyMap())
+                .endMetadata()
+                .withNewRoleRef()
+                .withKind(kind.name())
+                .withName(role)
+                .withApiGroup(DEFAULT_RBAC_API_GROUP)
+                .endRoleRef()
+                .addNewSubject()
+                .withKind("ServiceAccount")
+                .withName(serviceAccount)
+                .endSubject());
+    }
+}
diff --git a/...lla/deployment/src/main/java/io/quarkus/kubernetes/deployment/KubernetesCommonHelper.java b/...lla/deployment/src/main/java/io/quarkus/kubernetes/deployment/KubernetesCommonHelper.java
@@ -40,7 +40,6 @@
 import io.dekorate.kubernetes.decorator.AddMountDecorator;
 import io.dekorate.kubernetes.decorator.AddPvcVolumeDecorator;
 import io.dekorate.kubernetes.decorator.AddReadinessProbeDecorator;
-import io.dekorate.kubernetes.decorator.AddRoleBindingResourceDecorator;
 import io.dekorate.kubernetes.decorator.AddSecretVolumeDecorator;
 import io.dekorate.kubernetes.decorator.AddServiceAccountResourceDecorator;
 import io.dekorate.kubernetes.decorator.ApplicationContainerDecorator;
@@ -183,12 +182,13 @@ public static List<DecoratorBuildItem> createDecorators(Optional<Project> projec
 
         //Handle RBAC
         if (!roleBindings.isEmpty()) {
-            result.add(new DecoratorBuildItem(target, new ApplyServiceAccountNameDecorator()));
-            result.add(new DecoratorBuildItem(target, new AddServiceAccountResourceDecorator()));
+            result.add(new DecoratorBuildItem(target, new ApplyServiceAccountNameDecorator(name)));
+            result.add(new DecoratorBuildItem(target, new AddServiceAccountResourceDecorator(name)));
             roles.forEach(r -> result.add(new DecoratorBuildItem(target, new AddRoleResourceDecorator(name, r))));
             roleBindings.forEach(rb -> {
+                String rbName = Strings.isNotNullOrEmpty(rb.getName()) ? rb.getName() : name;
                 result.add(new DecoratorBuildItem(target,
-                        new AddRoleBindingResourceDecorator(rb.getName(), null, rb.getRole(),
+                        new AddRoleBindingResourceDecorator(rbName, name, rb.getRole(),
                                 rb.isClusterWide() ? AddRoleBindingResourceDecorator.RoleKind.ClusterRole
                                         : AddRoleBindingResourceDecorator.RoleKind.Role)));
                 labels.forEach(l -> {

diff --git a/.../src/test/java/io/quarkus/it/kubernetes/WithKubernetesClientAndExistingResourcesTest.java b/.../src/test/java/io/quarkus/it/kubernetes/WithKubernetesClientAndExistingResourcesTest.java
@@ -0,0 +1,65 @@
+package io.quarkus.it.kubernetes;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.Collections;
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.fabric8.kubernetes.api.model.apps.Deployment;
+import io.quarkus.bootstrap.model.AppArtifact;
+import io.quarkus.builder.Version;
+import io.quarkus.kubernetes.spi.CustomProjectRootBuildItem;
+import io.quarkus.test.ProdBuildResults;
+import io.quarkus.test.ProdModeTestResults;
+import io.quarkus.test.QuarkusProdModeTest;
+
+public class WithKubernetesClientAndExistingResourcesTest {
+    private static final String APPLICATION_NAME = "client-existing-resources";
+
+    @RegisterExtension
+    static final QuarkusProdModeTest config = new QuarkusProdModeTest()
+            .withApplicationRoot((jar) -> jar.addClasses(GreetingResource.class))
+            .setApplicationName(APPLICATION_NAME)
+            .setApplicationVersion("0.1-SNAPSHOT")
+            .withConfigurationResource("kubernetes-with-" + APPLICATION_NAME + ".properties")
+            .addCustomResourceEntry(Path.of("src", "main", "kubernetes", "kubernetes.yml"),
+                    "manifests/kubernetes-with-" + APPLICATION_NAME + "/kubernetes.yml")
+            .setForcedDependencies(Collections.singletonList(
+                    new AppArtifact("io.quarkus", "quarkus-kubernetes-client", Version.getVersion())))
+            .addBuildChainCustomizerEntries(
+                    new QuarkusProdModeTest.BuildChainCustomizerEntry(
+                            KubernetesWithCustomResourcesTest.CustomProjectRootBuildItemProducerProdMode.class,
+                            Collections.singletonList(CustomProjectRootBuildItem.class), Collections.emptyList()));
+
+    @ProdBuildResults
+    private ProdModeTestResults prodModeTestResults;
+
+    @Test
+    public void assertGeneratedResources() throws IOException {
+        final Path kubernetesDir = prodModeTestResults.getBuildDir().resolve("kubernetes");
+        assertThat(kubernetesDir)
+                .isDirectoryContaining(p -> p.getFileName().endsWith("kubernetes.json"))
+                .isDirectoryContaining(p -> p.getFileName().endsWith("kubernetes.yml"));
+        List<HasMetadata> kubernetesList = DeserializationUtil
+                .deserializeAsList(kubernetesDir.resolve("kubernetes.yml"));
+
+        assertThat(kubernetesList).filteredOn(h -> "Deployment".equals(h.getKind())).allSatisfy(h -> {
+            Deployment deployment = (Deployment) h;
+            assertThat(deployment.getSpec().getTemplate().getSpec().getServiceAccountName()).isEqualTo(APPLICATION_NAME);
+        });
+
+        assertThat(kubernetesList).filteredOn(h -> "ServiceAccount".equals(h.getKind())).singleElement().satisfies(h -> {
+            assertThat(h.getMetadata().getName()).isEqualTo(APPLICATION_NAME);
+        });
+
+        assertThat(kubernetesList).filteredOn(h -> "RoleBinding".equals(h.getKind())).singleElement().satisfies(h -> {
+            assertThat(h.getMetadata().getName()).isEqualTo(APPLICATION_NAME + "-view");
+        });
+    }
+}
diff --git a/...rkus-standard-way/src/test/resources/kubernetes-with-client-existing-resources.properties b/...rkus-standard-way/src/test/resources/kubernetes-with-client-existing-resources.properties
diff --git a/...way/src/test/resources/manifests/kubernetes-with-client-existing-resources/kubernetes.yml b/...way/src/test/resources/manifests/kubernetes-with-client-existing-resources/kubernetes.yml
@@ -0,0 +1,33 @@
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: my-service
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      name: my-service
+  template:
+    metadata:
+      labels:
+        name: my-service
+    spec:
+      containers:
+        - image: bitnami/mongodb:5.0
+          name: my-service
+---
+apiVersion: v1
+kind: Service
+metadata:
+  labels:
+    name: my-service
+  name: my-service
+spec:
+  ports:
+    - port: 27017
+      protocol: TCP
+      targetPort: 27017
+  selector:
+    name: my-service
+  type: ClusterIP