fix: only add PATCH if the dependent is using SSA

quarkiverse · Aug 28, 2024 · d6732ae · d6732ae
1 parent 57ea977
commit d6732ae
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 20 deletions.
diff --git a/annotations/src/main/java/io/quarkiverse/operatorsdk/annotations/RBACVerbs.java b/annotations/src/main/java/io/quarkiverse/operatorsdk/annotations/RBACVerbs.java
@@ -12,7 +12,6 @@ public class RBACVerbs {
     public static final String WATCH = "watch";
     public static final String DELETE = "delete";
     public static final String[] UPDATE_VERBS = new String[] { PATCH, UPDATE };
-    public static final String[] CREATE_VERBS = new String[] { CREATE, PATCH };
     public static final String[] READ_VERBS = new String[] { GET, LIST, WATCH };
     public static final String[] ALL_COMMON_VERBS;
 

diff --git a/...loyment/src/main/java/io/quarkiverse/operatorsdk/deployment/AddClusterRolesDecorator.java b/...loyment/src/main/java/io/quarkiverse/operatorsdk/deployment/AddClusterRolesDecorator.java
@@ -6,6 +6,7 @@
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.stream.Collectors;
@@ -20,11 +21,15 @@
 import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBuilder;
 import io.fabric8.kubernetes.api.model.rbac.PolicyRule;
 import io.fabric8.kubernetes.api.model.rbac.PolicyRuleBuilder;
+import io.javaoperatorsdk.operator.api.config.ConfigurationService;
 import io.javaoperatorsdk.operator.api.config.Utils;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter;
 import io.javaoperatorsdk.operator.processing.dependent.Creator;
 import io.javaoperatorsdk.operator.processing.dependent.Updater;
 import io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesDependentResource;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.ResourceUpdaterMatcher;
 import io.quarkiverse.operatorsdk.annotations.RBACVerbs;
 import io.quarkiverse.operatorsdk.runtime.DependentResourceSpecMetadata;
 import io.quarkiverse.operatorsdk.runtime.QuarkusControllerConfiguration;
@@ -113,41 +118,69 @@ private static Set<PolicyRule> getClusterRolePolicyRulesFromDependentResources(Q
             if (HasMetadata.class.isAssignableFrom(associatedResourceClass)) {
                 var resourceGroup = HasMetadata.getGroup(associatedResourceClass);
                 var resourcePlural = HasMetadata.getPlural(associatedResourceClass);
-                if (GenericKubernetesDependentResource.class.isAssignableFrom(dependentResourceClass)) {
+
+                final var verbs = new TreeSet<>(List.of(RBACVerbs.READ_VERBS));
+                if (Updater.class.isAssignableFrom(dependentResourceClass)) {
+                    verbs.addAll(List.of(RBACVerbs.UPDATE_VERBS));
+                }
+                if (Deleter.class.isAssignableFrom(dependentResourceClass)) {
+                    verbs.add(RBACVerbs.DELETE);
+                }
+                if (Creator.class.isAssignableFrom(dependentResourceClass)) {
+                    verbs.add(RBACVerbs.CREATE);
+                }
+
+                // Check if we're dealing with typeless Kubernetes resource or if we need to deal with SSA
+                if (KubernetesDependentResource.class.isAssignableFrom(dependentResourceClass)) {
                     try {
                         @SuppressWarnings({ "unchecked", "rawtypes" })
-                        final var genericKubernetesResource = Utils.instantiate(
-                                (Class<? extends GenericKubernetesDependentResource>) dependentResourceClass,
-                                GenericKubernetesDependentResource.class, ADD_CLUSTER_ROLES_DECORATOR);
-                        final var gvk = genericKubernetesResource.getGroupVersionKind();
-                        resourceGroup = gvk.getGroup();
-                        // todo: use plural form on GVK if available, see https://github.com/operator-framework/java-operator-sdk/pull/2515
-                        resourcePlural = Pluralize.toPlural(gvk.getKind());
+                        var kubeResource = Utils.instantiate(
+                                (Class<? extends KubernetesDependentResource>) dependentResourceClass,
+                                KubernetesDependentResource.class, ADD_CLUSTER_ROLES_DECORATOR);
+
+                        if (kubeResource instanceof GenericKubernetesDependentResource<? extends HasMetadata> genericKubeRes) {
+                            final var gvk = genericKubeRes.getGroupVersionKind();
+                            resourceGroup = gvk.getGroup();
+                            // todo: use plural form on GVK if available, see https://github.com/operator-framework/java-operator-sdk/pull/2515
+                            resourcePlural = Pluralize.toPlural(gvk.getKind());
+                        }
+
+                        // if we use SSA and the dependent resource class is not excluded from using SSA, we also need PATCH permissions for finalizer
+                        // todo: replace by using ConfigurationService.isUsingSSA once available see https://github.com/operator-framework/java-operator-sdk/pull/2516
+                        if (isUsingSSA(kubeResource, cri.getConfigurationService())) {
+                            verbs.add(RBACVerbs.PATCH);
+                        }
                     } catch (Exception e) {
                         log.warn("Ignoring " + dependentResourceClass.getName()
                                 + " for generic resource role processing as it cannot be instantiated", e);
                     }
                 }
-                final var verbs = new TreeSet<>(List.of(RBACVerbs.READ_VERBS));
                 final var dependentRule = new PolicyRuleBuilder()
                         .addToApiGroups(resourceGroup)
                         .addToResources(resourcePlural);
-                if (Updater.class.isAssignableFrom(dependentResourceClass)) {
-                    verbs.addAll(List.of(RBACVerbs.UPDATE_VERBS));
-                }
-                if (Deleter.class.isAssignableFrom(dependentResourceClass)) {
-                    verbs.add(RBACVerbs.DELETE);
-                }
-                if (Creator.class.isAssignableFrom(dependentResourceClass)) {
-                    verbs.addAll(List.of(RBACVerbs.CREATE_VERBS));
-                }
+
                 dependentRule.addToVerbs(verbs.toArray(String[]::new));
                 rules.add(dependentRule.build());
             }
         });
         return rules;
     }
 
