Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: improve RBAC generation for typeless dependents #937

Merged
merged 5 commits into from
Aug 28, 2024
Merged

feat: improve RBAC generation for typeless dependents #937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ca00bb1
refactor: merge rules as they are collected
metacosm Aug 26, 2024
ce5f6d0
feat: throw an exception if class is not found in index
metacosm Aug 26, 2024
12927d9
fix: use plural form instead of wildcard, adapt tests
metacosm Aug 26, 2024
a387f0b
refactor: address review comments
metacosm Aug 27, 2024
e3057ee
fix: only add PATCH if the dependent is using SSA
metacosm Aug 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ConfigurationUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,12 @@ public static <T> ClassInfo getClassInfoForInstantiation(AnnotationValue toInsta
Class<T> 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")
Expand Down
185 changes: 96 additions & 89 deletions ...loyment/src/main/java/io/quarkiverse/operatorsdk/deployment/AddClusterRolesDecorator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -32,6 +46,8 @@ public class AddClusterRolesDecorator extends ResourceProvidingDecorator<Kuberne
.build());
private static final String CR_API_VERSION = HasMetadata.getApiVersion(ClusterRole.class);
private static final String CR_KIND = HasMetadata.getKind(ClusterRole.class);
private static final Logger log = Logger.getLogger(AddClusterRolesDecorator.class);
private static final String ADD_CLUSTER_ROLES_DECORATOR = "AddClusterRolesDecorator";
private final Collection<QuarkusControllerConfiguration<?>> configs;

private final boolean validateCRDs;
Expand All @@ -57,21 +73,40 @@ public void visit(KubernetesListBuilder list) {
}

public static ClusterRole createClusterRole(QuarkusControllerConfiguration<?> cri) {
final var rules = new LinkedHashMap<String, PolicyRule>();
final var clusterRolePolicyRuleFromPrimaryResource = getClusterRolePolicyRuleFromPrimaryResource(cri);
final var primaryRuleKey = getKeyFor(clusterRolePolicyRuleFromPrimaryResource);
rules.put(primaryRuleKey, clusterRolePolicyRuleFromPrimaryResource);

Set<PolicyRule> 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<PolicyRule> newRules,
Map<String, PolicyRule> existingRules) {
newRules.forEach(newPolicyRule -> {
final var key = getKeyFor(newPolicyRule);
existingRules.merge(key, newPolicyRule, (existing, npr) -> {
Set<String> 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<PolicyRule> getClusterRolePolicyRulesFromDependentResources(QuarkusControllerConfiguration<?> cri) {
Set<PolicyRule> rules = new LinkedHashSet<>();
final Map<String, DependentResourceSpecMetadata<?, ?, ?>> dependentsMetadata = cri.getDependentsMetadata();
Expand All @@ -81,48 +116,71 @@ private static Set<PolicyRule> 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<? 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 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<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();
Expand All @@ -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<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) {
return controller + "-cluster-role";
}
Expand Down
Loading