diff --git a/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ConfigurationUtils.java b/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ConfigurationUtils.java index 96b9ac1ba..2f82960c6 100644 --- a/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ConfigurationUtils.java +++ b/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ConfigurationUtils.java @@ -80,7 +80,12 @@ public static ClassInfo getClassInfoForInstantiation(AnnotationValue toInsta Class interfaceClass, IndexView index) { final var expectedTypeDN = toInstantiate.asClass().name(); - return index.getClassByName(expectedTypeDN); + final var clazz = index.getClassByName(expectedTypeDN); + if (clazz == null) { + throw new IllegalStateException(expectedTypeDN + + " class was not found in Jandex index. If you see this in a test, don't forget to add the class to the application root when setting up the test."); + } + return clazz; } @SuppressWarnings("unchecked") diff --git a/core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/AddClusterRolesDecorator.java b/core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/AddClusterRolesDecorator.java index 80901c6ac..1951ae7c8 100644 --- a/core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/AddClusterRolesDecorator.java +++ b/core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/AddClusterRolesDecorator.java @@ -1,21 +1,35 @@ package io.quarkiverse.operatorsdk.deployment; -import java.util.*; -import java.util.function.Function; - -import org.jetbrains.annotations.NotNull; +import java.util.ArrayList; +import java.util.Collection; +import java.util.LinkedHashMap; +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; + +import org.jboss.logging.Logger; import io.dekorate.kubernetes.decorator.ResourceProvidingDecorator; +import io.fabric8.kubernetes.api.Pluralize; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.KubernetesListBuilder; import io.fabric8.kubernetes.api.model.rbac.ClusterRole; 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; @@ -32,6 +46,8 @@ public class AddClusterRolesDecorator extends ResourceProvidingDecorator> configs; private final boolean validateCRDs; @@ -57,21 +73,40 @@ public void visit(KubernetesListBuilder list) { } public static ClusterRole createClusterRole(QuarkusControllerConfiguration cri) { + final var rules = new LinkedHashMap(); + final var clusterRolePolicyRuleFromPrimaryResource = getClusterRolePolicyRuleFromPrimaryResource(cri); + final var primaryRuleKey = getKeyFor(clusterRolePolicyRuleFromPrimaryResource); + rules.put(primaryRuleKey, clusterRolePolicyRuleFromPrimaryResource); - Set collectedRules = new LinkedHashSet<>(); - collectedRules.add(getClusterRolePolicyRuleFromPrimaryResource(cri)); - collectedRules.addAll(getClusterRolePolicyRulesFromDependentResources(cri)); - collectedRules.addAll(cri.getAdditionalRBACRules()); + collectAndMergeIfNeededRulesFrom(getClusterRolePolicyRulesFromDependentResources(cri), rules); + collectAndMergeIfNeededRulesFrom(cri.getAdditionalRBACRules(), rules); return new ClusterRoleBuilder() .withNewMetadata() .withName(getClusterRoleName(cri.getName())) .endMetadata() - .addAllToRules(mergePolicyRulesOfSameGroupsAndKinds(collectedRules)) + .addAllToRules(rules.values()) .build(); } - @NotNull + private static void collectAndMergeIfNeededRulesFrom(Collection newRules, + Map existingRules) { + newRules.forEach(newPolicyRule -> { + final var key = getKeyFor(newPolicyRule); + existingRules.merge(key, newPolicyRule, (existing, npr) -> { + Set verbs1 = new TreeSet<>(existing.getVerbs()); + verbs1.addAll(npr.getVerbs()); + existing.setVerbs(new ArrayList<>(verbs1)); + return existing; + }); + }); + } + + private static String getKeyFor(PolicyRule rule) { + return rule.getApiGroups().stream().sorted().collect(Collectors.joining("-")) + "/" + + rule.getResources().stream().sorted().collect(Collectors.joining("-")); + } + private static Set getClusterRolePolicyRulesFromDependentResources(QuarkusControllerConfiguration cri) { Set rules = new LinkedHashSet<>(); final Map> dependentsMetadata = cri.getDependentsMetadata(); @@ -81,48 +116,71 @@ private static Set getClusterRolePolicyRulesFromDependentResources(Q // only process Kubernetes dependents if (HasMetadata.class.isAssignableFrom(associatedResourceClass)) { - String resourceGroup = HasMetadata.getGroup(associatedResourceClass); - String resourcePlural = HasMetadata.getPlural(associatedResourceClass); + var resourceGroup = HasMetadata.getGroup(associatedResourceClass); + var resourcePlural = HasMetadata.getPlural(associatedResourceClass); - // https://github.com/operator-framework/java-operator-sdk/pull/2515 - // Workaround for typeless resource, no necessary when this pull merged - if (GenericKubernetesDependentResource.class.isAssignableFrom(dependentResourceClass)) { - try { - // Only applied class with non-parameter constructor - if (Arrays.stream(dependentResourceClass.getConstructors()).anyMatch(i -> i.getParameterCount() == 0)) { - @SuppressWarnings("rawtypes") - GenericKubernetesDependentResource genericKubernetesResource = (GenericKubernetesDependentResource) dependentResourceClass - .getConstructor().newInstance(); - resourceGroup = genericKubernetesResource.getGroupVersionKind().getGroup(); - resourcePlural = "*"; - } - } catch (Exception e) { - throw new RuntimeException(e); - } - } - - final var dependentRule = new PolicyRuleBuilder() - .addToApiGroups(resourceGroup) - .addToResources(resourcePlural) - .addToVerbs(RBACVerbs.READ_VERBS); + final var verbs = new TreeSet<>(List.of(RBACVerbs.READ_VERBS)); if (Updater.class.isAssignableFrom(dependentResourceClass)) { - dependentRule.addToVerbs(RBACVerbs.UPDATE_VERBS); + verbs.addAll(List.of(RBACVerbs.UPDATE_VERBS)); } if (Deleter.class.isAssignableFrom(dependentResourceClass)) { - dependentRule.addToVerbs(RBACVerbs.DELETE); + verbs.add(RBACVerbs.DELETE); } if (Creator.class.isAssignableFrom(dependentResourceClass)) { - dependentRule.addToVerbs(RBACVerbs.CREATE); - if (!dependentRule.getVerbs().contains(RBACVerbs.PATCH)) { - dependentRule.addToVerbs(RBACVerbs.PATCH); + 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" }) + var kubeResource = Utils.instantiate( + (Class) dependentResourceClass, + KubernetesDependentResource.class, ADD_CLUSTER_ROLES_DECORATOR); + + if (kubeResource instanceof GenericKubernetesDependentResource 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 dependentRule = new PolicyRuleBuilder() + .addToApiGroups(resourceGroup) + .addToResources(resourcePlural); + + 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 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(); @@ -144,57 +202,6 @@ private static PolicyRule getClusterRolePolicyRuleFromPrimaryResource(QuarkusCon return rule.build(); } - /** - * Remove duplicated rules with same groups and resources, from which merge all verbs - * - * @param collectedRules may contain duplicated rules with same groups and resources, but different verbs - * @return no duplicated rules - */ - @NotNull - private static Set mergePolicyRulesOfSameGroupsAndKinds(Set collectedRules) { - Set mergedRules = new LinkedHashSet<>(); - collectedRules.stream() - .map(wrapEqualOfGroupsAndKinds()).forEach(i -> { - if (!mergedRules.add(i)) { - mergedRules.stream().filter(j -> Objects.equals(j, i)).findAny().ifPresent(r -> { - Set verbs1 = new LinkedHashSet<>(r.getVerbs()); - Set verbs2 = new LinkedHashSet<>(i.getVerbs()); - verbs1.addAll(verbs2); - r.setVerbs(verbs1.stream().toList()); - }); - } - }); - return mergedRules; - } - - @NotNull - private static Function wrapEqualOfGroupsAndKinds() { - return i -> new PolicyRule(i.getApiGroups(), i.getNonResourceURLs(), i.getResourceNames(), i.getResources(), - i.getVerbs()) { - @Override - public boolean equals(Object o) { - if (o == null) - return false; - if (o instanceof PolicyRule) { - if (Objects.equals( - this.getApiGroups().stream().sorted().toList(), - ((PolicyRule) o).getApiGroups().stream().sorted().toList())) { - return Objects.equals( - getResources().stream().sorted().toList(), - ((PolicyRule) o).getResources().stream().sorted().toList()); - } - } - return false; - } - - @Override - public int hashCode() { - // equals method called only with same hashCode - return 0; - } - }; - } - public static String getClusterRoleName(String controller) { return controller + "-cluster-role"; } 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 index 846dc0d94..4ddfa9976 100644 --- a/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/OperatorSDKTest.java +++ b/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/OperatorSDKTest.java @@ -14,6 +14,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import io.fabric8.kubernetes.api.Pluralize; import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.Secret; @@ -21,6 +22,7 @@ import io.fabric8.kubernetes.api.model.ServiceAccount; import io.fabric8.kubernetes.api.model.rbac.ClusterRole; import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBinding; +import io.fabric8.kubernetes.api.model.rbac.PolicyRule; import io.fabric8.kubernetes.api.model.rbac.RoleBinding; import io.fabric8.kubernetes.client.CustomResource; import io.fabric8.kubernetes.client.utils.Serialization; @@ -28,13 +30,26 @@ import io.quarkiverse.operatorsdk.annotations.RBACVerbs; import io.quarkiverse.operatorsdk.deployment.AddClusterRolesDecorator; import io.quarkiverse.operatorsdk.deployment.AddRoleBindingsDecorator; -import io.quarkiverse.operatorsdk.test.sources.*; +import io.quarkiverse.operatorsdk.test.sources.CRUDConfigMap; +import io.quarkiverse.operatorsdk.test.sources.CreateOnlyService; +import io.quarkiverse.operatorsdk.test.sources.Foo; +import io.quarkiverse.operatorsdk.test.sources.NonKubeResource; +import io.quarkiverse.operatorsdk.test.sources.ReadOnlySecret; +import io.quarkiverse.operatorsdk.test.sources.SimpleCR; +import io.quarkiverse.operatorsdk.test.sources.SimpleReconciler; +import io.quarkiverse.operatorsdk.test.sources.SimpleSpec; +import io.quarkiverse.operatorsdk.test.sources.SimpleStatus; +import io.quarkiverse.operatorsdk.test.sources.TestCR; +import io.quarkiverse.operatorsdk.test.sources.TestReconciler; +import io.quarkiverse.operatorsdk.test.sources.TypelessAnotherKubeResource; +import io.quarkiverse.operatorsdk.test.sources.TypelessKubeResource; import io.quarkus.test.ProdBuildResults; import io.quarkus.test.ProdModeTestResults; import io.quarkus.test.QuarkusProdModeTest; public class OperatorSDKTest { + public static final List READ_VERBS_LIST = Arrays.asList(READ_VERBS); private static final String APPLICATION_NAME = "test"; // Start unit test with your extension loaded @RegisterExtension @@ -49,6 +64,22 @@ public class OperatorSDKTest { @ProdBuildResults private ProdModeTestResults prodModeTestResults; + private static boolean hasReadAndAdditionalVerbsOnly(PolicyRule rule, String... additionalVerbs) { + final var verbs = rule.getVerbs(); + return (verbs.size() == READ_VERBS_LIST.size() + additionalVerbs.length) && verbs.containsAll(READ_VERBS_LIST) + && verbs.containsAll(List.of(additionalVerbs)); + } + + private static boolean isReadOnly(PolicyRule rule) { + return rule.getVerbs().equals(READ_VERBS_LIST); + } + + private static boolean hasOnlyCommonVerbs(PolicyRule rule) { + final var verbs = rule.getVerbs(); + return verbs.size() == ALL_COMMON_VERBS.length + && verbs.containsAll(Arrays.asList(ALL_COMMON_VERBS)); + } + @Test public void shouldCreateRolesAndRoleBindings() throws IOException { final var kubernetesDir = prodModeTestResults.getBuildDir().resolve("kubernetes"); @@ -82,33 +113,27 @@ public void shouldCreateRolesAndRoleBindings() throws IOException { })); assertTrue(rules.stream() .filter(rule -> rule.getResources().equals(List.of(HasMetadata.getPlural(Secret.class)))) - .anyMatch(rule -> rule.getVerbs().equals(Arrays.asList(READ_VERBS)))); + .anyMatch(OperatorSDKTest::isReadOnly)); assertTrue(rules.stream() .filter(rule -> rule.getResources().equals(List.of(HasMetadata.getPlural( Service.class)))) - .anyMatch(rule -> rule.getVerbs().containsAll(Arrays.asList(READ_VERBS)) - && rule.getVerbs().contains(CREATE))); + .anyMatch(rule -> hasReadAndAdditionalVerbsOnly(rule, CREATE))); assertTrue(rules.stream() .filter(rule -> rule.getResources().equals(List.of(HasMetadata.getPlural(ConfigMap.class)))) - .anyMatch(rule -> { - final var verbs = rule.getVerbs(); - return verbs.size() == ALL_COMMON_VERBS.length - && verbs.containsAll(Arrays.asList(ALL_COMMON_VERBS)); - })); + .anyMatch(OperatorSDKTest::hasOnlyCommonVerbs)); assertTrue(rules.stream() .filter(rule -> rule.getResources().equals(List.of(RBACRule.ALL))) .anyMatch(rule -> rule.getVerbs().equals(List.of(UPDATE)) && rule.getApiGroups().equals(List.of(RBACRule.ALL)))); - - // TODO: need update, https://github.com/operator-framework/java-operator-sdk/pull/2515 - // expected generic kubernetes resource: apiGroups is Group from GVK and resources should be '*' - // verbs should contain merged 'delete' - // count should be 1, as TypelessKubeResource and TypelessAnotherKubeResource have same GROUP + // Both typeless dependents are using the same GVK so the verbs associated with their policy rules should be merged into a single one + assertEquals(1, rules.stream() + .filter(rule -> rule.getApiGroups().equals(List.of(TypelessKubeResource.GROUP)) + && rule.getResources().equals(List.of(Pluralize.toPlural(TypelessKubeResource.KIND)))) + .count()); assertTrue(rules.stream() - .filter(rule -> rule.getApiGroups().equals(List.of(TypelessKubeResource.GROUP))) - .filter(rule -> rule.getResources().equals(List.of("*"))) - .filter(rule -> rule.getVerbs().contains("delete")) - .count() == 1); + .filter(rule -> rule.getApiGroups().equals(List.of(TypelessKubeResource.GROUP)) + && rule.getResources().equals(List.of(Pluralize.toPlural(TypelessKubeResource.KIND)))) + .allMatch(rule -> hasReadAndAdditionalVerbsOnly(rule, DELETE))); }); // check that we have a role binding for TestReconciler and that it uses the operator-level specified namespace diff --git a/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/sources/TypelessAnotherKubeResource.java b/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/sources/TypelessAnotherKubeResource.java index 14b718eff..fceabf764 100644 --- a/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/sources/TypelessAnotherKubeResource.java +++ b/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/sources/TypelessAnotherKubeResource.java @@ -1,14 +1,13 @@ package io.quarkiverse.operatorsdk.test.sources; +import static io.quarkiverse.operatorsdk.test.sources.TypelessKubeResource.*; + import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; import io.javaoperatorsdk.operator.processing.GroupVersionKind; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesDependentResource; public class TypelessAnotherKubeResource extends GenericKubernetesDependentResource implements Deleter { - public static final String GROUP = "crd.josdk.quarkiverse.io"; - public static final String KIND = "typelessAnother"; - public static final String VERSION = "v1"; private static final GroupVersionKind GVK = new GroupVersionKind(GROUP, VERSION, KIND); public TypelessAnotherKubeResource() {