Skip to content

Commit 0efdf85

Browse files
MikeEdgarmetacosm
MikeEdgar
and
metacosm
authored
Update lease access check to account for lease name and multiple rules (#2456)
 Signed-off-by: Michael Edgar <medgar@redhat.com> Signed-off-by: Chris Laprun <claprun@redhat.com> Co-authored-by: Chris Laprun <claprun@redhat.com>
1 parent dbfb4a6 commit 0efdf85

File tree

3 files changed

+119
-21
lines changed

3 files changed

+119
-21
lines changed

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

Lines changed: 32 additions & 12 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;
@@ -23,7 +27,7 @@ public class LeaderElectionManager {
2327

2428
public static final String NO_PERMISSION_TO_LEASE_RESOURCE_MESSAGE =
2529
"No permission to lease resource.";
26-
public static final String UNIVERSAL_VERB = "*";
30+
public static final String UNIVERSAL_VALUE = "*";
2731
public static final String COORDINATION_GROUP = "coordination.k8s.io";
2832
public static final String LEASES_RESOURCE = "leases";
2933

@@ -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,7 +60,8 @@ private void init(LeaderElectionConfiguration config) {
5560
log.error(message);
5661
throw new IllegalArgumentException(message);
5762
}
58-
final var lock = new LeaseLock(leaseNamespace, config.getLeaseName(), identity);
63+
leaseName = config.getLeaseName();
64+
final var lock = new LeaseLock(leaseNamespace, leaseName, identity);
5965
leaderElector = new LeaderElectorBuilder(
6066
configurationService.getKubernetesClient(),
6167
configurationService.getExecutorServiceManager().cachingExecutorService())
@@ -69,7 +75,7 @@ private void init(LeaderElectionConfiguration config) {
6975
// this is required to be false to receive stop event in all cases, thus stopLeading
7076
// is called always when leadership is lost/cancelled
7177
false,
72-
config.getLeaseName()))
78+
leaseName))
7379
.build();
7480
}
7581

@@ -126,19 +132,33 @@ 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 -> matchesValue(rule.getApiGroups(), COORDINATION_GROUP))
142+
.filter(rule -> matchesValue(rule.getResources(), LEASES_RESOURCE))
143+
.filter(rule -> rule.getResourceNames().isEmpty()
144+
|| rule.getResourceNames().contains(leaseName))
145+
.map(ResourceRule::getVerbs)
146+
.flatMap(Collection::stream)
147+
.distinct()
148+
.collect(Collectors.toList());
149+
if (verbsAllowed.contains(UNIVERSAL_VALUE) || verbsAllowed.containsAll(verbsRequired)) {
150+
return;
142151
}
152+
153+
var missingVerbs = verbsRequired.stream()
154+
.filter(Predicate.not(verbsAllowed::contains))
155+
.collect(Collectors.toList());
156+
157+
throw new OperatorException(NO_PERMISSION_TO_LEASE_RESOURCE_MESSAGE +
158+
" in namespace: " + leaseNamespace + "; missing required verbs: " + missingVerbs);
159+
}
160+
161+
private boolean matchesValue(Collection<String> values, String match) {
162+
return values.contains(match) || values.contains(UNIVERSAL_VALUE);
143163
}
144164
}

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

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

78
import org.junit.jupiter.api.AfterEach;
89
import org.junit.jupiter.api.Test;
910
import org.junit.jupiter.api.io.TempDir;
1011

12+
import io.fabric8.kubernetes.api.model.authorization.v1.ResourceRule;
13+
import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectRulesReview;
14+
import io.fabric8.kubernetes.api.model.authorization.v1.SubjectRulesReviewStatus;
1115
import io.fabric8.kubernetes.api.model.coordination.v1.Lease;
1216
import io.fabric8.kubernetes.client.Config;
1317
import io.javaoperatorsdk.operator.api.config.ConfigurationService;
1418
import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration;
1519

