Skip to content

Commit 62f60ea

Browse files
committed
improved permission checking
1 parent 18f396a commit 62f60ea

File tree

2 files changed

+40
-22
lines changed

2 files changed

+40
-22
lines changed

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

Lines changed: 39 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66
import org.slf4j.Logger;
77
import org.slf4j.LoggerFactory;
88

9+
import io.fabric8.kubernetes.api.model.authorization.v1.ResourceAttributesBuilder;
10+
import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectAccessReview;
11+
import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectAccessReviewSpecBuilder;
912
import io.fabric8.kubernetes.client.KubernetesClient;
10-
import io.fabric8.kubernetes.client.KubernetesClientException;
1113
import io.fabric8.kubernetes.client.extended.leaderelection.LeaderCallbacks;
1214
import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElectionConfig;
1315
import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElector;
@@ -28,8 +30,9 @@ public class LeaderElectionManager {
2830
private final ControllerManager controllerManager;
2931
private String identity;
3032
private CompletableFuture<?> leaderElectionFuture;
31-
private LeaderElectionConfig leaderElectionConfig;
3233
private KubernetesClient client;
34+
private String leaseName;
35+
private String leaseNamespace;
3336

3437
public LeaderElectionManager(ControllerManager controllerManager) {
3538
this.controllerManager = controllerManager;
@@ -38,7 +41,8 @@ public LeaderElectionManager(ControllerManager controllerManager) {
3841
public void init(LeaderElectionConfiguration config, KubernetesClient client) {
3942
this.client = client;
4043
this.identity = identity(config);
41-
final var leaseNamespace =
44+
this.leaseName = config.getLeaseName();
45+
leaseNamespace =
4246
config.getLeaseNamespace().orElseGet(
4347
() -> ConfigurationServiceProvider.instance().getClientConfiguration().getNamespace());
4448
if (leaseNamespace == null) {
@@ -47,21 +51,19 @@ public void init(LeaderElectionConfiguration config, KubernetesClient client) {
4751
log.error(message);
4852
throw new IllegalArgumentException(message);
4953
}
50-
final var lock = new LeaseLock(leaseNamespace, config.getLeaseName(), identity);
54+
final var lock = new LeaseLock(leaseNamespace, leaseName, identity);
5155
// releaseOnCancel is not used in the underlying implementation
52-
leaderElectionConfig = new LeaderElectionConfig(
53-
lock,
54-
config.getLeaseDuration(),
55-
config.getRenewDeadline(),
56-
config.getRetryPeriod(),
57-
leaderCallbacks(),
58-
true,
59-
config.getLeaseName());
60-
6156
leaderElector =
6257
new LeaderElectorBuilder(
6358
client, ExecutorServiceManager.instance().executorService())
64-
.withConfig(leaderElectionConfig)
59+
.withConfig(new LeaderElectionConfig(
60+
lock,
61+
config.getLeaseDuration(),
62+
config.getRenewDeadline(),
63+
config.getRetryPeriod(),
64+
leaderCallbacks(),
65+
true,
66+
config.getLeaseName()))
6567
.build();
6668
}
6769

@@ -110,14 +112,30 @@ public void stop() {
110112
}
111113

112114
private void checkLeaseAccess() {
113-
try {
114-
leaderElectionConfig.getLock().get(client);
115-
} catch (KubernetesClientException e) {
116-
if (e.getCode() == 403) {
117-
throw new OperatorException(NO_PERMISSION_TO_LEASE_RESOURCE_MESSAGE, e);
118-
} else {
119-
throw e;
115+
var verbs = new String[] {"create", "update", "get"};
116+
for (String verb : verbs) {
117+
var allowed = checkLeaseAccess(verb);
118+
if (!allowed) {
119+
throw new OperatorException(NO_PERMISSION_TO_LEASE_RESOURCE_MESSAGE);
120120
}
121121
}
122122
}
123+
124+
private boolean checkLeaseAccess(String verb) {
125+
var res = client.resource(selfSubjectReview(verb, leaseName, leaseNamespace)).create();
126+
return res.getStatus().getAllowed();
127+
}
128+
129+
private SelfSubjectAccessReview selfSubjectReview(String verb, String name, String namespace) {
130+
var res = new SelfSubjectAccessReview();
131+
res.setSpec(new SelfSubjectAccessReviewSpecBuilder()
132+
.withResourceAttributes(new ResourceAttributesBuilder()
133+
.withGroup("coordination.k8s.io")
134+
.withNamespace(namespace)
135+
.withName(name)
136+
.withVerb(verb)
137+
.build())
138+
.build());
139+
return res;
140+
}
123141
}

operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionPermissionIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import org.junit.jupiter.api.Test;
44

55
import io.fabric8.kubernetes.api.model.ConfigMap;
6+
import io.fabric8.kubernetes.api.model.authorization.v1.*;
67
import io.fabric8.kubernetes.api.model.rbac.Role;
78
import io.fabric8.kubernetes.api.model.rbac.RoleBinding;
89
import io.fabric8.kubernetes.client.ConfigBuilder;
@@ -68,5 +69,4 @@ private void applyRole() {
6869
.loadYaml(Role.class, this.getClass(), "leader-elector-stop-role-noaccess.yaml");
6970
adminClient.resource(role).createOrReplace();
7071
}
71-
7272
}

0 commit comments

Comments
 (0)