diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java index b916fdcc10..d0af5ee441 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java @@ -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; @@ -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"; @@ -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) { @@ -55,7 +60,8 @@ private void init(LeaderElectionConfiguration config) { log.error(message); throw new IllegalArgumentException(message); } - final var lock = new LeaseLock(leaseNamespace, config.getLeaseName(), identity); + leaseName = config.getLeaseName(); + final var lock = new LeaseLock(leaseNamespace, leaseName, identity); leaderElector = new LeaderElectorBuilder( configurationService.getKubernetesClient(), configurationService.getExecutorServiceManager().cachingExecutorService()) @@ -69,7 +75,7 @@ private void init(LeaderElectionConfiguration config) { // this is required to be false to receive stop event in all cases, thus stopLeading // is called always when leadership is lost/cancelled false, - config.getLeaseName())) + leaseName)) .build(); } @@ -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 values, String match) { + return values.contains(match) || values.contains(UNIVERSAL_VALUE); } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java index 98a5917230..ee56d6a99d 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java @@ -3,11 +3,15 @@ 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; @@ -15,6 +19,8 @@ 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; @@ -22,9 +28,9 @@ 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( @@ -48,7 +54,7 @@ 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()); } @@ -56,6 +62,64 @@ void testInitInferLeaseNamespace(@TempDir Path tempDir) throws IOException { @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); } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/MockKubernetesClient.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/MockKubernetesClient.java index eebf7a72d4..c9bff4a3d6 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/MockKubernetesClient.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/MockKubernetesClient.java @@ -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; @@ -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; @@ -32,12 +33,23 @@ public class MockKubernetesClient { public static KubernetesClient client(Class clazz) { - return client(clazz, null); + return client(clazz, null, null); + } + + public static KubernetesClient client(Class clazz, + Object selfSubjectReview) { + return client(clazz, null, selfSubjectReview); } - @SuppressWarnings({"unchecked", "rawtypes"}) public static KubernetesClient client(Class clazz, Consumer informerRunBehavior) { + return client(clazz, informerRunBehavior, null); + } + + @SuppressWarnings({"unchecked", "rawtypes"}) + public static KubernetesClient client(Class clazz, + Consumer informerRunBehavior, + Object selfSubjectReview) { final var client = mock(KubernetesClient.class); MixedOperation, Resource> resources = mock(MixedOperation.class); @@ -85,7 +97,9 @@ public static KubernetesClient client(Class 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); @@ -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; }