wip

Signed-off-by: Attila Mészáros <a_meszaros@apple.com>

Author: csviri

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

@@ -25,6 +25,7 @@
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.client.KubernetesClient;
 import io.fabric8.kubernetes.client.informers.SharedIndexInformer;
+import io.javaoperatorsdk.operator.OperatorException;
 
 @SuppressWarnings({"unchecked", "rawtypes"})
 public class AlwaysNewInformerPool extends AbstractInformerPool {
@@ -39,8 +40,21 @@ public <R extends HasMetadata> SharedIndexInformer<R> getInformer(
       String name,
       InformerClassifier<R> classifier,
       KubernetesClient client) {
+    var key = new ClassifierWithName(controllerName, name, classifier);
+    if (informers.containsKey(key)) {
+      throw new OperatorException(
+          "Informer already registered for controller: "
+              + controllerName
+              + ", event source: "
+              + name
+              + ", classifier: "
+              + classifier
+              + ". This pool creates a dedicated informer per controller/event source and never"
+              + " shares them, so requesting one twice for the same combination without releasing"
+              + " the previous one first would leak the earlier informer.");
+    }
     var informer = createInformer(classifier, client);
-    informers.put(new ClassifierWithName(controllerName, name, classifier), informer);
+    informers.put(key, informer);
     return informer;
   }
 

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/AlwaysNewInformerPoolTest.java

@@ -20,10 +20,12 @@
 
 import io.fabric8.kubernetes.client.KubernetesClient;
 import io.javaoperatorsdk.operator.MockKubernetesClient;
+import io.javaoperatorsdk.operator.OperatorException;
 import io.javaoperatorsdk.operator.api.config.BaseConfigurationService;
 import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -70,13 +72,33 @@ void createsSeparateInformerForDifferentEventSourceNames() {
   }
 
   @Test
-  void createsNewInformerEvenForAnIdenticalKey() {
+  void throwsWhenRequestingAnInformerForAnAlreadyRegisteredKey() {
     var classifier = classifier("default");
 
     pool.getInformer(CONTROLLER, ES_NAME, classifier, client);
+
+    // requesting the same controller+event source+classifier combination again without releasing
+    // first would otherwise silently overwrite the map entry and leak the earlier informer
+    assertThatThrownBy(() -> pool.getInformer(CONTROLLER, ES_NAME, classifier, client))
+        .isInstanceOf(OperatorException.class)
+        .hasMessageContaining(CONTROLLER)
+        .hasMessageContaining(ES_NAME)
+        .hasMessageContaining(classifier.toString());
+
+    // the earlier informer is left untouched, still registered exactly once
+    assertThat(pool.size()).isEqualTo(1);
+    verify(client, times(1)).resources(TestCustomResource.class);
+  }
+
+  @Test
+  void allowsReRequestingAnInformerAfterItWasReleased() {
+    var classifier = classifier("default");
+    pool.getInformer(CONTROLLER, ES_NAME, classifier, client);
+    pool.releaseInformer(CONTROLLER, ES_NAME, classifier);
+
+    // must not throw: releasing frees up the key for reuse
     pool.getInformer(CONTROLLER, ES_NAME, classifier, client);
 
-    // the same key overwrites the map entry, but a fresh informer is created on every call
     assertThat(pool.size()).isEqualTo(1);
     verify(client, times(2)).resources(TestCustomResource.class);
   }