feat: id provider for external dependent resources (#2970)

Signed-off-by: Attila Mészáros <[email protected]>
Co-authored-by: Martin Stefanko <[email protected]>

Author: csviri

docs/content/en/docs/documentation/dependent-resource-and-workflows/dependent-resources.md

@@ -355,6 +355,42 @@ as [integration tests](https://github.com/operator-framework/java-operator-sdk/t
 To see how bulk dependent resources interact with workflow conditions, please refer to this
 [integration test](https://github.com/operator-framework/java-operator-sdk/tree/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/bulkdependent/conidition).
 
+## Dependent Resources with External Resource
+
+Dependent resources are designed to manage also non-Kubernetes or external resources.
+To implement such dependent you can extend `AbstractExternalDependentResource` or one of its 
+[subclasses](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external).
+
+For Kubernetes resources we can have nice assumptions, like
+if there are multiple resources of the same type, we can select the target resource
+that dependent resource manages based on the name and namespace of the desired resource;
+or we can use a matcher based SSA in most of the cases if the resource is managed using SSA.
+
+### Selecting the target resource
+
+Unfortunately this is not true for external resources. So to make sure we are selecting
+the target resources from an event source, we provide a [mechanism](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractExternalDependentResource.java#L114-L138) that helps with that logic.
+Your POJO representing an external resource can implement [`ExternalResourceIDProvider`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/ExternalDependentIDProvider.java) : 
+
+```java
+
+public interface ExternalDependentIDProvider<T> {
+
+  T externalResourceId();
+}
+```
+
+That will provide an ID, what is used to check for equality for desired state and resources from event source caches.
+Not that if some reason this mechanism does not suit for you, you can simply 
+override [`selectTargetSecondaryResource`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractExternalDependentResource.java)
+method.
+
+### Matching external resources
+
+By default, external resources are matched using [equality](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractExternalDependentResource.java#L88-L92).
+So you can override equals of you POJO representing an external resource. 
+As an alternative you can always override the whole `match` method to completely customize matching.
+
 ## External State Tracking Dependent Resources
 
 It is sometimes necessary for a controller to track external (i.e. non-Kubernetes) state to

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

@@ -149,16 +149,8 @@ public Optional<R> getSecondaryResource(P primary, Context<P> context) {
    * @throws IllegalStateException if more than one candidate is found, in which case some other
    *     mechanism might be necessary to distinguish between candidate secondary resources
    */
-  protected Optional<R> selectTargetSecondaryResource(
-      Set<R> secondaryResources, P primary, Context<P> context) {
-    R desired = desired(primary, context);
-    var targetResources = secondaryResources.stream().filter(r -> r.equals(desired)).toList();
-    if (targetResources.size() > 1) {
-      throw new IllegalStateException(
-          "More than one secondary resource related to primary: " + targetResources);
-    }
-    return targetResources.isEmpty() ? Optional.empty() : Optional.of(targetResources.get(0));
-  }
+  protected abstract Optional<R> selectTargetSecondaryResource(
+      Set<R> secondaryResources, P primary, Context<P> context);
 
   private void throwIfNull(R desired, P primary, String descriptor) {
     if (desired == null) {

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

@@ -15,6 +15,10 @@
  */
 package io.javaoperatorsdk.operator.processing.dependent;
 
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.RecentOperationCacheFiller;
@@ -121,4 +125,30 @@ public void handleDeleteTargetResource(P primary, R resource, String key, Contex
   protected InformerEventSource getExternalStateEventSource() {
     return externalStateEventSource;
   }
+
+  @Override
+  protected Optional<R> selectTargetSecondaryResource(
+      Set<R> secondaryResources, P primary, Context<P> context) {
+    R desired = desired(primary, context);
+    List<R> targetResources;
+    if (desired instanceof ExternalDependentIDProvider<?> desiredWithId) {
+      targetResources =
+          secondaryResources.stream()
+              .filter(
+                  r ->
+                      ((ExternalDependentIDProvider<?>) r)
+                          .externalResourceId()
+                          .equals(desiredWithId.externalResourceId()))
+              .toList();
+    } else {
+      throw new IllegalStateException(
+          "Either implement ExternalDependentIDProvider or override this "
+              + " (selectTargetSecondaryResource) method.");
+    }
+    if (targetResources.size() > 1) {
+      throw new IllegalStateException(
+          "More than one secondary resource related to primary: " + targetResources);
+    }
+    return targetResources.isEmpty() ? Optional.empty() : Optional.of(targetResources.get(0));
+  }
 }

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

@@ -19,6 +19,7 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
@@ -120,6 +121,13 @@ public Result<R> match(R resource, P primary, Context<P> context) {
       return bulkDependentResource.match(resource, desired, primary, context);
     }
 
+    @Override
+    protected Optional<R> selectTargetSecondaryResource(
+        Set<R> secondaryResources, P primary, Context<P> context) {
+      throw new IllegalStateException(
+          "BulkDependentResource should not call selectTargetSecondaryResource.");
+    }
+
     @Override
     protected void onCreated(P primary, R created, Context<P> context) {
       asAbstractDependentResource().onCreated(primary, created, context);

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

@@ -0,0 +1,28 @@
+/*
+ * Copyright Java Operator SDK Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *         http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.javaoperatorsdk.operator.processing.dependent;
+
+/**
+ * Provides the identifier for an object that represents an external resource. This ID is used to
+ * select target resource for a dependent resource from the resources returned by `{@link
+ * io.javaoperatorsdk.operator.api.reconciler.Context#getSecondaryResources(Class)}`.
+ *
+ * @param <T>
+ */
+public interface ExternalDependentIDProvider<T> {
+
+  T externalResourceId();
+}

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java

@@ -23,7 +23,6 @@
 import io.fabric8.kubernetes.api.model.apps.Deployment;
 import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder;
 import io.fabric8.kubernetes.api.model.apps.DeploymentSpec;
-import io.fabric8.kubernetes.api.model.apps.DeploymentStatus;
 import io.fabric8.kubernetes.client.CustomResource;
 import io.fabric8.kubernetes.client.KubernetesClientException;
 import io.fabric8.kubernetes.client.http.HttpRequest;
@@ -130,29 +129,6 @@ void setsSpecCustomResourceWithReflection() {
     assertThat(tomcat.getSpec().getReplicas()).isEqualTo(1);
   }
 
-  @Test
-  void setsStatusWithReflection() {
-    Deployment deployment = new Deployment();
-    DeploymentStatus status = new DeploymentStatus();
-    status.setReplicas(2);
-
-    ReconcilerUtils.setStatus(deployment, status);
-
-    assertThat(deployment.getStatus().getReplicas()).isEqualTo(2);
-  }
-
-  @Test
-  void getsStatusWithReflection() {
-    Deployment deployment = new Deployment();
-    DeploymentStatus status = new DeploymentStatus();
-    status.setReplicas(2);
-    deployment.setStatus(status);
-
-    var res = ReconcilerUtils.getStatus(deployment);
-
-    assertThat(((DeploymentStatus) res).getReplicas()).isEqualTo(2);
-  }
-
   @Test
   void loadYamlAsBuilder() {
     DeploymentBuilder builder =

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

@@ -16,6 +16,7 @@
 package io.javaoperatorsdk.operator.processing.dependent;
 
 import java.util.Optional;
+import java.util.Set;
 
 import org.junit.jupiter.api.Test;
 
@@ -99,6 +100,20 @@ public Optional<ConfigMap> getSecondaryResource(
       return Optional.ofNullable(secondary);
     }
 
+    @Override
+    protected Optional<ConfigMap> selectTargetSecondaryResource(
+        Set<ConfigMap> secondaryResources,
+        TestCustomResource primary,
+        Context<TestCustomResource> context) {
+      if (secondaryResources.size() == 1) {
+        return Optional.of(secondaryResources.iterator().next());
+      } else if (secondaryResources.isEmpty()) {
+        return Optional.empty();
+      } else {
+        throw new IllegalStateException();
+      }
+    }
+
     @Override
     protected void onCreated(
         TestCustomResource primary, ConfigMap created, Context<TestCustomResource> context) {}

operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/multiplemanagedexternaldependenttype/MultipleManagedExternalDependentSameTypeIT.java

@@ -29,7 +29,7 @@
 class MultipleManagedExternalDependentSameTypeIT {
 
   @RegisterExtension
-  LocallyRunOperatorExtension operator =
+  LocallyRunOperatorExtension extension =
       LocallyRunOperatorExtension.builder()
           .withReconciler(new MultipleManagedExternalDependentResourceReconciler())
           .build();
@@ -42,15 +42,15 @@ class MultipleManagedExternalDependentSameTypeIT {
 
   @Test
   void handlesExternalCrudOperations() {
-    operator.create(testResource());
+    extension.create(testResource());
     assertResourceCreatedWithData(DEFAULT_SPEC_VALUE);
 
     var updatedResource = testResource();
     updatedResource.getSpec().setValue(UPDATED_SPEC_VALUE);
-    operator.replace(updatedResource);
+    extension.replace(updatedResource);
     assertResourceCreatedWithData(UPDATED_SPEC_VALUE);
 
-    operator.delete(testResource());
+    extension.delete(testResource());
     assertExternalResourceDeleted();
   }
 

operator-framework/src/test/java/io/javaoperatorsdk/operator/support/ExternalResource.java

@@ -18,9 +18,10 @@
 import java.util.Objects;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.javaoperatorsdk.operator.processing.dependent.ExternalDependentIDProvider;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
 
-public class ExternalResource {
+public class ExternalResource implements ExternalDependentIDProvider<String> {
 
   public static final String EXTERNAL_RESOURCE_NAME_DELIMITER = "#";
 
@@ -80,4 +81,9 @@ public static String toExternalResourceId(HasMetadata primary) {
         + EXTERNAL_RESOURCE_NAME_DELIMITER
         + primary.getMetadata().getNamespace();
   }
+
+  @Override
+  public String externalResourceId() {
+    return id;
+  }
 }

sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/schema/Schema.java

@@ -18,7 +18,9 @@
 import java.io.Serializable;
 import java.util.Objects;
 
-public class Schema implements Serializable {
+import io.javaoperatorsdk.operator.processing.dependent.ExternalDependentIDProvider;
+
+public class Schema implements Serializable, ExternalDependentIDProvider<String> {
 
   private final String name;
   private final String characterSet;
@@ -41,7 +43,7 @@ public boolean equals(Object o) {
     if (this == o) return true;
     if (o == null || getClass() != o.getClass()) return false;
     Schema schema = (Schema) o;
-    return Objects.equals(name, schema.name);
+    return Objects.equals(name, schema.name) && Objects.equals(characterSet, schema.characterSet);
   }
 
   @Override
@@ -53,4 +55,9 @@ public int hashCode() {
   public String toString() {
     return "Schema{" + "name='" + name + '\'' + ", characterSet='" + characterSet + '\'' + '}';
   }
+
+  @Override
+  public String externalResourceId() {
+    return name;
+  }
 }