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.

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

Fix RBAC generation issues #172

Merged
merged 6 commits into from
Dec 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@

import java.util.Map;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

import org.eclipse.microprofile.config.ConfigProvider;

import io.dekorate.kubernetes.decorator.ResourceProvidingDecorator;
import io.fabric8.kubernetes.api.model.KubernetesListBuilder;
import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBindingBuilder;
Expand Down Expand Up @@ -42,20 +45,9 @@ public void visit(KubernetesListBuilder list) {
.addNewSubject(null, "ServiceAccount", serviceAccountName, null)
.build());
} else if (config.watchAllNamespaces()) {
final var crBindingName = controllerName + "-cluster-role-binding";
// the decorator can be called several times but we only want to output the warning once
if (alreadyLogged.putIfAbsent(controllerName, new Object()) != null) {
OperatorSDKProcessor.log.warnv(
"''{0}'' controller is configured to watch all namespaces, this requires a ClusterRoleBinding for which we MUST specify the namespace of the operator ServiceAccount. However, at this information is not known at build time, we are leaving it blank and needs to be provided by the user by editing the ''{1}'' ClusterRoleBinding to provide the namespace in which the operator will be deployed.",
controllerName, crBindingName);
}
list.addToItems(new ClusterRoleBindingBuilder()
.withNewMetadata().withName(crBindingName)
.endMetadata()
.withNewRoleRef("rbac.authorization.k8s.io", "ClusterRole",
getClusterRoleName(controllerName))
.addNewSubject(null, "ServiceAccount", serviceAccountName, null)
.build());
handleClusterRoleBinding(list, serviceAccountName, controllerName,
controllerName + "-cluster-role-binding", "watch all namespaces",
getClusterRoleName(controllerName));
} else {
config.getEffectiveNamespaces().forEach(ns -> list.addToItems(
new RoleBindingBuilder()
Expand All @@ -71,14 +63,37 @@ public void visit(KubernetesListBuilder list) {

// if we validate the CRDs, also create a binding for the CRD validating role
if (validateCRDs) {
list.addToItems(new RoleBindingBuilder()
.withNewMetadata()
.withName(controllerName + "-crd-validating-role-binding")
.endMetadata()
.withNewRoleRef("rbac.authorization.k8s.io", "ClusterRole",
AddClusterRolesDecorator.JOSDK_CRD_VALIDATING_CLUSTER_ROLE)
.addNewSubject(null, "ServiceAccount", serviceAccountName, null)
.build());
final var crBindingName = controllerName + "-crd-validating-role-binding";
handleClusterRoleBinding(list, serviceAccountName, controllerName, crBindingName, "validate CRDs",
AddClusterRolesDecorator.JOSDK_CRD_VALIDATING_CLUSTER_ROLE);
}
}
}

private void handleClusterRoleBinding(KubernetesListBuilder list, String serviceAccountName,
String controllerName, String bindingName, String controllerConfMessage,
String clusterRoleName) {
final var namespace = ConfigProvider.getConfig()
.getOptionalValue("quarkus.kubernetes.namespace", String.class);
outputWarningIfNeeded(controllerName, bindingName, namespace, controllerConfMessage);
list.addToItems(new ClusterRoleBindingBuilder()
.withNewMetadata().withName(bindingName)
.endMetadata()
.withNewRoleRef("rbac.authorization.k8s.io", "ClusterRole", clusterRoleName)
.addNewSubject(null, "ServiceAccount", serviceAccountName, namespace.orElse(null))
.build());
}

private void outputWarningIfNeeded(String controllerName, String crBindingName,
Optional<String> namespace, String controllerConfMessage) {
// the decorator can be called several times but we only want to output the warning once
if (namespace.isEmpty()) {
if (alreadyLogged.putIfAbsent(controllerName + crBindingName, new Object()) == null) {
OperatorSDKProcessor.log.warnv(
"''{0}'' controller is configured to "
+ controllerConfMessage
+ ", this requires a ClusterRoleBinding for which we MUST specify the namespace of the operator ServiceAccount. This can be specified by setting the ''quarkus.kubernetes.namespace'' property. However, as this property is not set, we are leaving the namespace blank to be provided by the user by editing the ''{1}'' ClusterRoleBinding to provide the namespace in which the operator will be deployed.",
controllerName, crBindingName);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
package io.quarkiverse.operatorsdk.deployment;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Optional;
import java.util.Set;

import org.eclipse.microprofile.config.ConfigProvider;
import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.AnnotationValue;
import org.jboss.jandex.DotName;
Expand All @@ -12,6 +18,8 @@
import io.quarkiverse.operatorsdk.common.ConfigurationUtils;
import io.quarkiverse.operatorsdk.runtime.BuildTimeControllerConfiguration;
import io.quarkiverse.operatorsdk.runtime.BuildTimeOperatorConfiguration;
import io.smallrye.config.Converters;
import io.smallrye.config.Expressions;

class BuildTimeHybridControllerConfiguration {

Expand Down Expand Up @@ -75,4 +83,39 @@ String name(String resourceControllerClassName) {
return ConfigurationUtils.annotationValueOrDefault(
controllerAnnotation, "name", AnnotationValue::asString, () -> defaultControllerName);
}

Set<String> namespaces(String controllerName) {
// first check if we have a property for the namespaces, retrieving it without expanding it
final var config = ConfigProvider.getConfig();
var withoutExpansion = Expressions.withoutExpansion(
() -> config.getConfigValue("quarkus.operator-sdk.controllers."
+ controllerName + ".namespaces").getRawValue());

// check if the controller name is doubly quoted
if (withoutExpansion == null) {
withoutExpansion = Expressions.withoutExpansion(
() -> config.getConfigValue("quarkus.operator-sdk.controllers.\""
+ controllerName + "\".namespaces").getRawValue());
}

// check if the controller name is simply quoted
if (withoutExpansion == null) {
withoutExpansion = Expressions.withoutExpansion(
() -> config.getConfigValue("quarkus.operator-sdk.controllers.'"
+ controllerName + "'.namespaces").getRawValue());
}

if (withoutExpansion != null) {
// if we have a property, use it and convert it to a set of namespaces,
// potentially with unexpanded variable names as namespace names
final var converter = Converters.newCollectionConverter(
Converters.getImplicitConverter(String.class), ArrayList::new);
final var namespaces = converter.convert(withoutExpansion);
return new HashSet<>(namespaces);
}
return ConfigurationUtils.annotationValueOrDefault(controllerAnnotation,
"namespaces",
v -> new HashSet<>(Arrays.asList(v.asStringArray())),
Collections::emptySet);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ CRDGenerationInfo generate(OutputTargetBuildItem outputTarget, CRDConfiguration
if (!outputDir.exists()) {
outputDir.mkdirs();
}

// generate CRDs with detailed information
final var info = generator.forCRDVersions(crdConfig.versions).inOutputDir(outputDir).detailedGenerate();
final var crdDetailsPerNameAndVersion = info.getCRDDetailsPerNameAndVersion();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,16 @@ public void add(io.fabric8.crd.generator.CustomResourceInfo info, String crdName
+ crdName + " with version " + version);
}

final var converted = new CustomResourceInfo(
info.group(), version, info.kind(), info.singular(), info.plural(), info.shortNames(), info.storage(),
final var converted = augment(info, crdName, associatedControllerName);
versionsForCR.put(version, converted);
}

static CustomResourceInfo augment(io.fabric8.crd.generator.CustomResourceInfo info,
String crdName, String associatedControllerName) {
return new CustomResourceInfo(
info.group(), info.version(), info.kind(), info.singular(), info.plural(), info.shortNames(),
info.storage(),
info.served(), info.scope(), info.crClassName(),
info.specClassName(), info.statusClassName(), crdName, associatedControllerName);
versionsForCR.put(version, converted);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.function.BooleanSupplier;
import java.util.stream.Collectors;

import javax.enterprise.inject.Instance;
Expand All @@ -28,6 +28,7 @@

import com.fasterxml.jackson.databind.ObjectMapper;

import io.fabric8.crd.generator.CustomResourceInfo;
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.client.CustomResource;
import io.javaoperatorsdk.operator.ControllerUtils;
Expand Down Expand Up @@ -177,14 +178,37 @@ ConfigurationServiceBuildItem createConfigurationServiceAndOperator(
return new ConfigurationServiceBuildItem(Version.loadFromProperties(), controllerConfigs);
}

@BuildStep
private static class IsRBACEnabled implements BooleanSupplier {
private BuildTimeOperatorConfiguration config;

@Override
public boolean getAsBoolean() {
return !config.disableRbacGeneration;
}
}

@BuildStep(onlyIf = IsRBACEnabled.class)
public void addRBACForCustomResources(BuildProducer<DecoratorBuildItem> decorators,
GeneratedCRDInfoBuildItem generatedCRDs, ConfigurationServiceBuildItem configurations) {
final var mappings = generatedCRDs.getCRDGenerationInfo().getControllerToCustomResourceMappings();
var mappings = generatedCRDs.getCRDGenerationInfo().getControllerToCustomResourceMappings();
final var controllerConfigs = configurations.getControllerConfigs();
final var configs = new HashMap<String, QuarkusControllerConfiguration>(controllerConfigs.size());
controllerConfigs.forEach(c -> configs.put(c.getName(), c));

// if we didn't generate CRDs, we need to generate the CustomResourceInfo at this point
// because it doesn't otherwise exist
if (!buildTimeConfiguration.crd.generate || mappings.isEmpty()) {
final var controllerToCRIMap = new HashMap<String, io.quarkiverse.operatorsdk.common.CustomResourceInfo>(
configs.size());
configs.forEach((controllerName, config) -> {
final var augmented = CustomResourceControllerMapping.augment(
CustomResourceInfo.fromClass(config.getCustomResourceClass()),
config.getCRDName(), controllerName);
controllerToCRIMap.put(controllerName, augmented);
});
mappings = controllerToCRIMap;
}

decorators.produce(new DecoratorBuildItem(new AddClusterRolesDecorator(mappings, buildTimeConfiguration.crd.validate)));
decorators.produce(new DecoratorBuildItem(new AddRoleBindingsDecorator(configs,
buildTimeConfiguration.crd.validate)));
Expand Down Expand Up @@ -347,7 +371,7 @@ private Optional<QuarkusControllerConfiguration> createControllerConfiguration(
configExtractor.generationAware(),
crType,
configExtractor.delayedRegistration(),
getNamespaces(controllerAnnotation),
configExtractor.namespaces(name),
getFinalizer(controllerAnnotation, crdName),
getLabelSelector(controllerAnnotation));

Expand All @@ -368,14 +392,6 @@ private Optional<QuarkusControllerConfiguration> createControllerConfiguration(
return Optional.of(configuration);
}

private Set<String> getNamespaces(AnnotationInstance controllerAnnotation) {
return QuarkusControllerConfiguration.asSet(ConfigurationUtils.annotationValueOrDefault(
controllerAnnotation,
"namespaces",
AnnotationValue::asStringArray,
() -> new String[] {}));
}

private String getFinalizer(AnnotationInstance controllerAnnotation, String crdName) {
return ConfigurationUtils.annotationValueOrDefault(controllerAnnotation,
"finalizerName",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,10 @@ public class BuildTimeOperatorConfiguration {
*/
@ConfigItem
public Optional<String> delayRegistrationUntilEvent;

/**
* Whether Role-Based Access Control (RBAC) resources should be generated in the kubernetes manifests.
*/
@ConfigItem(defaultValue = "false")
public Boolean disableRbacGeneration;
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,6 @@ public QuarkusControllerConfiguration(
this.labelSelector = labelSelector;
}

public static Set<String> asSet(String[] namespaces) {
return namespaces == null || namespaces.length == 0
? Collections.emptySet()
: Set.of(namespaces);
}

// Needed for Quarkus to find the associated constructor parameter
public String getCrdName() {
return getCRDName();
Expand Down