feat: support cluster scoped and different namespace resources (#92)

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

Author: csviri

Diff for: README.md

@@ -169,6 +169,7 @@ spec:
                       protocol: TCP
                         
     - name: mutation_hook_config
+      clusterScoped: true
       # dependsOn relation means, that the resource will be reconciled only if all
       # the listed resources are already reconciled and ready (if ready post-condition is present).
       # This resource will be applied after the service and deployment are applied,

Diff for: docs/reference.md

@@ -21,6 +21,8 @@ It has several attributes:
 - **`name`** - is a mandatory unique (unique also regarding related resources) attribute.
   The resource is referenced by this name from other places, typically other resource templates and `JSCondition`.
   If it is used in a `JSCondition` the `name` must be a valid JavaScript variable name.
+- **`clusterScoped`** - a flag to indicate if the resource is cluster scoped. Default value is `false`. 
+  It is mandatory to set this for cluster scoped resources.
 - **`resource`** - is the desired state of the resource applied by default using Server Side Apply. The resource is templated using
   [qute templating engine](https://quarkus.io/guides/qute-reference), other resources can be referenced from the templates, see below.  
   There is a restriction, that the child resource is namespaced, and the namespace is always the same as the namespace of the `Glue`
@@ -51,6 +53,7 @@ See sample usage within `Glue` [here](https://github.com/csviri/kubernetes-glue-
 The following attributes can be defined for a related resource:
 
 - **`name`** - same as for child resource, unique identifier, used to reference the resource.
+- **`clusterScoped`** - if the related resource is cluster scoped. Default is `false`.
 - **`apiVersion`** - Kubernetes resource API Version of the resource
 - **`kind`** - Kubernetes kind property of the resource
 - **`resourceNames`** - list of string of the resource names within the same namespace as `Glue`.  
@@ -175,17 +178,6 @@ resources containing the same resource type.
 The templating and some of the Javascript condition is probably the most time-consuming and resource-intensive part which will 
 be continuously improved in the follow-up releases. 
 
-## Current limitations
-
-Note that none of the limitations are unsolvable, and will be continuously removed in the coming releases.
-
-1. Child resources and related resources are always namespace scoped resources, and in the same namespace as the
-   primary resource (`Glue` or the parent in the case of `GlueOperator`)
-
-2. ~~Related resource changes are not triggering the reconciliation.
-   Due to a bug in fabric8 client, after that is fixed, this is trivial to fix too:
-   https://github.com/fabric8io/kubernetes-client/issues/5729~~
-
 ## Samples
 
 1. [WebPage](https://github.com/csviri/kubernetes-glue-operator/tree/main/src/test/resources/sample/webpage) `GlueOperator`, serves a static website from the cluster.

Diff for: src/main/java/io/csviri/operator/glue/Utils.java

@@ -31,16 +31,13 @@ public static Map<String, GenericKubernetesResource> getActualResourcesByNameInW
     Map<String, GenericKubernetesResource> res = new HashMap<>();
     secondaryResources.forEach(sr -> {
       var dependentSpec = glue.getSpec().getChildResources().stream()
-          .filter(r -> Utils.getApiVersion(r).equals(sr.getApiVersion())
-              && Utils.getKind(r).equals(sr.getKind())
+          .filter(r ->
       // comparing the name from annotation since the resource name might be templated in spec
       // therefore "Utils.getName(relatedResourceSpec).equals(sr.getMetadata().getName())" would not
       // work
-              && r.getName()
-                  .equals(sr.getMetadata().getAnnotations().get(DEPENDENT_NAME_ANNOTATION_KEY))
-      // namespace not compared here, it should be done it is just not trivial, now it is limited to
-      // have one kind of resource in the workflow with the same resource name
-      ).findFirst();
+      r.getName()
+          .equals(sr.getMetadata().getAnnotations().get(DEPENDENT_NAME_ANNOTATION_KEY)))
+          .findFirst();
       dependentSpec.ifPresent(spec -> res.put(spec.getName(), sr));
     });
 
@@ -70,8 +67,9 @@ public static Map<String, GenericKubernetesResource> getRelatedResources(Glue gl
         (InformerEventSource<GenericKubernetesResource, Glue>) context
             .eventSourceRetriever()
             .getResourceEventSourceFor(GenericKubernetesResource.class, gvk.toString());
-    var namespace =
-        relatedResourceSpec.getNamespace() == null ? glue.getMetadata().getNamespace()
+
+    var namespace = relatedResourceSpec.isClusterScoped() ? null
+        : relatedResourceSpec.getNamespace() == null ? glue.getMetadata().getNamespace()
             : relatedResourceSpec.getNamespace();
 
     var res = new HashMap<String, GenericKubernetesResource>();

Diff for: src/main/java/io/csviri/operator/glue/customresource/glue/DependentResourceSpec.java

@@ -14,11 +14,13 @@ public class DependentResourceSpec {
   @Required
   private String name;
 
-  private String resourceTemplate;
+  private boolean clusterScoped = Boolean.FALSE;
 
   @PreserveUnknownFields
   private GenericKubernetesResource resource;
 
+  private String resourceTemplate;
+
   private List<String> dependsOn = new ArrayList<>();
 
   @PreserveUnknownFields
@@ -82,22 +84,32 @@ public DependentResourceSpec setResourceTemplate(String resourceTemplate) {
     return this;
   }
 
+  public boolean isClusterScoped() {
+    return clusterScoped;
+  }
+
+  public void setClusterScoped(boolean clusterScoped) {
+    this.clusterScoped = clusterScoped;
+  }
+
   @Override
   public boolean equals(Object o) {
     if (this == o)
       return true;
     if (o == null || getClass() != o.getClass())
       return false;
     DependentResourceSpec that = (DependentResourceSpec) o;
-    return Objects.equals(name, that.name)
+    return clusterScoped == that.clusterScoped && Objects.equals(name, that.name)
+        && Objects.equals(resource, that.resource)
         && Objects.equals(resourceTemplate, that.resourceTemplate)
-        && Objects.equals(resource, that.resource) && Objects.equals(dependsOn, that.dependsOn)
+        && Objects.equals(dependsOn, that.dependsOn)
         && Objects.equals(readyPostCondition, that.readyPostCondition)
         && Objects.equals(condition, that.condition);
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(name, resourceTemplate, resource, dependsOn, readyPostCondition, condition);
+    return Objects.hash(name, clusterScoped, resource, resourceTemplate, dependsOn,
+        readyPostCondition, condition);
   }
 }

Diff for: src/main/java/io/csviri/operator/glue/customresource/glue/RelatedResourceSpec.java

@@ -10,12 +10,13 @@ public class RelatedResourceSpec {
   // name for referencing the resource from templates and conditions (not name from object metadata)
   @Required
   private String name;
+  private String namespace;
 
   @Required
   private String apiVersion;
   @Required
   private String kind;
-  private String namespace;
+  private boolean clusterScoped = Boolean.FALSE;
   private List<String> resourceNames;
 
 
@@ -71,13 +72,22 @@ public boolean equals(Object o) {
     if (o == null || getClass() != o.getClass())
       return false;
     RelatedResourceSpec that = (RelatedResourceSpec) o;
-    return Objects.equals(name, that.name) && Objects.equals(apiVersion, that.apiVersion)
-        && Objects.equals(kind, that.kind) && Objects.equals(namespace, that.namespace)
+    return clusterScoped == that.clusterScoped && Objects.equals(name, that.name)
+        && Objects.equals(apiVersion, that.apiVersion) && Objects.equals(kind, that.kind)
+        && Objects.equals(namespace, that.namespace)
         && Objects.equals(resourceNames, that.resourceNames);
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(name, apiVersion, kind, namespace, resourceNames);
+    return Objects.hash(name, apiVersion, kind, clusterScoped, namespace, resourceNames);
+  }
+
+  public boolean isClusterScoped() {
+    return clusterScoped;
+  }
+
+  public void setClusterScoped(boolean clusterScoped) {
+    this.clusterScoped = clusterScoped;
   }
 }

Diff for: src/main/java/io/csviri/operator/glue/dependent/GCGenericDependentResource.java

@@ -7,11 +7,12 @@
 public class GCGenericDependentResource extends GenericDependentResource
     implements GarbageCollected<Glue> {
 
-  public GCGenericDependentResource(GenericKubernetesResource desired, String name) {
-    super(desired, name);
+  public GCGenericDependentResource(GenericKubernetesResource desired, String name,
+      boolean clusterScoped) {
+    super(desired, name, clusterScoped);
   }
 
-  public GCGenericDependentResource(String desiredTemplate, String name) {
-    super(desiredTemplate, name);
+  public GCGenericDependentResource(String desiredTemplate, String name, boolean clusterScoped) {
+    super(desiredTemplate, name, clusterScoped);
   }
 }

Diff for: src/main/java/io/csviri/operator/glue/dependent/GenericDependentResource.java

@@ -22,23 +22,27 @@ public class GenericDependentResource
   private final GenericKubernetesResource desired;
   private final String desiredTemplate;
   private final String name;
+  private final boolean clusterScoped;
 
   // optimize share between instances
   private final GenericTemplateHandler genericTemplateHandler = new GenericTemplateHandler();
 
-  public GenericDependentResource(GenericKubernetesResource desired, String name) {
+  public GenericDependentResource(GenericKubernetesResource desired, String name,
+      boolean clusterScoped) {
     super(new GroupVersionKind(desired.getApiVersion(), desired.getKind()));
     this.desired = desired;
     this.desiredTemplate = null;
     this.name = name;
+    this.clusterScoped = clusterScoped;
   }
 
-  public GenericDependentResource(String desiredTemplate, String name) {
+  public GenericDependentResource(String desiredTemplate, String name, boolean clusterScoped) {
     super(new GroupVersionKind(Utils.getApiVersionFromTemplate(desiredTemplate),
         Utils.getKindFromTemplate(desiredTemplate)));
     this.name = name;
     this.desiredTemplate = desiredTemplate;
     this.desired = null;
+    this.clusterScoped = clusterScoped;
   }
 
   @Override
@@ -53,8 +57,7 @@ protected GenericKubernetesResource desired(Glue primary,
     resultDesired.getMetadata().getAnnotations()
         .put(GlueReconciler.DEPENDENT_NAME_ANNOTATION_KEY, name);
 
-    // set only for cluster scoped when detection is ready
-    if (resultDesired.getMetadata().getNamespace() == null) {
+    if (resultDesired.getMetadata().getNamespace() == null && !clusterScoped) {
       resultDesired.getMetadata().setNamespace(primary.getMetadata().getNamespace());
     }
     return resultDesired;

Diff for: src/main/java/io/csviri/operator/glue/reconciler/glue/GlueReconciler.java

@@ -202,14 +202,19 @@ private void createAndAddDependentToWorkflow(Glue primary, Context<Glue> context
 
   private static GenericDependentResource createDependentResource(DependentResourceSpec spec,
       boolean leafDependent, Boolean resourceInSameNamespaceAsPrimary) {
-    if (leafDependent && resourceInSameNamespaceAsPrimary) {
+
+    if (leafDependent && resourceInSameNamespaceAsPrimary && !spec.isClusterScoped()) {
       return spec.getResourceTemplate() != null
-          ? new GCGenericDependentResource(spec.getResourceTemplate(), spec.getName())
-          : new GCGenericDependentResource(spec.getResource(), spec.getName());
+          ? new GCGenericDependentResource(spec.getResourceTemplate(), spec.getName(),
+              spec.isClusterScoped())
+          : new GCGenericDependentResource(spec.getResource(), spec.getName(),
+              spec.isClusterScoped());
     } else {
       return spec.getResourceTemplate() != null
-          ? new GenericDependentResource(spec.getResourceTemplate(), spec.getName())
-          : new GenericDependentResource(spec.getResource(), spec.getName());
+          ? new GenericDependentResource(spec.getResourceTemplate(), spec.getName(),
+              spec.isClusterScoped())
+          : new GenericDependentResource(spec.getResource(), spec.getName(),
+              spec.isClusterScoped());
     }
   }
 

Diff for: src/test/java/io/csviri/operator/glue/GlueTest.java

@@ -12,7 +12,10 @@
 import io.csviri.operator.glue.customresource.glue.Glue;
 import io.csviri.operator.glue.reconciler.ValidationAndErrorHandler;
 import io.fabric8.kubernetes.api.model.ConfigMap;
+import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
 import io.fabric8.kubernetes.api.model.Secret;
+import io.fabric8.kubernetes.client.dsl.NonDeletingOperation;
 import io.quarkus.test.junit.QuarkusTest;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -237,35 +240,88 @@ void nonUniqueNameResultsInErrorMessageOnStatus() {
     });
   }
 
+  @Test
+  void childInDifferentNamespace() {
+    Glue glue = create(TestUtils.loadGlue("/glue/ResourceInDifferentNamespace.yaml"));
+
+    await().untilAsserted(() -> {
+      var cmDifferentNS = client.configMaps().inNamespace("default")
+          .withName("configmap1");
+      var cm2 = get(ConfigMap.class, "configmap2");
+
+      assertThat(cmDifferentNS).isNotNull();
+      assertThat(cm2).isNotNull();
+    });
+
+    delete(glue);
+    await().timeout(TestUtils.GC_WAIT_TIMEOUT).untilAsserted(() -> {
+      var cmDifferentNS = client.configMaps().inNamespace("default")
+          .withName("configmap1").get();
+      var cm2 = get(ConfigMap.class, "configmap2");
+
+      assertThat(cmDifferentNS).isNull();
+      assertThat(cm2).isNull();
+    });
+  }
+
+  @Test
+  void clusterScopedChild() {
+    var glue = create(TestUtils.loadGlue("/glue/ClusterScopedChild.yaml"));
+    await().untilAsserted(() -> {
+      var ns = client.namespaces()
+          .withName("testnamespace");
+      assertThat(ns).isNotNull();
+    });
+
+    delete(glue);
+    await().timeout(TestUtils.GC_WAIT_TIMEOUT).untilAsserted(() -> {
+      var ns = client.namespaces()
+          .withName("testnamespace").get();
+      assertThat(ns).isNull();
+    });
+  }
+
+  @Test
+  void relatedResourceFromDifferentNamespace() {
+    client.resource(new ConfigMapBuilder()
+        .withMetadata(new ObjectMetaBuilder()
+            .withName("related-configmap")
+            .withNamespace("default")
+            .build())
+        .withData(Map.of("key1", "value1"))
+        .build()).createOr(NonDeletingOperation::update);
 
-  //
-  // @Disabled("Not supported in current version")
-  // @Test
-  // void childInDifferentNamespaceAsPrimary() {
-  // Glue w = extension
-  // .create(TestUtils.loadResoureFlow("/glue/ResourceInDifferentNamespace.yaml"));
-  //
-  // await().untilAsserted(() -> {
-  // var cmDifferentNS = extension.getKubernetesClient().configMaps().inNamespace("default")
-  // .withName("configmap1");
-  // var cm2 = extension.get(ConfigMap.class, "configmap2");
-  //
-  // assertThat(cmDifferentNS).isNotNull();
-  // assertThat(cm2).isNotNull();
-  // });
-  //
-  // extension.delete(w);
-  //
-  // await().untilAsserted(() -> {
-  // var cmDifferentNS = extension.getKubernetesClient().configMaps().inNamespace("default")
-  // .withName("configmap1");
-  // var cm2 = extension.get(ConfigMap.class, "configmap2");
-  //
-  // assertThat(cmDifferentNS).isNull();
-  // assertThat(cm2).isNull();
-  // });
-  //
-  // }
+    var glue = create(TestUtils.loadGlue("/glue/RelatedResourceFromDifferentNamespace.yaml"));
+
+    await().untilAsserted(() -> {
+      var cm = get(ConfigMap.class, "configmap1");
+      assertThat(cm).isNotNull();
+      assertThat(cm.getData()).containsEntry("copy-key", "value1");
+    });
+
+    delete(glue);
+    await().timeout(TestUtils.GC_WAIT_TIMEOUT).untilAsserted(() -> {
+      var cm = get(ConfigMap.class, "configmap1");
+      assertThat(cm).isNull();
+    });
+  }
+
+  @Test
+  void clusterScopedRelatedResource() {
+    var glue = create(TestUtils.loadGlue("/glue/ClusterScopedRelatedResource.yaml"));
+
+    await().untilAsserted(() -> {
+      var cm = get(ConfigMap.class, "configmap1");
+      assertThat(cm).isNotNull();
+      assertThat(cm.getData()).containsEntry("phase", "Active");
+    });
+
+    delete(glue);
+    await().timeout(TestUtils.GC_WAIT_TIMEOUT).untilAsserted(() -> {
+      var cm = get(ConfigMap.class, "configmap1");
+      assertThat(cm).isNull();
+    });
+  }
 
   private List<Glue> testWorkflowList(int num) {
     List<Glue> res = new ArrayList<>();
@@ -278,4 +334,6 @@ private List<Glue> testWorkflowList(int num) {
     return res;
   }
 
+
+
 }