fix: rebase on merge

operator-framework · May 30, 2023 · 2a0a4bf · 2a0a4bf
1 parent 6bd4a66
commit 2a0a4bf
Show file tree

Hide file tree

Showing 13 changed files with 79 additions and 41 deletions.
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
@@ -1,17 +1,14 @@
 package io.javaoperatorsdk.operator;
 
-import java.util.Arrays;
 import java.util.Arrays;
 import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
 
-import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectRulesReview;
-import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectRulesReviewSpecBuilder;
-import io.javaoperatorsdk.operator.api.config.ConfigurationService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import io.fabric8.kubernetes.api.model.authorization.v1.*;
+import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectRulesReview;
+import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectRulesReviewSpecBuilder;
 import io.fabric8.kubernetes.client.KubernetesClient;
 import io.fabric8.kubernetes.client.extended.leaderelection.LeaderCallbacks;
 import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElectionConfig;
@@ -27,12 +24,15 @@ 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 COORDINATION_GROUP = "coordination.k8s.io";
+  public static final String LEASES_RESOURCE = "leases";
 
   private LeaderElector leaderElector = null;
   private final ControllerManager controllerManager;
   private String identity;
   private CompletableFuture<?> leaderElectionFuture;
-  private KubernetesClient client;
+  private KubernetesClient kubernetesClient;
   private final ConfigurationService configurationService;
   private String leaseNamespace;
 
@@ -48,31 +48,31 @@ public boolean isLeaderElectionEnabled() {
     return configurationService.getLeaderElectionConfiguration().isPresent();
   }
 
-  public void init(LeaderElectionConfiguration config) {
+  private void init(LeaderElectionConfiguration config) {
     this.identity = identity(config);
     leaseNamespace =
-            config.getLeaseNamespace().orElseGet(
-                    () -> configurationService.getClientConfiguration().getNamespace());
+        config.getLeaseNamespace().orElseGet(
+            () -> configurationService.getClientConfiguration().getNamespace());
     if (leaseNamespace == null) {
       final var message =
-              "Lease namespace is not set and cannot be inferred. Leader election cannot continue.";
+          "Lease namespace is not set and cannot be inferred. Leader election cannot continue.";
       log.error(message);
       throw new IllegalArgumentException(message);
     }
     final var lock = new LeaseLock(leaseNamespace, config.getLeaseName(), identity);
     // releaseOnCancel is not used in the underlying implementation
     leaderElector = new LeaderElectorBuilder(
-            kubernetesClient, configurationService.getExecutorServiceManager().cachingExecutorService())
-            .withConfig(
-                    new LeaderElectionConfig(
-                            lock,
-                            config.getLeaseDuration(),
-                            config.getRenewDeadline(),
-                            config.getRetryPeriod(),
-                            leaderCallbacks(),
-                            true,
-                            config.getLeaseName()))
-            .build();
+        kubernetesClient, configurationService.getExecutorServiceManager().cachingExecutorService())
+        .withConfig(
+            new LeaderElectionConfig(
+                lock,
+                config.getLeaseDuration(),
+                config.getRenewDeadline(),
+                config.getRetryPeriod(),
+                leaderCallbacks(),
+                true,
+                config.getLeaseName()))
+        .build();
   }
 
 
@@ -122,16 +122,16 @@ private void checkLeaseAccess() {
     var verbs = Arrays.asList("create", "update", "get");
     SelfSubjectRulesReview review = new SelfSubjectRulesReview();
     review.setSpec(new SelfSubjectRulesReviewSpecBuilder().withNamespace(leaseNamespace).build());
-    var reviewResult = client.resource(review).create();
+    var reviewResult = kubernetesClient.resource(review).create();
     log.debug("SelfSubjectRulesReview result: {}", reviewResult);
     var foundRule = reviewResult.getStatus().getResourceRules().stream()
-            .filter(rule -> rule.getApiGroups().contains("coordination.k8s.io")
-                    && rule.getResources().contains("leases")
-                    && (rule.getVerbs().containsAll(verbs)) || rule.getVerbs().contains("*"))
-            .findAny();
+        .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);
+          " in namespace: " + leaseNamespace);
     }
   }
 }
diff --git a/...peratorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java b/...peratorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java
@@ -2,7 +2,6 @@
 
 import java.util.*;
 
