feat: allow easier configuration of matcher (#2760)

* feat: allow easier configuration of matcher

Signed-off-by: Chris Laprun <[email protected]>

* refactor: avoid recording default matcher

Signed-off-by: Chris Laprun <[email protected]>

* refactor: make matcher configurable instead of settable

Signed-off-by: Chris Laprun <[email protected]>

---------

Signed-off-by: Chris Laprun <[email protected]>

Author: metacosm

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

@@ -27,5 +27,24 @@ boolean createResourceOnlyIfNotExistingWithSSA() default
    */
   BooleanWithUndefined useSSA() default BooleanWithUndefined.UNDEFINED;
 
+  /**
+   * The underlying Informer event source configuration
+   *
+   * @return the {@link Informer} configuration
+   */
   Informer informer() default @Informer;
+
+  /**
+   * The specific matcher implementation to use when Server-Side Apply (SSA) is used, when case the
+   * default one isn't working appropriately. Typically, this could be needed to cover border cases
+   * with some Kubernetes resources that are modified by their controllers to normalize or add
+   * default values, which could result in infinite loops with the default matcher. Using a specific
+   * matcher could also be an optimization decision if determination of whether two resources match
+   * can be done faster than what can be done with the default exhaustive algorithm.
+   *
+   * @return the class of the specific matcher to use for the associated dependent resources
+   * @since 5.1
+   */
+  Class<? extends SSABasedGenericKubernetesResourceMatcher> matcher() default
+      SSABasedGenericKubernetesResourceMatcher.class;
 }

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

@@ -22,10 +22,22 @@ public KubernetesDependentResourceConfig<R> configFrom(
         DEFAULT_CREATE_RESOURCE_ONLY_IF_NOT_EXISTING_WITH_SSA;
 
     Boolean useSSA = null;
+    SSABasedGenericKubernetesResourceMatcher<R> matcher =
+        SSABasedGenericKubernetesResourceMatcher.getInstance();
     if (configAnnotation != null) {
       createResourceOnlyIfNotExistingWithSSA =
           configAnnotation.createResourceOnlyIfNotExistingWithSSA();
       useSSA = configAnnotation.useSSA().asBoolean();
+
+      // check if we have a specific matcher
+      Class<? extends KubernetesDependentResource<?, ?>> dependentResourceClass =
+          (Class<? extends KubernetesDependentResource<?, ?>>) spec.getDependentResourceClass();
+      final var context =
+          Utils.contextFor(
+              controllerConfig, dependentResourceClass, configAnnotation.annotationType());
+      matcher =
+          Utils.instantiate(
+              configAnnotation.matcher(), SSABasedGenericKubernetesResourceMatcher.class, context);
     }
 
     var informerConfiguration =
@@ -35,7 +47,7 @@ public KubernetesDependentResourceConfig<R> configFrom(
             controllerConfig);
 
     return new KubernetesDependentResourceConfig<>(
-        useSSA, createResourceOnlyIfNotExistingWithSSA, informerConfiguration);
+        useSSA, createResourceOnlyIfNotExistingWithSSA, informerConfiguration, matcher);
   }
 
   @SuppressWarnings({"unchecked"})

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

