Skip to content

Commit

Permalink
Update lease access check to account for lease name and multiple rules
Browse files Browse the repository at this point in the history 
Signed-off-by: Michael Edgar <medgar@redhat.com>
  • Loading branch information
@MikeEdgar
MikeEdgar committed Jun 26, 2024
1 parent 8f77f68 commit 109e831
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 19 deletions.
40 changes: 30 additions & 10 deletions operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
package io.javaoperatorsdk.operator;

import java.util.Arrays;
import java.util.Collection;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.function.Predicate;
import java.util.stream.Collectors;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import io.fabric8.kubernetes.api.model.authorization.v1.ResourceRule;
import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectRulesReview;
import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectRulesReviewSpecBuilder;
import io.fabric8.kubernetes.client.extended.leaderelection.LeaderCallbacks;
Expand All @@ -23,7 +27,7 @@ public class LeaderElectionManager {

public static final String NO_PERMISSION_TO_LEASE_RESOURCE_MESSAGE =
"No permission to lease resource.";
public static final String UNIVERSAL_VERB = "*";
public static final String UNIVERSAL_VALUE = "*";
public static final String COORDINATION_GROUP = "coordination.k8s.io";
public static final String LEASES_RESOURCE = "leases";

Expand All @@ -33,6 +37,7 @@ public class LeaderElectionManager {
private CompletableFuture<?> leaderElectionFuture;
private final ConfigurationService configurationService;
private String leaseNamespace;
private String leaseName;

LeaderElectionManager(ControllerManager controllerManager,
ConfigurationService configurationService) {
Expand All @@ -55,6 +60,7 @@ private void init(LeaderElectionConfiguration config) {
log.error(message);
throw new IllegalArgumentException(message);
}
leaseName = config.getLeaseName();
final var lock = new LeaseLock(leaseNamespace, config.getLeaseName(), identity);
leaderElector = new LeaderElectorBuilder(
configurationService.getKubernetesClient(),
Expand Down Expand Up @@ -126,19 +132,33 @@ public void stop() {
}

private void checkLeaseAccess() {
var verbs = Arrays.asList("create", "update", "get");
var verbsRequired = Arrays.asList("create", "update", "get");
SelfSubjectRulesReview review = new SelfSubjectRulesReview();
review.setSpec(new SelfSubjectRulesReviewSpecBuilder().withNamespace(leaseNamespace).build());
var reviewResult = configurationService.getKubernetesClient().resource(review).create();
log.debug("SelfSubjectRulesReview result: {}", reviewResult);
var foundRule = reviewResult.getStatus().getResourceRules().stream()
.filter(rule -> rule.getApiGroups().contains(COORDINATION_GROUP)
&& rule.getResources().contains(LEASES_RESOURCE)
&& (rule.getVerbs().containsAll(verbs)) || rule.getVerbs().contains(UNIVERSAL_VERB))
.findAny();
if (foundRule.isEmpty()) {
throw new OperatorException(NO_PERMISSION_TO_LEASE_RESOURCE_MESSAGE +
" in namespace: " + leaseNamespace);
var verbsAllowed = reviewResult.getStatus().getResourceRules().stream()
.filter(rule -> matchesValue(rule.getApiGroups(), COORDINATION_GROUP))
.filter(rule -> matchesValue(rule.getResources(), LEASES_RESOURCE))
.filter(rule -> rule.getResourceNames().isEmpty()
|| rule.getResourceNames().contains(leaseName))
.map(ResourceRule::getVerbs)
.flatMap(Collection::stream)
.distinct()
.collect(Collectors.toList());
if (verbsAllowed.contains(UNIVERSAL_VALUE) || verbsAllowed.containsAll(verbsRequired)) {
return;
}

var missingVerbs = verbsRequired.stream()
.filter(Predicate.not(verbsAllowed::contains))
.collect(Collectors.toList());

throw new OperatorException(NO_PERMISSION_TO_LEASE_RESOURCE_MESSAGE +
" in namespace: " + leaseNamespace + "; missing required verbs: " + missingVerbs);
}

private boolean matchesValue(Collection<String> values, String match) {
return values.contains(match) || values.contains(UNIVERSAL_VALUE);
}
}
72 changes: 68 additions & 4 deletions ...r-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,34 @@
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;

import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

import io.fabric8.kubernetes.api.model.authorization.v1.ResourceRule;
import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectRulesReview;
import io.fabric8.kubernetes.api.model.authorization.v1.SubjectRulesReviewStatus;
import io.fabric8.kubernetes.api.model.coordination.v1.Lease;
import io.fabric8.kubernetes.client.Config;
import io.javaoperatorsdk.operator.api.config.ConfigurationService;
import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration;

import static io.fabric8.kubernetes.client.Config.KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY;
import static io.fabric8.kubernetes.client.Config.KUBERNETES_NAMESPACE_FILE;
import static io.javaoperatorsdk.operator.LeaderElectionManager.COORDINATION_GROUP;
import static io.javaoperatorsdk.operator.LeaderElectionManager.LEASES_RESOURCE;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

class LeaderElectionManagerTest {

private LeaderElectionManager leaderElectionManager() {
private LeaderElectionManager leaderElectionManager(Object selfSubjectReview) {
ControllerManager controllerManager = mock(ControllerManager.class);
final var kubernetesClient = MockKubernetesClient.client(Lease.class);
final var kubernetesClient = MockKubernetesClient.client(Lease.class, selfSubjectReview);
when(kubernetesClient.getConfiguration()).thenReturn(Config.autoConfigure(null));
var configurationService =
ConfigurationService.newOverriddenConfigurationService(
Expand All @@ -48,14 +54,72 @@ void testInitInferLeaseNamespace(@TempDir Path tempDir) throws IOException {
System.setProperty(KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false");
System.setProperty(KUBERNETES_NAMESPACE_FILE, namespacePath.toString());

final var leaderElectionManager = leaderElectionManager();
final var leaderElectionManager = leaderElectionManager(null);
leaderElectionManager.start();
assertTrue(leaderElectionManager.isLeaderElectionEnabled());
}

@Test
void testFailedToInitInferLeaseNamespace() {
System.setProperty(KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false");
assertThrows(IllegalArgumentException.class, () -> leaderElectionManager().start());
final var leaderElectionManager = leaderElectionManager(null);
assertThrows(IllegalArgumentException.class, leaderElectionManager::start);
}

@Test
void testInitPermissionsMultipleRulesWithResourceName(@TempDir Path tempDir) throws IOException {
var namespace = "foo";
var namespacePath = tempDir.resolve("namespace");
Files.writeString(namespacePath, namespace);

System.setProperty(KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false");
System.setProperty(KUBERNETES_NAMESPACE_FILE, namespacePath.toString());

SelfSubjectRulesReview review = new SelfSubjectRulesReview();
review.setStatus(new SubjectRulesReviewStatus());
var resourceRule1 = new ResourceRule();
resourceRule1.setApiGroups(Arrays.asList(COORDINATION_GROUP));
resourceRule1.setResources(Arrays.asList(LEASES_RESOURCE));
resourceRule1.setResourceNames(Arrays.asList("test"));
resourceRule1.setVerbs(Arrays.asList("get", "update"));
var resourceRule2 = new ResourceRule();
resourceRule2.setApiGroups(Arrays.asList(COORDINATION_GROUP));
resourceRule2.setResources(Arrays.asList(LEASES_RESOURCE));
resourceRule2.setVerbs(Arrays.asList("create"));
review.getStatus().setResourceRules(Arrays.asList(resourceRule1, resourceRule2));

final var leaderElectionManager = leaderElectionManager(review);
leaderElectionManager.start();
assertTrue(leaderElectionManager.isLeaderElectionEnabled());
}

@Test
void testFailedToInitMissingPermission(@TempDir Path tempDir) throws IOException {
var namespace = "foo";
var namespacePath = tempDir.resolve("namespace");
Files.writeString(namespacePath, namespace);

System.setProperty(KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false");
System.setProperty(KUBERNETES_NAMESPACE_FILE, namespacePath.toString());

SelfSubjectRulesReview review = new SelfSubjectRulesReview();
review.setStatus(new SubjectRulesReviewStatus());
var resourceRule1 = new ResourceRule();
resourceRule1.setApiGroups(Arrays.asList(COORDINATION_GROUP));
resourceRule1.setResources(Arrays.asList(LEASES_RESOURCE));
resourceRule1.setVerbs(Arrays.asList("get"));
var resourceRule2 = new ResourceRule();
resourceRule2.setApiGroups(Arrays.asList(COORDINATION_GROUP));
resourceRule2.setResources(Arrays.asList(LEASES_RESOURCE));
resourceRule2.setVerbs(Arrays.asList("update"));
var resourceRule3 = new ResourceRule();
resourceRule3.setApiGroups(Arrays.asList(COORDINATION_GROUP));
resourceRule3.setResources(Arrays.asList(LEASES_RESOURCE));
resourceRule3.setResourceNames(Arrays.asList("some-other-lease"));
resourceRule3.setVerbs(Arrays.asList("create"));
review.getStatus().setResourceRules(Arrays.asList(resourceRule1, resourceRule2, resourceRule3));

final var leaderElectionManager = leaderElectionManager(review);
assertThrows(OperatorException.class, leaderElectionManager::start);
}
}
24 changes: 19 additions & 5 deletions operator-framework-core/src/test/java/io/javaoperatorsdk/operator/MockKubernetesClient.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.javaoperatorsdk.operator;

import java.util.Arrays;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.Executors;
import java.util.function.Consumer;
Expand All @@ -20,7 +21,7 @@

import static io.javaoperatorsdk.operator.LeaderElectionManager.COORDINATION_GROUP;
import static io.javaoperatorsdk.operator.LeaderElectionManager.LEASES_RESOURCE;
import static io.javaoperatorsdk.operator.LeaderElectionManager.UNIVERSAL_VERB;
import static io.javaoperatorsdk.operator.LeaderElectionManager.UNIVERSAL_VALUE;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyLong;
import static org.mockito.Mockito.anyString;
Expand All @@ -32,12 +33,23 @@
public class MockKubernetesClient {

public static <T extends HasMetadata> KubernetesClient client(Class<T> clazz) {
return client(clazz, null);
return client(clazz, null, null);
}

public static <T extends HasMetadata> KubernetesClient client(Class<T> clazz,
Object selfSubjectReview) {
return client(clazz, null, selfSubjectReview);
}

@SuppressWarnings({"unchecked", "rawtypes"})
public static <T extends HasMetadata> KubernetesClient client(Class<T> clazz,
Consumer<Void> informerRunBehavior) {
return client(clazz, informerRunBehavior, null);
}

@SuppressWarnings({"unchecked", "rawtypes"})
public static <T extends HasMetadata> KubernetesClient client(Class<T> clazz,
Consumer<Void> informerRunBehavior,
Object selfSubjectReview) {
final var client = mock(KubernetesClient.class);
MixedOperation<T, KubernetesResourceList<T>, Resource<T>> resources =
mock(MixedOperation.class);
Expand Down Expand Up @@ -85,7 +97,9 @@ public static <T extends HasMetadata> KubernetesClient client(Class<T> clazz,
var selfSubjectResourceResourceMock = mock(NamespaceableResource.class);
when(client.resource(any(SelfSubjectRulesReview.class)))
.thenReturn(selfSubjectResourceResourceMock);
when(selfSubjectResourceResourceMock.create()).thenReturn(allowSelfSubjectReview());
when(selfSubjectResourceResourceMock.create())
.thenReturn(Optional.ofNullable(selfSubjectReview)
.orElseGet(MockKubernetesClient::allowSelfSubjectReview));

final var apiGroupDSL = mock(ApiextensionsAPIGroupDSL.class);
when(client.apiextensions()).thenReturn(apiGroupDSL);
Expand All @@ -107,7 +121,7 @@ private static Object allowSelfSubjectReview() {
var resourceRule = new ResourceRule();
resourceRule.setApiGroups(Arrays.asList(COORDINATION_GROUP));
resourceRule.setResources(Arrays.asList(LEASES_RESOURCE));
resourceRule.setVerbs(Arrays.asList(UNIVERSAL_VERB));
resourceRule.setVerbs(Arrays.asList(UNIVERSAL_VALUE));
review.getStatus().setResourceRules(Arrays.asList(resourceRule));
return review;
}
Expand Down

0 comments on commit 109e831

Please sign in to comment.