clusterctl: Support Namespace resource in clusterctl alpha topology plan

[apricote]

User Story

As an operator I would like to produce a topology plan using clusterctl alpha topology plan for resource files that include non-namespaced resources, such as Namespace.

Detailed Description

We currently create a combined manifest for the Namespace+Cluster+ClusterClass resources using kustomize. We then use the output from kustomize build . > manifests.yaml to create a plan using clusterctl alpha topology plan -f manifests.yaml, but this fails because the Namespace resource does not have metadata.namespace set (because it is not a namespaced resource):

Error: input failed validation: all the objects in the input should belong to the same namespace

Instead I would like clusterctl alpha topology plan to ignore non-namespaced resources when verifying if all resources belong to the same namespace. At least I would like it to ignore Namespace resources.

Anything else you would like to add:

Ignoring Namespace resources is easily implemented, but we probably want a solution that is more robust:

diff --git a/cmd/clusterctl/client/cluster/topology.go b/cmd/clusterctl/client/cluster/topology.go
index 40ca1fa1d..54f26fa41 100644
--- a/cmd/clusterctl/client/cluster/topology.go
+++ b/cmd/clusterctl/client/cluster/topology.go
@@ -681,6 +681,10 @@ func clusterClassUsesTemplate(cc *clusterv1.ClusterClass, templateRef *corev1.Ob
 func uniqueNamespaces(objs []*unstructured.Unstructured) []string {
        ns := sets.NewString()
        for _, obj := range objs {
+               if obj.GetKind() == "Namespace" {
+                       continue
+               }
+
                // Note: treat empty namespace (namespace not set) as a unique namespace.
                // If some have a namespace set and some do not. It is safer to consider them as
                // objects from different namespaces.

/kind feature
/area clusterctl
/area topology

[sbueringer]

An alternative could be to check if the name of the Namespace resource matches the namespace of the namespaced resources.

[fabriziopandini]

I'm not against making alpha topology plan tolerant of the Namespace object, however, I have some concerns about the approach.

Having a ClusterClass and the Cluster in a single template IMO is a bad practice because it promotes the idea of having one ClusterClass for each Cluster, which is an anti-pattern; the rationale behind this is explained in the doc.

This also contrasts with our stack being oriented a promoting best practices. e.g

• clusterctl repositories expect the ClusterClass to be in a separated file, so it can be deployed only once vs being duplicated for every cluster
• alpha topology plan accepts more files, thus allowing to have one file for the Cluster and one file for the ClusterClass (reusable for other Clusters)

Last but not least I'm also -1 to extend this to support non-namespaced resources different from the Cluster for the sake of keeping the command as scoped as possible.

[fabriziopandini]

/milestone v1.2

[apricote]

Having a ClusterClass and the Cluster in a single template IMO is a bad practice because it promotes the idea of having one ClusterClass for each Cluster, which is an anti-pattern; the rationale behind this is explained in the doc.

We currently deploy the clusters into seperate namespaces to isolate the resources and to enable fine-grained permissions. Once #5673 is implemented, I will be happy to move away from 1-1 Relationship of ClusterClasses and Clusters.

Last but not least I'm also -1 to extend this to support non-namespaced resources different from the Cluster for the sake of keeping the command as scoped as possible.

That certainly makes sense. It just felt weird to add a "special case" for the Namespace resource.

If everyone is fine with this, I would like to implement the approach @sbueringer suggested and add the metadata.name of kind: Namespace to the ns Set.

[sbueringer]

From the perspective that we enable deploying the corresponding Namespace as part of the cluster-template that sounds fine to me.

Assuming that Fabrizo meant "different from the Namespace" here

Last but not least I'm also -1 to extend this to support non-namespaced resources different from the Cluster for the sake of keeping the command as scoped as possible.

+1 from my side

[apricote]

I updated the title to match the new scope and will begin implementing this now for Namespace resources..

/assign