Add classNamespace to topology (#11352)

- Add documentation on securing cross-namespace access for CC
- Add ByClusterClassRef index
- Support cross-ns CC rebase

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

Author: Danil-Grigorev

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"`
 
+	// classNamespace is the namespace of the ClusterClass object to create the topology.
+	// If the namespace is empty or not set, it is defaulted to the namespace of the cluster object.
+	// Value must follow the DNS1123Subdomain syntax.
+	// +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,8 +30,44 @@ import (
 const (
 	// ClusterClassNameField is used by the Cluster controller to index Clusters by ClusterClass name.
 	ClusterClassNameField = "spec.topology.class"
+
+	// ClusterClassRefPath is used by the Cluster controller to index Clusters by ClusterClass name and namespace.
+	ClusterClassRefPath = "spec.topology.classRef"
+
+	// clusterClassRefFmt is used to correctly format class ref index key.
+	clusterClassRefFmt = "%s/%s"
 )
 
+// ByClusterClassRef adds the cluster class name  index to the
+// managers cache.
+func ByClusterClassRef(ctx context.Context, mgr ctrl.Manager) error {
+	if err := mgr.GetCache().IndexField(ctx, &clusterv1.Cluster{},
+		ClusterClassRefPath,
+		ClusterByClusterClassRef,
+	); err != nil {
+		return errors.Wrap(err, "error setting index field")
+	}
+	return nil
+}
+
+// ClusterByClusterClassRef contains the logic to index Clusters by ClusterClass name and namespace.
+func ClusterByClusterClassRef(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 {
+		key := cluster.GetClassKey()
+		return []string{fmt.Sprintf(clusterClassRefFmt, key.Namespace, key.Name)}
+	}
+	return nil
+}
+
+// ClusterClassRef returns ClusterClass index key to be used for search.
+func ClusterClassRef(cc *clusterv1.ClusterClass) string {
+	return fmt.Sprintf(clusterClassRefFmt, cc.GetNamespace(), cc.GetName())
+}
+
 // ByClusterClassName adds the cluster class name  index to the
 // managers cache.
 func ByClusterClassName(ctx context.Context, mgr ctrl.Manager) error {

api/v1beta1/index/cluster_test.go

@@ -20,12 +20,13 @@ import (
 	"testing"
 
 	. "github.com/onsi/gomega"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 )
 
-func TestClusterByClassName(t *testing.T) {
+func TestClusterByClusterClassRef(t *testing.T) {
 	testCases := []struct {
 		name     string
 		object   client.Object
@@ -39,20 +40,40 @@ func TestClusterByClassName(t *testing.T) {
 		{
 			name: "when cluster has a valid Topology",
 			object: &clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "cluster",
+					Namespace: "default",
+				},
 				Spec: clusterv1.ClusterSpec{
 					Topology: &clusterv1.Topology{
 						Class: "class1",
 					},
 				},
 			},
-			expected: []string{"class1"},
+			expected: []string{"default/class1"},
+		},
+		{
+			name: "when cluster has a valid Topology with namespace specified",
+			object: &clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "cluster",
+					Namespace: "default",
+				},
+				Spec: clusterv1.ClusterSpec{
+					Topology: &clusterv1.Topology{
+						Class:          "class1",
+						ClassNamespace: "other",
+					},
+				},
+			},
+			expected: []string{"other/class1"},
 		},
 	}
 
 	for _, test := range testCases {
 		t.Run(test.name, func(t *testing.T) {
 			g := NewWithT(t)
-			got := ClusterByClusterClassClassName(test.object)
+			got := ClusterByClusterClassRef(test.object)
 			g.Expect(got).To(Equal(test.expected))
 		})
 	}

api/v1beta1/index/index.go

@@ -36,7 +36,7 @@ func AddDefaultIndexes(ctx context.Context, mgr ctrl.Manager) error {
 	}
 
 	if feature.Gates.Enabled(feature.ClusterTopology) {
-		if err := ByClusterClassName(ctx, mgr); err != nil {
+		if err := ByClusterClassRef(ctx, mgr); err != nil {
 			return err
 		}
 	}

api/v1beta1/zz_generated.openapi.go

@@ -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,87 @@ 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
+```
+
+
+#### 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

config/crd/bases/cluster.x-k8s.io_clusters.yaml

@@ -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
 

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

@@ -492,8 +492,9 @@ 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.ClusterClassRefPath: index.ClusterClassRef(clusterClass),
+		},
 	); err != nil {
 		return nil
 	}

internal/apis/core/v1alpha4/conversion.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))
+		})
+	}
+}