From ff50f6bad69885369eadbd502b54acbe55f895b4 Mon Sep 17 00:00:00 2001 From: Jan Schoone <6106846+jschoone@users.noreply.github.com> Date: Thu, 22 Oct 2026 13:06:32 +0200 Subject: [PATCH 1/3] fix: default the namespace of providerRef and cluster references to the owning object external.Get in CAPI >= 1.11 uses the namespace of the reference instead of an explicit namespace argument. A ClusterStack with a providerRef without namespace (as documented) could therefore not find its template anymore. Always resolve the template in the namespace of the ClusterStack, like before, default the namespace of the Cluster references used for the addon builtins and add a test for a providerRef without namespace. Signed-off-by: Jan Schoone <6106846+jschoone@users.noreply.github.com> --- .../controller/clusteraddon_controller.go | 6 ++- .../controller/clusterstack_controller.go | 11 +++-- .../clusterstack_controller_test.go | 48 +++++++++++++++++++ 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/internal/controller/clusteraddon_controller.go b/internal/controller/clusteraddon_controller.go index d1626a23..72f86305 100644 --- a/internal/controller/clusteraddon_controller.go +++ b/internal/controller/clusteraddon_controller.go @@ -1276,11 +1276,15 @@ func helmTemplateClusterAddon(chartPath string, helmTemplate []byte, kubernetesV // initializeBuiltins takes a map of keys to object references, attempts to get the referenced objects, and returns a map of keys to the actual objects. // These objects are a map[string]interface{} so that they can be used as values in the template. -func initializeBuiltins(ctx context.Context, c client.Client, referenceMap map[string]corev1.ObjectReference, _ *clusterv1.Cluster) (map[string]interface{}, error) { +func initializeBuiltins(ctx context.Context, c client.Client, referenceMap map[string]corev1.ObjectReference, cluster *clusterv1.Cluster) (map[string]interface{}, error) { valueLookUp := make(map[string]interface{}) for name, ref := range referenceMap { objectRef := referenceMap[name] + // external.Get uses the namespace of the reference. References of a Cluster are in the namespace of the Cluster. + if objectRef.Namespace == "" { + objectRef.Namespace = cluster.Namespace + } obj, err := external.Get(ctx, c, &objectRef) if err != nil { return nil, fmt.Errorf("failed to get object %s: %w", ref.Name, err) diff --git a/internal/controller/clusterstack_controller.go b/internal/controller/clusterstack_controller.go index 40821a7c..70e54ec3 100644 --- a/internal/controller/clusterstack_controller.go +++ b/internal/controller/clusterstack_controller.go @@ -278,10 +278,15 @@ func (r *ClusterStackReconciler) getOrCreateClusterStackRelease(ctx context.Cont } func (r *ClusterStackReconciler) createOrUpdateProviderClusterStackRelease(ctx context.Context, name string, clusterStack *csov1alpha1.ClusterStack) (*corev1.ObjectReference, error) { + // The template always lives in the namespace of the ClusterStack, the namespace of the providerRef is optional + // and ignored. external.Get uses the namespace of the reference, so it has to be set explicitly. + templateRef := clusterStack.Spec.ProviderRef.DeepCopy() + templateRef.Namespace = clusterStack.Namespace + // get template object where the object is based on - from, err := external.Get(ctx, r.Client, clusterStack.Spec.ProviderRef) + from, err := external.Get(ctx, r.Client, templateRef) if err != nil { - return nil, fmt.Errorf("ProviderClusterStackReleaseTemplate %q for kind %q not found: %w", clusterStack.Spec.ProviderRef.Name, clusterStack.Spec.ProviderRef.GetObjectKind(), err) + return nil, fmt.Errorf("ProviderClusterStackReleaseTemplate %q for kind %q not found: %w", templateRef.Name, templateRef.GetObjectKind(), err) } ref := &corev1.ObjectReference{ @@ -306,7 +311,7 @@ func (r *ClusterStackReconciler) createOrUpdateProviderClusterStackRelease(ctx c input := &generateProviderObjectInput{ ExistingObject: existingObject, Template: from, - TemplateRef: clusterStack.Spec.ProviderRef, + TemplateRef: templateRef, Namespace: clusterStack.Namespace, Name: name, Labels: clusterStack.Labels, diff --git a/internal/controller/clusterstack_controller_test.go b/internal/controller/clusterstack_controller_test.go index 3cded26a..43bf3f79 100644 --- a/internal/controller/clusterstack_controller_test.go +++ b/internal/controller/clusterstack_controller_test.go @@ -501,6 +501,54 @@ var _ = Describe("ClusterStackReconciler", func() { }) }) + Context("Tests with a providerRef without namespace", func() { + var clusterStackReleaseTagV1Key types.NamespacedName + + BeforeEach(func() { + clusterStack.Spec = csov1alpha1.ClusterStackSpec{ + Provider: "docker", + Name: "ferrol", + KubernetesVersion: "1.27", + Versions: []string{"v1"}, + AutoSubscribe: false, + // The namespace is intentionally left empty: it is optional in the documented + // examples and has to default to the namespace of the ClusterStack. + ProviderRef: &corev1.ObjectReference{ + APIVersion: "infrastructure.clusterstack.x-k8s.io/v1alpha1", + Kind: "TestInfrastructureProviderClusterStackReleaseTemplate", + Name: "provider-test1", + }, + } + Expect(testEnv.Create(ctx, clusterStack)).To(Succeed()) + + cs, err := clusterstack.New(clusterStack.Spec.Provider, clusterStack.Spec.Name, clusterStack.Spec.KubernetesVersion, "v1") + Expect(err).To(BeNil()) + clusterStackReleaseTagV1Key = types.NamespacedName{Name: cs.String(), Namespace: testNs.Name} + }) + + AfterEach(func() { + Expect(testEnv.Cleanup(ctx, clusterStack)).To(Succeed()) + }) + + It("creates the ProviderClusterStackRelease in the namespace of the ClusterStack", func() { + Eventually(func() error { + var clusterStackRelease csov1alpha1.ClusterStackRelease + return testEnv.Get(ctx, clusterStackReleaseTagV1Key, &clusterStackRelease) + }, timeout, interval).Should(BeNil()) + + Eventually(func() error { + ref := &corev1.ObjectReference{ + APIVersion: "infrastructure.clusterstack.x-k8s.io/v1alpha1", + Kind: "TestInfrastructureProviderClusterStackRelease", + Name: clusterStackReleaseTagV1Key.Name, + Namespace: testNs.Name, + } + _, err := external.Get(ctx, testEnv.GetClient(), ref) + return err + }, timeout, interval).Should(BeNil()) + }) + }) + Context("Tests with multiple versions", func() { var ( clusterStackReleaseTagV1 string From 9db472ea0d5736fdf0b2414b1afc3d43f0df7852 Mon Sep 17 00:00:00 2001 From: Jan Schoone <6106846+jschoone@users.noreply.github.com> Date: Thu, 22 Oct 2026 13:07:13 +0200 Subject: [PATCH 2/3] fix: restore kube-system as default namespace for resources cleaned up in the workload cluster cleanUpResources talks to the workload cluster, where resources without a namespace belong to the addon namespace (kube-system). The namespace of the ClusterAddon object in the management cluster is not meaningful there. Signed-off-by: Jan Schoone <6106846+jschoone@users.noreply.github.com> --- internal/controller/clusteraddon_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/clusteraddon_controller.go b/internal/controller/clusteraddon_controller.go index 72f86305..e13e90a3 100644 --- a/internal/controller/clusteraddon_controller.go +++ b/internal/controller/clusteraddon_controller.go @@ -642,7 +642,7 @@ func cleanUpResources(ctx context.Context, in *templateAndApplyClusterAddonInput for _, resource := range extraResources { if resource.Namespace == "" { - resource.Namespace = in.clusterAddon.Namespace + resource.Namespace = clusterAddonNamespace } dr, err := kube.GetDynamicResourceInterface(resource.Namespace, in.restConfig, resource.GroupVersionKind()) if err != nil { From 486b4188762fdfbcd1f8aea71a138e3e4f689e0c Mon Sep 17 00:00:00 2001 From: Jan Schoone <6106846+jschoone@users.noreply.github.com> Date: Thu, 22 Oct 2026 14:35:21 +0200 Subject: [PATCH 3/3] fix: consider spec.topology.classNamespace when checking whether ClusterClasses are in use Clusters of any namespace can use the ClusterClasses of a namespace since CAPI 1.11. The in-use checks of the ClusterStack and ClusterStackRelease webhooks and controllers only looked at the clusters of the own namespace, so a ClusterStackRelease could be deleted while a cluster in another namespace still used its ClusterClass, and clusters of the own namespace using a ClusterClass of another namespace were wrongly counted. Add pkg/clusterclass with a helper listing the clusters that use the ClusterClasses of a namespace and a test for getUsedClusterClasses. Signed-off-by: Jan Schoone <6106846+jschoone@users.noreply.github.com> --- api/v1alpha1/clusterstack_webhook.go | 19 ++--- api/v1alpha1/clusterstackrelease_webhook.go | 19 ++--- internal/controller/utils.go | 23 ++---- internal/controller/utils_test.go | 92 +++++++++++++++++++++ pkg/clusterclass/clusterclass.go | 62 ++++++++++++++ 5 files changed, 179 insertions(+), 36 deletions(-) create mode 100644 internal/controller/utils_test.go create mode 100644 pkg/clusterclass/clusterclass.go diff --git a/api/v1alpha1/clusterstack_webhook.go b/api/v1alpha1/clusterstack_webhook.go index 2fd1da73..30b40852 100644 --- a/api/v1alpha1/clusterstack_webhook.go +++ b/api/v1alpha1/clusterstack_webhook.go @@ -22,12 +22,12 @@ import ( "reflect" "strings" + "github.com/SovereignCloudStack/cluster-stack-operator/pkg/clusterclass" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/clusterstack" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/version" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" - clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -159,20 +159,17 @@ func (r *ClusterStackWebhook) ValidateDelete(ctx context.Context, obj runtime.Ob } func (r *ClusterStackWebhook) getClustersUsingClusterStack(ctx context.Context, clusterStack *ClusterStack) ([]string, error) { - clusterList := &clusterv1.ClusterList{} - if err := r.List(ctx, clusterList, &client.ListOptions{Namespace: clusterStack.Namespace}); err != nil { + // clusters of all namespaces can use the ClusterClasses of this namespace (spec.topology.classNamespace) + clusterList, err := clusterclass.ListClustersUsingClassesOf(ctx, r.Client, clusterStack.Namespace) + if err != nil { return nil, fmt.Errorf("failed to list clusters: %w", err) } - clusters := make([]string, 0, len(clusterList.Items)) + clusters := make([]string, 0, len(clusterList)) // list the names of all ClusterClasses that are referenced in Cluster objects - for i := range clusterList.Items { - if clusterList.Items[i].Spec.Topology == nil { - continue - } - - clusterClass := clusterList.Items[i].Spec.Topology.Class + for i := range clusterList { + clusterClass := clusterList[i].Spec.Topology.Class if clusterClass == "" { continue } @@ -185,7 +182,7 @@ func (r *ClusterStackWebhook) getClustersUsingClusterStack(ctx context.Context, clusterStack.Spec.Channel == cs.Version.Channel && clusterStack.Spec.Name == cs.Name && clusterStack.Spec.KubernetesVersion == cs.KubernetesVersion.StringWithDot() { - clusters = append(clusters, clusterList.Items[i].Name) + clusters = append(clusters, clusterList[i].Name) } } diff --git a/api/v1alpha1/clusterstackrelease_webhook.go b/api/v1alpha1/clusterstackrelease_webhook.go index 67c14765..35142cbd 100644 --- a/api/v1alpha1/clusterstackrelease_webhook.go +++ b/api/v1alpha1/clusterstackrelease_webhook.go @@ -21,10 +21,10 @@ import ( "fmt" "strings" + "github.com/SovereignCloudStack/cluster-stack-operator/pkg/clusterclass" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/clusterstack" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" - clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -85,20 +85,17 @@ func (r *ClusterStackReleaseWebhook) ValidateDelete(ctx context.Context, obj run } func (r *ClusterStackReleaseWebhook) getClustersUsingClusterStackRelease(ctx context.Context, clusterStackRelease *ClusterStackRelease) ([]string, error) { - clusterList := &clusterv1.ClusterList{} - if err := r.List(ctx, clusterList, &client.ListOptions{Namespace: clusterStackRelease.Namespace}); err != nil { + // clusters of all namespaces can use the ClusterClasses of this namespace (spec.topology.classNamespace) + clusterList, err := clusterclass.ListClustersUsingClassesOf(ctx, r.Client, clusterStackRelease.Namespace) + if err != nil { return nil, fmt.Errorf("failed to list clusters: %w", err) } - clusters := make([]string, 0, len(clusterList.Items)) + clusters := make([]string, 0, len(clusterList)) // list the names of all ClusterClasses that are referenced in Cluster objects - for i := range clusterList.Items { - if clusterList.Items[i].Spec.Topology == nil { - continue - } - - clusterClass := clusterList.Items[i].Spec.Topology.Class + for i := range clusterList { + clusterClass := clusterList[i].Spec.Topology.Class clusterClassFormated, err := clusterstack.NewFromClusterClassProperties(clusterClass) if err != nil { return nil, fmt.Errorf("failed to read properties from clusterClass string: %w", err) @@ -112,7 +109,7 @@ func (r *ClusterStackReleaseWebhook) getClustersUsingClusterStackRelease(ctx con clusterClassFormated.Provider == clusterStackReleaseFormatted.Provider && clusterClassFormated.KubernetesVersion == clusterStackReleaseFormatted.KubernetesVersion && clusterClassFormated.Version.String() == clusterStackReleaseFormatted.Version.String() { - clusters = append(clusters, clusterList.Items[i].Name) + clusters = append(clusters, clusterList[i].Name) } } diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 145b72b6..d2e7b21d 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -20,28 +20,23 @@ import ( "context" "fmt" - clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" + "github.com/SovereignCloudStack/cluster-stack-operator/pkg/clusterclass" "sigs.k8s.io/controller-runtime/pkg/client" ) func getUsedClusterClasses(ctx context.Context, c client.Client, namespace string) ([]string, error) { - clusterList := &clusterv1.ClusterList{} - if err := c.List(ctx, clusterList, &client.ListOptions{Namespace: namespace}); err != nil { - return nil, fmt.Errorf("failed to list clusters: %w", err) + // clusters of all namespaces can use the ClusterClasses of this namespace (spec.topology.classNamespace) + clusters, err := clusterclass.ListClustersUsingClassesOf(ctx, c, namespace) + if err != nil { + return nil, fmt.Errorf("failed to list clusters using ClusterClasses of namespace %s: %w", namespace, err) } - usedClusterClasses := make([]string, 0, len(clusterList.Items)) + usedClusterClasses := make([]string, 0, len(clusters)) // list the names of all ClusterClasses that are referenced in Cluster objects - for i := range clusterList.Items { - cluster := clusterList.Items[i] - if cluster.Spec.Topology == nil { - continue - } - if cluster.Spec.Topology.Class != "" { - // Use classNamespace if set, otherwise use cluster namespace - // This ensures we track ClusterClasses in the correct namespace - usedClusterClasses = append(usedClusterClasses, cluster.Spec.Topology.Class) + for i := range clusters { + if clusters[i].Spec.Topology.Class != "" { + usedClusterClasses = append(usedClusterClasses, clusters[i].Spec.Topology.Class) } } diff --git a/internal/controller/utils_test.go b/internal/controller/utils_test.go new file mode 100644 index 00000000..54810382 --- /dev/null +++ b/internal/controller/utils_test.go @@ -0,0 +1,92 @@ +/* +Copyright 2026 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("getUsedClusterClasses", func() { + var ( + nsA, nsB *corev1.Namespace + clusters []*clusterv1.Cluster + ) + + newCluster := func(name string, ns *corev1.Namespace, class, classNamespace string) *clusterv1.Cluster { + return &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: ns.Name, + Finalizers: []string{clusterv1.ClusterFinalizer}, + }, + Spec: clusterv1.ClusterSpec{ + Topology: &clusterv1.Topology{ + Class: class, + ClassNamespace: classNamespace, + Version: testKubernetesVersion, + }, + }, + } + } + + BeforeEach(func() { + var err error + nsA, err = testEnv.CreateNamespace(ctx, "used-classes-a") + Expect(err).NotTo(HaveOccurred()) + nsB, err = testEnv.CreateNamespace(ctx, "used-classes-b") + Expect(err).NotTo(HaveOccurred()) + + clusters = []*clusterv1.Cluster{ + // uses the ClusterClass of its own namespace A + newCluster("local-a", nsA, "docker-ferrol-1-27-v1", ""), + // lives in A, but uses the ClusterClass of namespace B + newCluster("cross-a-to-b", nsA, "docker-ferrol-1-27-v2", nsB.Name), + // lives in B and uses the ClusterClass of its own namespace B + newCluster("local-b", nsB, "docker-ferrol-1-27-v3", ""), + // lives in B, but uses the ClusterClass of namespace A + newCluster("cross-b-to-a", nsB, "docker-ferrol-1-27-v4", nsA.Name), + } + for _, cluster := range clusters { + Expect(testEnv.Create(ctx, cluster)).To(Succeed()) + } + }) + + AfterEach(func() { + Eventually(func() error { + objs := []client.Object{nsA, nsB} + for _, cluster := range clusters { + objs = append(objs, cluster) + } + return testEnv.Cleanup(ctx, objs...) + }, timeout, interval).Should(Succeed()) + }) + + It("takes the classNamespace of the clusters into account", func() { + Eventually(func() ([]string, error) { + return getUsedClusterClasses(ctx, testEnv, nsA.Name) + }, timeout, interval).Should(ConsistOf("docker-ferrol-1-27-v1", "docker-ferrol-1-27-v4")) + + Eventually(func() ([]string, error) { + return getUsedClusterClasses(ctx, testEnv, nsB.Name) + }, timeout, interval).Should(ConsistOf("docker-ferrol-1-27-v2", "docker-ferrol-1-27-v3")) + }) +}) diff --git a/pkg/clusterclass/clusterclass.go b/pkg/clusterclass/clusterclass.go new file mode 100644 index 00000000..219b8783 --- /dev/null +++ b/pkg/clusterclass/clusterclass.go @@ -0,0 +1,62 @@ +/* +Copyright 2026 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package clusterclass contains helpers to find out which clusters use ClusterClasses of a namespace. +package clusterclass + +import ( + "context" + "fmt" + + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// Namespace returns the namespace of the ClusterClass that is used by the cluster. +// Since CAPI 1.11 a ClusterClass can live in a different namespace than the Cluster (spec.topology.classNamespace). +// It returns an empty string if the cluster does not use a ClusterClass. +func Namespace(cluster *clusterv1.Cluster) string { + if cluster.Spec.Topology == nil { + return "" + } + + if cluster.Spec.Topology.ClassNamespace != "" { + return cluster.Spec.Topology.ClassNamespace + } + + return cluster.Namespace +} + +// ListClustersUsingClassesOf returns all clusters, in any namespace, that use a ClusterClass of the given namespace. +func ListClustersUsingClassesOf(ctx context.Context, c client.Reader, namespace string) ([]clusterv1.Cluster, error) { + clusterList := &clusterv1.ClusterList{} + if err := c.List(ctx, clusterList); err != nil { + return nil, fmt.Errorf("failed to list clusters: %w", err) + } + + clusters := make([]clusterv1.Cluster, 0, len(clusterList.Items)) + for i := range clusterList.Items { + if clusterList.Items[i].Spec.Topology == nil { + continue + } + + if Namespace(&clusterList.Items[i]) == namespace { + clusters = append(clusters, clusterList.Items[i]) + } + } + + return clusters, nil +}