+    private static boolean isUsingSSA(KubernetesDependentResource<?, ?> dependentResource,
+            ConfigurationService configurationService) {
+        if (dependentResource instanceof ResourceUpdaterMatcher) {
+            return false;
+        }
+        Optional<Boolean> useSSAConfig = dependentResource.configuration()
+                .flatMap(KubernetesDependentResourceConfig::useSSA);
+        // don't use SSA for certain resources by default, only if explicitly overriden
+        if (useSSAConfig.isEmpty()
+                && configurationService.defaultNonSSAResource().contains(dependentResource.resourceType())) {
+            return false;
+        }
+        return useSSAConfig.orElse(configurationService.ssaBasedCreateUpdateMatchForDependentResources());
+    }
+
     private static PolicyRule getClusterRolePolicyRuleFromPrimaryResource(QuarkusControllerConfiguration<?> cri) {
         final var rule = new PolicyRuleBuilder();
         final var resourceClass = cri.getResourceClass();

diff --git a/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/OperatorSDKTest.java b/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/OperatorSDKTest.java
@@ -117,7 +117,7 @@ public void shouldCreateRolesAndRoleBindings() throws IOException {
                     assertTrue(rules.stream()
                             .filter(rule -> rule.getResources().equals(List.of(HasMetadata.getPlural(
                                     Service.class))))
-                            .anyMatch(rule -> hasReadAndAdditionalVerbsOnly(rule, CREATE_VERBS)));
+                            .anyMatch(rule -> hasReadAndAdditionalVerbsOnly(rule, CREATE)));
                     assertTrue(rules.stream()
                             .filter(rule -> rule.getResources().equals(List.of(HasMetadata.getPlural(ConfigMap.class))))
                             .anyMatch(OperatorSDKTest::hasOnlyCommonVerbs));