fix fabric8io#4363: refining patch logic and adding metadata compare …

…utility

CHANGELOG.md

@@ -21,6 +21,7 @@
 * Fix #4201: Removed sendAsync from the individual http client implementations
 * Fix #4355: for exec, attach, upload, and copy operations the container id/name will be validated or chosen prior to the remote call.  You may also use the kubectl.kubernetes.io/default-container annotation to specify the default container.
 * Fix #4530: generalizing the Serialization logic to allow for primitive values and clarifying the type expectations.
+* Fix #4363: exposed ResourceCompare.metadataChanged
 
 #### Dependency Upgrade
 
@@ -35,6 +36,7 @@
 * Fix #4515: files located at the root of jars named model.properties, e.g. core.properties, have been removed
 * Fix #4579: the implicit registration of resource and list types that happens when using the resource(class) methods has been removed. This makes the behavior of the client more predictable as that was an undocumented side-effect.  If you expect to see instances of a custom type from an untyped api call - typically KubernetesClient.load, KubernetesClient.resourceList, KubernetesClient.resource(InputStream|String), then you must either create a META-INF/services/io.fabric8.kubernetes.api.model.KubernetesResource file (see above #3923), or make calls to KubernetesDeserializer.registerCustomKind - however since KubernetesDeserializer is an internal class that mechanism is not preferred.
 * Fix #4597: remove the deprecated support for `javax.validation.constraints.NotNull` in the `crd-generator`, to mark a property as `required` it needs to be annotated with `io.fabric8.generator.annotation.Required`
+* Fix #4363: deprecated existing ResourceCompare methods such as compareKubernetesResource as they are not for general use
 
 ### 6.2.0 (2022-10-20)
 

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/ResourceCompare.java

