Skip to content

Commit

Permalink
feat: support typeless kubernetes resources when generating manifests (…
Browse files Browse the repository at this point in the history 
…#931)

* 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

* cleanup code-checking problems

* fix missing assertion

* Refactor AddClusterRolesDecorator for more understandable

---------

Co-authored-by: cc <cc@xpie.cn>
  • Loading branch information
@chendouble @xpie
chendouble and xpie authored Aug 26, 2024
1 parent cc34493 commit 3ad74ac
Show file tree
Hide file tree
Showing 5 changed files with 162 additions and 40 deletions.
138 changes: 111 additions & 27 deletions ...loyment/src/main/java/io/quarkiverse/operatorsdk/deployment/AddClusterRolesDecorator.java
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
package io.quarkiverse.operatorsdk.deployment;

import java.util.Collection;
import java.util.Map;
import java.util.*;
import java.util.function.Function;

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;
Expand Down Expand Up @@ -53,40 +57,53 @@ 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");
Set<PolicyRule> collectedRules = new LinkedHashSet<>();
collectedRules.add(getClusterRolePolicyRuleFromPrimaryResource(cri));
collectedRules.addAll(getClusterRolePolicyRulesFromDependentResources(cri));
collectedRules.addAll(cri.getAdditionalRBACRules());

rule.addToApiGroups(HasMetadata.getGroup(resourceClass))
.addToVerbs(RBACVerbs.ALL_COMMON_VERBS)
.build();

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<PolicyRule> getClusterRolePolicyRulesFromDependentResources(QuarkusControllerConfiguration<?> cri) {
Set<PolicyRule> rules = new LinkedHashSet<>();
final Map<String, DependentResourceSpecMetadata<?, ?, ?>> dependentsMetadata = cri.getDependentsMetadata();
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);

// 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(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);
Expand All @@ -100,15 +117,82 @@ public static ClusterRole createClusterRole(QuarkusControllerConfiguration<?> cr
dependentRule.addToVerbs(RBACVerbs.PATCH);
}
}
clusterRoleBuilder.addToRules(dependentRule.build());
rules.add(dependentRule.build());
}

});
return rules;
}

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);

// add additional RBAC rules
clusterRoleBuilder.addAllToRules(cri.getAdditionalRBACRules());
// 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");

return clusterRoleBuilder.build();
rule.addToApiGroups(HasMetadata.getGroup(resourceClass))
.addToVerbs(RBACVerbs.ALL_COMMON_VERBS)
.build();
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<PolicyRule> mergePolicyRulesOfSameGroupsAndKinds(Set<PolicyRule> collectedRules) {
Set<PolicyRule> 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<String> verbs1 = new LinkedHashSet<>(r.getVerbs());
Set<String> verbs2 = new LinkedHashSet<>(i.getVerbs());
verbs1.addAll(verbs2);
r.setVerbs(verbs1.stream().toList());
});
}
});
return mergedRules;
}

@NotNull
private static Function<PolicyRule, PolicyRule> 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) {
Expand Down
25 changes: 13 additions & 12 deletions core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/OperatorSDKTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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 -> {
Expand Down Expand Up @@ -108,6 +99,16 @@ 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
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
Expand Down
4 changes: 3 additions & 1 deletion core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/sources/TestReconciler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<TestCR> {
Expand Down
18 changes: 18 additions & 0 deletions ...nt/src/test/java/io/quarkiverse/operatorsdk/test/sources/TypelessAnotherKubeResource.java
View file
Original file line number Diff line number Diff line change
@@ -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<TestCR> implements Deleter<TestCR> {

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);
}

}
17 changes: 17 additions & 0 deletions ...eployment/src/test/java/io/quarkiverse/operatorsdk/test/sources/TypelessKubeResource.java
View file
Original file line number Diff line number Diff line change
@@ -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<TestCR> {

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);
}

}

0 comments on commit 3ad74ac

Please sign in to comment.