@@ -37,6 +37,7 @@ public abstract class KubernetesDependentResource<R extends HasMetadata, P exten
     implements ConfiguredDependentResource<KubernetesDependentResourceConfig<R>> {
 
   private static final Logger log = LoggerFactory.getLogger(KubernetesDependentResource.class);
+
   private final boolean garbageCollected = this instanceof GarbageCollected;
   private KubernetesDependentResourceConfig<R> kubernetesDependentResourceConfig;
   private volatile Boolean useSSA;
@@ -112,7 +113,9 @@ public Result<R> match(R actualResource, R desired, P primary, Context<P> contex
     addMetadata(true, actualResource, desired, primary, context);
     if (useSSA(context)) {
       matches =
-          SSABasedGenericKubernetesResourceMatcher.getInstance()
+          configuration()
+              .map(KubernetesDependentResourceConfig::matcher)
+              .orElse(SSABasedGenericKubernetesResourceMatcher.getInstance())
               .matches(actualResource, desired, context);
     } else {
       matches =

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

@@ -10,14 +10,18 @@ public class KubernetesDependentResourceConfig<R extends HasMetadata> {
   private final Boolean useSSA;
   private final boolean createResourceOnlyIfNotExistingWithSSA;
   private final InformerConfiguration<R> informerConfig;
+  private final SSABasedGenericKubernetesResourceMatcher<R> matcher;
 
   public KubernetesDependentResourceConfig(
       Boolean useSSA,
       boolean createResourceOnlyIfNotExistingWithSSA,
-      InformerConfiguration<R> informerConfig) {
+      InformerConfiguration<R> informerConfig,
+      SSABasedGenericKubernetesResourceMatcher<R> matcher) {
     this.useSSA = useSSA;
     this.createResourceOnlyIfNotExistingWithSSA = createResourceOnlyIfNotExistingWithSSA;
     this.informerConfig = informerConfig;
+    this.matcher =
+        matcher != null ? matcher : SSABasedGenericKubernetesResourceMatcher.getInstance();
   }
 
   public boolean createResourceOnlyIfNotExistingWithSSA() {
@@ -31,4 +35,8 @@ public Boolean useSSA() {
   public InformerConfiguration<R> informerConfig() {
     return informerConfig;
   }
+
+  public SSABasedGenericKubernetesResourceMatcher<R> matcher() {
+    return matcher;
+  }
 }

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

@@ -8,6 +8,7 @@ public final class KubernetesDependentResourceConfigBuilder<R extends HasMetadat
   private boolean createResourceOnlyIfNotExistingWithSSA;
   private Boolean useSSA = null;
   private InformerConfiguration<R> informerConfiguration;
+  private SSABasedGenericKubernetesResourceMatcher<R> matcher;
 
   public KubernetesDependentResourceConfigBuilder() {}
 
@@ -29,8 +30,14 @@ public KubernetesDependentResourceConfigBuilder<R> withKubernetesDependentInform
     return this;
   }
 
+  public KubernetesDependentResourceConfigBuilder<R> withSSAMatcher(
+      SSABasedGenericKubernetesResourceMatcher<R> matcher) {
+    this.matcher = matcher;
+    return this;
+  }
+
   public KubernetesDependentResourceConfig<R> build() {
     return new KubernetesDependentResourceConfig<>(
-        useSSA, createResourceOnlyIfNotExistingWithSSA, informerConfiguration);
+        useSSA, createResourceOnlyIfNotExistingWithSSA, informerConfiguration, matcher);
   }
 }

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

@@ -22,6 +22,7 @@
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -39,6 +40,7 @@ void setup() {
     when(mockedContext.getClient()).thenReturn(client);
 
     final var configurationService = mock(ConfigurationService.class);
+    when(configurationService.shouldUseSSA(any(), any(), any())).thenReturn(true);
     final var controllerConfiguration = mock(ControllerConfiguration.class);
     when(controllerConfiguration.getConfigurationService()).thenReturn(configurationService);
     when(controllerConfiguration.fieldManager()).thenReturn("controller");
@@ -239,17 +241,34 @@ void testSanitizeState_daemonSetWithResources_withMismatch() {
   @ParameterizedTest
   @ValueSource(booleans = {true, false})
   void testCustomMatcher_returnsExpectedMatchBasedOnReadOnlyLabel(boolean readOnly) {
+    var dr = new ConfigMapDR();
+    dr.configureWith(
+        new KubernetesDependentResourceConfigBuilder()
+            .withSSAMatcher(new ReadOnlyAwareMatcher())
+            .build());
     var desiredConfigMap =
         loadResource("configmap.empty-owner-reference-desired.yaml", ConfigMap.class);
     desiredConfigMap.getData().put("key1", "another value");
     var actualConfigMap = loadResource("configmap.empty-owner-reference.yaml", ConfigMap.class);
     actualConfigMap.getMetadata().getLabels().put("readonly", Boolean.toString(readOnly));
 
-    var matcher = new ReadOnlyAwareMatcher<ConfigMap>();
-    assertThat(matcher.matches(actualConfigMap, desiredConfigMap, mockedContext))
+    ConfigMap ignoredPrimary = null;
+    assertThat(
+            dr.match(
+                    actualConfigMap,
+                    desiredConfigMap,
+                    ignoredPrimary,
+                    (Context<ConfigMap>) mockedContext)
+                .matched())
         .isEqualTo(readOnly);
   }
 
+  private static class ConfigMapDR extends KubernetesDependentResource<ConfigMap, ConfigMap> {
+    public ConfigMapDR() {
+      super(ConfigMap.class);
+    }
+  }
+
   private static class ReadOnlyAwareMatcher<T extends HasMetadata>
       extends SSABasedGenericKubernetesResourceMatcher<T> {
     @Override