Skip to content

Commit

Permalink
fix: generate proper RoleRef name for ClusterRoleBindings (#959)
Browse files Browse the repository at this point in the history
An improper name was generated in case a controller watched all
namespaces.

Fixes #869

Signed-off-by: Chris Laprun <[email protected]>
  • Loading branch information
metacosm committed Sep 18, 2024
1 parent 739972f commit a2aef63
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ private List<HasMetadata> bindingsFor(QuarkusControllerConfiguration<?> controll
if (controllerConfiguration.watchCurrentNamespace()) {
// create a RoleBinding that will be applied in the current namespace if watching only the current NS
itemsToAdd.add(createRoleBinding(roleBindingName, serviceAccountName, null,
createDefaultRoleRef(getClusterRoleName(controllerName))));
createDefaultRoleRef(controllerName)));
// add additional Role Bindings
controllerConfiguration.getAdditionalRBACRoleRefs().forEach(
roleRef -> {
Expand Down Expand Up @@ -106,7 +106,7 @@ private List<HasMetadata> bindingsFor(QuarkusControllerConfiguration<?> controll
desiredWatchedNamespaces
.forEach(ns -> {
itemsToAdd.add(createRoleBinding(roleBindingName, serviceAccountName, ns,
createDefaultRoleRef(getClusterRoleName(controllerName))));
createDefaultRoleRef(controllerName)));
//add additional Role Bindings
controllerConfiguration.getAdditionalRBACRoleRefs()
.forEach(roleRef -> {
Expand All @@ -124,7 +124,7 @@ public static String getCRDValidatingBindingName(String controllerName) {
return controllerName + "-crd-validating-role-binding";
}

private static String getClusterRoleBindingName(String controllerName) {
public static String getClusterRoleBindingName(String controllerName) {
return controllerName + "-cluster-role-binding";
}

Expand All @@ -142,7 +142,9 @@ public static String getSpecificRoleBindingName(String controllerName, RoleRef r

private static RoleRef createDefaultRoleRef(String controllerName) {
return new RoleRefBuilder()
.withApiGroup(RBAC_AUTHORIZATION_GROUP).withKind(CLUSTER_ROLE).withName(controllerName)
.withApiGroup(RBAC_AUTHORIZATION_GROUP)
.withKind(CLUSTER_ROLE)
.withName(getClusterRoleName(controllerName))
.build();
}

Expand All @@ -167,7 +169,7 @@ private static ClusterRoleBinding createClusterRoleBinding(String serviceAccount
String controllerName, String bindingName, String controllerConfMessage,
RoleRef roleRef) {
outputWarningIfNeeded(controllerName, bindingName, controllerConfMessage);
roleRef = roleRef == null ? createDefaultRoleRef(serviceAccountName) : roleRef;
roleRef = roleRef == null ? createDefaultRoleRef(controllerName) : roleRef;
final var ns = deployNamespace.orElse(null);
log.infov("Creating ''{0}'' ClusterRoleBinding to be applied to ''{1}'' namespace", bindingName, ns);
return new ClusterRoleBindingBuilder()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package io.quarkiverse.operatorsdk.test;

import static io.quarkiverse.operatorsdk.annotations.RBACVerbs.*;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.io.FileInputStream;
import java.io.IOException;
import java.nio.file.Files;
import java.util.Arrays;
import java.util.List;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.fabric8.kubernetes.api.model.ConfigMap;
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.rbac.ClusterRole;
import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBinding;
import io.fabric8.kubernetes.client.utils.Serialization;
import io.quarkiverse.operatorsdk.deployment.AddClusterRolesDecorator;
import io.quarkiverse.operatorsdk.deployment.AddRoleBindingsDecorator;
import io.quarkiverse.operatorsdk.test.sources.WatchAllReconciler;
import io.quarkus.test.ProdBuildResults;
import io.quarkus.test.ProdModeTestResults;
import io.quarkus.test.QuarkusProdModeTest;

public class WatchAllNamespacesTest {
private static final String APPLICATION_NAME = "watch-all-namespaces-test";
// Start unit test with your extension loaded
@RegisterExtension
static final QuarkusProdModeTest config = new QuarkusProdModeTest()
.setApplicationName(APPLICATION_NAME)
.withApplicationRoot((jar) -> jar.addClasses(WatchAllReconciler.class));

@ProdBuildResults
private ProdModeTestResults prodModeTestResults;

@Test
public void shouldCreateRolesAndRoleBindings() throws IOException {
final var kubernetesDir = prodModeTestResults.getBuildDir().resolve("kubernetes");
final var kubeManifest = kubernetesDir.resolve("kubernetes.yml");
Assertions.assertTrue(Files.exists(kubeManifest));
final var kubeIS = new FileInputStream(kubeManifest.toFile());
// use unmarshall version with parameters map to ensure code goes through the proper processing wrt multiple documents
@SuppressWarnings("unchecked")
final var kubeResources = (List<HasMetadata>) Serialization.unmarshal(kubeIS);

// check cluster role
final var clusterRoleName = AddClusterRolesDecorator.getClusterRoleName(WatchAllReconciler.NAME);
//make sure the target role exists because otherwise the test will succeed without actually checking anything
assertTrue(kubeResources.stream()
.anyMatch(i -> clusterRoleName.equals(i.getMetadata().getName())));
kubeResources.stream()
.filter(i -> clusterRoleName.equals(i.getMetadata().getName()))
.map(ClusterRole.class::cast)
.forEach(cr -> {
final var rules = cr.getRules();
assertEquals(1, rules.size());

var rule = rules.get(0);
assertEquals(List.of(HasMetadata.getGroup(ConfigMap.class)), rule.getApiGroups());
final var resources = rule.getResources();
final var plural = HasMetadata.getPlural(ConfigMap.class);
// status is void so shouldn't be present in resources
assertEquals(List.of(plural, plural + "/finalizers"), resources);
assertEquals(Arrays.asList(ALL_COMMON_VERBS), rule.getVerbs());
});

// check that we have a cluster role binding that is mapped to the proper ClusterRole
final var clusterRoleBindingName = AddRoleBindingsDecorator.getClusterRoleBindingName(WatchAllReconciler.NAME);
assertTrue(kubeResources.stream().anyMatch(i -> clusterRoleBindingName.equals(i.getMetadata().getName())));
kubeResources.stream()
.filter(i -> clusterRoleBindingName.equals(i.getMetadata().getName()))
.map(ClusterRoleBinding.class::cast)
.forEach(rb -> assertEquals(clusterRoleName, rb.getRoleRef().getName()));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package io.quarkiverse.operatorsdk.test.sources;

import io.fabric8.kubernetes.api.model.ConfigMap;
import io.javaoperatorsdk.operator.api.reconciler.Constants;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;

@ControllerConfiguration(name = WatchAllReconciler.NAME, namespaces = Constants.WATCH_ALL_NAMESPACES)
public class WatchAllReconciler implements Reconciler<ConfigMap> {

public static final String NAME = "watchall";

@Override
public UpdateControl<ConfigMap> reconcile(ConfigMap configMap, Context<ConfigMap> context) throws Exception {
return UpdateControl.noUpdate();
}
}

0 comments on commit a2aef63

Please sign in to comment.