Skip to content

Commit d310190

Browse files
metacosmmetacosm
committed
feat: cache generated RBACs since code is called once per k8s flavor
1 parent d1aa473 commit d310190

File tree

2 files changed

+72
-68
lines changed

2 files changed

+72
-68
lines changed

‎core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/AddRoleBindingsDecorator.java‎

Lines changed: 67 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,22 @@
22

33
import static io.quarkiverse.operatorsdk.deployment.AddClusterRolesDecorator.getClusterRoleName;
44

5+
import java.util.ArrayList;
56
import java.util.Collection;
6-
import java.util.HashSet;
7+
import java.util.List;
78
import java.util.Optional;
8-
import java.util.Set;
99
import java.util.concurrent.ConcurrentHashMap;
1010
import java.util.concurrent.ConcurrentMap;
1111

1212
import org.eclipse.microprofile.config.ConfigProvider;
1313
import org.jboss.logging.Logger;
1414

1515
import io.dekorate.kubernetes.decorator.ResourceProvidingDecorator;
16+
import io.fabric8.kubernetes.api.model.HasMetadata;
1617
import io.fabric8.kubernetes.api.model.KubernetesListBuilder;
18+
import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBinding;
1719
import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBindingBuilder;
20+
import io.fabric8.kubernetes.api.model.rbac.RoleBinding;
1821
import io.fabric8.kubernetes.api.model.rbac.RoleBindingBuilder;
1922
import io.quarkiverse.operatorsdk.runtime.BuildTimeOperatorConfiguration;
2023
import io.quarkiverse.operatorsdk.runtime.QuarkusControllerConfiguration;
@@ -28,7 +31,7 @@ public class AddRoleBindingsDecorator extends ResourceProvidingDecorator<Kuberne
2831
protected static final String SERVICE_ACCOUNT = "ServiceAccount";
2932
private final Collection<QuarkusControllerConfiguration> configs;
3033
private final BuildTimeOperatorConfiguration operatorConfiguration;
31-
private static final ConcurrentMap<String, Object> alreadyLogged = new ConcurrentHashMap<>();
34+
private static final ConcurrentMap<QuarkusControllerConfiguration, List<HasMetadata>> cachedBindings = new ConcurrentHashMap<>();
3235
private static final Optional<String> deployNamespace = ConfigProvider.getConfig()
3336
.getOptionalValue("quarkus.kubernetes.namespace", String.class);
3437

