feat: generic matcher for Kubernetes dependent resource

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java

@@ -1,11 +1,14 @@
 package io.javaoperatorsdk.operator;
 
+import java.io.IOException;
+import java.io.InputStream;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.util.Locale;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.api.model.ObjectMeta;
+import io.fabric8.kubernetes.client.utils.Serialization;
 import io.javaoperatorsdk.operator.api.reconciler.Constants;
 import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
@@ -111,7 +114,15 @@ public static Object getSpec(HasMetadata resource) {
       Method getSpecMethod = resource.getClass().getMethod("getSpec");
       return getSpecMethod.invoke(resource);
     } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) {
-      throw new IllegalStateException(e);
+      throw new IllegalStateException("No spec found on resource", e);
+    }
+  }
+
+  public static <T> T loadYaml(Class<T> clazz, Class loader, String yaml) {
+    try (InputStream is = loader.getResourceAsStream(yaml)) {
+      return Serialization.unmarshal(is, clazz);
+    } catch (IOException ex) {
+      throw new IllegalStateException("Cannot find yaml on classpath: " + yaml);
     }
   }
 

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

@@ -16,9 +16,9 @@
 /** An interface from which to retrieve configuration information. */
 public interface ConfigurationService {
 
-  Cloner DEFAULT_CLONER = new Cloner() {
-    private final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
+  ObjectMapper OBJECT_MAPPER = new ObjectMapper();
 
+  Cloner DEFAULT_CLONER = new Cloner() {
     @Override
     public HasMetadata clone(HasMetadata object) {
       try {
@@ -122,4 +122,8 @@ default ExecutorService getExecutorService() {
   default boolean closeClientOnStop() {
     return true;
   }
+
+  default ObjectMapper getObjectMapper() {
+    return OBJECT_MAPPER;
+  }
 }

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

@@ -0,0 +1,43 @@
+package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
+
+import io.fabric8.kubernetes.api.model.ConfigMap;
+import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.fabric8.kubernetes.api.model.Secret;
+import io.fabric8.zjsonpatch.JsonDiff;
+import io.javaoperatorsdk.operator.ReconcilerUtils;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+public class DesiredValueMatcher implements ResourceMatcher {
+
+  private final ObjectMapper objectMapper;
+
+  public DesiredValueMatcher(ObjectMapper objectMapper) {
+    this.objectMapper = objectMapper;
+  }
+
+  @Override
+  public boolean match(HasMetadata actualResource, HasMetadata desiredResource, Context context) {
+    if (actualResource instanceof Secret) {
+      return ResourceComparators.compareSecretData((Secret) desiredResource,
+          (Secret) actualResource);
+    }
+    if (actualResource instanceof ConfigMap) {
+      return ResourceComparators.compareConfigMapData((ConfigMap) desiredResource,
+          (ConfigMap) actualResource);
+    }
+    // reflection will be replaced by this:
+    // https://github.com/fabric8io/kubernetes-client/issues/3816
+    var desiredSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(desiredResource));
+    var actualSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(actualResource));
+    var diffJsonPatch = JsonDiff.asJson(desiredSpecNode, actualSpecNode);
+    for (int i = 0; i < diffJsonPatch.size(); i++) {
+      String operation = diffJsonPatch.get(i).get("op").asText();
+      if (!operation.equals("add")) {
+        return false;
+      }
+    }
+    return true;
+  }
+}

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

@@ -8,7 +8,6 @@
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.client.KubernetesClient;
-import io.javaoperatorsdk.operator.ReconcilerUtils;
 import io.javaoperatorsdk.operator.api.config.ConfigurationService;
 import io.javaoperatorsdk.operator.api.config.Utils;
 import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration;
@@ -32,6 +31,7 @@ public abstract class KubernetesDependentResource<R extends HasMetadata, P exten
   protected KubernetesClient client;
   private InformerEventSource<R, P> informerEventSource;
   private boolean addOwnerReference;
+  protected ResourceMatcher resourceMatcher;
 
   @Override
   public void configureWith(KubernetesDependentResourceConfig config) {
@@ -40,7 +40,7 @@ public void configureWith(KubernetesDependentResourceConfig config) {
   }
 
   @SuppressWarnings("unchecked")
-  private void configureWith(ConfigurationService service, String labelSelector,
+  private void configureWith(ConfigurationService configService, String labelSelector,
       Set<String> namespaces, boolean addOwnerReference) {
     final var primaryResourcesRetriever =
         (this instanceof PrimaryResourcesRetriever) ? (PrimaryResourcesRetriever<R>) this
@@ -50,26 +50,28 @@ private void configureWith(ConfigurationService service, String labelSelector,
             ? (AssociatedSecondaryResourceIdentifier<P>) this
             : ResourceID::fromResource;
     InformerConfiguration<R, P> ic =
-        InformerConfiguration.from(service, resourceType())
+        InformerConfiguration.from(configService, resourceType())
             .withLabelSelector(labelSelector)
             .withNamespaces(namespaces)
             .withPrimaryResourcesRetriever(primaryResourcesRetriever)
             .withAssociatedSecondaryResourceIdentifier(secondaryResourceIdentifier)
             .build();
-    this.addOwnerReference = addOwnerReference;
-    informerEventSource = new InformerEventSource<>(ic, client);
+    configureWith(configService, new InformerEventSource<>(ic, client), addOwnerReference);
   }
 
   /**
    * Use to share informers between event more resources.
-   *
+   * 
+   * @param configurationService get configs
    * @param informerEventSource informer to use
    * @param addOwnerReference to the created resource
    */
-  public void configureWith(InformerEventSource<R, P> informerEventSource,
+  public void configureWith(ConfigurationService configurationService,
+      InformerEventSource<R, P> informerEventSource,
       boolean addOwnerReference) {
     this.informerEventSource = informerEventSource;
     this.addOwnerReference = addOwnerReference;
+    initResourceMatcherIfNotSet(configurationService);
   }
 
   protected void beforeCreateOrUpdate(R desired, P primary) {
@@ -79,8 +81,8 @@ protected void beforeCreateOrUpdate(R desired, P primary) {
   }
 
   @Override
-  protected boolean match(R actual, R target, Context context) {
-    return ReconcilerUtils.specsEqual(actual, target);
+  protected boolean match(R actualResource, R desiredResource, Context context) {
+    return resourceMatcher.match(actualResource, desiredResource, context);
   }
 
   @SuppressWarnings("unchecked")
@@ -107,6 +109,7 @@ protected R update(R actual, R target, P primary, Context context) {
 
   @Override
   public Optional<EventSource> eventSource(EventSourceContext<P> context) {
+    initResourceMatcherIfNotSet(context.getConfigurationService());
     if (informerEventSource == null) {
       configureWith(context.getConfigurationService(), null, null,
           KubernetesDependent.ADD_OWNER_REFERENCE_DEFAULT);
@@ -144,4 +147,16 @@ public Optional<R> getResource(P primaryResource) {
   public void setKubernetesClient(KubernetesClient kubernetesClient) {
     this.client = kubernetesClient;
   }
+
+  /**
+   * Override this method to configure resource matcher
+   *
+   * @param configurationService config service to mainly access object mapper
+   */
+  protected void initResourceMatcherIfNotSet(ConfigurationService configurationService) {
+    if (resourceMatcher == null) {
+      resourceMatcher = new DesiredValueMatcher(configurationService.getObjectMapper());
+    }
+  }
+
 }

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

@@ -0,0 +1,20 @@
+package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
+
+import java.util.Objects;
+
+import io.fabric8.kubernetes.api.model.ConfigMap;
+import io.fabric8.kubernetes.api.model.Secret;
+
+public class ResourceComparators {
+
+  public static boolean compareConfigMapData(ConfigMap c1, ConfigMap c2) {
+    return Objects.equals(c1.getData(), c2.getData()) &&
+        Objects.equals(c1.getBinaryData(), c2.getBinaryData());
+  }
+
+  public static boolean compareSecretData(Secret s1, Secret s2) {
+    return Objects.equals(s1.getType(), s2.getType()) &&
+        Objects.equals(s1.getData(), s2.getData()) &&
+        Objects.equals(s1.getStringData(), s2.getStringData());
+  }
+}

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

@@ -0,0 +1,9 @@
+package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
+
+import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+
+public interface ResourceMatcher {
+
+  boolean match(HasMetadata actualResource, HasMetadata desiredResource, Context context);
+}

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/DesiredValueMatcherTest.java

@@ -0,0 +1,50 @@
+package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
+
+import org.junit.jupiter.api.Test;
+
+import io.fabric8.kubernetes.api.model.apps.Deployment;
+import io.javaoperatorsdk.operator.ReconcilerUtils;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class DesiredValueMatcherTest {
+
+  DesiredValueMatcher desiredValueMatcher = new DesiredValueMatcher(new ObjectMapper());
+
+  @Test
+  void checksIfDesiredValuesAreTheSame() {
+    var target1 = createDeployment();
+    var desired1 = createDeployment();
+    assertThat(desiredValueMatcher.match(target1, desired1, null)).isTrue();
+
+    var target2 = createDeployment();
+    var desired2 = createDeployment();
+    target2.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val");
+    assertThat(desiredValueMatcher.match(target2, desired2, null))
+        .withFailMessage("Additive changes should be ok")
+        .isTrue();
+
+    var target3 = createDeployment();
+    var desired3 = createDeployment();
+    desired3.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val");
+    assertThat(desiredValueMatcher.match(target3, desired3, null))
+        .withFailMessage("Removed value should not be ok")
+        .isFalse();
+
+    var target4 = createDeployment();
+    var desired4 = createDeployment();
+    target4.getSpec().setReplicas(2);
+    assertThat(desiredValueMatcher.match(target4, desired4, null))
+        .withFailMessage("Changed values are not ok")
+        .isFalse();
+  }
+
+  Deployment createDeployment() {
+    Deployment deployment =
+        ReconcilerUtils.loadYaml(
+            Deployment.class, DesiredValueMatcherTest.class, "nginx-deployment.yaml");
+    return deployment;
+  }
+}

operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/nginx-deployment.yaml

@@ -0,0 +1,21 @@
+apiVersion: apps/v1 # for versions before 1.9.0 use apps/v1beta2
+kind: Deployment
+metadata:
+  name: "test"
+spec:
+  progressDeadlineSeconds: 600
+  revisionHistoryLimit: 10
+  selector:
+    matchLabels:
+      app: "test-dependent"
+  replicas: 1
+  template:
+    metadata:
+      labels:
+        app: "test-dependent"
+    spec:
+      containers:
+      - name: nginx
+        image: nginx:1.17.0
+        ports:
+        - containerPort: 80

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonedependent/StandaloneDependentTestReconciler.java

@@ -3,7 +3,6 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.List;
-import java.util.Objects;
 
 import io.fabric8.kubernetes.api.model.apps.Deployment;
 import io.fabric8.kubernetes.client.KubernetesClient;
@@ -72,14 +71,5 @@ protected Deployment desired(StandaloneDependentTestCustomResource primary, Cont
       deployment.getMetadata().setNamespace(primary.getMetadata().getNamespace());
       return deployment;
     }
-
-    @Override
-    protected boolean match(Deployment actual, Deployment target, Context context) {
-      return Objects.equals(actual.getSpec().getReplicas(), target.getSpec().getReplicas()) &&
-          actual.getSpec().getTemplate().getSpec().getContainers().get(0).getImage()
-              .equals(
-                  target.getSpec().getTemplate().getSpec().getContainers().get(0).getImage());
-    }
-
   }
 }

sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/DeploymentDependentResource.java

@@ -43,11 +43,4 @@ protected Deployment desired(Tomcat tomcat, Context context) {
         .build();
     return deployment;
   }
-
-  @Override
-  public boolean match(Deployment fetched, Deployment target, Context context) {
-    return fetched.getSpec().getTemplate().getSpec().getContainers().get(0).getImage()
-        .equals(target.getSpec().getTemplate().getSpec().getContainers().get(0).getImage());
-  }
-
 }

sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java

@@ -62,7 +62,7 @@ public UpdateControl<WebPage> reconcile(WebPage webPage, Context context) {
     WebPageStatus status = new WebPageStatus();
 
     waitUntilConfigMapAvailable(webPage);
-    status.setHtmlConfigMap(configMapDR.getResource(webPage).get().getMetadata().getName());
+    status.setHtmlConfigMap(configMapDR.getResource(webPage).orElseThrow().getMetadata().getName());
     status.setAreWeGood("Yes!");
     status.setErrorMessage(null);
     webPage.setStatus(status);
@@ -114,12 +114,6 @@ protected Deployment desired(WebPage webPage, Context context) {
             return deployment;
           }
 
-          @Override
-          protected boolean match(Deployment actual, Deployment target, Context context) {
-            // todo comparator
-            return true;
-          }
-
           @Override
           protected Class<Deployment> resourceType() {
             return Deployment.class;
@@ -140,11 +134,6 @@ protected Service desired(WebPage webPage, Context context) {
             return service;
           }
 
-          protected boolean match(Service actual, Service target, Context context) {
-            // todo comparator
-            return true;
-          }
-
           @Override
           protected Class<Service> resourceType() {
             return Service.class;