feat: ResourceIDMapper for external event sources, external dependents and bulk dependents (#3020)

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

Author: csviri

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

@@ -369,19 +369,22 @@ or we can use a matcher based SSA in most of the cases if the resource is manage
 ### 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) : 
+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#L133-L147) that helps with that logic.
+[`ResourceIDMapper`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceIDMapper.java)
+maps the resource to and ID and the ID of desired and actual resource is checked for equality.
+
+Your POJO representing an external resource can implement [`ResourceIDProvider`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceIDProvider.java). 
+The default `ResourceIDMapper` implementation works on top of resource which implements the `ResourceIDProvider`:
 
 ```java
 
-public interface ExternalDependentIDProvider<T> {
+public interface ResourceIDProvider<T> {
 
-  T externalResourceId();
+  T resourceId();
 }
 ```
 
-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 
+Note that if those approaches does not work for your use case, 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.
 

docs/content/en/docs/migration/v5-2-migration.md

@@ -0,0 +1,44 @@
+---
+title: Migrating from v5.1 to v5.2
+description: Migrating from v5.1 to v5.2
+---
+
+Version 5.2 brings some breaking changes to certain components. This document provides
+a migration guide for these changes. For all the new features, see the release notes.
+
+## Custom ID types across multiple components using ResourceIDMapper and ResourceIDProvider 
+
+Working with the id of a resource is needed across various components in the framework.
+Until this version, the components provided by the framework assumed that you could easily
+convert the id of a resource into a String representation. For example, 
+[BulkDependentResources](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkDependentResource.java#L46)
+worked with a `Map<String,R>` of resources, where the id was always of type String. 
+
+Mainly because of the need to manage external dependent resources more elegantly,
+we introduced a cross-cutting concept: [`ResourceIDMapper`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceIDMapper.java),
+which gets the ID of a resource. This is used across various components, see:
+
+ - [`ExternalResourceCachingEventSource`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/ExternalResourceCachingEventSource.java#L66)
+ - [`ExternalBulkDependentResource`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/ExternalBulkDependentResource.java)
+ - [`AbstractExternalDependentResource`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractExternalDependentResource.java#L39)
+   and its subclasses.
+   
+We also added [`ResourceIDProvider`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceIDProvider.java),
+which you can implement in your Pojo representing a resource. 
+
+The easiest way to migrate to this new approach is to implement this interface for your (external) resource
+and set the ID type generics for the components above. The default implementation of the `ResourceIDMapper`
+works with `ResourceIDProvider` (see [related implementation](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceIDMapper.java#L52)).
+
+If you cannot implement `ResourceIDProvider` because, for example, the class that represents the external resource is generated and final,
+you can always set a custom `ResourceIDMapper` on the components above.
+
+See also:
+ - related issue: [link](https://github.com/operator-framework/java-operator-sdk/issues/2972)
+ - related pull requests: 
+   - [2970](https://github.com/operator-framework/java-operator-sdk/pull/2970)
+   - [3020](https://github.com/operator-framework/java-operator-sdk/pull/3020)
+
+
+
+

docs/content/fileList.txt

@@ -0,0 +1,71 @@
+/*
+ * 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;
+
+import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.javaoperatorsdk.operator.processing.event.ResourceID;
+import io.javaoperatorsdk.operator.processing.event.source.ExternalResourceCachingEventSource;
+
+/**
+ * Provides id for the target resource. This mapper is used across multiple components of the
+ * framework, like:
+ *
+ * <ul>
+ *   <li>{@link io.javaoperatorsdk.operator.processing.dependent.AbstractExternalDependentResource}
+ *   <li>{@link ExternalResourceCachingEventSource}
+ *   <li>{@link io.javaoperatorsdk.operator.processing.dependent.KubernetesBulkDependentResource}
+ * </ul>
+ *
+ * @see ResourceIDProvider<ID>
+ */
+public interface ResourceIDMapper<R, ID> {
+
+  /**
+   * @return id for the target resource.
+   */
+  ID idFor(R resource);
+
+  /**
+   * Can be used if a polling event source handles only single secondary resource and the id is
+   * String. See also docs for: {@link ExternalResourceCachingEventSource}; or in {@link
+   * io.javaoperatorsdk.operator.processing.dependent.AbstractExternalDependentResource} when there
+   * is always only one secondary resource for that dependent resource. By definition cannot be used
+   * for a BulkDependent resources.
+   *
+   * @return static id mapper, all resources are mapped for same id.
+   * @param <R> secondary resource type
+   */
+  static <R> ResourceIDMapper<R, String> singleResourceResourceIDMapper() {
+    // the result could be any string, by definition would work with any value
+    return r -> "irrelevant";
+  }
+
+  @SuppressWarnings({"rawtypes", "unchecked"})
+  static <R, ID> ResourceIDMapper<R, ID> resourceIdProviderMapper() {
+    return r -> {
+      if (r instanceof ResourceIDProvider resourceIDProvider) {
+        return (ID) resourceIDProvider.resourceId();
+      } else {
+        throw new IllegalStateException(
+            "Resource does not implement ExternalDependentIDProvider: " + r.getClass());
+      }
+    };
+  }
+
+  static <R extends HasMetadata> ResourceIDMapper<R, ResourceID> kubernetesResourceIdMapper() {
+    return ResourceID::fromResource;
+  }
+}

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

@@ -0,0 +1,38 @@
+/*
+ * 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;
+
+/**
+ * Provides the identifier for an object that represents a resource. This ID is used:
+ *
+ * <ul>
+ *   <li>to select the target external resource for a dependent resource from the resources returned
+ *       by {@link io.javaoperatorsdk.operator.api.reconciler.Context#getSecondaryResources(Class)},
+ *   <li>used in {@link ResourceIDMapper} for event sources in external resources. But also for bulk
+ *       dependent resource see {@link
+ *       io.javaoperatorsdk.operator.processing.dependent.ExternalBulkDependentResource},
+ *   <li>and external event sources, see {@link
+ *       io.javaoperatorsdk.operator.processing.event.source.ExternalResourceCachingEventSource}
+ * </ul>
+ *
+ * @see ResourceIDMapper
+ * @param <ID> type of the id
+ */
+public interface ResourceIDProvider<ID> {
+
+  /** ID for the resource POJO that implement this interface. */
+  ID resourceId();
+}

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

@@ -63,7 +63,7 @@ protected AbstractDependentResource(String name) {
 
     dependentResourceReconciler =
         this instanceof BulkDependentResource
-            ? new BulkDependentResourceReconciler<>((BulkDependentResource<R, P>) this)
+            ? new BulkDependentResourceReconciler<>((BulkDependentResource<R, P, ?>) this)
             : new SingleDependentResourceReconciler<>(this);
     this.name = name == null ? DependentResource.defaultNameFor(this.getClass()) : name;
   }

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

@@ -22,19 +22,22 @@
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.RecentOperationCacheFiller;
+import io.javaoperatorsdk.operator.processing.ResourceIDMapper;
 import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
 import io.javaoperatorsdk.operator.processing.event.source.EventSource;
 import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource;
 
 public abstract class AbstractExternalDependentResource<
-        R, P extends HasMetadata, T extends EventSource<R, P>>
+        R, P extends HasMetadata, T extends EventSource<R, P>, ID>
     extends AbstractEventSourceHolderDependentResource<R, P, T> {
 
   private final boolean isDependentResourceWithExplicitState =
       this instanceof DependentResourceWithExplicitState;
   private final boolean isBulkDependentResource = this instanceof BulkDependentResource;
 
+  protected ResourceIDMapper<R, ID> resourceIDMapper = ResourceIDMapper.resourceIdProviderMapper();
+
   @SuppressWarnings("rawtypes")
   private DependentResourceWithExplicitState dependentResourceWithExplicitState;
 
@@ -131,24 +134,23 @@ 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.");
-    }
+    var desiredID = resourceIDMapper.idFor(desired);
+    targetResources =
+        secondaryResources.stream()
+            .filter(r -> resourceIDMapper.idFor(r).equals(desiredID))
+            .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));
   }
+
+  public ResourceIDMapper<R, ID> resourceIDMapper() {
+    return resourceIDMapper;
+  }
+
+  public void setResourceIDMapper(ResourceIDMapper<R, ID> resourceIDMapper) {
+    this.resourceIDMapper = resourceIDMapper;
+  }
 }

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

@@ -27,10 +27,11 @@
  * implementing this interface will typically also implement one or more additional interfaces such
  * as {@link Creator}, {@link Updater}, {@link Deleter}.
  *
+ * @param <ID> type of the id to distinguish resource
  * @param <R> the dependent resource type
  * @param <P> the primary resource type
  */
-public interface BulkDependentResource<R, P extends HasMetadata> {
+public interface BulkDependentResource<R, P extends HasMetadata, ID> {
 
   /**
    * Retrieves a map of desired secondary resources associated with the specified primary resource,
@@ -42,7 +43,7 @@ public interface BulkDependentResource<R, P extends HasMetadata> {
    * @return a Map associating desired secondary resources with the specified primary via arbitrary
    *     identifiers
    */
-  default Map<String, R> desiredResources(P primary, Context<P> context) {
+  default Map<ID, R> desiredResources(P primary, Context<P> context) {
     throw new IllegalStateException(
         "Implement desiredResources in case a non read-only bulk dependent resource");
   }
@@ -57,7 +58,7 @@ default Map<String, R> desiredResources(P primary, Context<P> context) {
    * @return a Map associating actual secondary resources with the specified primary via arbitrary
    *     identifiers
    */
-  Map<String, R> getSecondaryResources(P primary, Context<P> context);
+  Map<ID, R> getSecondaryResources(P primary, Context<P> context);
 
   /**
    * Deletes the actual resource identified by the specified key if it's not in the set of desired
@@ -69,7 +70,7 @@ default Map<String, R> desiredResources(P primary, Context<P> context) {
    * @param key key of the resource
    * @param context actual context
    */
-  void deleteTargetResource(P primary, R resource, String key, Context<P> context);
+  void deleteTargetResource(P primary, R resource, ID key, Context<P> context);
 
   /**
    * Determines whether the specified secondary resource matches the desired state with target index

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

@@ -29,19 +29,19 @@
 import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult;
 import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result;
 
-class BulkDependentResourceReconciler<R, P extends HasMetadata>
+class BulkDependentResourceReconciler<R, P extends HasMetadata, ID>
     implements DependentResourceReconciler<R, P> {
 
-  private final BulkDependentResource<R, P> bulkDependentResource;
+  private final BulkDependentResource<R, P, ID> bulkDependentResource;
 
-  BulkDependentResourceReconciler(BulkDependentResource<R, P> bulkDependentResource) {
+  BulkDependentResourceReconciler(BulkDependentResource<R, P, ID> bulkDependentResource) {
     this.bulkDependentResource = bulkDependentResource;
   }
 
   @Override
   public ReconcileResult<R> reconcile(P primary, Context<P> context) {
 
-    Map<String, R> actualResources = bulkDependentResource.getSecondaryResources(primary, context);
+    Map<ID, R> actualResources = bulkDependentResource.getSecondaryResources(primary, context);
     if (!(bulkDependentResource instanceof Creator<?, ?>)
         && !(bulkDependentResource instanceof Deleter<?>)
         && !(bulkDependentResource instanceof Updater<?, ?>)) {
@@ -73,7 +73,7 @@ public void delete(P primary, Context<P> context) {
   }
 
   private void deleteExtraResources(
-      Set<String> expectedKeys, Map<String, R> actualResources, P primary, Context<P> context) {
+      Set<ID> expectedKeys, Map<ID, R> actualResources, P primary, Context<P> context) {
     actualResources.forEach(
         (key, value) -> {
           if (!expectedKeys.contains(key)) {
@@ -90,13 +90,13 @@ private void deleteExtraResources(
    * @param <P>
    */
   @Ignore
-  private static class BulkDependentResourceInstance<R, P extends HasMetadata>
+  private static class BulkDependentResourceInstance<R, P extends HasMetadata, ID>
       extends AbstractDependentResource<R, P> implements Creator<R, P>, Deleter<P>, Updater<R, P> {
-    private final BulkDependentResource<R, P> bulkDependentResource;
+    private final BulkDependentResource<R, P, ID> bulkDependentResource;
     private final R desired;
 
     private BulkDependentResourceInstance(
-        BulkDependentResource<R, P> bulkDependentResource, R desired) {
+        BulkDependentResource<R, P, ID> bulkDependentResource, R desired) {
       this.bulkDependentResource = bulkDependentResource;
       this.desired = desired;
     }

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

@@ -18,5 +18,5 @@
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter;
 
-public interface CRUDBulkDependentResource<R, P extends HasMetadata>
-    extends BulkDependentResource<R, P>, Creator<R, P>, BulkUpdater<R, P>, Deleter<P> {}
+public interface CRUDBulkDependentResource<R, P extends HasMetadata, ID>
+    extends BulkDependentResource<R, P, ID>, Creator<R, P>, BulkUpdater<R, P>, Deleter<P> {}