@@ -19,10 +19,13 @@
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import io.fabric8.kubernetes.api.model.KubernetesList;
+import io.fabric8.kubernetes.api.model.ObjectMeta;
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
 
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 public class ResourceCompare {
   private ResourceCompare() {
@@ -47,6 +50,7 @@ private ResourceCompare() {
    *
    * @return boolean value whether both resources are actually equal or not
    */
+  @Deprecated
   public static <T> boolean equals(T left, T right) {
     ObjectMapper jsonMapper = Serialization.jsonMapper();
     if (left == null && right == null) {
@@ -66,6 +70,7 @@ public static <T> boolean equals(T left, T right) {
     }
   }
 
+  @Deprecated
   public static boolean compareKubernetesList(Map<String, Object> leftJson, Map<String, Object> rightJson) {
     List<Map<String, Object>> leftItems = (List<Map<String, Object>>) leftJson.get(ITEMS);
     List<Map<String, Object>> rightItems = (List<Map<String, Object>>) rightJson.get(ITEMS);
@@ -85,6 +90,7 @@ public static boolean compareKubernetesList(Map<String, Object> leftJson, Map<St
     return true;
   }
 
+  @Deprecated
   public static boolean compareKubernetesResource(Map<String, Object> leftJson, Map<String, Object> rightJson) {
     return isEqualMetadata(leftJson, rightJson) &&
         isEqualSpec(leftJson, rightJson);
@@ -139,4 +145,22 @@ private static boolean isLeftMapSupersetOfRight(Map<String, Object> leftMap, Map
     }
     return true;
   }
+
+  /**
+   * Compare the two metadata objects after they have been stripped of api server managed fields, such as uid
+   *
+   * @return true if the metadata are the same ignoring
+   */
+  public static boolean metadataChanged(ObjectMeta object1, ObjectMeta object2) {
+    return !Objects.equals(withoutRuntimeState(object1), withoutRuntimeState(object2));
+  }
+
+  static ObjectMeta withoutRuntimeState(ObjectMeta meta) {
+    if (meta == null) {
+      return null;
+    }
+    ObjectMetaBuilder builder = new ObjectMetaBuilder(meta);
+    return builder.withCreationTimestamp(null).withDeletionTimestamp(null).withGeneration(null).withResourceVersion(null)
+        .withSelfLink(null).withUid(null).build();
+  }
 }

kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/ResourceCompareTest.java

@@ -18,6 +18,7 @@
 import io.fabric8.kubernetes.api.model.IntOrString;
 import io.fabric8.kubernetes.api.model.KubernetesList;
 import io.fabric8.kubernetes.api.model.KubernetesListBuilder;
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
 import io.fabric8.kubernetes.api.model.Pod;
 import io.fabric8.kubernetes.api.model.PodBuilder;
 import io.fabric8.kubernetes.api.model.ReplicationController;
@@ -149,4 +150,10 @@ void testEqualsWhenOneResourceIsNull() {
   void testEqualsWhenBothNull() {
     assertTrue(ResourceCompare.equals(null, null));
   }
+
+  @Test
+  void testMetadataChanged() {
+    assertFalse(ResourceCompare.metadataChanged(new ObjectMetaBuilder().withResourceVersion("1").build(),
+        new ObjectMetaBuilder().withResourceVersion("2").build()));
+  }
 }

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/PatchUtils.java

@@ -19,13 +19,13 @@
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.SerializationFeature;
-import io.fabric8.kubernetes.api.model.ObjectMeta;
+import com.fasterxml.jackson.databind.node.ObjectNode;
 import io.fabric8.kubernetes.client.KubernetesClientException;
+import io.fabric8.kubernetes.client.utils.ResourceCompare;
 import io.fabric8.kubernetes.client.utils.Serialization;
 import io.fabric8.zjsonpatch.JsonDiff;
 
-import java.util.Map;
-import java.util.function.BiFunction;
+import java.util.Optional;
 
 public class PatchUtils {
 
@@ -39,51 +39,45 @@ private PatchUtils() {
 
   private static class SingletonHolder {
     public static final ObjectMapper patchMapper;
-    public static final ObjectMapper yamlMapper;
 
     static {
       patchMapper = Serialization.jsonMapper().copy();
-      patchMapper.addMixIn(ObjectMeta.class, ObjectMetaMixIn.class);
+      // if this isn't set the patches are still correct, but not as compact for some reason - array values are added individually
       patchMapper.setConfig(patchMapper.getSerializationConfig().without(SerializationFeature.WRITE_EMPTY_JSON_ARRAYS));
-
-      yamlMapper = Serialization.yamlMapper().copy();
-      yamlMapper.addMixIn(ObjectMeta.class, ObjectMetaMixIn.class);
-      yamlMapper.setConfig(yamlMapper.getSerializationConfig().without(SerializationFeature.WRITE_EMPTY_JSON_ARRAYS));
     }
   }
 
   public static String withoutRuntimeState(Object object, Format format, boolean omitStatus) {
-    return withoutRuntimeState(object, format, omitStatus, (m, v) -> {
-      try {
-        return m.writeValueAsString(v);
-      } catch (JsonProcessingException e) {
-        throw KubernetesClientException.launderThrowable(e);
-      }
-    });
+    ObjectMapper mapper = format == Format.JSON ? Serialization.jsonMapper() : Serialization.yamlMapper();
+    try {
+      return mapper.writeValueAsString(withoutRuntimeState(object, omitStatus));
+    } catch (JsonProcessingException e) {
+      throw KubernetesClientException.launderThrowable(e);
+    }
   }
 
+  /**
+   * See also {@link ResourceCompare#withoutRuntimeState}
+   */
   static JsonNode withoutRuntimeState(Object object, boolean omitStatus) {
-    return withoutRuntimeState(object, Format.JSON, omitStatus, (m, v) -> m.convertValue(v, JsonNode.class));
-  }
-
-  static <T> T withoutRuntimeState(Object object, Format format, boolean omitStatus,
-      BiFunction<ObjectMapper, Object, T> result) {
-    ObjectMapper mapper = SingletonHolder.patchMapper;
-    if (format == Format.YAML) {
-      mapper = SingletonHolder.yamlMapper;
-    }
-    Object value = object;
+    ObjectNode raw = SingletonHolder.patchMapper.convertValue(object, ObjectNode.class);
+    Optional.ofNullable(raw.get("metadata")).filter(ObjectNode.class::isInstance).map(ObjectNode.class::cast).ifPresent(m -> {
+      m.remove("creationTimestamp");
+      m.remove("deletionTimestamp");
+      m.remove("generation");
+      m.remove("resourceVersion");
+      m.remove("selfLink");
+      m.remove("uid");
+    });
     if (omitStatus) {
-      Map<?, ?> raw = mapper.convertValue(object, Map.class);
       raw.remove("status");
-      value = raw;
     }
-    return result.apply(mapper, value);
+    return raw;
   }
 
   public static String jsonDiff(Object current, Object updated, boolean omitStatus) {
     try {
-      return SingletonHolder.patchMapper.writeValueAsString(
+      return Serialization.jsonMapper().writeValueAsString(
           JsonDiff.asJson(withoutRuntimeState(current, omitStatus), withoutRuntimeState(updated, omitStatus)));
     } catch (JsonProcessingException e) {
       throw KubernetesClientException.launderThrowable(e);