feat: use fabric8 client json mapper by default (#1382)

Also remove constants to avoid mismatched configurations

Co-authored-by: Chris Laprun <[email protected]>

Author: csviri

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractConfigurationService.java

@@ -13,9 +13,16 @@
 public class AbstractConfigurationService implements ConfigurationService {
   private final Map<String, ControllerConfiguration> configurations = new ConcurrentHashMap<>();
   private final Version version;
+  private final Cloner cloner;
 
   public AbstractConfigurationService(Version version) {
     this.version = version;
+    this.cloner = ConfigurationService.super.getResourceCloner();
+  }
+
+  public AbstractConfigurationService(Version version, Cloner cloner) {
+    this.version = version;
+    this.cloner = cloner;
   }
 
   protected <R extends HasMetadata> void register(ControllerConfiguration<R> config) {
@@ -77,10 +84,12 @@ protected <R extends HasMetadata> String keyFor(Reconciler<R> reconciler) {
     return ReconcilerUtils.getNameFor(reconciler);
   }
 
+  @SuppressWarnings("unused")
   protected ControllerConfiguration getFor(String reconcilerName) {
     return configurations.get(reconcilerName);
   }
 
+  @SuppressWarnings("unused")
   protected Stream<ControllerConfiguration> controllerConfigurations() {
     return configurations.values().stream();
   }
@@ -94,4 +103,9 @@ public Set<String> getKnownReconcilerNames() {
   public Version getVersion() {
     return version;
   }
+
+  @Override
+  public Cloner getResourceCloner() {
+    return cloner;
+  }
 }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java

@@ -15,6 +15,10 @@ public BaseConfigurationService(Version version) {
     super(version);
   }
 
+  public BaseConfigurationService(Version version, Cloner cloner) {
+    super(version, cloner);
+  }
+
   public BaseConfigurationService() {
     this(Utils.loadFromProperties());
   }

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

@@ -7,6 +7,7 @@
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.client.Config;
 import io.fabric8.kubernetes.client.CustomResource;
+import io.fabric8.kubernetes.client.utils.Serialization;
 import io.javaoperatorsdk.operator.api.monitoring.Metrics;
 import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResourceFactory;
@@ -17,20 +18,6 @@
 /** An interface from which to retrieve configuration information. */
 public interface ConfigurationService {
 
-  ObjectMapper OBJECT_MAPPER = new ObjectMapper();
-
-  Cloner DEFAULT_CLONER = new Cloner() {
-    @SuppressWarnings("unchecked")
-    @Override
-    public HasMetadata clone(HasMetadata 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 reconciler
    *
@@ -93,12 +80,25 @@ default int concurrentReconciliationThreads() {
   }
 
   /**
-   * Used to clone custom resources.
+   * Used to clone custom resources. It is strongly suggested that implementors override this method
+   * since the default implementation creates a new {@link Cloner} instance each time this method is
+   * called.
    *
-   * @return the ObjectMapper to use
+   * @return the configured {@link Cloner}
    */
   default Cloner getResourceCloner() {
-    return DEFAULT_CLONER;
+    return new Cloner() {
+      @SuppressWarnings("unchecked")
+      @Override
+      public HasMetadata clone(HasMetadata object) {
+        try {
+          final var mapper = getObjectMapper();
+          return mapper.readValue(mapper.writeValueAsString(object), object.getClass());
+        } catch (JsonProcessingException e) {
+          throw new IllegalStateException(e);
+        }
+      }
+    };
   }
 
   int DEFAULT_TERMINATION_TIMEOUT_SECONDS = 10;
@@ -126,7 +126,7 @@ default boolean closeClientOnStop() {
   }
 
   default ObjectMapper getObjectMapper() {
-    return OBJECT_MAPPER;
+    return Serialization.jsonMapper();
   }
 
   default DependentResourceFactory dependentResourceFactory() {

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

@@ -81,7 +81,7 @@ public ConfigurationServiceOverrider withObjectMapper(ObjectMapper objectMapper)
   }
 
   public ConfigurationService build() {
-    return new BaseConfigurationService(original.getVersion()) {
+    return new BaseConfigurationService(original.getVersion(), cloner) {
       @Override
       public Set<String> getKnownReconcilerNames() {
         return original.getKnownReconcilerNames();
@@ -102,11 +102,6 @@ public int concurrentReconciliationThreads() {
         return threadNumber;
       }
 
-      @Override
-      public Cloner getResourceCloner() {
-        return cloner;
-      }
-
       @Override
       public int getTerminationTimeoutSeconds() {
         return timeoutSeconds;

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

@@ -7,7 +7,7 @@
 
 import io.fabric8.kubernetes.api.model.ObjectMeta;
 import io.fabric8.kubernetes.api.model.apps.Deployment;
-import io.javaoperatorsdk.operator.api.config.ConfigurationService;
+import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider;
 import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
 import io.javaoperatorsdk.operator.sample.standalonedependent.StandaloneDependentTestCustomResource;
 import io.javaoperatorsdk.operator.sample.standalonedependent.StandaloneDependentTestCustomResourceSpec;
@@ -52,7 +52,7 @@ void executeUpdateForTestingCacheUpdateForGetResource() {
 
     awaitForDeploymentReadyReplicas(1);
 
-    var clonedCr = ConfigurationService.DEFAULT_CLONER.clone(createdCR);
+    var clonedCr = ConfigurationServiceProvider.instance().getResourceCloner().clone(createdCR);
     clonedCr.getSpec().setReplicaCount(2);
     operator.replace(clonedCr);