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

fix: use service account name instead of deployment name #953

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

fix: use service account name instead of deployment name #953

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7526b60
fix: use service account name instead of deployment name
metacosm Sep 9, 2024
28e5354
wip: use effective SA from Quarkus
metacosm Sep 13, 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
44 changes: 44 additions & 0 deletions .../java/io/quarkiverse/operatorsdk/bundle/ConfiguredServiceAccountNameShouldBeUsedTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package io.quarkiverse.operatorsdk.bundle;

import static io.quarkiverse.operatorsdk.bundle.Utils.BUNDLE;
import static io.quarkiverse.operatorsdk.bundle.Utils.getCSVFor;
import static org.junit.jupiter.api.Assertions.*;

import java.io.IOException;
import java.nio.file.Files;

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

import io.quarkiverse.operatorsdk.bundle.sources.First;
import io.quarkiverse.operatorsdk.bundle.sources.ReconcilerWithNoCsvMetadata;
import io.quarkus.test.ProdBuildResults;
import io.quarkus.test.ProdModeTestResults;
import io.quarkus.test.QuarkusProdModeTest;

public class ConfiguredServiceAccountNameShouldBeUsedTest {

public static final String APPLICATION_NAME = "configured-service-account-name";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@metacosm isn't this conflicting your other draft PR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is, I created the other one to show current progress and discuss things with @iocanel. This one will get a workaround with the current code to provide a fix but, ultimately, the proper way to handle things should be what has been started in the other PR.

public static final String SA_NAME = "my-operator-sa";
public static final String NS_NAME = "some-namespace";
@RegisterExtension
static final QuarkusProdModeTest config = new QuarkusProdModeTest()
.setApplicationName(APPLICATION_NAME)
.withApplicationRoot((jar) -> jar
.addClasses(First.class, ReconcilerWithNoCsvMetadata.class))
.overrideConfigKey("quarkus.kubernetes.rbac.service-accounts." + SA_NAME + ".namespace", NS_NAME);

@ProdBuildResults
private ProdModeTestResults prodModeTestResults;

@Test
public void shouldWriteBundleEvenWhenCsvMetadataIsNotUsed() throws IOException {
final var bundle = prodModeTestResults.getBuildDir().resolve(BUNDLE);
assertTrue(Files.exists(bundle.resolve(APPLICATION_NAME)));
final var csv = getCSVFor(bundle, APPLICATION_NAME);
final var deployment = csv.getSpec().getInstall().getSpec().getDeployments().get(0);
assertEquals(SA_NAME, deployment.getName());
assertEquals(SA_NAME, deployment.getSpec().getTemplate().getSpec().getServiceAccount());
}

}
213 changes: 119 additions & 94 deletions ...loyment/src/main/java/io/quarkiverse/operatorsdk/deployment/AddRoleBindingsDecorator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

import org.eclipse.microprofile.config.ConfigProvider;
import org.jboss.logging.Logger;

import io.dekorate.kubernetes.decorator.Decorator;
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.apps.DeploymentBuilder;
import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBinding;
import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBindingBuilder;
import io.fabric8.kubernetes.api.model.rbac.RoleBinding;
Expand All @@ -24,100 +25,29 @@
import io.fabric8.kubernetes.api.model.rbac.RoleRefBuilder;
import io.quarkiverse.operatorsdk.runtime.BuildTimeOperatorConfiguration;
import io.quarkiverse.operatorsdk.runtime.QuarkusControllerConfiguration;
import io.quarkus.kubernetes.deployment.AddServiceAccountResourceDecorator;