1620
import static io.fabric8.kubernetes.client.Config.KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY;
1721
import static io.fabric8.kubernetes.client.Config.KUBERNETES_NAMESPACE_FILE;
22+
import static io.javaoperatorsdk.operator.LeaderElectionManager.COORDINATION_GROUP;
23+
import static io.javaoperatorsdk.operator.LeaderElectionManager.LEASES_RESOURCE;
1824
import static org.junit.jupiter.api.Assertions.assertThrows;
1925
import static org.junit.jupiter.api.Assertions.assertTrue;
2026
import static org.mockito.Mockito.mock;
2127
import static org.mockito.Mockito.when;
2228

2329
class LeaderElectionManagerTest {
2430

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

51-
final var leaderElectionManager = leaderElectionManager();
57+
final var leaderElectionManager = leaderElectionManager(null);
5258
leaderElectionManager.start();
5359
assertTrue(leaderElectionManager.isLeaderElectionEnabled());
5460
}
5561

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

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

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package io.javaoperatorsdk.operator;
22

33
import java.util.Arrays;
4+
import java.util.Optional;
45
import java.util.concurrent.CompletableFuture;
56
import java.util.concurrent.Executors;
67
import java.util.function.Consumer;
@@ -20,7 +21,7 @@
2021

2122
import static io.javaoperatorsdk.operator.LeaderElectionManager.COORDINATION_GROUP;
2223
import static io.javaoperatorsdk.operator.LeaderElectionManager.LEASES_RESOURCE;
23-
import static io.javaoperatorsdk.operator.LeaderElectionManager.UNIVERSAL_VERB;
24+
import static io.javaoperatorsdk.operator.LeaderElectionManager.UNIVERSAL_VALUE;
2425
import static org.mockito.Mockito.any;
2526
import static org.mockito.Mockito.anyLong;
2627
import static org.mockito.Mockito.anyString;
@@ -32,12 +33,23 @@
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, null);
37+
}
38+
39+
public static <T extends HasMetadata> KubernetesClient client(Class<T> clazz,
40+
Object selfSubjectReview) {
41+
return client(clazz, null, selfSubjectReview);
3642
}
3743

38-
@SuppressWarnings({"unchecked", "rawtypes"})
3944
public static <T extends HasMetadata> KubernetesClient client(Class<T> clazz,
4045
Consumer<Void> informerRunBehavior) {
46+
return client(clazz, informerRunBehavior, null);
47+
}
48+
49+
@SuppressWarnings({"unchecked", "rawtypes"})
50+
public static <T extends HasMetadata> KubernetesClient client(Class<T> clazz,
51+
Consumer<Void> informerRunBehavior,
52+
Object selfSubjectReview) {
4153
final var client = mock(KubernetesClient.class);
4254
MixedOperation<T, KubernetesResourceList<T>, Resource<T>> resources =
4355
mock(MixedOperation.class);
@@ -85,7 +97,9 @@ public static <T extends HasMetadata> KubernetesClient client(Class<T> clazz,
8597
var selfSubjectResourceResourceMock = mock(NamespaceableResource.class);
8698
when(client.resource(any(SelfSubjectRulesReview.class)))
8799
.thenReturn(selfSubjectResourceResourceMock);
88-
when(selfSubjectResourceResourceMock.create()).thenReturn(allowSelfSubjectReview());
100+
when(selfSubjectResourceResourceMock.create())
101+
.thenReturn(Optional.ofNullable(selfSubjectReview)
102+
.orElseGet(MockKubernetesClient::allowSelfSubjectReview));
89103

90104
final var apiGroupDSL = mock(ApiextensionsAPIGroupDSL.class);
91105
when(client.apiextensions()).thenReturn(apiGroupDSL);
@@ -107,7 +121,7 @@ private static Object allowSelfSubjectReview() {
107121
var resourceRule = new ResourceRule();
108122
resourceRule.setApiGroups(Arrays.asList(COORDINATION_GROUP));
109123
resourceRule.setResources(Arrays.asList(LEASES_RESOURCE));
110-
resourceRule.setVerbs(Arrays.asList(UNIVERSAL_VERB));
124+
resourceRule.setVerbs(Arrays.asList(UNIVERSAL_VALUE));
111125
review.getStatus().setResourceRules(Arrays.asList(resourceRule));
112126
return review;
113127
}

0 commit comments

Comments
 (0)