-import com.fasterxml.jackson.databind.ObjectMapper;
 import io.fabric8.kubernetes.api.model.ConfigMap;
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.api.model.Secret;
@@ -12,7 +11,6 @@
 import io.javaoperatorsdk.operator.processing.dependent.kubernetes.processors.GenericResourceUpdatePreProcessor;
 
 import com.fasterxml.jackson.databind.JsonNode;
-import io.javaoperatorsdk.operator.processing.dependent.kubernetes.processors.GenericResourceUpdatePreProcessor;
 import com.fasterxml.jackson.databind.ObjectMapper;
 
 public class GenericKubernetesResourceMatcher<R extends HasMetadata, P extends HasMetadata>
@@ -186,15 +184,16 @@ public static <R extends HasMetadata> Result<R> match(R desired, R actualResourc
 
     if (considerMetadata) {
       Optional<Result<R>> res =
-              matchMetadata(desired, actualResource, labelsAndAnnotationsEquality, objectMapper);
+          matchMetadata(desired, actualResource, labelsAndAnnotationsEquality, objectMapper);
       if (res.isPresent()) {
         return res.orElseThrow();
       }
     }
 
     final ResourceUpdatePreProcessor<R> processor =
-            GenericResourceUpdatePreProcessor.processorFor((Class<R>) desired.getClass());
-    final var matched = processor.matches(actualResource, desired, specEquality, ignoredPaths);
+        GenericResourceUpdatePreProcessor.processorFor((Class<R>) desired.getClass());
+    final var matched =
+        processor.matches(actualResource, desired, specEquality, objectMapper, ignoredPaths);
     return Result.computed(matched, desired);
   }
 

diff --git a/.../javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceUpdatePreProcessor.java b/.../javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceUpdatePreProcessor.java
@@ -6,10 +6,10 @@
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.zjsonpatch.JsonDiff;
-import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 
 import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
 
 import static io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesResourceMatcher.*;
 
