Skip to content

Commit 6b6b9a1

Browse files
MikeEdgarMikeEdgar
committed
Update lease access check to account for lease name and multiple rules
Signed-off-by: Michael Edgar <medgar@redhat.com>
1 parent dcc3283 commit 6b6b9a1

File tree

3 files changed

+109
-16
lines changed

3 files changed

+109
-16
lines changed

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
package io.javaoperatorsdk.operator;
22

33
import java.util.Arrays;
4+
import java.util.Collection;
45
import java.util.UUID;
56
import java.util.concurrent.CompletableFuture;
7+
import java.util.function.Predicate;
8+
import java.util.stream.Collectors;
69

710
import org.slf4j.Logger;
811
import org.slf4j.LoggerFactory;
912

13+
import io.fabric8.kubernetes.api.model.authorization.v1.ResourceRule;
1014
import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectRulesReview;
1115
import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectRulesReviewSpecBuilder;
1216
import io.fabric8.kubernetes.client.extended.leaderelection.LeaderCallbacks;
@@ -33,6 +37,7 @@ public class LeaderElectionManager {
3337
private CompletableFuture<?> leaderElectionFuture;
3438
private final ConfigurationService configurationService;
3539
private String leaseNamespace;
40+
private String leaseName;
3641

3742
LeaderElectionManager(ControllerManager controllerManager,
3843
ConfigurationService configurationService) {
@@ -55,6 +60,7 @@ private void init(LeaderElectionConfiguration config) {
5560
log.error(message);
5661
throw new IllegalArgumentException(message);
5762
}
63+
leaseName = config.getLeaseName();
5864
final var lock = new LeaseLock(leaseNamespace, config.getLeaseName(), identity);
5965
leaderElector = new LeaderElectorBuilder(
6066
configurationService.getKubernetesClient(),
@@ -126,19 +132,28 @@ public void stop() {
126132
}
127133

128134
private void checkLeaseAccess() {
129-
var verbs = Arrays.asList("create", "update", "get");
135+
var verbsRequired = Arrays.asList("create", "update", "get");
130136
SelfSubjectRulesReview review = new SelfSubjectRulesReview();
131137
review.setSpec(new SelfSubjectRulesReviewSpecBuilder().withNamespace(leaseNamespace).build());
132138
var reviewResult = configurationService.getKubernetesClient().resource(review).create();
133139
log.debug("SelfSubjectRulesReview result: {}", reviewResult);
134-
var foundRule = reviewResult.getStatus().getResourceRules().stream()
135-
.filter(rule -> rule.getApiGroups().contains(COORDINATION_GROUP)
136-
&& rule.getResources().contains(LEASES_RESOURCE)
137-
&& (rule.getVerbs().containsAll(verbs)) || rule.getVerbs().contains(UNIVERSAL_VERB))
138-
.findAny();
139-
if (foundRule.isEmpty()) {
140-
throw new OperatorException(NO_PERMISSION_TO_LEASE_RESOURCE_MESSAGE +
141-
" in namespace: " + leaseNamespace);
140+
var verbsAllowed = reviewResult.getStatus().getResourceRules().stream()
141+
.filter(rule -> rule.getApiGroups().contains(COORDINATION_GROUP))
142+
.filter(rule -> rule.getResources().contains(LEASES_RESOURCE))
143+
.filter(rule -> rule.getResourceNames().isEmpty() || rule.getResourceNames().contains(leaseName))
144+
.map(ResourceRule::getVerbs)
145+
.flatMap(Collection::stream)
146+
.distinct()
147+
.collect(Collectors.toList());
148+
if (verbsAllowed.contains(UNIVERSAL_VERB) || verbsAllowed.containsAll(verbsRequired)) {
149+
return;
142150
}
151+
152+
var missingVerbs = verbsRequired.stream()
153+
.filter(Predicate.not(verbsAllowed::contains))
154+
.collect(Collectors.toList());
155+
156+
throw new OperatorException(NO_PERMISSION_TO_LEASE_RESOURCE_MESSAGE +
157+
" in namespace: " + leaseNamespace + "; missing required verbs: " + missingVerbs);
143158
}
144159
}

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java

Lines changed: 71 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,28 +3,37 @@
33
import java.io.IOException;
44
import java.nio.file.Files;
55
import java.nio.file.Path;
6+
import java.util.Arrays;
7+
import java.util.Optional;
68

79
import org.junit.jupiter.api.AfterEach;
810
import org.junit.jupiter.api.Test;
911
import org.junit.jupiter.api.io.TempDir;
1012

13+
import io.fabric8.kubernetes.api.model.authorization.v1.ResourceRule;
14+
import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectRulesReview;
15+
import io.fabric8.kubernetes.api.model.authorization.v1.SubjectRulesReviewStatus;
1116
import io.fabric8.kubernetes.api.model.coordination.v1.Lease;
1217
import io.fabric8.kubernetes.client.Config;
1318
import io.javaoperatorsdk.operator.api.config.ConfigurationService;
1419
import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration;
1520

1621
import static io.fabric8.kubernetes.client.Config.KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY;
1722
import static io.fabric8.kubernetes.client.Config.KUBERNETES_NAMESPACE_FILE;
23+
import static io.javaoperatorsdk.operator.LeaderElectionManager.COORDINATION_GROUP;
24+
import static io.javaoperatorsdk.operator.LeaderElectionManager.LEASES_RESOURCE;
1825
import static org.junit.jupiter.api.Assertions.assertThrows;
1926
import static org.junit.jupiter.api.Assertions.assertTrue;
2027
import static org.mockito.Mockito.mock;
2128
import static org.mockito.Mockito.when;
2229

2330
class LeaderElectionManagerTest {
2431

25-
private LeaderElectionManager leaderElectionManager() {
32+
private LeaderElectionManager leaderElectionManager(Optional<Object> selfSubjectReview) {
2633
ControllerManager controllerManager = mock(ControllerManager.class);
27-
final var kubernetesClient = MockKubernetesClient.client(Lease.class);
34+
final var kubernetesClient = selfSubjectReview
35+
.map(review -> MockKubernetesClient.client(Lease.class, () -> review))
36+
.orElseGet(() -> MockKubernetesClient.client(Lease.class));
2837
when(kubernetesClient.getConfiguration()).thenReturn(Config.autoConfigure(null));
2938
var configurationService =
3039
ConfigurationService.newOverriddenConfigurationService(
@@ -48,14 +57,72 @@ void testInitInferLeaseNamespace(@TempDir Path tempDir) throws IOException {
4857
System.setProperty(KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false");
4958
System.setProperty(KUBERNETES_NAMESPACE_FILE, namespacePath.toString());
5059

51-
final var leaderElectionManager = leaderElectionManager();
60+
final var leaderElectionManager = leaderElectionManager(Optional.empty());
5261
leaderElectionManager.start();
5362
assertTrue(leaderElectionManager.isLeaderElectionEnabled());
5463
}
5564

5665
@Test
5766
void testFailedToInitInferLeaseNamespace() {
5867
System.setProperty(KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false");
59-
assertThrows(IllegalArgumentException.class, () -> leaderElectionManager().start());
68+
final var leaderElectionManager = leaderElectionManager(Optional.empty());
69+
assertThrows(IllegalArgumentException.class, leaderElectionManager::start);
70+
}
71+
72+
@Test
73+
void testInitPermissionsMultipleRulesWithResourceName(@TempDir Path tempDir) throws IOException {
74+
var namespace = "foo";
75+
var namespacePath = tempDir.resolve("namespace");
76+
Files.writeString(namespacePath, namespace);
77+
78+
System.setProperty(KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false");
79+
System.setProperty(KUBERNETES_NAMESPACE_FILE, namespacePath.toString());
80+
81+
SelfSubjectRulesReview review = new SelfSubjectRulesReview();
82+
review.setStatus(new SubjectRulesReviewStatus());
83+
var resourceRule1 = new ResourceRule();
84+
resourceRule1.setApiGroups(Arrays.asList(COORDINATION_GROUP));
85+
resourceRule1.setResources(Arrays.asList(LEASES_RESOURCE));
86+
resourceRule1.setResourceNames(Arrays.asList("test"));
87+
resourceRule1.setVerbs(Arrays.asList("get", "update"));
88+
var resourceRule2 = new ResourceRule();
89+
resourceRule2.setApiGroups(Arrays.asList(COORDINATION_GROUP));
90+
resourceRule2.setResources(Arrays.asList(LEASES_RESOURCE));
91+
resourceRule2.setVerbs(Arrays.asList("create"));
92+
review.getStatus().setResourceRules(Arrays.asList(resourceRule1, resourceRule2));
93+
94+
final var leaderElectionManager = leaderElectionManager(Optional.of(review));
95+
leaderElectionManager.start();
96+
assertTrue(leaderElectionManager.isLeaderElectionEnabled());
97+
}
98+
99+
@Test
100+
void testFailedToInitMissingPermission(@TempDir Path tempDir) throws IOException {
101+
var namespace = "foo";
102+
var namespacePath = tempDir.resolve("namespace");
103+
Files.writeString(namespacePath, namespace);
104+
105+
System.setProperty(KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false");
106+
System.setProperty(KUBERNETES_NAMESPACE_FILE, namespacePath.toString());
107+
108+
SelfSubjectRulesReview review = new SelfSubjectRulesReview();
109+
review.setStatus(new SubjectRulesReviewStatus());
110+
var resourceRule1 = new ResourceRule();
111+
resourceRule1.setApiGroups(Arrays.asList(COORDINATION_GROUP));
112+
resourceRule1.setResources(Arrays.asList(LEASES_RESOURCE));
113+
resourceRule1.setVerbs(Arrays.asList("get"));
114+
var resourceRule2 = new ResourceRule();
115+
resourceRule2.setApiGroups(Arrays.asList(COORDINATION_GROUP));
116+
resourceRule2.setResources(Arrays.asList(LEASES_RESOURCE));
117+
resourceRule2.setVerbs(Arrays.asList("update"));
118+
var resourceRule3 = new ResourceRule();
119+
resourceRule3.setApiGroups(Arrays.asList(COORDINATION_GROUP));
120+
resourceRule3.setResources(Arrays.asList(LEASES_RESOURCE));
121+
resourceRule3.setResourceNames(Arrays.asList("some-other-lease"));
122+
resourceRule3.setVerbs(Arrays.asList("create"));
123+
review.getStatus().setResourceRules(Arrays.asList(resourceRule1, resourceRule2, resourceRule3));
124+
125+
final var leaderElectionManager = leaderElectionManager(Optional.of(review));
126+
assertThrows(OperatorException.class, leaderElectionManager::start);
60127
}
61128
}

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/MockKubernetesClient.java

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import java.util.concurrent.CompletableFuture;
55
import java.util.concurrent.Executors;
66
import java.util.function.Consumer;
7+
import java.util.function.Supplier;
78

89
import io.fabric8.kubernetes.api.model.HasMetadata;
910
import io.fabric8.kubernetes.api.model.KubernetesResourceList;
@@ -32,12 +33,22 @@
3233
public class MockKubernetesClient {
3334

3435
public static <T extends HasMetadata> KubernetesClient client(Class<T> clazz) {
35-
return client(clazz, null);
36+
return client(clazz, null, MockKubernetesClient::allowSelfSubjectReview);
37+
}
38+
39+
public static <T extends HasMetadata> KubernetesClient client(Class<T> clazz, Supplier<Object> selfSubjectReview) {
40+
return client(clazz, null, selfSubjectReview);
3641
}
3742

38-
@SuppressWarnings({"unchecked", "rawtypes"})
3943
public static <T extends HasMetadata> KubernetesClient client(Class<T> clazz,
4044
Consumer<Void> informerRunBehavior) {
45+
return client(clazz, informerRunBehavior, MockKubernetesClient::allowSelfSubjectReview);
46+
}
47+
48+
@SuppressWarnings({"unchecked", "rawtypes"})
49+
public static <T extends HasMetadata> KubernetesClient client(Class<T> clazz,
50+
Consumer<Void> informerRunBehavior,
51+
Supplier<Object> selfSubjectReview) {
4152
final var client = mock(KubernetesClient.class);
4253
MixedOperation<T, KubernetesResourceList<T>, Resource<T>> resources =
4354
mock(MixedOperation.class);
@@ -85,7 +96,7 @@ public static <T extends HasMetadata> KubernetesClient client(Class<T> clazz,
8596
var selfSubjectResourceResourceMock = mock(NamespaceableResource.class);
8697
when(client.resource(any(SelfSubjectRulesReview.class)))
8798
.thenReturn(selfSubjectResourceResourceMock);
88-
when(selfSubjectResourceResourceMock.create()).thenReturn(allowSelfSubjectReview());
99+
when(selfSubjectResourceResourceMock.create()).thenReturn(selfSubjectReview.get());
89100

90101
final var apiGroupDSL = mock(ApiextensionsAPIGroupDSL.class);
91102
when(client.apiextensions()).thenReturn(apiGroupDSL);

0 commit comments

Comments
 (0)