From 875f369c35b2fbf4079fc765cdf5ec10b63f7577 Mon Sep 17 00:00:00 2001 From: cc Date: Thu, 22 Aug 2024 21:59:51 +0800 Subject: [PATCH 1/4] Support typeless kubernetes resources when generating manifests Typeless kubernetes resource has no Group and Plural value, so generating wrong policy rule. Commit summary: 1) Use ApiGroup value from GVK and '*' for Plural value if dependent resource is assignable from GenericKubernetesDependentResource 2) Merge verbs in rule for optimization if with same ApiGroups and Resources --- .../deployment/AddClusterRolesDecorator.java | 79 +++++++++++++++++-- .../operatorsdk/test/OperatorSDKTest.java | 17 ++-- .../test/sources/TestReconciler.java | 4 +- .../sources/TypelessAnotherKubeResource.java | 18 +++++ .../test/sources/TypelessKubeResource.java | 17 ++++ 5 files changed, 115 insertions(+), 20 deletions(-) create mode 100644 core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/sources/TypelessAnotherKubeResource.java create mode 100644 core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/sources/TypelessKubeResource.java 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 4e98add7f..ea4849456 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,17 +1,20 @@ package io.quarkiverse.operatorsdk.deployment; -import java.util.Collection; -import java.util.Map; +import java.util.*; + +import org.jetbrains.annotations.NotNull; import io.dekorate.kubernetes.decorator.ResourceProvidingDecorator; 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.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.quarkiverse.operatorsdk.annotations.RBACVerbs; import io.quarkiverse.operatorsdk.runtime.DependentResourceSpecMetadata; import io.quarkiverse.operatorsdk.runtime.QuarkusControllerConfiguration; @@ -78,15 +81,32 @@ public static ClusterRole createClusterRole(QuarkusControllerConfiguration cr .addToRules(rule.build()); final Map> dependentsMetadata = cri.getDependentsMetadata(); + Set collectedRules = new LinkedHashSet(); dependentsMetadata.forEach((name, spec) -> { final var dependentResourceClass = spec.getDependentResourceClass(); final var associatedResourceClass = spec.getDependentType(); // only process Kubernetes dependents if (HasMetadata.class.isAssignableFrom(associatedResourceClass)) { + String resourceGroup = HasMetadata.getGroup(associatedResourceClass); + String resourcePlural = HasMetadata.getPlural(associatedResourceClass); + + if (GenericKubernetesDependentResource.class.isAssignableFrom(dependentResourceClass)) { + try { + // Only applied class with non-parameter constructor + if (Arrays.stream(dependentResourceClass.getConstructors()).anyMatch(i -> i.getParameterCount() == 0)) { + GenericKubernetesDependentResource genericKubernetesResource = (GenericKubernetesDependentResource) dependentResourceClass + .getConstructor().newInstance(); + resourceGroup = genericKubernetesResource.getGroupVersionKind().getGroup(); + resourcePlural = "*"; + } + } catch (Exception e) { + throw new RuntimeException(e); + } + } final var dependentRule = new PolicyRuleBuilder() - .addToApiGroups(HasMetadata.getGroup(associatedResourceClass)) - .addToResources(HasMetadata.getPlural(associatedResourceClass)) + .addToApiGroups(resourceGroup) + .addToResources(resourcePlural) .addToVerbs(RBACVerbs.READ_VERBS); if (Updater.class.isAssignableFrom(dependentResourceClass)) { dependentRule.addToVerbs(RBACVerbs.UPDATE_VERBS); @@ -100,17 +120,62 @@ public static ClusterRole createClusterRole(QuarkusControllerConfiguration cr dependentRule.addToVerbs(RBACVerbs.PATCH); } } - clusterRoleBuilder.addToRules(dependentRule.build()); + collectedRules.add(dependentRule.build()); } }); - // add additional RBAC rules - clusterRoleBuilder.addAllToRules(cri.getAdditionalRBACRules()); + collectedRules.addAll(cri.getAdditionalRBACRules()); + Set normalizedRules = getNormalizedRules(collectedRules); + clusterRoleBuilder.addToRules(normalizedRules.toArray(PolicyRule[]::new)); return clusterRoleBuilder.build(); } + @NotNull + private static Set getNormalizedRules(Set collectedRules) { + Set normalizedRules = new LinkedHashSet(); + collectedRules.stream().map(i -> { + return 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())) { + if (Objects.equals( + getResources().stream().sorted().toList(), + ((PolicyRule) o).getResources().stream().sorted().toList())) { + return true; + } + } + } + return false; + } + + @Override + public int hashCode() { + // equals method called only with same hashCode + return 0; + } + }; + }).forEach(i -> { + if (!normalizedRules.add(i)) { + normalizedRules.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 normalizedRules; + } + 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 d27c8710d..885e49e69 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 @@ -28,17 +28,7 @@ import io.quarkiverse.operatorsdk.annotations.RBACVerbs; import io.quarkiverse.operatorsdk.deployment.AddClusterRolesDecorator; import io.quarkiverse.operatorsdk.deployment.AddRoleBindingsDecorator; -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.*; import io.quarkus.test.ProdBuildResults; import io.quarkus.test.ProdModeTestResults; import io.quarkus.test.QuarkusProdModeTest; @@ -53,6 +43,7 @@ public class OperatorSDKTest { .withApplicationRoot((jar) -> jar .addClasses(TestReconciler.class, TestCR.class, CRUDConfigMap.class, ReadOnlySecret.class, CreateOnlyService.class, NonKubeResource.class, Foo.class, + TypelessKubeResource.class, TypelessAnotherKubeResource.class, SimpleReconciler.class, SimpleCR.class, SimpleSpec.class, SimpleStatus.class)); @ProdBuildResults @@ -79,7 +70,7 @@ public void shouldCreateRolesAndRoleBindings() throws IOException { .map(ClusterRole.class::cast) .forEach(cr -> { final var rules = cr.getRules(); - assertEquals(5, rules.size()); + assertEquals(6, rules.size()); assertTrue(rules.stream() .filter(rule -> rule.getApiGroups().equals(List.of(HasMetadata.getGroup(TestCR.class)))) .anyMatch(rule -> { @@ -108,6 +99,8 @@ public void shouldCreateRolesAndRoleBindings() throws IOException { .filter(rule -> rule.getResources().equals(List.of(RBACRule.ALL))) .anyMatch(rule -> rule.getVerbs().equals(List.of(UPDATE)) && rule.getApiGroups().equals(List.of(RBACRule.ALL)))); + rules.stream().filter(rule -> rule.getApiGroups().equals(List.of(TypelessKubeResource.GROUP))) + .anyMatch(rule -> rule.getResources().equals(List.of("*"))); }); // 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/TestReconciler.java b/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/sources/TestReconciler.java index 0217ed525..3b788188a 100644 --- a/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/sources/TestReconciler.java +++ b/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/sources/TestReconciler.java @@ -12,7 +12,9 @@ @Dependent(type = CRUDConfigMap.class), @Dependent(type = ReadOnlySecret.class), @Dependent(type = CreateOnlyService.class), - @Dependent(type = NonKubeResource.class) + @Dependent(type = NonKubeResource.class), + @Dependent(type = TypelessKubeResource.class), + @Dependent(type = TypelessAnotherKubeResource.class) }) @RBACRule(verbs = RBACVerbs.UPDATE, apiGroups = RBACRule.ALL, resources = RBACRule.ALL) public class TestReconciler implements Reconciler { 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 new file mode 100644 index 000000000..14b718eff --- /dev/null +++ b/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/sources/TypelessAnotherKubeResource.java @@ -0,0 +1,18 @@ +package io.quarkiverse.operatorsdk.test.sources; + +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() { + super(GVK); + } + +} diff --git a/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/sources/TypelessKubeResource.java b/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/sources/TypelessKubeResource.java new file mode 100644 index 000000000..e6da7caf7 --- /dev/null +++ b/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/sources/TypelessKubeResource.java @@ -0,0 +1,17 @@ +package io.quarkiverse.operatorsdk.test.sources; + +import io.javaoperatorsdk.operator.processing.GroupVersionKind; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesDependentResource; + +public class TypelessKubeResource extends GenericKubernetesDependentResource { + + public static final String GROUP = "crd.josdk.quarkiverse.io"; + public static final String KIND = "typeless"; + public static final String VERSION = "v1"; + private static final GroupVersionKind GVK = new GroupVersionKind(GROUP, VERSION, KIND); + + public TypelessKubeResource() { + super(GVK); + } + +} From 0a1cbd8932fd701082f0a1f061c9b89adaa7184f Mon Sep 17 00:00:00 2001 From: cc Date: Thu, 22 Aug 2024 22:25:28 +0800 Subject: [PATCH 2/4] cleanup code-checking problems --- .../deployment/AddClusterRolesDecorator.java | 66 +++++++++---------- 1 file changed, 32 insertions(+), 34 deletions(-) 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 ea4849456..8dbd25046 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 @@ -81,7 +81,7 @@ public static ClusterRole createClusterRole(QuarkusControllerConfiguration cr .addToRules(rule.build()); final Map> dependentsMetadata = cri.getDependentsMetadata(); - Set collectedRules = new LinkedHashSet(); + Set collectedRules = new LinkedHashSet<>(); dependentsMetadata.forEach((name, spec) -> { final var dependentResourceClass = spec.getDependentResourceClass(); final var associatedResourceClass = spec.getDependentType(); @@ -95,6 +95,7 @@ public static ClusterRole createClusterRole(QuarkusControllerConfiguration cr 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(); @@ -134,45 +135,42 @@ public static ClusterRole createClusterRole(QuarkusControllerConfiguration cr @NotNull private static Set getNormalizedRules(Set collectedRules) { - Set normalizedRules = new LinkedHashSet(); - collectedRules.stream().map(i -> { - return 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())) { + Set normalizedRules = new LinkedHashSet<>(); + collectedRules.stream() + .map(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( - getResources().stream().sorted().toList(), - ((PolicyRule) o).getResources().stream().sorted().toList())) { - return true; + 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; } - return false; - } - @Override - public int hashCode() { - // equals method called only with same hashCode - return 0; - } - }; - }).forEach(i -> { - if (!normalizedRules.add(i)) { - normalizedRules.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()); + @Override + public int hashCode() { + // equals method called only with same hashCode + return 0; + } + }).forEach(i -> { + if (!normalizedRules.add(i)) { + normalizedRules.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 normalizedRules; } From 2f3fe6db7621c7746b72e5c61d13e88543d528fb Mon Sep 17 00:00:00 2001 From: cc Date: Sat, 24 Aug 2024 06:04:27 +0800 Subject: [PATCH 3/4] fix missing assertion --- .../deployment/AddClusterRolesDecorator.java | 6 ++++++ .../quarkiverse/operatorsdk/test/OperatorSDKTest.java | 10 ++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) 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 8dbd25046..444b76324 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 @@ -133,6 +133,12 @@ public static ClusterRole createClusterRole(QuarkusControllerConfiguration cr return clusterRoleBuilder.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 getNormalizedRules(Set collectedRules) { Set normalizedRules = new LinkedHashSet<>(); 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 885e49e69..22272d369 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 @@ -99,8 +99,14 @@ public void shouldCreateRolesAndRoleBindings() throws IOException { .filter(rule -> rule.getResources().equals(List.of(RBACRule.ALL))) .anyMatch(rule -> rule.getVerbs().equals(List.of(UPDATE)) && rule.getApiGroups().equals(List.of(RBACRule.ALL)))); - rules.stream().filter(rule -> rule.getApiGroups().equals(List.of(TypelessKubeResource.GROUP))) - .anyMatch(rule -> rule.getResources().equals(List.of("*"))); + // 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 + 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); }); // check that we have a role binding for TestReconciler and that it uses the operator-level specified namespace From 58a49c8b9770f6108ebee1c68ee478ee4d8d532a Mon Sep 17 00:00:00 2001 From: cc Date: Sat, 24 Aug 2024 23:06:08 +0800 Subject: [PATCH 4/4] Refactor AddClusterRolesDecorator for more understandable --- .../deployment/AddClusterRolesDecorator.java | 127 ++++++++++-------- .../operatorsdk/test/OperatorSDKTest.java | 2 + 2 files changed, 73 insertions(+), 56 deletions(-) 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 444b76324..80901c6ac 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,6 +1,7 @@ package io.quarkiverse.operatorsdk.deployment; import java.util.*; +import java.util.function.Function; import org.jetbrains.annotations.NotNull; @@ -56,32 +57,24 @@ public void visit(KubernetesListBuilder list) { } public static ClusterRole createClusterRole(QuarkusControllerConfiguration cri) { - final var rule = new PolicyRuleBuilder(); - final var resourceClass = cri.getResourceClass(); - final var plural = HasMetadata.getPlural(resourceClass); - rule.addToResources(plural); - - // if the resource has a non-Void status, also add the status resource - if (cri.isStatusPresentAndNotVoid()) { - rule.addToResources(plural + "/status"); - } - // add finalizers sub-resource because it's used in several contexts, even in the absence of finalizers - // see: https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#ownerreferencespermissionenforcement - rule.addToResources(plural + "/finalizers"); - - rule.addToApiGroups(HasMetadata.getGroup(resourceClass)) - .addToVerbs(RBACVerbs.ALL_COMMON_VERBS) - .build(); + Set collectedRules = new LinkedHashSet<>(); + collectedRules.add(getClusterRolePolicyRuleFromPrimaryResource(cri)); + collectedRules.addAll(getClusterRolePolicyRulesFromDependentResources(cri)); + collectedRules.addAll(cri.getAdditionalRBACRules()); - final var clusterRoleBuilder = new ClusterRoleBuilder() + return new ClusterRoleBuilder() .withNewMetadata() .withName(getClusterRoleName(cri.getName())) .endMetadata() - .addToRules(rule.build()); + .addAllToRules(mergePolicyRulesOfSameGroupsAndKinds(collectedRules)) + .build(); + } + @NotNull + private static Set getClusterRolePolicyRulesFromDependentResources(QuarkusControllerConfiguration cri) { + Set rules = new LinkedHashSet<>(); final Map> dependentsMetadata = cri.getDependentsMetadata(); - Set collectedRules = new LinkedHashSet<>(); dependentsMetadata.forEach((name, spec) -> { final var dependentResourceClass = spec.getDependentResourceClass(); final var associatedResourceClass = spec.getDependentType(); @@ -91,6 +84,8 @@ public static ClusterRole createClusterRole(QuarkusControllerConfiguration cr String resourceGroup = HasMetadata.getGroup(associatedResourceClass); String 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 @@ -105,6 +100,7 @@ public static ClusterRole createClusterRole(QuarkusControllerConfiguration cr throw new RuntimeException(e); } } + final var dependentRule = new PolicyRuleBuilder() .addToApiGroups(resourceGroup) .addToResources(resourcePlural) @@ -121,16 +117,31 @@ public static ClusterRole createClusterRole(QuarkusControllerConfiguration cr dependentRule.addToVerbs(RBACVerbs.PATCH); } } - collectedRules.add(dependentRule.build()); + rules.add(dependentRule.build()); } - }); - // add additional RBAC rules - collectedRules.addAll(cri.getAdditionalRBACRules()); - Set normalizedRules = getNormalizedRules(collectedRules); - clusterRoleBuilder.addToRules(normalizedRules.toArray(PolicyRule[]::new)); + return rules; + } - return clusterRoleBuilder.build(); + private static PolicyRule getClusterRolePolicyRuleFromPrimaryResource(QuarkusControllerConfiguration cri) { + final var rule = new PolicyRuleBuilder(); + final var resourceClass = cri.getResourceClass(); + final var plural = HasMetadata.getPlural(resourceClass); + rule.addToResources(plural); + + // if the resource has a non-Void status, also add the status resource + if (cri.isStatusPresentAndNotVoid()) { + rule.addToResources(plural + "/status"); + } + + // add finalizers sub-resource because it's used in several contexts, even in the absence of finalizers + // see: https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#ownerreferencespermissionenforcement + rule.addToResources(plural + "/finalizers"); + + rule.addToApiGroups(HasMetadata.getGroup(resourceClass)) + .addToVerbs(RBACVerbs.ALL_COMMON_VERBS) + .build(); + return rule.build(); } /** @@ -140,36 +151,12 @@ public static ClusterRole createClusterRole(QuarkusControllerConfiguration cr * @return no duplicated rules */ @NotNull - private static Set getNormalizedRules(Set collectedRules) { - Set normalizedRules = new LinkedHashSet<>(); + private static Set mergePolicyRulesOfSameGroupsAndKinds(Set collectedRules) { + Set mergedRules = new LinkedHashSet<>(); collectedRules.stream() - .map(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; - } - }).forEach(i -> { - if (!normalizedRules.add(i)) { - normalizedRules.stream().filter(j -> Objects.equals(j, i)).findAny().ifPresent(r -> { + .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); @@ -177,7 +164,35 @@ public int hashCode() { }); } }); - return normalizedRules; + 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) { 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 22272d369..846dc0d94 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 @@ -99,6 +99,8 @@ public void shouldCreateRolesAndRoleBindings() throws IOException { .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