@@ -19,9 +19,9 @@ public interface ResourceUpdatePreProcessor<R extends HasMetadata> {
 
   R replaceSpecOnActual(R actual, R desired, Context<?> context);
 
-  default boolean matches(R actual, R desired, boolean equality, String[] ignoredPaths) {
+  default boolean matches(R actual, R desired, boolean equality, ObjectMapper objectMapper,
+      String[] ignoredPaths) {
 
-    var objectMapper = ConfigurationServiceProvider.instance().getObjectMapper();
     var desiredNode = objectMapper.valueToTree(desired);
     var actualNode = objectMapper.valueToTree(actual);
     var wholeDiffJsonPatch = JsonDiff.asJson(desiredNode, actualNode);

diff --git a/...cessing/dependent/kubernetes/processors/ClusterRoleBindingResourceUpdatePreProcessor.java b/...cessing/dependent/kubernetes/processors/ClusterRoleBindingResourceUpdatePreProcessor.java
@@ -4,6 +4,8 @@
 
 import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBinding;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
+
 public class ClusterRoleBindingResourceUpdatePreProcessor
     extends GenericResourceUpdatePreProcessor<ClusterRoleBinding> {
 
@@ -15,6 +17,7 @@ protected void updateClonedActual(ClusterRoleBinding actual, ClusterRoleBinding
 
   @Override
   public boolean matches(ClusterRoleBinding actual, ClusterRoleBinding desired, boolean equality,
+      ObjectMapper objectMapper,
       String[] ignoredPaths) {
     return Objects.equals(actual.getRoleRef(), desired.getRoleRef()) &&
         Objects.equals(actual.getSubjects(), desired.getSubjects());

diff --git a/...tor/processing/dependent/kubernetes/processors/ClusterRoleResourceUpdatePreProcessor.java b/...tor/processing/dependent/kubernetes/processors/ClusterRoleResourceUpdatePreProcessor.java
@@ -4,6 +4,8 @@
 
 import io.fabric8.kubernetes.api.model.rbac.ClusterRole;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
+
 public class ClusterRoleResourceUpdatePreProcessor
     extends GenericResourceUpdatePreProcessor<ClusterRole> {
 
@@ -15,6 +17,7 @@ protected void updateClonedActual(ClusterRole actual, ClusterRole desired) {
 
   @Override
   public boolean matches(ClusterRole actual, ClusterRole desired, boolean equality,
+      ObjectMapper objectMapper,
       String[] ignoredPaths) {
     return Objects.equals(actual.getRules(), desired.getRules()) &&
         Objects.equals(actual.getAggregationRule(), desired.getAggregationRule());

diff --git a/...rator/processing/dependent/kubernetes/processors/ConfigMapResourceUpdatePreProcessor.java b/...rator/processing/dependent/kubernetes/processors/ConfigMapResourceUpdatePreProcessor.java
@@ -4,6 +4,8 @@
 
 import io.fabric8.kubernetes.api.model.ConfigMap;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
+
 public class ConfigMapResourceUpdatePreProcessor
     extends GenericResourceUpdatePreProcessor<ConfigMap> {
 
@@ -16,6 +18,7 @@ protected void updateClonedActual(ConfigMap actual, ConfigMap desired) {
 
   @Override
   public boolean matches(ConfigMap actual, ConfigMap desired, boolean equality,
+      ObjectMapper objectMapper,
       String[] ignoredPaths) {
     return Objects.equals(actual.getImmutable(), desired.getImmutable()) &&
         Objects.equals(actual.getData(), desired.getData()) &&

diff --git a/...tor/processing/dependent/kubernetes/processors/RoleBindingResourceUpdatePreProcessor.java b/...tor/processing/dependent/kubernetes/processors/RoleBindingResourceUpdatePreProcessor.java
@@ -4,6 +4,8 @@
 
 import io.fabric8.kubernetes.api.model.rbac.RoleBinding;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
+
 public class RoleBindingResourceUpdatePreProcessor
     extends GenericResourceUpdatePreProcessor<RoleBinding> {
 
@@ -15,6 +17,7 @@ protected void updateClonedActual(RoleBinding actual, RoleBinding desired) {
 
   @Override
   public boolean matches(RoleBinding actual, RoleBinding desired, boolean equality,
+      ObjectMapper objectMapper,
       String[] ignoredPaths) {
     return Objects.equals(actual.getRoleRef(), desired.getRoleRef()) &&
         Objects.equals(actual.getSubjects(), desired.getSubjects());

diff --git a/...k/operator/processing/dependent/kubernetes/processors/RoleResourceUpdatePreProcessor.java b/...k/operator/processing/dependent/kubernetes/processors/RoleResourceUpdatePreProcessor.java
@@ -4,6 +4,8 @@
 
 import io.fabric8.kubernetes.api.model.rbac.Role;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
+
 public class RoleResourceUpdatePreProcessor extends GenericResourceUpdatePreProcessor<Role> {
 
   @Override
@@ -12,7 +14,8 @@ protected void updateClonedActual(Role actual, Role desired) {
   }
 
   @Override
-  public boolean matches(Role actual, Role desired, boolean equality, String[] ignoredPaths) {
+  public boolean matches(Role actual, Role desired, boolean equality, ObjectMapper objectMapper,
+      String[] ignoredPaths) {
     return Objects.equals(actual.getRules(), desired.getRules());
   }
 }
diff --git a/...operator/processing/dependent/kubernetes/processors/SecretResourceUpdatePreProcessor.java b/...operator/processing/dependent/kubernetes/processors/SecretResourceUpdatePreProcessor.java
@@ -4,6 +4,8 @@
 
 import io.fabric8.kubernetes.api.model.Secret;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
+
 public class SecretResourceUpdatePreProcessor extends GenericResourceUpdatePreProcessor<Secret> {
 
   @Override
@@ -15,7 +17,8 @@ protected void updateClonedActual(Secret actual, Secret desired) {
   }
 
   @Override
-  public boolean matches(Secret actual, Secret desired, boolean equality, String[] ignoredPaths) {
+  public boolean matches(Secret actual, Secret desired, boolean equality, ObjectMapper objectMapper,
+      String[] ignoredPaths) {
     return Objects.equals(actual.getImmutable(), desired.getImmutable()) &&
         Objects.equals(actual.getType(), desired.getType()) &&
         Objects.equals(actual.getData(), desired.getData()) &&

diff --git a/...tor/processing/dependent/kubernetes/processors/ServiceAccountResourceUpdateProcessor.java b/...tor/processing/dependent/kubernetes/processors/ServiceAccountResourceUpdateProcessor.java
@@ -4,6 +4,8 @@
 
 import io.fabric8.kubernetes.api.model.ServiceAccount;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
+
 public class ServiceAccountResourceUpdateProcessor
     extends GenericResourceUpdatePreProcessor<ServiceAccount> {
 
@@ -16,6 +18,7 @@ protected void updateClonedActual(ServiceAccount actual, ServiceAccount desired)
 
   @Override
   public boolean matches(ServiceAccount actual, ServiceAccount desired, boolean equality,
+      ObjectMapper objectMapper,
       String[] ignoredPaths) {
     return Objects.equals(actual.getAutomountServiceAccountToken(),
         desired.getAutomountServiceAccountToken()) &&

diff --git a/...r-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java b/...r-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java
@@ -9,7 +9,7 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
 
-import io.fabric8.kubernetes.client.KubernetesClient;
+import io.fabric8.kubernetes.api.model.coordination.v1.Lease;
 import io.javaoperatorsdk.operator.api.config.BaseConfigurationService;
 import io.javaoperatorsdk.operator.api.config.ConfigurationService;
 import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration;
@@ -27,7 +27,7 @@ class LeaderElectionManagerTest {
   @BeforeEach
   void setUp() {
     ControllerManager controllerManager = mock(ControllerManager.class);
-    final var kubernetesClient = mock(KubernetesClient.class);
+    final var kubernetesClient = MockKubernetesClient.client(Lease.class);
     var configurationService =
         ConfigurationService.newOverriddenConfigurationService(new BaseConfigurationService(),
             o -> o.withLeaderElectionConfiguration(new LeaderElectionConfiguration("test")));

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
@@ -1,18 +1,23 @@
 package io.javaoperatorsdk.operator;
 
+import java.util.Arrays;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Executors;
 import java.util.function.Consumer;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.api.model.KubernetesResourceList;
+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.client.KubernetesClient;
 import io.fabric8.kubernetes.client.V1ApiextensionAPIGroupDSL;
 import io.fabric8.kubernetes.client.dsl.*;
 import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElectorBuilder;
 import io.fabric8.kubernetes.client.informers.SharedIndexInformer;
 import io.fabric8.kubernetes.client.informers.cache.Indexer;
 
+import static io.javaoperatorsdk.operator.LeaderElectionManager.*;
 import static org.mockito.Mockito.*;
 
 public class MockKubernetesClient {
@@ -61,6 +66,10 @@ public static <T extends HasMetadata> KubernetesClient client(Class<T> clazz,
     when(client.resources(clazz)).thenReturn(resources);
     when(client.leaderElector())
         .thenReturn(new LeaderElectorBuilder(client, Executors.newSingleThreadExecutor()));
+    var selfSubjectResourceResourceMock = mock(NamespaceableResource.class);
+    when(client.resource(any(SelfSubjectRulesReview.class)))
+        .thenReturn(selfSubjectResourceResourceMock);
+    when(selfSubjectResourceResourceMock.create()).thenReturn(allowSelfSubjectReview());
 
     final var apiGroupDSL = mock(ApiextensionsAPIGroupDSL.class);
     when(client.apiextensions()).thenReturn(apiGroupDSL);
@@ -72,4 +81,15 @@ public static <T extends HasMetadata> KubernetesClient client(Class<T> clazz,
 
     return client;
   }
+
+  private static Object allowSelfSubjectReview() {
+    SelfSubjectRulesReview review = new SelfSubjectRulesReview();
+    review.setStatus(new SubjectRulesReviewStatus());
+    var resourceRule = new ResourceRule();
+    resourceRule.setApiGroups(Arrays.asList(COORDINATION_GROUP));
+    resourceRule.setResources(Arrays.asList(LEASES_RESOURCE));
+    resourceRule.setVerbs(Arrays.asList(UNIVERSAL_VERB));
+    review.getStatus().setResourceRules(Arrays.asList(resourceRule));
+    return review;
+  }
 }
diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionPermissionIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionPermissionIT.java
@@ -9,7 +9,6 @@
 import io.fabric8.kubernetes.client.ConfigBuilder;
 import io.fabric8.kubernetes.client.KubernetesClient;
 import io.fabric8.kubernetes.client.KubernetesClientBuilder;
-import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider;
 import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
@@ -46,7 +45,6 @@ void operatorStopsIfNoLeaderElectionPermission() {
 
     assertThat(exception.getCause().getMessage())
         .contains(NO_PERMISSION_TO_LEASE_RESOURCE_MESSAGE);
-    ConfigurationServiceProvider.reset();
   }