matchers ignore list

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

@@ -92,7 +92,7 @@ public Optional<T> computedDesired() {
    * @return a {@link Result} encapsulating whether the resource matched its desired state and this
    *         associated state if it was computed as part of the matching process. Use the static
    *         convenience methods ({@link Result#nonComputed(boolean)} and
-   *         {@link Result#computed(boolean, Object)})
+   *         {@link Result#computed(boolean, Object)}) to create your return {@link Result}.
    */
   Result<R> match(R actualResource, P primary, Context<P> context);
 }

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

@@ -145,7 +145,8 @@ public Result<R> match(R actualResource, P primary, Context<P> context) {
 
   @SuppressWarnings("unused")
   public Result<R> match(R actualResource, R desired, P primary, Context<P> context) {
-    return GenericKubernetesResourceMatcher.match(desired, actualResource, false);
+    return GenericKubernetesResourceMatcher.match(desired, actualResource, false,
+        false, false);
   }
 
   protected void handleDelete(P primary, R secondary, Context<P> context) {

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

@@ -11,6 +11,7 @@
 import io.javaoperatorsdk.operator.ReconcilerUtils;
 import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.processing.dependent.Matcher;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
@@ -21,44 +22,94 @@ class GenericKubernetesResourceMatcherTest {
 
   private static final Context context = mock(Context.class);
 
+  Deployment actual = createDeployment();
+  Deployment desired = createDeployment();
+  TestDependentResource dependentResource = new TestDependentResource(desired);
+  Matcher matcher =
+      GenericKubernetesResourceMatcher.matcherFor(Deployment.class, dependentResource);
+
   @BeforeAll
   static void setUp() {
     final var controllerConfiguration = mock(ControllerConfiguration.class);
     when(context.getControllerConfiguration()).thenReturn(controllerConfiguration);
   }
 
   @Test
-  void checksIfDesiredValuesAreTheSame() {
-    var actual = createDeployment();
-    final var desired = createDeployment();
-    final var dependentResource = new TestDependentResource(desired);
-    final var matcher =
-        GenericKubernetesResourceMatcher.matcherFor(Deployment.class, dependentResource);
+  void matchesTrivialCases() {
     assertThat(matcher.match(actual, null, context).matched()).isTrue();
-    assertThat(matcher.match(actual, null, context).computedDesired().isPresent()).isTrue();
-    assertThat(matcher.match(actual, null, context).computedDesired().get()).isEqualTo(desired);
+    assertThat(matcher.match(actual, null, context).computedDesired()).isPresent();
+    assertThat(matcher.match(actual, null, context).computedDesired()).contains(desired);
+  }
 
+  @Test
+  void matchesAdditiveOnlyChanges() {
     actual.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val");
     assertThat(matcher.match(actual, null, context).matched())
-        .withFailMessage("Additive changes should be ok")
+        .withFailMessage("Additive changes should not cause a mismatch by default")
         .isTrue();
+  }
 
+  @Test
+  void matchesWithStrongSpecEquality() {
+    actual.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val");
     assertThat(GenericKubernetesResourceMatcher
-        .match(dependentResource, actual, null, context, true, true).matched())
-        .withFailMessage("Strong equality does not ignore additive changes on spec")
+        .match(dependentResource, actual, null, context, true, true,
+            true)
+        .matched())
+        .withFailMessage("Adding values should fail matching when strong equality is required")
         .isFalse();
+  }
 
+  @Test
+  void doesNotMatchRemovedValues() {
     actual = createDeployment();
     assertThat(matcher.match(actual, createPrimary("removed"), context).matched())
-        .withFailMessage("Removed value should not be ok")
+        .withFailMessage("Removing values in metadata should lead to a mismatch")
         .isFalse();
+  }
 
+  @Test
+  void doesNotMatchChangedValues() {
     actual = createDeployment();
     actual.getSpec().setReplicas(2);
     assertThat(matcher.match(actual, null, context).matched())
-        .withFailMessage("Changed values are not ok")
+        .withFailMessage("Should not have matched because values have changed")
         .isFalse();
+  }
+
+  @Test
+  void doesNotMatchChangedValuesWhenNoIgnoredPathsAreProvided() {
+    actual = createDeployment();
+    actual.getSpec().setReplicas(2);
+    assertThat(GenericKubernetesResourceMatcher
+        .match(dependentResource, actual, null, context, true).matched())
+        .withFailMessage(
+            "Should not have matched because values have changed and no ignored path is provided")
+        .isFalse();
+  }
 
+  @Test
+  void doesNotAttemptToMatchIgnoredPaths() {
+    actual = createDeployment();
+    actual.getSpec().setReplicas(2);
+    assertThat(GenericKubernetesResourceMatcher
+        .match(dependentResource, actual, null, context, false, "/spec/replicas").matched())
+        .withFailMessage("Should not have compared ignored paths")
+        .isTrue();
+  }
+
+  @Test
+  void ignoresWholeSubPath() {
+    actual = createDeployment();
+    actual.getSpec().getTemplate().getMetadata().getLabels().put("additional-key", "val");
+    assertThat(GenericKubernetesResourceMatcher
+        .match(dependentResource, actual, null, context, false, "/spec/template").matched())
+        .withFailMessage("Should match when only changes impact ignored sub-paths")
+        .isTrue();
+  }
+
+  @Test
+  void matchesMetadata() {
     actual = new DeploymentBuilder(createDeployment())
         .editOrNewMetadata()
         .addToAnnotations("test", "value")
@@ -70,9 +121,16 @@ void checksIfDesiredValuesAreTheSame() {
         .isTrue();
 
     assertThat(GenericKubernetesResourceMatcher
-        .match(dependentResource, actual, null, context, true).matched())
+        .match(dependentResource, actual, null, context, true, true, true).matched())
         .withFailMessage("Annotations should matter when metadata is considered")
         .isFalse();
+
+    assertThat(GenericKubernetesResourceMatcher
+        .match(dependentResource, actual, null, context, true, false).matched())
+        .withFailMessage(
+            "Should match when strong equality is not considered and only additive changes are made")
+        .isTrue();
+
   }
 
   Deployment createDeployment() {

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

@@ -0,0 +1,57 @@
+package io.javaoperatorsdk.operator;
+
+import java.time.Duration;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
+import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
+import io.javaoperatorsdk.operator.sample.servicestrictmatcher.ServiceDependentResource;
+import io.javaoperatorsdk.operator.sample.servicestrictmatcher.ServiceStrictMatcherSpec;
+import io.javaoperatorsdk.operator.sample.servicestrictmatcher.ServiceStrictMatcherTestCustomResource;
+import io.javaoperatorsdk.operator.sample.servicestrictmatcher.ServiceStrictMatcherTestReconciler;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
+
+public class ServiceStrictMatcherIT {
+
+  @RegisterExtension
+  LocallyRunOperatorExtension operator =
+      LocallyRunOperatorExtension.builder().withReconciler(new ServiceStrictMatcherTestReconciler())
+          .build();
+
+
+  @Test
+  void testTheMatchingDoesNoTTriggersFurtherUpdates() {
+    var resource = operator.create(testResource());
+
+    await().untilAsserted(() -> {
+      assertThat(operator.getReconcilerOfType(ServiceStrictMatcherTestReconciler.class)
+          .getNumberOfExecutions()).isEqualTo(1);
+    });
+
+    // make an update to spec to reconcile again
+    resource.getSpec().setValue(2);
+    operator.replace(resource);
+
+    await().pollDelay(Duration.ofMillis(300)).untilAsserted(() -> {
+      assertThat(operator.getReconcilerOfType(ServiceStrictMatcherTestReconciler.class)
+          .getNumberOfExecutions()).isEqualTo(2);
+      assertThat(ServiceDependentResource.updated.get()).isZero();
+    });
+  }
+
+
+  ServiceStrictMatcherTestCustomResource testResource() {
+    var res = new ServiceStrictMatcherTestCustomResource();
+    res.setSpec(new ServiceStrictMatcherSpec());
+    res.getSpec().setValue(1);
+    res.setMetadata(new ObjectMetaBuilder()
+        .withName("test1")
+        .build());
+    return res;
+  }
+
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/servicestrictmatcher/ServiceDependentResource.java

@@ -0,0 +1,62 @@
+package io.javaoperatorsdk.operator.sample.servicestrictmatcher;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import io.fabric8.kubernetes.api.model.Service;
+import io.javaoperatorsdk.operator.ServiceStrictMatcherIT;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.processing.dependent.Matcher;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesResourceMatcher;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent;
+
+import static io.javaoperatorsdk.operator.ReconcilerUtils.loadYaml;
+
+@KubernetesDependent
+public class ServiceDependentResource
+    extends CRUDKubernetesDependentResource<Service, ServiceStrictMatcherTestCustomResource> {
+
+  public static AtomicInteger updated = new AtomicInteger(0);
+
+  public ServiceDependentResource() {
+    super(Service.class);
+  }
+
+  @Override
+  protected Service desired(ServiceStrictMatcherTestCustomResource primary,
+      Context<ServiceStrictMatcherTestCustomResource> context) {
+    Service service = loadYaml(Service.class, ServiceStrictMatcherIT.class, "service.yaml");
+    service.getMetadata().setName(primary.getMetadata().getName());
+    service.getMetadata().setNamespace(primary.getMetadata().getNamespace());
+    Map<String, String> labels = new HashMap<>();
+    labels.put("app", "deployment-name");
+    service.getSpec().setSelector(labels);
+    return service;
+  }
+
+  @Override
+  public Matcher.Result<Service> match(Service actualResource,
+      ServiceStrictMatcherTestCustomResource primary,
+      Context<ServiceStrictMatcherTestCustomResource> context) {
+    return GenericKubernetesResourceMatcher.match(this, actualResource, primary, context, false,
+        false,
+        "/spec/ports",
+        "/spec/clusterIP",
+        "/spec/clusterIPs",
+        "/spec/externalTrafficPolicy",
+        "/spec/internalTrafficPolicy",
+        "/spec/ipFamilies",
+        "/spec/ipFamilyPolicy",
+        "/spec/sessionAffinity");
+  }
+
+  @Override
+  public Service update(Service actual, Service target,
+      ServiceStrictMatcherTestCustomResource primary,
+      Context<ServiceStrictMatcherTestCustomResource> context) {
+    updated.addAndGet(1);
+    return super.update(actual, target, primary, context);
+  }
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/servicestrictmatcher/ServiceStrictMatcherSpec.java

@@ -0,0 +1,15 @@
+package io.javaoperatorsdk.operator.sample.servicestrictmatcher;
+
+public class ServiceStrictMatcherSpec {
+
+  private int value;
+
+  public int getValue() {
+    return value;
+  }
+
+  public ServiceStrictMatcherSpec setValue(int value) {
+    this.value = value;
+    return this;
+  }
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/servicestrictmatcher/ServiceStrictMatcherTestCustomResource.java

@@ -0,0 +1,16 @@
+package io.javaoperatorsdk.operator.sample.servicestrictmatcher;
+
+import io.fabric8.kubernetes.api.model.Namespaced;
+import io.fabric8.kubernetes.client.CustomResource;
+import io.fabric8.kubernetes.model.annotation.Group;
+import io.fabric8.kubernetes.model.annotation.ShortNames;
+import io.fabric8.kubernetes.model.annotation.Version;
+
+@Group("sample.javaoperatorsdk")
+@Version("v1")
+@ShortNames("ssm")
+public class ServiceStrictMatcherTestCustomResource
+    extends CustomResource<ServiceStrictMatcherSpec, Void>
+    implements Namespaced {
+
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/servicestrictmatcher/ServiceStrictMatcherTestReconciler.java

@@ -0,0 +1,29 @@
+package io.javaoperatorsdk.operator.sample.servicestrictmatcher;
+
+import java.util.concurrent.atomic.AtomicInteger;
+
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
+import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
+import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
+
+@ControllerConfiguration(dependents = {@Dependent(type = ServiceDependentResource.class)})
+public class ServiceStrictMatcherTestReconciler
+    implements Reconciler<ServiceStrictMatcherTestCustomResource> {
+
+
+  private final AtomicInteger numberOfExecutions = new AtomicInteger(0);
+
+  @Override
+  public UpdateControl<ServiceStrictMatcherTestCustomResource> reconcile(
+      ServiceStrictMatcherTestCustomResource resource,
+      Context<ServiceStrictMatcherTestCustomResource> context) {
+    numberOfExecutions.addAndGet(1);
+    return UpdateControl.noUpdate();
+  }
+
+  public int getNumberOfExecutions() {
+    return numberOfExecutions.get();
+  }
+}

operator-framework/src/test/resources/io/javaoperatorsdk/operator/service.yaml

@@ -0,0 +1,12 @@
+apiVersion: v1
+kind: Service
+metadata:
+  name: ""
+spec:
+  selector:
+    app: ""
+  ports:
+    - protocol: TCP
+      port: 80
+      targetPort: 80
+  type: NodePort