fix: wire Updater/Builder in DependentResource, better owner handling

Author: metacosm

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

@@ -7,48 +7,44 @@
 import io.javaoperatorsdk.operator.processing.event.source.ResourceCache;
 import io.javaoperatorsdk.operator.processing.event.source.ResourceEventSource;
 
-public class DependentResource<R extends HasMetadata, P extends HasMetadata> {
+public class DependentResource<R extends HasMetadata, P extends HasMetadata>
+    implements Builder<R, P>, Updater<R, P> {
 
   private final DefaultDependentResourceConfiguration<R, P> configuration;
-  private final Builder<R> builder;
-  private final Updater<R> updater;
+  private final Builder<R, P> builder;
+  private final Updater<R, P> updater;
   private final Fetcher<R> fetcher;
   private ResourceEventSource<R, P> source;
 
   public DependentResource(DefaultDependentResourceConfiguration<R, P> configuration,
-      Builder<R> builder, Updater<R> updater, Fetcher<R> fetcher) {
+      Builder<R, P> builder, Updater<R, P> updater, Fetcher<R> fetcher) {
     this.configuration = configuration;
     this.builder = builder;
     this.updater = updater;
     this.fetcher = fetcher;
   }
 
-  public R build() {
-    if (getConfiguration().creatable()) {
-      throw new IllegalStateException(
-          "Should be implemented if DependentResource is configured as creatable");
-    }
-    throw new IllegalStateException("Should not be called if DependentResource is not creatable");
+  @Override
+  public R buildFor(P primary) {
+    return builder.buildFor(primary);
   }
 
   public ResourceCache<R> getCache() {
     return source.getResourceCache();
   }
 
   public R fetchFor(HasMetadata owner) {
-    return getCache().get(ResourceID.fromResource(owner)).orElse(null);
+    return fetcher != null ? fetcher.fetchFor(owner, getCache())
+        : getCache().get(ResourceID.fromResource(owner)).orElse(null);
   }
 
   public DependentResourceConfiguration<R, P> getConfiguration() {
     return configuration;
   }
 
-  public R update(R fetched) {
-    if (getConfiguration().updatable()) {
-      throw new IllegalStateException(
-          "Should be implemented if DependentResource is configured as updatable");
-    }
-    throw new IllegalStateException("Should not be called if DependentResource is not updatable");
+  @Override
+  public R update(R fetched, P primary) {
+    return updater.update(fetched, primary);
   }
 
   public void setSource(ResourceEventSource<R, P> source) {

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

@@ -115,22 +115,30 @@ public UpdateControl<R> reconcile(R resource, Context<R> context) {
       var dependentResource = dependent.fetchFor(resource);
       if (conf.creatable() && dependentResource == null) {
         // we need to create the dependent
-        dependentResource = dependent.build();
-
-        if (conf.owned()) {
-          final var metadata = resource.getMetadata();
-          final var updatedMetadata = new ObjectMetaBuilder(dependentResource.getMetadata())
-              .addNewOwnerReference()
-              .withUid(metadata.getUid())
-              .withApiVersion(resource.getApiVersion())
-              .withName(metadata.getName())
-              .withKind(resource.getKind())
-              .endOwnerReference().build();
-          dependentResource.setMetadata(updatedMetadata);
-        }
-
+        dependentResource = dependent.buildFor(resource);
+        log.info("Creating '{}' {} dependent in namespace {} for '{}' {}",
+            dependentResource.getMetadata().getName(), conf.getResourceTypeName(),
+            dependentResource.getMetadata().getNamespace(), resource.getMetadata().getName(),
+            configuration.getResourceTypeName());
       } else if (conf.updatable()) {
-        dependentResource = dependent.update(dependentResource);
+        dependentResource = dependent.update(dependentResource, resource);
+        log.info("Updating '{}' {} dependent in namespace {} for '{}' {}",
+            dependentResource.getMetadata().getName(), conf.getResourceTypeName(),
+            dependentResource.getMetadata().getNamespace(), resource.getMetadata().getName(),
+            configuration.getResourceTypeName());
+      }
+
+      // create/add the owner reference if needed
+      if (conf.owned()) {
+        final var metadata = resource.getMetadata();
+        final var updatedMetadata = new ObjectMetaBuilder(dependentResource.getMetadata())
+            .addNewOwnerReference()
+            .withUid(metadata.getUid())
+            .withApiVersion(resource.getApiVersion())
+            .withName(metadata.getName())
+            .withKind(resource.getKind())
+            .endOwnerReference().build();
+        dependentResource.setMetadata(updatedMetadata);
       }
 
       // send the changes to the cluster