feat: Cloner interface for Custom Resource instead of ObjectMapper (#611)

Author: csviri

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Cloner.java

@@ -0,0 +1,9 @@
+package io.javaoperatorsdk.operator.api.config;
+
+import io.fabric8.kubernetes.client.CustomResource;
+
+public interface Cloner {
+
+  CustomResource<?, ?> clone(CustomResource<?, ?> object);
+
+}

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java

@@ -9,12 +9,24 @@
 import io.javaoperatorsdk.operator.Metrics;
 import io.javaoperatorsdk.operator.api.ResourceController;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 
 /** An interface from which to retrieve configuration information. */
 public interface ConfigurationService {
 
-  ObjectMapper OBJECT_MAPPER = new ObjectMapper();
+  Cloner DEFAULT_CLONER = new Cloner() {
+    private final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
+
+    @Override
+    public CustomResource<?, ?> clone(CustomResource<?, ?> object) {
+      try {
+        return OBJECT_MAPPER.readValue(OBJECT_MAPPER.writeValueAsString(object), object.getClass());
+      } catch (JsonProcessingException e) {
+        throw new IllegalStateException(e);
+      }
+    }
+  };
 
   /**
    * Retrieves the configuration associated with the specified controller
@@ -79,14 +91,12 @@ default int concurrentReconciliationThreads() {
   }
 
   /**
-   * The {@link ObjectMapper} that the operator should use to de-/serialize resources. This is
-   * particularly useful when frameworks can configure a specific mapper that should also be used by
-   * the SDK.
-   *
+   * Used to clone custom resources.
+   * 
    * @return the ObjectMapper to use
    */
-  default ObjectMapper getObjectMapper() {
-    return OBJECT_MAPPER;
+  default Cloner getResourceCloner() {
+    return DEFAULT_CLONER;
   }
 
   int DEFAULT_TERMINATION_TIMEOUT_SECONDS = 10;

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java

@@ -7,15 +7,13 @@
 import io.javaoperatorsdk.operator.Metrics;
 import io.javaoperatorsdk.operator.api.ResourceController;
 
-import com.fasterxml.jackson.databind.ObjectMapper;
-
 public class ConfigurationServiceOverrider {
   private final ConfigurationService original;
   private Metrics metrics;
   private Config clientConfig;
   private boolean checkCR;
   private int threadNumber;
-  private ObjectMapper mapper;
+  private Cloner cloner;
   private int timeoutSeconds;
 
   public ConfigurationServiceOverrider(
@@ -24,7 +22,7 @@ public ConfigurationServiceOverrider(
     this.clientConfig = original.getClientConfiguration();
     this.checkCR = original.checkCRDAndValidateLocalModel();
     this.threadNumber = original.concurrentReconciliationThreads();
-    this.mapper = original.getObjectMapper();
+    this.cloner = original.getResourceCloner();
     this.timeoutSeconds = original.getTerminationTimeoutSeconds();
     this.metrics = original.getMetrics();
   }
@@ -45,8 +43,8 @@ public ConfigurationServiceOverrider withConcurrentReconciliationThreads(int thr
     return this;
   }
 
-  public ConfigurationServiceOverrider withObjectMapper(ObjectMapper mapper) {
-    this.mapper = mapper;
+  public ConfigurationServiceOverrider withResourceCloner(Cloner cloner) {
+    this.cloner = cloner;
     return this;
   }
 
@@ -94,8 +92,8 @@ public int concurrentReconciliationThreads() {
       }
 
       @Override
-      public ObjectMapper getObjectMapper() {
-        return mapper;
+      public Cloner getResourceCloner() {
+        return cloner;
       }
 
       @Override

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSource.java

@@ -13,15 +13,13 @@
 import io.fabric8.kubernetes.client.informers.SharedIndexInformer;
 import io.fabric8.kubernetes.client.informers.cache.Cache;
 import io.javaoperatorsdk.operator.MissingCRDException;
+import io.javaoperatorsdk.operator.api.config.Cloner;
 import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
 import io.javaoperatorsdk.operator.processing.ConfiguredController;
 import io.javaoperatorsdk.operator.processing.ResourceCache;
 import io.javaoperatorsdk.operator.processing.event.AbstractEventSource;
 import io.javaoperatorsdk.operator.processing.event.CustomResourceID;
 
-import com.fasterxml.jackson.core.JsonProcessingException;
-import com.fasterxml.jackson.databind.ObjectMapper;
-
 import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName;
 import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID;
 import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getVersion;
@@ -40,15 +38,14 @@ public class CustomResourceEventSource<T extends CustomResource<?, ?>> extends A
   private final ConfiguredController<T> controller;
   private final Map<String, SharedIndexInformer<T>> sharedIndexInformers =
       new ConcurrentHashMap<>();
-  private final ObjectMapper cloningObjectMapper;
+
   private final CustomResourceEventFilter<T> filter;
   private final OnceWhitelistEventFilterEventFilter<T> onceWhitelistEventFilterEventFilter;
-
+  private final Cloner cloner;
 
   public CustomResourceEventSource(ConfiguredController<T> controller) {
     this.controller = controller;
-    this.cloningObjectMapper =
-        controller.getConfiguration().getConfigurationService().getObjectMapper();
+    this.cloner = controller.getConfiguration().getConfigurationService().getResourceCloner();
 
     var filters = new CustomResourceEventFilter[] {
         CustomResourceEventFilters.finalizerNeededAndApplied(),
@@ -160,7 +157,7 @@ public Optional<T> getCustomResource(CustomResourceID resourceID) {
     if (resource == null) {
       return Optional.empty();
     } else {
-      return Optional.of(clone(resource));
+      return Optional.of((T) (cloner.clone(resource)));
     }
   }
 
@@ -176,15 +173,6 @@ public SharedIndexInformer<T> getInformer(String namespace) {
     return getInformers().get(Objects.requireNonNullElse(namespace, ANY_NAMESPACE_MAP_KEY));
   }
 
-  private T clone(T customResource) {
-    try {
-      return (T) cloningObjectMapper.readValue(
-          cloningObjectMapper.writeValueAsString(customResource), customResource.getClass());
-    } catch (JsonProcessingException e) {
-      throw new IllegalStateException(e);
-    }
-  }
-
   /**
    * This will ensure that the next event received after this method is called will not be filtered
    * out.
@@ -196,4 +184,5 @@ public void whitelistNextEvent(CustomResourceID customResourceID) {
       onceWhitelistEventFilterEventFilter.whitelistNextEvent(customResourceID);
     }
   }
+
 }

Diff for: operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilterTest.java

@@ -103,8 +103,8 @@ public void eventNotFilteredByCustomPredicateIfFinalizerIsRequired() {
             oldResource.getStatus().getConfigMapStatus(),
             newResource.getStatus().getConfigMapStatus()));
 
-    when(config.getConfigurationService().getObjectMapper())
-        .thenReturn(ConfigurationService.OBJECT_MAPPER);
+    when(config.getConfigurationService().getResourceCloner())
+        .thenReturn(ConfigurationService.DEFAULT_CLONER);
 
     var controller = new TestConfiguredController(config);
     var eventSource = new CustomResourceEventSource<>(controller);
@@ -142,8 +142,8 @@ public TestControllerConfig(String finalizer, boolean generationAware,
           TestCustomResource.class,
           mock(ConfigurationService.class));
 
-      when(getConfigurationService().getObjectMapper())
-          .thenReturn(ConfigurationService.OBJECT_MAPPER);
+      when(getConfigurationService().getResourceCloner())
+          .thenReturn(ConfigurationService.DEFAULT_CLONER);
     }
   }
 

Diff for: operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSourceTest.java

@@ -164,8 +164,8 @@ public TestConfiguration(boolean generationAware) {
           null,
           TestCustomResource.class,
           mock(ConfigurationService.class));
-      when(getConfigurationService().getObjectMapper())
-          .thenReturn(ConfigurationService.OBJECT_MAPPER);
+      when(getConfigurationService().getResourceCloner())
+          .thenReturn(ConfigurationService.DEFAULT_CLONER);
       when(getConfigurationService().getMetrics())
           .thenReturn(Metrics.NOOP);
     }