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

Update lease access check to account for lease name and multiple rules #2456

Merged
merged 2 commits into from
Jul 2, 2024
Merged

Update lease access check to account for lease name and multiple rules #2456

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
109e831
Update lease access check to account for lease name and multiple rules
MikeEdgar Jun 26, 2024
244f40e
refactor: minor
metacosm Jul 2, 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
40 changes: 30 additions & 10 deletions operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading