cross-ns CC: Add classNamespace to topology

Signed-off-by: Danil-Grigorev <[email protected]>

api/v1beta1/cluster_types.go

@@ -17,6 +17,7 @@ limitations under the License.
 package v1beta1
 
 import (
+	"cmp"
 	"fmt"
 	"net"
 	"strings"
@@ -517,6 +518,15 @@ type Topology struct {
 	// The name of the ClusterClass object to create the topology.
 	Class string `json:"class"`
 
+	// The namespace of the ClusterClass object to create the topology. Empty namespace assumes the namespace of the cluster object.
+	// Class namespace changes are not supported by the rebase procedure, as different CC namespace uses namespace-local templates.
+	// Cluster templates namespace modification is not allowed.
+	// +optional
+	// +kubebuilder:validation:MinLength=1
+	// +kubebuilder:validation:MaxLength=253
+	// +kubebuilder:validation:Pattern="^[a-z0-9](?:[-a-z0-9]*[a-z0-9])?(?:\\.[a-z0-9](?:[-a-z0-9]*[a-z0-9])?)*$"
+	ClassNamespace string `json:"classNamespace,omitempty"`
+
 	// The Kubernetes version of the cluster.
 	Version string `json:"version"`
 
@@ -1045,7 +1055,9 @@ func (c *Cluster) GetClassKey() types.NamespacedName {
 	if c.Spec.Topology == nil {
 		return types.NamespacedName{}
 	}
-	return types.NamespacedName{Namespace: c.GetNamespace(), Name: c.Spec.Topology.Class}
+
+	namespace := cmp.Or(c.Spec.Topology.ClassNamespace, c.Namespace)
+	return types.NamespacedName{Namespace: namespace, Name: c.Spec.Topology.Class}
 }
 
 // GetConditions returns the set of conditions for this object.

api/v1beta1/index/cluster.go

@@ -30,6 +30,8 @@ import (
 const (
 	// ClusterClassNameField is used by the Cluster controller to index Clusters by ClusterClass name.
 	ClusterClassNameField = "spec.topology.class"
+	// ClusterClassNamespaceField is used by the Cluster controller to index Clusters by ClusterClass namespace.
+	ClusterClassNamespaceField = "spec.topology.classNamespace"
 )
 
 // ByClusterClassName adds the cluster class name  index to the
@@ -44,6 +46,18 @@ func ByClusterClassName(ctx context.Context, mgr ctrl.Manager) error {
 	return nil
 }
 
+// ByClusterClassNamespace adds the cluster class namespace index to the
+// managers cache.
+func ByClusterClassNamespace(ctx context.Context, mgr ctrl.Manager) error {
+	if err := mgr.GetCache().IndexField(ctx, &clusterv1.Cluster{},
+		ClusterClassNamespaceField,
+		ClusterByClusterClassNamespace,
+	); err != nil {
+		return errors.Wrap(err, "error setting index field")
+	}
+	return nil
+}
+
 // ClusterByClusterClassClassName contains the logic to index Clusters by ClusterClass name.
 func ClusterByClusterClassClassName(o client.Object) []string {
 	cluster, ok := o.(*clusterv1.Cluster)
@@ -55,3 +69,15 @@ func ClusterByClusterClassClassName(o client.Object) []string {
 	}
 	return nil
 }
+
+// ClusterByClusterClassNamespace contains the logic to index Clusters by ClusterClass namespace.
+func ClusterByClusterClassNamespace(o client.Object) []string {
+	cluster, ok := o.(*clusterv1.Cluster)
+	if !ok {
+		panic(fmt.Sprintf("Expected Cluster but got a %T", o))
+	}
+	if cluster.Spec.Topology != nil {
+		return []string{cluster.GetClassKey().Namespace}
+	}
+	return nil
+}

api/v1beta1/index/index.go

@@ -39,6 +39,10 @@ func AddDefaultIndexes(ctx context.Context, mgr ctrl.Manager) error {
 		if err := ByClusterClassName(ctx, mgr); err != nil {
 			return err
 		}
+
+		if err := ByClusterClassNamespace(ctx, mgr); err != nil {
+			return err
+		}
 	}
 
 	if feature.Gates.Enabled(feature.MachinePool) {

docs/book/src/tasks/experimental-features/cluster-class/write-clusterclass.md

@@ -15,7 +15,7 @@ flexible enough to be used in as many Clusters as possible by supporting variant
     * [Defining a custom naming strategy for MachineDeployment objects](#defining-a-custom-naming-strategy-for-machinedeployment-objects)
     * [Defining a custom naming strategy for MachinePool objects](#defining-a-custom-naming-strategy-for-machinepool-objects)
 * [Advanced features of ClusterClass with patches](#advanced-features-of-clusterclass-with-patches)
-    * [MachineDeployment variable overrides](#machinedeployment-variable-overrides)
+    * [MachineDeployment variable overrides](#machinedeployment-and-machinepool-variable-overrides)
     * [Builtin variables](#builtin-variables)
     * [Complex variable types](#complex-variable-types)
     * [Using variable values in JSON patches](#using-variable-values-in-json-patches)
@@ -438,11 +438,94 @@ spec:
         template: "{{ .cluster.name }}-{{ .machinePool.topologyName }}-{{ .random }}"
 ```
 
+### Defining a custom namespace for ClusterClass object
+
+As a user, I may need to create a `Cluster` from a `ClusterClass` object that exists only in a different namespace. To uniquely identify the `ClusterClass`, a `NamespacedName` ref is constructed from combination of:
+* `cluster.spec.topology.classNamespace` - namespace of the `ClusterClass` object.
+* `cluster.spec.topology.class` - name of the `ClusterClass` object.
+
+Example of the `Cluster` object with the `name/namespace` reference:
+
+```yaml
+apiVersion: cluster.x-k8s.io/v1beta1
+kind: Cluster
+metadata:
+  name: my-docker-cluster
+  namespace: default
+spec:
+  topology:
+    class: docker-clusterclass-v0.1.0
+    classNamespace: default
+    version: v1.22.4
+    controlPlane:
+      replicas: 3
+    workers:
+      machineDeployments:
+      - class: default-worker
+        name: md-0
+        replicas: 4
+        failureDomain: region
+```
+
+<aside class="note warning">
+
+<h1>Cluster rebase across namespaces</h1>
+
+Changing `classNamespace` is not supported in rebase procedure, while changing `class` reference to a different `ClusterClass` from the same namespace performs regular `Cluster` rebase procedure.
+
+</aside>
+
+#### Securing cross-namespace reference to the ClusterClass
+
+It is often desirable to restrict free cross-namespace `ClusterClass` access for the `Cluster` object. This can be implemented by defining a [`ValidatingAdmissionPolicy`](https://kubernetes.io/docs/reference/access-authn-authz/validating-admission-policy/#what-is-validating-admission-policy) on the `Cluster` object.
+
+An example of such policy may be:
+
+```yaml
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingAdmissionPolicy
+metadata:
+  name: "cluster-class-ref.cluster.x-k8s.io"
+spec:
+  failurePolicy: Fail
+  paramKind:
+    apiVersion: v1
+    kind: Secret
+  matchConstraints:
+    resourceRules:
+    - apiGroups:   ["cluster.x-k8s.io"]
+      apiVersions: ["v1beta1"]
+      operations:  ["CREATE", "UPDATE"]
+      resources:   ["clusters"]
+  validations:
+    - expression: "!has(object.spec.topology.classNamespace) || object.spec.topology.classNamespace in params.data"
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingAdmissionPolicyBinding
+metadata:
+  name: "cluster-class-ref-binding.cluster.x-k8s.io"
+spec:
+  policyName: "cluster-class-ref.cluster.x-k8s.io"
+  validationActions: [Deny]
+  paramRef:
+    name: "allowed-namespaces.cluster-class-ref.cluster.x-k8s.io"
+    namespace: "default"
+    parameterNotFoundAction: Deny
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: "allowed-namespaces.cluster-class-ref.cluster.x-k8s.io"
+  namespace: "default"
+data:
+  default: ""
+```
+
 ## Advanced features of ClusterClass with patches
 
 This section will explain more advanced features of ClusterClass patches.
 
-### MachineDeployment & MachinePool variable overrides
+### MachineDeployment and MachinePool variable overrides
 
 If you want to use many variations of MachineDeployments in Clusters, you can either define
 a MachineDeployment class for every variation or you can define patches and variables to

internal/apis/core/v1alpha4/conversion.go

@@ -42,6 +42,7 @@ func (src *Cluster) ConvertTo(dstRaw conversion.Hub) error {
 		if dst.Spec.Topology == nil {
 			dst.Spec.Topology = &clusterv1.Topology{}
 		}
+		dst.Spec.Topology.ClassNamespace = restored.Spec.Topology.ClassNamespace
 		dst.Spec.Topology.Variables = restored.Spec.Topology.Variables
 		dst.Spec.Topology.ControlPlane.Variables = restored.Spec.Topology.ControlPlane.Variables
 

internal/controllers/topology/cluster/cluster_controller.go

@@ -492,8 +492,10 @@ func (r *Reconciler) clusterClassToCluster(ctx context.Context, o client.Object)
 	if err := r.Client.List(
 		ctx,
 		clusterList,
-		client.MatchingFields{index.ClusterClassNameField: clusterClass.Name},
-		client.InNamespace(clusterClass.Namespace),
+		client.MatchingFields{
+			index.ClusterClassNameField:      clusterClass.Name,
+			index.ClusterClassNamespaceField: clusterClass.Namespace,
+		},
 	); err != nil {
 		return nil
 	}

internal/controllers/topology/cluster/cluster_controller_test.go

@@ -52,6 +52,7 @@ import (
 var (
 	clusterName1                           = "cluster1"
 	clusterName2                           = "cluster2"
+	clusterName3                           = "cluster3"
 	clusterClassName1                      = "class1"
 	clusterClassName2                      = "class2"
 	infrastructureMachineTemplateName1     = "inframachinetemplate1"
@@ -854,6 +855,19 @@ func setupTestEnvForIntegrationTests(ns *corev1.Namespace) (func() error, error)
 				Build()).
 		Build()
 
+	// Cross ns referencing cluster
+	cluster3 := builder.Cluster(ns.Name, clusterName3).
+		WithTopology(
+			builder.ClusterTopology().
+				WithClass(clusterClass.Name).
+				WithClassNamespace("other").
+				WithMachineDeployment(machineDeploymentTopology2).
+				WithMachinePool(machinePoolTopology2).
+				WithVersion("1.21.0").
+				WithControlPlaneReplicas(1).
+				Build()).
+		Build()
+
 	// Setup kubeconfig secrets for the clusters, so the ClusterCacheTracker works.
 	cluster1Secret := kubeconfig.GenerateSecret(cluster1, kubeconfig.FromEnvTestConfig(env.Config, cluster1))
 	cluster2Secret := kubeconfig.GenerateSecret(cluster2, kubeconfig.FromEnvTestConfig(env.Config, cluster2))
@@ -876,6 +890,7 @@ func setupTestEnvForIntegrationTests(ns *corev1.Namespace) (func() error, error)
 		clusterClassForRebase,
 		cluster1,
 		cluster2,
+		cluster3,
 		cluster1Secret,
 		cluster2Secret,
 	}
@@ -1577,3 +1592,56 @@ func TestReconciler_ValidateCluster(t *testing.T) {
 		})
 	}
 }
+
+func TestClusterClassToCluster(t *testing.T) {
+	utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.ClusterTopology, true)
+	g := NewWithT(t)
+
+	ns, err := env.CreateNamespace(ctx, "cluster-reconcile-namespace")
+	g.Expect(err).ToNot(HaveOccurred())
+
+	// Create the objects needed for the integration test:
+	cleanup, err := setupTestEnvForIntegrationTests(ns)
+	g.Expect(err).ToNot(HaveOccurred())
+
+	// Defer a cleanup function that deletes each of the objects created during setupTestEnvForIntegrationTests.
+	defer func() {
+		g.Expect(cleanup()).To(Succeed())
+	}()
+
+	tests := []struct {
+		name         string
+		clusterClass *clusterv1.ClusterClass
+		expected     []reconcile.Request
+	}{
+		{
+			name:         "ClusterClass change should request reconcile for the referenced class",
+			clusterClass: builder.ClusterClass(ns.Name, clusterClassName1).Build(),
+			expected: []reconcile.Request{
+				{NamespacedName: client.ObjectKeyFromObject(builder.Cluster(ns.Name, clusterName1).Build())},
+				{NamespacedName: client.ObjectKeyFromObject(builder.Cluster(ns.Name, clusterName2).Build())},
+			},
+		},
+		{
+			name:         "ClusterClass with no matching name and namespace should not trigger reconcile",
+			clusterClass: builder.ClusterClass("other", clusterClassName2).Build(),
+			expected:     []reconcile.Request{},
+		},
+		{
+			name:         "Different ClusterClass with matching name and namespace should trigger reconcile",
+			clusterClass: builder.ClusterClass("other", clusterClassName1).Build(),
+			expected: []reconcile.Request{
+				{NamespacedName: client.ObjectKeyFromObject(builder.Cluster(ns.Name, clusterName3).Build())},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(*testing.T) {
+			r := &Reconciler{Client: env.GetClient()}
+
+			requests := r.clusterClassToCluster(ctx, tt.clusterClass)
+			g.Expect(requests).To(ConsistOf(tt.expected))
+		})
+	}
+}

internal/controllers/topology/cluster/suite_test.go

@@ -60,11 +60,14 @@ func TestMain(m *testing.M) {
 		if err := index.AddDefaultIndexes(ctx, mgr); err != nil {
 			panic(fmt.Sprintf("unable to setup index: %v", err))
 		}
-		// Set up the ClusterClassName index explicitly here. This index is ordinarily created in
+		// Set up the ClusterClassName and ClusterClassNamespace index explicitly here. This index is ordinarily created in
 		// index.AddDefaultIndexes. That doesn't happen here because the ClusterClass feature flag is not set.
 		if err := index.ByClusterClassName(ctx, mgr); err != nil {
 			panic(fmt.Sprintf("unable to setup index: %v", err))
 		}
+		if err := index.ByClusterClassNamespace(ctx, mgr); err != nil {
+			panic(fmt.Sprintf("unable to setup index: %v", err))
+		}
 	}
 	setupReconcilers := func(ctx context.Context, mgr ctrl.Manager) {
 		clusterCache, err := clustercache.SetupWithManager(ctx, mgr, clustercache.Options{

internal/webhooks/clusterclass.go

@@ -379,12 +379,15 @@ func (webhook *ClusterClass) classNamesFromMPWorkerClass(w clusterv1.WorkersClas
 func (webhook *ClusterClass) getClustersUsingClusterClass(ctx context.Context, clusterClass *clusterv1.ClusterClass) ([]clusterv1.Cluster, error) {
 	clusters := &clusterv1.ClusterList{}
 	err := webhook.Client.List(ctx, clusters,
-		client.MatchingFields{index.ClusterClassNameField: clusterClass.Name},
-		client.InNamespace(clusterClass.Namespace),
+		client.MatchingFields{
+			index.ClusterClassNameField:      clusterClass.Name,
+			index.ClusterClassNamespaceField: clusterClass.Namespace,
+		},
 	)
 	if err != nil {
 		return nil, err
 	}
+
 	return clusters.Items, nil
 }