fix: use SSA matcher to filter events

closes: operator-framework#2249

Signed-off-by: Steve Hawkins <[email protected]>

Author: shawkins

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

@@ -23,6 +23,7 @@
 import io.fabric8.kubernetes.api.model.apps.Deployment;
 import io.fabric8.kubernetes.api.model.apps.ReplicaSet;
 import io.fabric8.kubernetes.api.model.apps.StatefulSet;
+import io.fabric8.kubernetes.client.KubernetesClient;
 import io.fabric8.kubernetes.client.utils.KubernetesSerialization;
 import io.javaoperatorsdk.operator.OperatorException;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
@@ -79,10 +80,14 @@ public static <L extends HasMetadata> SSABasedGenericKubernetesResourceMatcher<L
   private static final Logger log =
       LoggerFactory.getLogger(SSABasedGenericKubernetesResourceMatcher.class);
 
-  @SuppressWarnings("unchecked")
   public boolean matches(R actual, R desired, Context<?> context) {
-    var optionalManagedFieldsEntry =
-        checkIfFieldManagerExists(actual, context.getControllerConfiguration().fieldManager());
+    return matches(
+        actual, desired, context.getControllerConfiguration().fieldManager(), context.getClient(), false);
+  }
+
+  @SuppressWarnings("unchecked")
+  public boolean matches(R actual, R desired, String fieldManager, KubernetesClient client, boolean compareActual) {
+    var optionalManagedFieldsEntry = checkIfFieldManagerExists(actual, fieldManager);
     // If no field is managed by our controller, that means the controller hasn't touched the
     // resource yet and the resource probably doesn't match the desired state. Not matching here
     // means that the resource will need to be updated and since this will be done using SSA, the
@@ -93,7 +98,7 @@ public boolean matches(R actual, R desired, Context<?> context) {
 
     var managedFieldsEntry = optionalManagedFieldsEntry.orElseThrow();
 
-    var objectMapper = context.getClient().getKubernetesSerialization();
+    var objectMapper = client.getKubernetesSerialization();
     var actualMap = objectMapper.convertValue(actual, Map.class);
     var desiredMap = objectMapper.convertValue(desired, Map.class);
     if (LoggingUtils.isNotSensitiveResource(desired)) {
@@ -108,7 +113,18 @@ public boolean matches(R actual, R desired, Context<?> context) {
         managedFieldsEntry.getFieldsV1().getAdditionalProperties(),
         objectMapper);
 
-    removeIrrelevantValues(desiredMap);
+    if (compareActual) {
+      var prunedDesired = new HashMap<String, Object>(actualMap.size());
+      keepOnlyManagedFields(
+          prunedDesired,
+          desiredMap,
+          managedFieldsEntry.getFieldsV1().getAdditionalProperties(),
+          objectMapper);
+      
+      desiredMap = prunedDesired;
+    } else {
+      removeIrrelevantValues(desiredMap);
+    }
 
     var matches = prunedActual.equals(desiredMap);
     if (!matches && log.isDebugEnabled() && LoggingUtils.isNotSensitiveResource(desired)) {
@@ -438,7 +454,7 @@ private static Map.Entry<Integer, Map<String, Object>> selectListEntryBasedOnKey
     return new AbstractMap.SimpleEntry<>(lastIndex, possibleTargets.get(0));
   }
 
-  private Optional<ManagedFieldsEntry> checkIfFieldManagerExists(R actual, String fieldManager) {
+  public Optional<ManagedFieldsEntry> checkIfFieldManagerExists(R actual, String fieldManager) {
     var targetManagedFields =
         actual.getMetadata().getManagedFields().stream()
             // Only the apply operations are interesting for us since those were created properly be

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java

@@ -14,6 +14,7 @@
 import io.fabric8.kubernetes.client.informers.ResourceEventHandler;
 import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.SSABasedGenericKubernetesResourceMatcher;
 import io.javaoperatorsdk.operator.processing.event.Event;
 import io.javaoperatorsdk.operator.processing.event.EventHandler;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
@@ -68,6 +69,8 @@ public class InformerEventSource<R extends HasMetadata, P extends HasMetadata>
   private final PrimaryToSecondaryIndex<R> primaryToSecondaryIndex;
   private final PrimaryToSecondaryMapper<P> primaryToSecondaryMapper;
   private final String id = UUID.randomUUID().toString();
+  private final KubernetesClient client;
+  private final String fieldManager;
 
   public InformerEventSource(
       InformerEventSourceConfiguration<R> configuration, EventSourceContext<P> context) {
@@ -77,18 +80,20 @@ public InformerEventSource(
         context
             .getControllerConfiguration()
             .getConfigurationService()
-            .parseResourceVersionsForEventFilteringAndCaching());
+            .parseResourceVersionsForEventFilteringAndCaching(),
+        context.getControllerConfiguration().fieldManager());
   }
 
   InformerEventSource(InformerEventSourceConfiguration<R> configuration, KubernetesClient client) {
-    this(configuration, client, false);
+    this(configuration, client, false, "manager");
   }
 
   @SuppressWarnings({"unchecked", "rawtypes"})
   private InformerEventSource(
       InformerEventSourceConfiguration<R> configuration,
       KubernetesClient client,
-      boolean parseResourceVersions) {
+      boolean parseResourceVersions,
+      String fieldManager) {
     super(
         configuration.name(),
         configuration
@@ -112,6 +117,8 @@ private InformerEventSource(
     onUpdateFilter = informerConfig.getOnUpdateFilter();
     onDeleteFilter = informerConfig.getOnDeleteFilter();
     genericFilter = informerConfig.getGenericFilter();
+    this.client = client;
+    this.fieldManager = fieldManager;
   }
 
   @Override
@@ -215,10 +222,22 @@ private boolean isEventKnownFromAnnotation(R newObject, R oldObject) {
       if (id.equals(parts[0])) {
         if (oldObject == null && parts.length == 1) {
           known = true;
-        } else if (oldObject != null
-            && parts.length == 2
-            && oldObject.getMetadata().getResourceVersion().equals(parts[1])) {
-          known = true;
+        } else if (oldObject != null && parts.length == 2) {
+          if (oldObject.getMetadata().getResourceVersion().equals(parts[1])) {
+            known = true;
+          } else {
+            // if the resource version doesn't match, there's a chance that it's due
+            // to a change that is not meaningful, but causes the matcher logic between
+            // desired and newObject to see a change.
+            // TODO: this likely should be conditional on useSSA, not just the presence of the 
+            // field manager
+            var ssaMatcher = SSABasedGenericKubernetesResourceMatcher.getInstance();
+            if (ssaMatcher.checkIfFieldManagerExists(newObject, fieldManager).isPresent()) {
+              known = ssaMatcher.matches(oldObject, newObject, fieldManager, client, true);
+            } else {
+              // do we even need to worry about this case 
+            }
+          }
         }
       }
     }

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

@@ -114,7 +114,25 @@ void addedLabelInDesiredMakesMatchFail() {
 
     assertThat(matcher.matches(actualConfigMap, desiredConfigMap, mockedContext)).isFalse();
   }
+  
+  @Test
+  void compareActualSame() {
+    var actualConfigMap = loadResource("configmap.empty-owner-reference.yaml", ConfigMap.class);
 
+    assertThat(matcher.matches(actualConfigMap, actualConfigMap, mockedContext.getControllerConfiguration().fieldManager(), mockedContext.getClient(), true)).isTrue();
+  }
+  
+  @Test
+  void compareActualSameWithUnownedChange() {
+    var actualConfigMap = loadResource("configmap.empty-owner-reference.yaml", ConfigMap.class);
+    
+    var newConfigMap = loadResource("configmap.empty-owner-reference.yaml", ConfigMap.class);
+    newConfigMap.getMetadata().setLabels(Map.of("newlabel", "val"));
+
+    assertThat(matcher.matches(newConfigMap, actualConfigMap, mockedContext.getControllerConfiguration().fieldManager(), mockedContext.getClient(), true)).isTrue();
+    assertThat(matcher.matches(actualConfigMap, newConfigMap, mockedContext.getControllerConfiguration().fieldManager(), mockedContext.getClient(), true)).isTrue();
+  }
+  
   @Test
   @SuppressWarnings("unchecked")
   void sortListItemsTest() {