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 1 commit
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());
}

}
178 changes: 100 additions & 78 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 @@ -13,9 +13,11 @@
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,37 +26,128 @@
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 final Collection<QuarkusControllerConfiguration<?>> configs;
private final BuildTimeOperatorConfiguration operatorConfiguration;
private static final Logger log = Logger.getLogger(AddRoleBindingsDecorator.class);
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 Collection<QuarkusControllerConfiguration<?>> configs;
private final BuildTimeOperatorConfiguration operatorConfiguration;

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

public static String getCRDValidatingBindingName(String controllerName) {
return controllerName + "-crd-validating-role-binding";
}

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

public static String getRoleBindingName(String controllerName) {
return controllerName + "-role-binding";
}

public static String getSpecificRoleBindingName(String controllerName, String roleRefName) {
return roleRefName + "-" + getRoleBindingName(controllerName);
}

public static String getSpecificRoleBindingName(String controllerName, RoleRef roleRef) {
return getSpecificRoleBindingName(controllerName, roleRef.getName());
}

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

private static RoleBinding createRoleBinding(String roleBindingName,
String serviceAccountName,
String targetNamespace,
RoleRef roleRef) {
final var nsMsg = (targetNamespace == null ? "current" : "'" + targetNamespace + "'") + " namespace";
log.infov("Creating ''{0}'' RoleBinding to be applied to {1}", roleBindingName, nsMsg);
return new RoleBindingBuilder()
.withNewMetadata()
.withName(roleBindingName)
.withNamespace(targetNamespace)
.endMetadata()
.withRoleRef(roleRef)
.addNewSubject(null, SERVICE_ACCOUNT, serviceAccountName,
deployNamespace.orElse(null))
.build();
}

private static ClusterRoleBinding createClusterRoleBinding(String serviceAccountName,
String controllerName, String bindingName, String controllerConfMessage,
RoleRef roleRef) {
outputWarningIfNeeded(controllerName, bindingName, controllerConfMessage);
roleRef = roleRef == null ? createDefaultRoleRef(serviceAccountName) : roleRef;
final var ns = deployNamespace.orElse(null);
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)
.endSubject()
.build();
}

private static 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()) {
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) {
final var serviceAccountName = getMandatoryDeploymentMetadata(list).getName();
final var serviceAccountName = getServiceAccountName(list);
configs.forEach(config -> {
final var toAdd = cachedBindings.computeIfAbsent(config, c -> bindingsFor(c, serviceAccountName));
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,
String serviceAccountName) {
final var controllerName = controllerConfiguration.getName();
Expand Down Expand Up @@ -119,75 +212,4 @@ private List<HasMetadata> bindingsFor(QuarkusControllerConfiguration<?> controll

return itemsToAdd;
}

public static String getCRDValidatingBindingName(String controllerName) {
return controllerName + "-crd-validating-role-binding";
}

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

public static String getRoleBindingName(String controllerName) {
return controllerName + "-role-binding";
}

public static String getSpecificRoleBindingName(String controllerName, String roleRefName) {
return roleRefName + "-" + getRoleBindingName(controllerName);
}

public static String getSpecificRoleBindingName(String controllerName, RoleRef roleRef) {
return getSpecificRoleBindingName(controllerName, roleRef.getName());
}

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

private static RoleBinding createRoleBinding(String roleBindingName,
String serviceAccountName,
String targetNamespace,
RoleRef roleRef) {
final var nsMsg = (targetNamespace == null ? "current" : "'" + targetNamespace + "'") + " namespace";
log.infov("Creating ''{0}'' RoleBinding to be applied to {1}", roleBindingName, nsMsg);
return new RoleBindingBuilder()
.withNewMetadata()
.withName(roleBindingName)
.withNamespace(targetNamespace)
.endMetadata()
.withRoleRef(roleRef)
.addNewSubject(null, SERVICE_ACCOUNT, serviceAccountName,
deployNamespace.orElse(null))
.build();
}

private static ClusterRoleBinding createClusterRoleBinding(String serviceAccountName,
String controllerName, String bindingName, String controllerConfMessage,
RoleRef roleRef) {
outputWarningIfNeeded(controllerName, bindingName, controllerConfMessage);
roleRef = roleRef == null ? createDefaultRoleRef(serviceAccountName) : roleRef;
final var ns = deployNamespace.orElse(null);
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)
.endSubject()
.build();
}

private static 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()) {
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);
}
}
}
Loading