@@ -41,82 +44,83 @@ public AddRoleBindingsDecorator(Collection<QuarkusControllerConfiguration> confi
4144
@Override
4245
public void visit(KubernetesListBuilder list) {
4346
final var serviceAccountName = getMandatoryDeploymentMetadata(list).getName();
44-
configs.forEach(controllerConfiguration -> {
45-
QuarkusControllerConfiguration<?> config = (QuarkusControllerConfiguration<?>) controllerConfiguration;
46-
String controllerName = config.getName();
47-
final var roleBindingName = controllerName + "-role-binding";
48-
if (config.watchCurrentNamespace()) {
49-
// create a RoleBinding that will be applied in the current namespace if watching only the current NS
50-
log.infov("Creating ''{0}'' RoleBinding to be applied to current namespace", roleBindingName);
51-
list.addToItems(new RoleBindingBuilder()
52-
.withNewMetadata()
53-
.withName(roleBindingName)
54-
.endMetadata()
55-
.withNewRoleRef(RBAC_AUTHORIZATION_GROUP, CLUSTER_ROLE,
56-
getClusterRoleName(controllerName))
57-
.addNewSubject(null, SERVICE_ACCOUNT, serviceAccountName, null)
58-
.build());
59-
} else if (config.watchAllNamespaces()) {
60-
handleClusterRoleBinding(list, serviceAccountName, controllerName,
61-
controllerName + "-cluster-role-binding", "watch all namespaces",
62-
getClusterRoleName(controllerName));
63-
} else {
64-
// retrieve which namespaces should be used to generate either from annotation or from the build time configuration
65-
var desiredWatchedNamespaces = config.getNamespaces();
66-
final var buildTimeConfig = operatorConfiguration.controllers.get(controllerName);
67-
if (buildTimeConfig != null) {
68-
desiredWatchedNamespaces = buildTimeConfig.generateWithWatchedNamespaces
69-
.<Set<String>> map(HashSet::new)
70-
.orElse(desiredWatchedNamespaces);
71-
}
72-
73-
// create a RoleBinding using either the provided deployment namespace or the desired watched namespace name
74-
desiredWatchedNamespaces.forEach(ns -> {
75-
log.infov(
76-
"Creating ''{0}'' RoleBinding to be applied to ''{1}'' namespace",
77-
roleBindingName, ns);
78-
list.addToItems(new RoleBindingBuilder()
79-
.withNewMetadata()
80-
.withName(roleBindingName)
81-
.withNamespace(ns)
82-
.endMetadata()
83-
.withNewRoleRef(RBAC_AUTHORIZATION_GROUP, CLUSTER_ROLE,
84-
getClusterRoleName(controllerName))
85-
.addNewSubject(null, SERVICE_ACCOUNT, serviceAccountName, deployNamespace.orElse(null))
86-
.build());
87-
});
88-
}
89-
90-
// if we validate the CRDs, also create a binding for the CRD validating role
91-
if (operatorConfiguration.crd.validate) {
92-
final var crBindingName = controllerName + "-crd-validating-role-binding";
93-
handleClusterRoleBinding(list, serviceAccountName, controllerName, crBindingName, "validate CRDs",
94-
AddClusterRolesDecorator.JOSDK_CRD_VALIDATING_CLUSTER_ROLE);
95-
}
47+
configs.forEach(config -> {
48+
final var toAdd = cachedBindings.computeIfAbsent(config, c -> bindingsFor(c, serviceAccountName));
49+
list.addAllToItems(toAdd);
9650
});
9751
}
9852

99-
private void handleClusterRoleBinding(KubernetesListBuilder list, String serviceAccountName,
53+
private List<HasMetadata> bindingsFor(QuarkusControllerConfiguration<?> controllerConfiguration,
54+
String serviceAccountName) {
55+
final var controllerName = controllerConfiguration.getName();
56+
57+
// retrieve which namespaces should be used to generate either from annotation or from the build time configuration
58+
final var desiredWatchedNamespaces = controllerConfiguration.getNamespaces();
59+
60+
// if we validate the CRDs, also create a binding for the CRD validating role
61+
List<HasMetadata> itemsToAdd;
62+
if (operatorConfiguration.crd.validate) {
63+
final var crBindingName = controllerName + "-crd-validating-role-binding";
64+
final var crdValidatorRoleBinding = createClusterRoleBinding(serviceAccountName, controllerName,
65+
crBindingName, "validate CRDs",
66+
AddClusterRolesDecorator.JOSDK_CRD_VALIDATING_CLUSTER_ROLE);
67+
itemsToAdd = new ArrayList<>(desiredWatchedNamespaces.size() + 1);
68+
itemsToAdd.add(crdValidatorRoleBinding);
69+
} else {
70+
itemsToAdd = new ArrayList<>(desiredWatchedNamespaces.size());
71+
}
72+
73+
final var roleBindingName = controllerName + "-role-binding";
74+
if (controllerConfiguration.watchCurrentNamespace()) {
75+
// create a RoleBinding that will be applied in the current namespace if watching only the current NS
76+
itemsToAdd.add(createRoleBinding(roleBindingName, controllerName, serviceAccountName, null));
77+
} else if (controllerConfiguration.watchAllNamespaces()) {
78+
itemsToAdd.add(createClusterRoleBinding(serviceAccountName, controllerName,
79+
controllerName + "-cluster-role-binding", "watch all namespaces",
80+
getClusterRoleName(controllerName)));
81+
} else {
82+
// create a RoleBinding using either the provided deployment namespace or the desired watched namespace name
83+
desiredWatchedNamespaces
84+
.forEach(ns -> itemsToAdd.add(createRoleBinding(roleBindingName, controllerName, serviceAccountName, ns)));
85+
}
86+
87+
return itemsToAdd;
88+
}
89+
90+
private static RoleBinding createRoleBinding(String roleBindingName, String controllerName,
91+
String serviceAccountName, String namespace) {
92+
final var nsMsg = (namespace == null ? "current" : "'" + namespace + "'") + " namespace";
93+
log.infov("Creating ''{0}'' RoleBinding to be applied to {1}", roleBindingName, nsMsg);
94+
return new RoleBindingBuilder()
95+
.withNewMetadata()
96+
.withName(roleBindingName)
97+
.withNamespace(deployNamespace.orElse(namespace))
98+
.endMetadata()
99+
.withNewRoleRef(RBAC_AUTHORIZATION_GROUP, CLUSTER_ROLE, getClusterRoleName(controllerName))
100+
.addNewSubject(null, SERVICE_ACCOUNT, serviceAccountName,
101+
deployNamespace.orElse(null))
102+
.build();
103+
}
104+
105+
private static ClusterRoleBinding createClusterRoleBinding(String serviceAccountName,
100106
String controllerName, String bindingName, String controllerConfMessage,
101107
String clusterRoleName) {
102108
outputWarningIfNeeded(controllerName, bindingName, controllerConfMessage);
103109
final var ns = deployNamespace.orElse(null);
104110
log.infov("Creating ''{0}'' ClusterRoleBinding to be applied to ''{1}'' namespace", bindingName, ns);
105-
list.addToItems(new ClusterRoleBindingBuilder()
111+
return new ClusterRoleBindingBuilder()
106112
.withNewMetadata().withName(bindingName)
107113
.endMetadata()
108114
.withNewRoleRef(RBAC_AUTHORIZATION_GROUP, CLUSTER_ROLE, clusterRoleName)
109115
.addNewSubject()
110-
.withKind(SERVICE_ACCOUNT).withName(serviceAccountName).withNamespace(
111-
ns)
116+
.withKind(SERVICE_ACCOUNT).withName(serviceAccountName).withNamespace(ns)
112117
.endSubject()
113-
.build());
118+
.build();
114119
}
115120

116-
private void outputWarningIfNeeded(String controllerName, String crBindingName, String controllerConfMessage) {
121+
private static void outputWarningIfNeeded(String controllerName, String crBindingName, String controllerConfMessage) {
117122
// the decorator can be called several times but we only want to output the warning once
118-
if (deployNamespace.isEmpty()
119-
&& alreadyLogged.putIfAbsent(controllerName + crBindingName, new Object()) == null) {
123+
if (deployNamespace.isEmpty()) {
120124
log.warnv(
121125
"''{0}'' controller is configured to "
122126
+ controllerConfMessage

‎docs/modules/ROOT/pages/includes/quarkus-operator-sdk.adoc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ endif::add-copy-button-to-env-var[]
150150
ifndef::add-copy-button-to-env-var[]
151151
Environment variable: `+++QUARKUS_OPERATOR_SDK_DISABLE_RBAC_GENERATION+++`
152152
endif::add-copy-button-to-env-var[]
153-
--|boolean
153+
--|boolean
154154
|`false`
155155

156156

@@ -339,10 +339,10 @@ a|icon:lock[title=Fixed at build time] [[quarkus-operator-sdk_quarkus.operator-s
339339

340340
[.description]
341341
--
342-
An optional list of comma-separated watched namespace names that will be used to generate manifests at build time.
343-
Note that this is provided as a means to quickly deploy a specific controller to test it by applying the generated manifests to the target cluster. If empty, no manifests will be generated. The namespace in which the controller will be deployed will be the currently configured namespace as specified by your `.kube/config` file, unless you specify the target deployment namespace using the `quarkus.kubernetes.namespace` property.
342+
An optional list of comma-separated watched namespace names that will be used to generate manifests at build time.
343+
Note that this is provided as a means to quickly deploy a specific controller to test it by applying the generated manifests to the target cluster. If empty, no manifests will be generated. The namespace in which the controller will be deployed will be the currently configured namespace as specified by your `.kube/config` file, unless you specify the target deployment namespace using the `quarkus.kubernetes.namespace` property.
344344

345-
As this functionality cannot handle namespaces that are not know until runtime (because the generation happens during build time), we recommend that you use a different mechanism such as OLM or Helm charts to deploy your operator in production.
345+
As this functionality cannot handle namespaces that are not know until runtime (because the generation happens during build time), we recommend that you use a different mechanism such as OLM or Helm charts to deploy your operator in production.
346346
This replaces the previous `namespaces` property which was confusing and against Quarkus best practices since it existed both at build time and runtime. That property wasn't also adequately capturing the fact that namespaces that wouldn't be known until runtime would render whatever got generated at build time invalid as far as generated manifests were concerned.
347347

348348
ifdef::add-copy-button-to-env-var[]
@@ -351,7 +351,7 @@ endif::add-copy-button-to-env-var[]
351351
ifndef::add-copy-button-to-env-var[]
352352
Environment variable: `+++QUARKUS_OPERATOR_SDK_CONTROLLERS__CONTROLLERS__GENERATE_WITH_WATCHED_NAMESPACES+++`
353353
endif::add-copy-button-to-env-var[]
354-
--|list of string
354+
--|list of string
355355
|
356356

357357

0 commit comments

Comments
 (0)