feat: move name is directly to dependent resource (#2250)

* feat: move name is directly to dependent resource

- use this name when throwing aggregate exception

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

* refactor to use a dedicated interface for setting the name

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

* refactor: add default implementation for name() (#2255)

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

---------

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

Author: csviri

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java

@@ -68,4 +68,9 @@ static String defaultNameFor(Class<? extends DependentResource> dependentResourc
   default boolean isDeletable() {
     return this instanceof Deleter;
   }
+
+  default String name() {
+    return defaultNameFor(getClass());
+  }
+
 }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/NameSetter.java

@@ -0,0 +1,7 @@
+package io.javaoperatorsdk.operator.api.reconciler.dependent;
+
+public interface NameSetter {
+
+  void setName(String name);
+
+}

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

@@ -11,13 +11,14 @@
 import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.NameSetter;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult;
 import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
 
 @Ignore
 public abstract class AbstractDependentResource<R, P extends HasMetadata>
-    implements DependentResource<R, P> {
+    implements DependentResource<R, P>, NameSetter {
   private static final Logger log = LoggerFactory.getLogger(AbstractDependentResource.class);
 
   private final boolean creatable = this instanceof Creator;
@@ -29,14 +30,21 @@ public abstract class AbstractDependentResource<R, P extends HasMetadata>
   private ResourceDiscriminator<R, P> resourceDiscriminator;
   private final DependentResourceReconciler<R, P> dependentResourceReconciler;
 
+  protected String name;
+
   @SuppressWarnings({"unchecked"})
   protected AbstractDependentResource() {
+    this(null);
+  }
+
+  protected AbstractDependentResource(String name) {
     creator = creatable ? (Creator<R, P>) this : null;
     updater = updatable ? (Updater<R, P>) this : null;
 
     dependentResourceReconciler = this instanceof BulkDependentResource
         ? new BulkDependentResourceReconciler<>((BulkDependentResource<R, P>) this)
         : new SingleDependentResourceReconciler<>(this);
+    this.name = name == null ? DependentResource.defaultNameFor(this.getClass()) : name;
   }
 
   /**
@@ -176,4 +184,13 @@ protected boolean isUpdatable() {
   public boolean isDeletable() {
     return deletable;
   }
+
+  @Override
+  public String name() {
+    return name;
+  }
+
+  public void setName(String name) {
+    this.name = name;
+  }
 }

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

@@ -31,6 +31,11 @@ public abstract class AbstractEventSourceHolderDependentResource<R, P extends Ha
   protected String eventSourceNameToUse;
 
   protected AbstractEventSourceHolderDependentResource(Class<R> resourceType) {
+    this(resourceType, null);
+  }
+
+  protected AbstractEventSourceHolderDependentResource(Class<R> resourceType, String name) {
+    super(name);
     this.resourceType = resourceType;
   }
 

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

@@ -43,7 +43,12 @@ public abstract class KubernetesDependentResource<R extends HasMetadata, P exten
 
   @SuppressWarnings("unchecked")
   public KubernetesDependentResource(Class<R> resourceType) {
-    super(resourceType);
+    this(resourceType, null);
+  }
+
+  @SuppressWarnings("unchecked")
+  public KubernetesDependentResource(Class<R> resourceType, String name) {
+    super(resourceType, name);
 
     usingCustomResourceUpdateMatcher = this instanceof ResourceUpdaterMatcher;
     updaterMatcher = usingCustomResourceUpdateMatcher

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

@@ -18,7 +18,7 @@
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
 
 @SuppressWarnings("rawtypes")
-public abstract class AbstractWorkflowExecutor<P extends HasMetadata> {
+abstract class AbstractWorkflowExecutor<P extends HasMetadata> {
 
   protected final Workflow<P> workflow;
   protected final P primary;
@@ -133,17 +133,16 @@ protected <R> void registerOrDeregisterEventSourceBasedOnActivation(
       boolean activationConditionMet,
       DependentResourceNode<R, P> dependentResourceNode) {
     if (dependentResourceNode.getActivationCondition().isPresent()) {
+      final var dr = dependentResourceNode.getDependentResource();
+      final var eventSourceRetriever = context.eventSourceRetriever();
       if (activationConditionMet) {
         var eventSource =
-            dependentResourceNode.getDependentResource().eventSource(context.eventSourceRetriever()
-                .eventSourceContextForDynamicRegistration());
+            dr.eventSource(eventSourceRetriever.eventSourceContextForDynamicRegistration());
         var es = eventSource.orElseThrow();
-        context.eventSourceRetriever()
-            .dynamicallyRegisterEventSource(dependentResourceNode.getName(), es);
+        eventSourceRetriever.dynamicallyRegisterEventSource(dr.name(), es);
 
       } else {
-        context.eventSourceRetriever()
-            .dynamicallyDeRegisterEventSource(dependentResourceNode.getName());
+        eventSourceRetriever.dynamicallyDeRegisterEventSource(dr.name());
       }
     }
   }

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

@@ -12,8 +12,10 @@
 import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceReferencer;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.NameSetter;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware;
 
+import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET;
 import static io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow.THROW_EXCEPTION_AUTOMATICALLY_DEFAULT;
 
 @SuppressWarnings("rawtypes")
@@ -77,13 +79,14 @@ public Workflow<P> resolve(KubernetesClient client,
       ControllerConfiguration<P> configuration) {
     final var alreadyResolved = new HashMap<String, DependentResourceNode>(orderedSpecs.size());
     for (DependentResourceSpec spec : orderedSpecs) {
-      final var node = new DependentResourceNode(spec.getName(),
+      final var dependentResource = resolve(spec, client, configuration);
+      final var node = new DependentResourceNode(
           spec.getReconcileCondition(),
           spec.getDeletePostCondition(),
           spec.getReadyCondition(),
           spec.getActivationCondition(),
-          resolve(spec, client, configuration));
-      alreadyResolved.put(node.getName(), node);
+          dependentResource);
+      alreadyResolved.put(dependentResource.name(), node);
       spec.getDependsOn()
           .forEach(depend -> node.addDependsOnRelation(alreadyResolved.get(depend)));
     }
@@ -104,6 +107,11 @@ private <R> DependentResource<R, P> resolve(DependentResourceSpec<R, P> spec,
         configuration.getConfigurationService().dependentResourceFactory()
             .createFrom(spec, configuration);
 
+    final var name = spec.getName();
+    if (name != null && !NO_VALUE_SET.equals(name) && dependentResource instanceof NameSetter) {
+      ((NameSetter) dependentResource).setName(name);
+    }
+
     if (dependentResource instanceof KubernetesClientAware) {
       ((KubernetesClientAware) dependentResource).setKubernetesClient(client);
     }

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

@@ -20,7 +20,7 @@
  * @param <P> primary resource
  */
 @SuppressWarnings("rawtypes")
-public class DefaultWorkflow<P extends HasMetadata> implements Workflow<P> {
+class DefaultWorkflow<P extends HasMetadata> implements Workflow<P> {
 
   private final Map<String, DependentResourceNode> dependentResourceNodes;
   private final Set<DependentResourceNode> topLevelResources;
@@ -77,9 +77,9 @@ private Map<String, DependentResourceNode> toMap(Set<DependentResourceNode> node
           bottomLevelResource.remove(dependsOn);
         }
       }
-      map.put(node.getName(), node);
+      map.put(node.getDependentResource().name(), node);
     }
-    if (topLevelResources.size() == 0) {
+    if (topLevelResources.isEmpty()) {
       throw new IllegalStateException(
           "No top-level dependent resources found. This might indicate a cyclic Set of DependentResourceNode has been provided.");
     }

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

@@ -8,29 +8,24 @@
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 
 @SuppressWarnings("rawtypes")
-public class DependentResourceNode<R, P extends HasMetadata> {
+class DependentResourceNode<R, P extends HasMetadata> {
 
   private final List<DependentResourceNode> dependsOn = new LinkedList<>();
   private final List<DependentResourceNode> parents = new LinkedList<>();
-  private final String name;
+
   private Condition<R, P> reconcilePrecondition;
   private Condition<R, P> deletePostcondition;
   private Condition<R, P> readyPostcondition;
   private Condition<R, P> activationCondition;
   private final DependentResource<R, P> dependentResource;
 
-  DependentResourceNode(String name, DependentResource<R, P> dependentResource) {
-    this(name, null, null, null, null, dependentResource);
-  }
-
   DependentResourceNode(DependentResource<R, P> dependentResource) {
-    this(getNameFor(dependentResource), null, null, null, null, dependentResource);
+    this(null, null, null, null, dependentResource);
   }
 
-  public DependentResourceNode(String name, Condition<R, P> reconcilePrecondition,
+  public DependentResourceNode(Condition<R, P> reconcilePrecondition,
       Condition<R, P> deletePostcondition, Condition<R, P> readyPostcondition,
       Condition<R, P> activationCondition, DependentResource<R, P> dependentResource) {
-    this.name = name;
     this.reconcilePrecondition = reconcilePrecondition;
     this.deletePostcondition = deletePostcondition;
     this.readyPostcondition = readyPostcondition;
@@ -55,16 +50,10 @@ public List<DependentResourceNode> getParents() {
     return parents;
   }
 
-  public String getName() {
-    return name;
-  }
-
-
   public Optional<Condition<R, P>> getReconcilePrecondition() {
     return Optional.ofNullable(reconcilePrecondition);
   }
 
-
   public Optional<Condition<R, P>> getDeletePostcondition() {
     return Optional.ofNullable(deletePostcondition);
   }
@@ -106,18 +95,12 @@ public boolean equals(Object o) {
       return false;
     }
     DependentResourceNode<?, ?> that = (DependentResourceNode<?, ?>) o;
-    return name.equals(that.name);
+    return this.getDependentResource().name().equals(that.getDependentResource().name());
   }
 
   @Override
   public int hashCode() {
-    return name.hashCode();
-  }
-
-  @SuppressWarnings("rawtypes")
-  static String getNameFor(DependentResource dependentResource) {
-    return DependentResource.defaultNameFor(dependentResource.getClass()) + "#"
-        + dependentResource.hashCode();
+    return this.getDependentResource().name().hashCode();
   }
 
   @Override

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

@@ -21,14 +21,9 @@ public class WorkflowBuilder<P extends HasMetadata> {
   private boolean isCleaner = false;
 
   public WorkflowBuilder<P> addDependentResource(DependentResource dependentResource) {
-    return addDependentResource(dependentResource, null);
-  }
-
-  public WorkflowBuilder<P> addDependentResource(DependentResource dependentResource, String name) {
-    currentNode = name == null ? new DependentResourceNode<>(dependentResource)
-        : new DependentResourceNode<>(name, dependentResource);
+    currentNode = new DependentResourceNode<>(dependentResource);
     isCleaner = isCleaner || dependentResource.isDeletable();
-    final var actualName = currentNode.getName();
+    final var actualName = dependentResource.name();
     dependentResourceNodes.put(actualName, currentNode);
     return this;
   }
@@ -70,8 +65,7 @@ public WorkflowBuilder<P> withActivationCondition(Condition activationCondition)
 
   DependentResourceNode getNodeByDependentResource(DependentResource<?, ?> dependentResource) {
     // first check by name
-    final var node =
-        dependentResourceNodes.get(DependentResourceNode.getNameFor(dependentResource));
+    final var node = dependentResourceNodes.get(dependentResource.name());
     if (node != null) {
       return node;
     } else {