fix: race condition on get when add received after informer cache read (#3237)

Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Steven Hawkins <shawkins@redhat.com>

Author: 3 people

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

@@ -191,8 +191,8 @@ public void handleRecentResourceCreate(ResourceID resourceID, R resource) {
 
   @Override
   public Optional<R> get(ResourceID resourceID) {
+    // The order of reading from these caches matters
     Optional<R> resource = temporaryResourceCache.getResourceFromCache(resourceID);
-    var res = cache.get(resourceID);
     if (comparableResourceVersions
         && resource.isPresent()
         && ReconcilerUtilsInternal.compareResourceVersions(
@@ -201,13 +201,16 @@ public Optional<R> get(ResourceID resourceID) {
             > 0) {
       log.debug("Latest resource found in temporary cache for Resource ID: {}", resourceID);
       return resource;
+    } else {
+      // this needs to happen after comparison to ensure correctness
+      var resFromInformer = cache.get(resourceID);
+      log.debug(
+          "Resource {} in temp cache; {}found in informer cache" + ", for Resource ID: {}",
+          resource.isPresent() ? "obsolete" : "not found",
+          resFromInformer.isPresent() ? "" : "not ",
+          resourceID);
+      return resFromInformer;
     }
-    log.debug(
-        "Resource not found, or older, in temporary cache. Found in informer cache {}, for"
-            + " Resource ID: {}",
-        res.isPresent(),
-        resourceID);
-    return res;
   }
 
   /**

operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/cachingfilteringupdate/CachingFilteringUpdateReconciler.java

@@ -43,27 +43,39 @@ public UpdateControl<CachingFilteringUpdateCustomResource> reconcile(
       CachingFilteringUpdateCustomResource resource,
       Context<CachingFilteringUpdateCustomResource> context) {
 
-    context.resourceOperations().serverSideApply(prepareCM(resource));
+    context.resourceOperations().serverSideApply(prepareCM(resource, 1));
     var cachedCM = context.getSecondaryResource(ConfigMap.class);
     if (cachedCM.isEmpty()) {
       issueFound.set(true);
       throw new IllegalStateException("Error for resource: " + ResourceID.fromResource(resource));
     }
 
+    var updated = context.resourceOperations().serverSideApply(prepareCM(resource, 2));
+    cachedCM = context.getSecondaryResource(ConfigMap.class);
+    if (!cachedCM
+        .orElseThrow()
+        .getMetadata()
+        .getResourceVersion()
+        .equals(updated.getMetadata().getResourceVersion())) {
+      issueFound.set(true);
+      throw new IllegalStateException(
+          "Update error for resource: " + ResourceID.fromResource(resource));
+    }
+
     ensureStatusExists(resource);
     resource.getStatus().setUpdated(true);
     return UpdateControl.patchStatus(resource);
   }
 
-  private static ConfigMap prepareCM(CachingFilteringUpdateCustomResource p) {
+  private static ConfigMap prepareCM(CachingFilteringUpdateCustomResource p, int num) {
     var cm =
         new ConfigMapBuilder()
             .withMetadata(
                 new ObjectMetaBuilder()
                     .withName(p.getMetadata().getName())
                     .withNamespace(p.getMetadata().getNamespace())
                     .build())
-            .withData(Map.of("name", p.getMetadata().getName()))
+            .withData(Map.of("name", p.getMetadata().getName(), "num", "" + num))
             .build();
     cm.addOwnerReference(p);
     return cm;