diff --git a/api/v1beta1/cluster_types.go b/api/v1beta1/cluster_types.go index bbce8f181311..d4d9d1b6d065 100644 --- a/api/v1beta1/cluster_types.go +++ b/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. diff --git a/api/v1beta1/index/cluster.go b/api/v1beta1/index/cluster.go index d61d8c36f90d..b48595e7d3cc 100644 --- a/api/v1beta1/index/cluster.go +++ b/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 { diff --git a/api/v1beta1/index/cluster_test.go b/api/v1beta1/index/cluster_test.go index 4205103c7cf5..3eefe041a665 100644 --- a/api/v1beta1/index/cluster_test.go +++ b/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)) }) } diff --git a/api/v1beta1/index/index.go b/api/v1beta1/index/index.go index c6a17bf175bc..818e1283dabd 100644 --- a/api/v1beta1/index/index.go +++ b/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 } } diff --git a/api/v1beta1/zz_generated.openapi.go b/api/v1beta1/zz_generated.openapi.go index 3167c70da4e7..09e3e3aa97ec 100644 --- a/api/v1beta1/zz_generated.openapi.go +++ b/api/v1beta1/zz_generated.openapi.go @@ -4461,6 +4461,13 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_Topology(ref common.ReferenceCallb Format: "", }, }, + "classNamespace": { + SchemaProps: spec.SchemaProps{ + Description: "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.", + Type: []string{"string"}, + Format: "", + }, + }, "version": { SchemaProps: spec.SchemaProps{ Description: "The Kubernetes version of the cluster.", diff --git a/config/crd/bases/cluster.x-k8s.io_clusters.yaml b/config/crd/bases/cluster.x-k8s.io_clusters.yaml index 4ad16356145b..2e46a2b837a9 100644 --- a/config/crd/bases/cluster.x-k8s.io_clusters.yaml +++ b/config/crd/bases/cluster.x-k8s.io_clusters.yaml @@ -928,6 +928,15 @@ spec: description: The name of the ClusterClass object to create the topology. type: string + classNamespace: + description: |- + 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. + maxLength: 253 + minLength: 1 + pattern: ^[a-z0-9](?:[-a-z0-9]*[a-z0-9])?(?:\.[a-z0-9](?:[-a-z0-9]*[a-z0-9])?)*$ + type: string controlPlane: description: controlPlane describes the cluster control plane. properties: diff --git a/docs/book/src/tasks/experimental-features/cluster-class/write-clusterclass.md b/docs/book/src/tasks/experimental-features/cluster-class/write-clusterclass.md index 1c722cfcf5f9..6d79f33549e2 100644 --- a/docs/book/src/tasks/experimental-features/cluster-class/write-clusterclass.md +++ b/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,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 diff --git a/internal/apis/core/v1alpha4/conversion.go b/internal/apis/core/v1alpha4/conversion.go index cd69ca35d2ba..8e2ed513d13d 100644 --- a/internal/apis/core/v1alpha4/conversion.go +++ b/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 diff --git a/internal/apis/core/v1alpha4/zz_generated.conversion.go b/internal/apis/core/v1alpha4/zz_generated.conversion.go index 14df0dfc9112..f5c0433a653d 100644 --- a/internal/apis/core/v1alpha4/zz_generated.conversion.go +++ b/internal/apis/core/v1alpha4/zz_generated.conversion.go @@ -1757,6 +1757,7 @@ func Convert_v1alpha4_Topology_To_v1beta1_Topology(in *Topology, out *v1beta1.To func autoConvert_v1beta1_Topology_To_v1alpha4_Topology(in *v1beta1.Topology, out *Topology, s conversion.Scope) error { out.Class = in.Class + // WARNING: in.ClassNamespace requires manual conversion: does not exist in peer-type out.Version = in.Version out.RolloutAfter = (*metav1.Time)(unsafe.Pointer(in.RolloutAfter)) if err := Convert_v1beta1_ControlPlaneTopology_To_v1alpha4_ControlPlaneTopology(&in.ControlPlane, &out.ControlPlane, s); err != nil { diff --git a/internal/controllers/topology/cluster/cluster_controller.go b/internal/controllers/topology/cluster/cluster_controller.go index 30c140734fe0..3ebd33e3c023 100644 --- a/internal/controllers/topology/cluster/cluster_controller.go +++ b/internal/controllers/topology/cluster/cluster_controller.go @@ -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 } diff --git a/internal/controllers/topology/cluster/cluster_controller_test.go b/internal/controllers/topology/cluster/cluster_controller_test.go index 61fc1d8dd810..9b6b3419c76b 100644 --- a/internal/controllers/topology/cluster/cluster_controller_test.go +++ b/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)) + }) + } +} diff --git a/internal/controllers/topology/cluster/suite_test.go b/internal/controllers/topology/cluster/suite_test.go index 715f4a52bd47..945e3520d8c5 100644 --- a/internal/controllers/topology/cluster/suite_test.go +++ b/internal/controllers/topology/cluster/suite_test.go @@ -62,7 +62,7 @@ func TestMain(m *testing.M) { } // Set up the ClusterClassName 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 { + if err := index.ByClusterClassRef(ctx, mgr); err != nil { panic(fmt.Sprintf("unable to setup index: %v", err)) } } diff --git a/internal/topology/check/compatibility.go b/internal/topology/check/compatibility.go index 2ede647df1a4..3b524599bad9 100644 --- a/internal/topology/check/compatibility.go +++ b/internal/topology/check/compatibility.go @@ -107,20 +107,6 @@ func LocalObjectTemplatesAreCompatible(current, desired clusterv1.LocalObjectTem currentGK.Kind, desiredGK.Kind), )) } - allErrs = append(allErrs, LocalObjectTemplatesAreInSameNamespace(current, desired, pathPrefix)...) - return allErrs -} - -// LocalObjectTemplatesAreInSameNamespace checks if two referenced objects are in the same namespace. -func LocalObjectTemplatesAreInSameNamespace(current, desired clusterv1.LocalObjectTemplate, pathPrefix *field.Path) field.ErrorList { - var allErrs field.ErrorList - if current.Ref.Namespace != desired.Ref.Namespace { - allErrs = append(allErrs, field.Forbidden( - pathPrefix.Child("ref", "namespace"), - fmt.Sprintf("templates must be in the same namespace as the ClusterClass (%s)", - current.Ref.Namespace), - )) - } return allErrs } diff --git a/internal/topology/check/compatibility_test.go b/internal/topology/check/compatibility_test.go index e66d4223f7eb..9b22f7c0aba3 100644 --- a/internal/topology/check/compatibility_test.go +++ b/internal/topology/check/compatibility_test.go @@ -210,7 +210,7 @@ func TestLocalObjectTemplatesAreCompatible(t *testing.T) { APIVersion: "test.group.io/versiontwo", }, } - incompatibleNamespaceChangeTemplate := clusterv1.LocalObjectTemplate{ + compatibleNamespaceChangeTemplate := clusterv1.LocalObjectTemplate{ Ref: &corev1.ObjectReference{ Namespace: "different", Name: "foo", @@ -253,15 +253,15 @@ func TestLocalObjectTemplatesAreCompatible(t *testing.T) { wantErr: false, }, { - name: "Block change to template API Group", + name: "Allow change to template namespace", current: template, - desired: incompatibleAPIGroupChangeTemplate, - wantErr: true, + desired: compatibleNamespaceChangeTemplate, + wantErr: false, }, { - name: "Block change to template namespace", + name: "Block change to template API Group", current: template, - desired: incompatibleNamespaceChangeTemplate, + desired: incompatibleAPIGroupChangeTemplate, wantErr: true, }, { @@ -409,11 +409,11 @@ func TestClusterClassesAreCompatible(t *testing.T) { Name: "baz", Namespace: "default", } - compatibleRef := &corev1.ObjectReference{ + compatibleRefOther := &corev1.ObjectReference{ APIVersion: "group.test.io/another-foo", Kind: "barTemplate", Name: "another-baz", - Namespace: "default", + Namespace: "other", } tests := []struct { @@ -450,7 +450,7 @@ func TestClusterClassesAreCompatible(t *testing.T) { }, { - name: "pass for compatible clusterClasses", + name: "pass for compatible clusterClasses (with different namespace)", current: builder.ClusterClass(metav1.NamespaceDefault, "class1"). WithInfrastructureClusterTemplate( builder.InfrastructureClusterTemplate(metav1.NamespaceDefault, "infra1").Build()). @@ -473,26 +473,26 @@ func TestClusterClassesAreCompatible(t *testing.T) { builder.BootstrapTemplate(metav1.NamespaceDefault, "bootstrap1").Build()). Build()). Build(), - desired: builder.ClusterClass(metav1.NamespaceDefault, "class1"). + desired: builder.ClusterClass("other", "class1"). WithInfrastructureClusterTemplate( - builder.InfrastructureClusterTemplate(metav1.NamespaceDefault, "infra1").Build()). + builder.InfrastructureClusterTemplate("other", "infra1").Build()). WithControlPlaneTemplate( - refToUnstructured(compatibleRef)). + refToUnstructured(compatibleRefOther)). WithControlPlaneInfrastructureMachineTemplate( - refToUnstructured(compatibleRef)). + refToUnstructured(compatibleRefOther)). WithWorkerMachineDeploymentClasses( *builder.MachineDeploymentClass("aa"). WithInfrastructureTemplate( - builder.InfrastructureMachineTemplate(metav1.NamespaceDefault, "infra1").Build()). + builder.InfrastructureMachineTemplate("other", "infra1").Build()). WithBootstrapTemplate( - builder.BootstrapTemplate(metav1.NamespaceDefault, "bootstrap1").Build()). + builder.BootstrapTemplate("other", "bootstrap1").Build()). Build()). WithWorkerMachinePoolClasses( *builder.MachinePoolClass("bb"). WithInfrastructureTemplate( - builder.InfrastructureMachinePoolTemplate(metav1.NamespaceDefault, "infra1").Build()). + builder.InfrastructureMachinePoolTemplate("other", "infra1").Build()). WithBootstrapTemplate( - builder.BootstrapTemplate(metav1.NamespaceDefault, "bootstrap1").Build()). + builder.BootstrapTemplate("other", "bootstrap1").Build()). Build()). Build(), wantErr: false, diff --git a/internal/webhooks/clusterclass.go b/internal/webhooks/clusterclass.go index 65fa7a2d69cd..70bdf0458cc4 100644 --- a/internal/webhooks/clusterclass.go +++ b/internal/webhooks/clusterclass.go @@ -379,12 +379,14 @@ 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.ClusterClassRefPath: index.ClusterClassRef(clusterClass), + }, ) if err != nil { return nil, err } + return clusters.Items, nil } diff --git a/internal/webhooks/clusterclass_test.go b/internal/webhooks/clusterclass_test.go index 49b8928529ad..c1f6a257fd20 100644 --- a/internal/webhooks/clusterclass_test.go +++ b/internal/webhooks/clusterclass_test.go @@ -91,7 +91,7 @@ func TestClusterClassDefaultNamespaces(t *testing.T) { fakeClient := fake.NewClientBuilder(). WithScheme(fakeScheme). - WithIndex(&clusterv1.Cluster{}, index.ClusterClassNameField, index.ClusterByClusterClassClassName). + WithIndex(&clusterv1.Cluster{}, index.ClusterClassRefPath, index.ClusterByClusterClassRef). Build() // Create the webhook and add the fakeClient as its client. @@ -1865,7 +1865,7 @@ func TestClusterClassValidation(t *testing.T) { // Sets up the fakeClient for the test case. fakeClient := fake.NewClientBuilder(). WithScheme(fakeScheme). - WithIndex(&clusterv1.Cluster{}, index.ClusterClassNameField, index.ClusterByClusterClassClassName). + WithIndex(&clusterv1.Cluster{}, index.ClusterClassRefPath, index.ClusterByClusterClassRef). Build() // Pin the compatibility version used in variable CEL validation to 1.29, so we don't have to continuously refactor @@ -2512,7 +2512,7 @@ func TestClusterClassValidationWithClusterAwareChecks(t *testing.T) { fakeClient := fake.NewClientBuilder(). WithScheme(fakeScheme). WithObjects(tt.clusters...). - WithIndex(&clusterv1.Cluster{}, index.ClusterClassNameField, index.ClusterByClusterClassClassName). + WithIndex(&clusterv1.Cluster{}, index.ClusterClassRefPath, index.ClusterByClusterClassRef). Build() // Create the webhook and add the fakeClient as its client. @@ -2527,6 +2527,69 @@ func TestClusterClassValidationWithClusterAwareChecks(t *testing.T) { } } +func TestGetClustersUsingClusterClass(t *testing.T) { + // NOTE: ClusterTopology feature flag is disabled by default, thus preventing to create or update ClusterClasses. + // Enabling the feature flag temporarily for this test. + utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.ClusterTopology, true) + + topology := builder.ClusterTopology().WithClass("class1") + + tests := []struct { + name string + clusterClass *clusterv1.ClusterClass + clusters []client.Object + expectErr bool + expectClusters []client.Object + }{ + { + name: "ClusterClass should return clusters referencing it", + clusterClass: builder.ClusterClass("default", "class1").Build(), + clusters: []client.Object{ + builder.Cluster("default", "cluster1").WithTopology(topology.Build()).Build(), + builder.Cluster("default", "cluster2").Build(), + builder.Cluster("other", "cluster2").WithTopology(topology.DeepCopy().WithClassNamespace("default").Build()).Build(), + builder.Cluster("other", "cluster3").WithTopology(topology.Build()).Build(), + }, + expectClusters: []client.Object{ + builder.Cluster("default", "cluster1").Build(), + builder.Cluster("other", "cluster2").Build(), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + // Sets up the fakeClient for the test case. + fakeClient := fake.NewClientBuilder(). + WithScheme(fakeScheme). + WithObjects(tt.clusters...). + WithIndex(&clusterv1.Cluster{}, index.ClusterClassRefPath, index.ClusterByClusterClassRef). + Build() + + // Create the webhook and add the fakeClient as its client. + webhook := &ClusterClass{Client: fakeClient} + clusters, err := webhook.getClustersUsingClusterClass(ctx, tt.clusterClass) + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + + clusterKeys := []client.ObjectKey{} + for _, c := range clusters { + clusterKeys = append(clusterKeys, client.ObjectKeyFromObject(&c)) + } + expectedKeys := []client.ObjectKey{} + for _, c := range tt.expectClusters { + expectedKeys = append(expectedKeys, client.ObjectKeyFromObject(c)) + } + g.Expect(clusterKeys).To(Equal(expectedKeys)) + }) + } +} + func TestValidateAutoscalerAnnotationsForClusterClass(t *testing.T) { tests := []struct { name string diff --git a/util/test/builder/builders.go b/util/test/builder/builders.go index dd2ae1dc881f..762b418fe835 100644 --- a/util/test/builder/builders.go +++ b/util/test/builder/builders.go @@ -114,7 +114,7 @@ func (c *ClusterBuilder) Build() *clusterv1.Cluster { // ClusterTopologyBuilder contains the fields needed to build a testable ClusterTopology. type ClusterTopologyBuilder struct { - class string + class, classNamespace string workers *clusterv1.WorkersTopology version string controlPlaneReplicas int32 @@ -136,6 +136,12 @@ func (c *ClusterTopologyBuilder) WithClass(class string) *ClusterTopologyBuilder return c } +// WithClassNamespace adds the passed classNamespace to the ClusterTopologyBuilder. +func (c *ClusterTopologyBuilder) WithClassNamespace(ns string) *ClusterTopologyBuilder { + c.classNamespace = ns + return c +} + // WithVersion adds the passed version to the ClusterTopologyBuilder. func (c *ClusterTopologyBuilder) WithVersion(version string) *ClusterTopologyBuilder { c.version = version @@ -181,9 +187,10 @@ func (c *ClusterTopologyBuilder) WithVariables(vars ...clusterv1.ClusterVariable // Build returns a testable cluster Topology object with any values passed to the builder. func (c *ClusterTopologyBuilder) Build() *clusterv1.Topology { t := &clusterv1.Topology{ - Class: c.class, - Workers: c.workers, - Version: c.version, + Class: c.class, + ClassNamespace: c.classNamespace, + Workers: c.workers, + Version: c.version, ControlPlane: clusterv1.ControlPlaneTopology{ Replicas: &c.controlPlaneReplicas, MachineHealthCheck: c.controlPlaneMHC,