draft of a generalization of the update matcher

Author: shawkins

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java

@@ -6,6 +6,7 @@
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
 
 import io.fabric8.kubernetes.api.model.ConfigMap;
 import io.fabric8.kubernetes.api.model.HasMetadata;
@@ -24,6 +25,8 @@ public class GenericKubernetesResourceMatcher<R extends HasMetadata, P extends H
   private static final String OP = "op";
   public static final String METADATA_LABELS = "/metadata/labels";
   public static final String METADATA_ANNOTATIONS = "/metadata/annotations";
+  // without knowing the CRD we cannot ignore status as it may not be a subresource, so if it's included we expect it to match
+  private static Set<String> NOT_OTHER_FIELDS = Set.of(SPEC, "/metadata", "/apiVersion", "/kind");
 
   private static final String PATH = "path";
   private static final String[] EMPTY_ARRAY = {};
@@ -192,11 +195,11 @@ public static <R extends HasMetadata, P extends HasMetadata> Result<R> match(R d
       }
     }
 
-    final var matched = matchSpec(actualResource, desired, specEquality, context, ignoredPaths);
+    final var matched = match(actualResource, desired, specEquality, context, ignoredPaths);
     return Result.computed(matched, desired);
   }
 
-  private static <R extends HasMetadata> boolean matchSpec(R actual, R desired, boolean equality,
+  private static <R extends HasMetadata> boolean match(R actual, R desired, boolean equality,
       Context<?> context,
       String[] ignoredPaths) {
 
@@ -208,25 +211,37 @@ private static <R extends HasMetadata> boolean matchSpec(R actual, R desired, bo
     final List<String> ignoreList =
         ignoredPaths != null && ignoredPaths.length > 0 ? Arrays.asList(ignoredPaths)
             : Collections.emptyList();
-    // reflection will be replaced by this:
-    // https://github.com/fabric8io/kubernetes-client/issues/3816
-    var specDiffJsonPatch = getDiffsImpactingPathsWithPrefixes(wholeDiffJsonPatch, SPEC);
+    boolean specMatch = match(equality, wholeDiffJsonPatch, ignoreList, SPEC);
+    if (!specMatch) {
+      return false;
+    }
+    // expect everything else to be equal
+    var names = desiredNode.fieldNames();
+    List<String> prefixes = new ArrayList<>();
+    while (names.hasNext()) {
+      String prefix = "/" + names.next();
+      if (!NOT_OTHER_FIELDS.contains(prefix)) {
+        prefixes.add(prefix);
+      }
+    }
+    return match(true, wholeDiffJsonPatch, ignoreList, prefixes.toArray(String[]::new));
+  }
+
+  private static boolean match(boolean equality, JsonNode wholeDiffJsonPatch, final List<String> ignoreList, String... prefixes) {
+    var diffJsonPatch = getDiffsImpactingPathsWithPrefixes(wholeDiffJsonPatch, prefixes);
     // In case of equality is set to true, no diffs are allowed, so we return early if diffs exist
     // On contrary (if equality is false), "add" is allowed for cases when for some
     // resources Kubernetes fills-in values into spec.
-    if (equality && !specDiffJsonPatch.isEmpty()) {
+    if (diffJsonPatch.isEmpty()) {
+      return true;
+    }
+    if (equality) {
       return false;
     }
-    if (!equality && !ignoreList.isEmpty()) {
-      if (!allDiffsOnIgnoreList(specDiffJsonPatch, ignoreList)) {
-        return false;
-      }
-    } else {
-      if (!allDiffsAreAddOps(specDiffJsonPatch)) {
-        return false;
-      }
+    if (!ignoreList.isEmpty() && allDiffsOnIgnoreList(diffJsonPatch, ignoreList)) {
+      return true;
     }
-    return true;
+    return allDiffsAreAddOps(diffJsonPatch);
   }
 
   private static boolean allDiffsOnIgnoreList(List<JsonNode> metadataJSonDiffs,

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ClusterRoleBindingResourceUpdaterMatcher.java

@@ -1,49 +1,61 @@
 package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher;
 
-import java.util.Map;
-
-import io.fabric8.kubernetes.api.model.*;
-import io.fabric8.kubernetes.api.model.discovery.v1.EndpointSlice;
-import io.fabric8.kubernetes.api.model.rbac.ClusterRole;
-import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBinding;
-import io.fabric8.kubernetes.api.model.rbac.Role;
-import io.fabric8.kubernetes.api.model.rbac.RoleBinding;
-import io.javaoperatorsdk.operator.ReconcilerUtils;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.introspect.ClassIntrospector.MixInResolver;
+import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.fabric8.kubernetes.api.model.KubernetesResource;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 import io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesResourceMatcher;
 import io.javaoperatorsdk.operator.processing.dependent.kubernetes.ResourceUpdaterMatcher;
 
+import java.util.Map;
+
 public class GenericResourceUpdaterMatcher<R extends HasMetadata> implements
     ResourceUpdaterMatcher<R> {
 
   private static final ResourceUpdaterMatcher<?> INSTANCE = new GenericResourceUpdaterMatcher<>();
 
-  @SuppressWarnings("rawtypes")
-  private static final Map<Class, ResourceUpdaterMatcher> processors = Map.of(
-      Secret.class, new SecretResourceUpdaterMatcher(),
-      ConfigMap.class, new ConfigMapResourceUpdaterMatcher(),
-      ServiceAccount.class, new ServiceAccountResourceUpdaterMatcher(),
-      Role.class, new RoleResourceUpdaterMatcher(),
-      ClusterRole.class, new ClusterRoleResourceUpdaterMatcher(),
-      RoleBinding.class, new RoleBindingResourceUpdaterMatcher(),
-      ClusterRoleBinding.class, new ClusterRoleBindingResourceUpdaterMatcher(),
-      Endpoints.class, new EndpointsResourceUpdaterMatcher(),
-      EndpointSlice.class, new EndpointSliceResourceUpdateMatcher());
+  @JsonInclude(JsonInclude.Include.ALWAYS)
+  private static class NullIncludingMixIn {
+  }
+
+  // we need a specialized mapper so that all fields, including the null ones, can be gleaning from the desired state
+  private static ObjectMapper MAPPER = new ObjectMapper();
+  static {
+    MAPPER.setMixInResolver(new MixInResolver() {
+
+      @Override
+      public Class<?> findMixInClassFor(Class<?> cls) {
+        if (KubernetesResource.class.isAssignableFrom(cls)) {
+          return NullIncludingMixIn.class;
+        }
+        return null;
+      }
+
+      @Override
+      public MixInResolver copy() {
+        return this;
+      }
+    });
+  }
 
   protected GenericResourceUpdaterMatcher() {}
 
   @SuppressWarnings("unchecked")
   public static <R extends HasMetadata> ResourceUpdaterMatcher<R> updaterMatcherFor(
       Class<R> resourceType) {
-    final var processor = processors.get(resourceType);
-    return processor != null ? processor : (ResourceUpdaterMatcher<R>) INSTANCE;
+    return (ResourceUpdaterMatcher<R>) INSTANCE;
   }
 
+  @Override
   public R updateResource(R actual, R desired, Context<?> context) {
-    var clonedActual = context.getControllerConfiguration().getConfigurationService()
-        .getResourceCloner().clone(actual);
+    Map<String, Object> actualMap = MAPPER.convertValue(actual, Map.class);
+    Map<String, Object> desiredMap = MAPPER.convertValue(desired, Map.class);
+    desiredMap.remove("metadata"); // handled separately below
+    actualMap.putAll(desiredMap); // will also preserve additionalProperties
+    var clonedActual = (R) MAPPER.convertValue(actualMap, desired.getClass());
     updateLabelsAndAnnotation(clonedActual, desired);
-    updateClonedActual(clonedActual, desired);
     return clonedActual;
   }
 
@@ -53,15 +65,6 @@ public boolean matches(R actual, R desired, Context<?> context) {
         false, false, context).matched();
   }
 
-  protected void updateClonedActual(R actual, R desired) {
-    updateSpec(actual, desired);
-  }
-
-  public static <K extends HasMetadata> void updateSpec(K actual, K desired) {
-    var desiredSpec = ReconcilerUtils.getSpec(desired);
-    ReconcilerUtils.setSpec(actual, desiredSpec);
-  }
-
   public static <K extends HasMetadata> void updateLabelsAndAnnotation(K actual, K desired) {
     actual.getMetadata().getLabels().putAll(desired.getMetadata().getLabels());
     actual.getMetadata().getAnnotations().putAll(desired.getMetadata().getAnnotations());