@SuppressWarnings("rawtypes")
public class AddRoleBindingsDecorator extends ResourceProvidingDecorator<KubernetesListBuilder> {

private static final Logger log = Logger.getLogger(AddRoleBindingsDecorator.class);
protected static final String RBAC_AUTHORIZATION_GROUP = "rbac.authorization.k8s.io";
public static final String CLUSTER_ROLE = "ClusterRole";
protected static final String RBAC_AUTHORIZATION_GROUP = "rbac.authorization.k8s.io";
public static final RoleRef CRD_VALIDATING_ROLE_REF = new RoleRef(RBAC_AUTHORIZATION_GROUP, CLUSTER_ROLE,
JOSDK_CRD_VALIDATING_CLUSTER_ROLE_NAME);
protected static final String SERVICE_ACCOUNT = "ServiceAccount";
private static final Logger log = Logger.getLogger(AddRoleBindingsDecorator.class);
private static final ConcurrentMap<QuarkusControllerConfiguration, List<HasMetadata>> cachedBindings = new ConcurrentHashMap<>();
private final Collection<QuarkusControllerConfiguration<?>> configs;
private final BuildTimeOperatorConfiguration operatorConfiguration;
private static final ConcurrentMap<QuarkusControllerConfiguration, List<HasMetadata>> cachedBindings = new ConcurrentHashMap<>();
private static final Optional<String> deployNamespace = ConfigProvider.getConfig()
.getOptionalValue("quarkus.kubernetes.namespace", String.class);
public static final RoleRef CRD_VALIDATING_ROLE_REF = new RoleRef(RBAC_AUTHORIZATION_GROUP, CLUSTER_ROLE,
JOSDK_CRD_VALIDATING_CLUSTER_ROLE_NAME);
private final String serviceAccountName;
private final String serviceAccountNamespace;

public AddRoleBindingsDecorator(Collection<QuarkusControllerConfiguration<?>> configs,
BuildTimeOperatorConfiguration operatorConfiguration) {
BuildTimeOperatorConfiguration operatorConfiguration, String serviceAccountName, String serviceAccountNamespace) {
this.configs = configs;
this.operatorConfiguration = operatorConfiguration;
}

@Override
public void visit(KubernetesListBuilder list) {
final var serviceAccountName = getMandatoryDeploymentMetadata(list).getName();
configs.forEach(config -> {
final var toAdd = cachedBindings.computeIfAbsent(config, c -> bindingsFor(c, serviceAccountName));
list.addAllToItems(toAdd);
});
}

private List<HasMetadata> bindingsFor(QuarkusControllerConfiguration<?> controllerConfiguration,
String serviceAccountName) {
final var controllerName = controllerConfiguration.getName();

// retrieve which namespaces should be used to generate either from annotation or from the build time configuration
final var desiredWatchedNamespaces = controllerConfiguration.getNamespaces();

// if we validate the CRDs, also create a binding for the CRD validating role
List<HasMetadata> itemsToAdd;
if (operatorConfiguration.crd().validate()) {
final var crBindingName = getCRDValidatingBindingName(controllerName);
final var crdValidatorRoleBinding = createClusterRoleBinding(serviceAccountName, controllerName,
crBindingName, "validate CRDs", CRD_VALIDATING_ROLE_REF);
itemsToAdd = new ArrayList<>(desiredWatchedNamespaces.size() + 1);
itemsToAdd.add(crdValidatorRoleBinding);
} else {
itemsToAdd = new ArrayList<>(desiredWatchedNamespaces.size());
}

final var roleBindingName = getRoleBindingName(controllerName);
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))));
// add additional Role Bindings
controllerConfiguration.getAdditionalRBACRoleRefs().forEach(
roleRef -> {
final var specificRoleBindingName = getSpecificRoleBindingName(controllerName, roleRef);
itemsToAdd.add(createRoleBinding(specificRoleBindingName, serviceAccountName, null, roleRef));
});
} else if (controllerConfiguration.watchAllNamespaces()) {
itemsToAdd.add(createClusterRoleBinding(serviceAccountName, controllerName,
getClusterRoleBindingName(controllerName), "watch all namespaces",
null));
// add additional cluster role bindings only if they target cluster roles
controllerConfiguration.getAdditionalRBACRoleRefs().forEach(
roleRef -> {
if (!CLUSTER_ROLE.equals(roleRef.getKind())) {
log.warnv("Cannot create a ClusterRoleBinding for RoleRef ''{0}'' because it's not a ClusterRole",
roleRef);
} else {
itemsToAdd.add(createClusterRoleBinding(serviceAccountName, controllerName,
roleRef.getName() + "-" + getClusterRoleBindingName(controllerName),
"watch all namespaces", roleRef));
}
});
} else {
// create a RoleBinding using either the provided deployment namespace or the desired watched namespace name
desiredWatchedNamespaces
.forEach(ns -> {
itemsToAdd.add(createRoleBinding(roleBindingName, serviceAccountName, ns,
createDefaultRoleRef(getClusterRoleName(controllerName))));
//add additional Role Bindings
controllerConfiguration.getAdditionalRBACRoleRefs()
.forEach(roleRef -> {
final var specificRoleBindingName = getSpecificRoleBindingName(controllerName, roleRef);
itemsToAdd.add(createRoleBinding(specificRoleBindingName, serviceAccountName,
ns, roleRef));
});
});
}

return itemsToAdd;
this.serviceAccountName = serviceAccountName;
this.serviceAccountNamespace = serviceAccountNamespace;
}

public static String getCRDValidatingBindingName(String controllerName) {
Expand All @@ -142,12 +72,11 @@ 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();
}

private static RoleBinding createRoleBinding(String roleBindingName,
String serviceAccountName,
private RoleBinding createRoleBinding(String roleBindingName,
String targetNamespace,
RoleRef roleRef) {
final var nsMsg = (targetNamespace == null ? "current" : "'" + targetNamespace + "'") + " namespace";
Expand All @@ -158,36 +87,132 @@ private static RoleBinding createRoleBinding(String roleBindingName,
.withNamespace(targetNamespace)
.endMetadata()
.withRoleRef(roleRef)
.addNewSubject(null, SERVICE_ACCOUNT, serviceAccountName,
deployNamespace.orElse(null))
.addNewSubject(null, SERVICE_ACCOUNT, getServiceAccountName(),
getNamespace())
.build();
}

private static ClusterRoleBinding createClusterRoleBinding(String serviceAccountName,
private String getServiceAccountName() {
return serviceAccountName;
}

private String getNamespace() {
return serviceAccountNamespace;
}

private ClusterRoleBinding createClusterRoleBinding(
String controllerName, String bindingName, String controllerConfMessage,
RoleRef roleRef) {
outputWarningIfNeeded(controllerName, bindingName, controllerConfMessage);
roleRef = roleRef == null ? createDefaultRoleRef(serviceAccountName) : roleRef;
final var ns = deployNamespace.orElse(null);
roleRef = roleRef == null ? createDefaultRoleRef(controllerName) : roleRef;
final var ns = getNamespace();
log.infov("Creating ''{0}'' ClusterRoleBinding to be applied to ''{1}'' namespace", bindingName, ns);
return new ClusterRoleBindingBuilder()
.withNewMetadata().withName(bindingName)
.endMetadata()
.withRoleRef(roleRef)
.addNewSubject()
.withKind(SERVICE_ACCOUNT).withName(serviceAccountName).withNamespace(ns)
.withKind(SERVICE_ACCOUNT).withName(getServiceAccountName()).withNamespace(ns)
.endSubject()
.build();
}

private static void outputWarningIfNeeded(String controllerName, String crBindingName, String controllerConfMessage) {
private void outputWarningIfNeeded(String controllerName, String crBindingName, String controllerConfMessage) {
// the decorator can be called several times but we only want to output the warning once
if (deployNamespace.isEmpty()) {
if (getNamespace() == null) {
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);
}
}

@Override
public void visit(KubernetesListBuilder list) {
configs.forEach(config -> {
final var toAdd = cachedBindings.computeIfAbsent(config, this::bindingsFor);
list.addAllToItems(toAdd);
});
}

@Override
@SuppressWarnings("unchecked")
public Class<? extends Decorator>[] after() {
return new Class[] { AddServiceAccountResourceDecorator.class };
}

private String getServiceAccountName(KubernetesListBuilder list) {
final var items = list.getVisitableMap()
.map(visitables -> visitables.get("items"))
.orElseThrow(() -> new IllegalStateException("Items not found in generated resources list"));
final var deployment = items.stream().filter(visitable -> visitable instanceof DeploymentBuilder)
.map(DeploymentBuilder.class::cast)
.map(DeploymentBuilder::build)
.findFirst()
.orElseThrow(() -> new IllegalStateException("Deployment not found in generated resources list"));
return Optional.ofNullable(deployment.getSpec().getTemplate().getSpec().getServiceAccountName())
.orElseThrow(() -> new IllegalStateException("Service account name not found in generated resources list"));
}

private List<HasMetadata> bindingsFor(QuarkusControllerConfiguration<?> controllerConfiguration) {
final var controllerName = controllerConfiguration.getName();

// retrieve which namespaces should be used to generate either from annotation or from the build time configuration
final var desiredWatchedNamespaces = controllerConfiguration.getNamespaces();

// if we validate the CRDs, also create a binding for the CRD validating role
List<HasMetadata> itemsToAdd;
if (operatorConfiguration.crd().validate()) {
final var crBindingName = getCRDValidatingBindingName(controllerName);
final var crdValidatorRoleBinding = createClusterRoleBinding(controllerName,
crBindingName, "validate CRDs", CRD_VALIDATING_ROLE_REF);
itemsToAdd = new ArrayList<>(desiredWatchedNamespaces.size() + 1);
itemsToAdd.add(crdValidatorRoleBinding);
} else {
itemsToAdd = new ArrayList<>(desiredWatchedNamespaces.size());
}

final var roleBindingName = getRoleBindingName(controllerName);
if (controllerConfiguration.watchCurrentNamespace()) {
// create a RoleBinding that will be applied in the current namespace if watching only the current NS
itemsToAdd.add(createRoleBinding(roleBindingName, null, createDefaultRoleRef(controllerName)));
// add additional Role Bindings
controllerConfiguration.getAdditionalRBACRoleRefs().forEach(
roleRef -> {
final var specificRoleBindingName = getSpecificRoleBindingName(controllerName, roleRef);
itemsToAdd.add(createRoleBinding(specificRoleBindingName, null, roleRef));
});
} else if (controllerConfiguration.watchAllNamespaces()) {
itemsToAdd.add(createClusterRoleBinding(controllerName,
getClusterRoleBindingName(controllerName), "watch all namespaces",
null));
// add additional cluster role bindings only if they target cluster roles
controllerConfiguration.getAdditionalRBACRoleRefs().forEach(
roleRef -> {
if (!CLUSTER_ROLE.equals(roleRef.getKind())) {
log.warnv("Cannot create a ClusterRoleBinding for RoleRef ''{0}'' because it's not a ClusterRole",
roleRef);
} else {
itemsToAdd.add(createClusterRoleBinding(controllerName,
roleRef.getName() + "-" + getClusterRoleBindingName(controllerName),
"watch all namespaces", roleRef));
}
});
} else {
// create a RoleBinding using either the provided deployment namespace or the desired watched namespace name
desiredWatchedNamespaces
.forEach(ns -> {
itemsToAdd.add(createRoleBinding(roleBindingName, ns, createDefaultRoleRef(controllerName)));
//add additional Role Bindings
controllerConfiguration.getAdditionalRBACRoleRefs()
.forEach(roleRef -> {
final var specificRoleBindingName = getSpecificRoleBindingName(controllerName, roleRef);
itemsToAdd.add(createRoleBinding(specificRoleBindingName, ns, roleRef));
});
});
}

return itemsToAdd;
}
}
8 changes: 6 additions & 2 deletions .../deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/RBACAugmentationStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package io.quarkiverse.operatorsdk.deployment;

import java.util.List;
import java.util.function.BooleanSupplier;

import io.quarkiverse.operatorsdk.runtime.BuildTimeOperatorConfiguration;
import io.quarkus.deployment.annotations.BuildProducer;
import io.quarkus.deployment.annotations.BuildStep;
import io.quarkus.kubernetes.spi.DecoratorBuildItem;
import io.quarkus.kubernetes.spi.KubernetesEffectiveServiceAccountBuildItem;

public class RBACAugmentationStep {

Expand All @@ -21,13 +23,15 @@ public boolean getAsBoolean() {

@BuildStep(onlyIf = IsRBACEnabled.class)
void augmentRBACForResources(BuildTimeOperatorConfiguration buildTimeConfiguration,
List<KubernetesEffectiveServiceAccountBuildItem> effectiveServiceAccounts,
BuildProducer<DecoratorBuildItem> decorators,
ControllerConfigurationsBuildItem configurations) {

final var effectiveServiceAccount = effectiveServiceAccounts.get(0); // todo: fix
final var configs = configurations.getControllerConfigs().values();
decorators.produce(new DecoratorBuildItem(
new AddClusterRolesDecorator(configs, buildTimeConfiguration.crd().validate())));
decorators.produce(new DecoratorBuildItem(
new AddRoleBindingsDecorator(configs, buildTimeConfiguration)));
new AddRoleBindingsDecorator(configs, buildTimeConfiguration, effectiveServiceAccount.getServiceAccountName(),
effectiveServiceAccount.getNamespace())));
}
}
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<packaging>pom</packaging>
<name>Quarkus - Operator SDK - Parent</name>
<properties>
<quarkus.version>3.14.2</quarkus.version>
<quarkus.version>999-SNAPSHOT</quarkus.version>
<java-operator-sdk.version>4.9.4</java-operator-sdk.version>
</properties>
<scm>
Expand Down