From 724d3ae4afe3b1143f3019f6b97ee1bf116cc77f Mon Sep 17 00:00:00 2001 From: Gianluca Mardente Date: Thu, 9 Jul 2026 15:41:42 +0200 Subject: [PATCH] (bug) drop namespaces RBAC dependency from migration The migrate initContainer (added in v1.11.0 to move per-cluster status from Classifier.status onto ClassifierReport.status) checks each stale/pending entry in the deprecated status arrays by Get-ing that cluster's Namespace, to decide whether the entry refers to a cluster that's since been deleted. classifier-manager-role has never granted get on core namespaces, so any upgrade that still has real data in those arrays (going from pre-v1.11.0 straight to a current release, skipping the intermediate versions that already ran the migration once and cleared the fields) hits: migrating classifier default-classifier: checking namespace : namespaces "" is forbidden: ... and the initContainer crash-loops, so classifier-manager never becomes Ready. Instead of granting the new namespaces permission, check whether the cluster the entry actually refers to (SveltosCluster or CAPI Cluster) still exists, via migrationClusterExists. classifier-manager-role already grants get;list;watch on both sveltosclusters and cluster.x-k8s.io/clusters for other reconcilers, so this needs no new RBAC. --- migration.go | 44 +++++++++++++++++++++++++-------- migration_test.go | 63 +++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 90 insertions(+), 17 deletions(-) diff --git a/migration.go b/migration.go index 51e48f2..6295bc9 100644 --- a/migration.go +++ b/migration.go @@ -34,6 +34,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" "sigs.k8s.io/controller-runtime/pkg/client" libsveltosv1beta1 "github.com/projectsveltos/libsveltos/api/v1beta1" @@ -116,16 +117,16 @@ func migrateOneClassifier(ctx context.Context, c client.Client, } for _, d := range byCluster { - ns := &corev1.Namespace{} - if err := c.Get(ctx, types.NamespacedName{Name: d.ref.Namespace}, ns); err != nil { - if apierrors.IsNotFound(err) { - log.V(logs.LogDebug).Info("skipping stale cluster entry (namespace gone)", - "classifier", classifier.Name, - "cluster", d.ref.Name, - "namespace", d.ref.Namespace) - continue - } - return fmt.Errorf("checking namespace %s: %w", d.ref.Namespace, err) + exists, err := migrationClusterExists(ctx, c, &d.ref) + if err != nil { + return fmt.Errorf("checking cluster %s/%s: %w", d.ref.Namespace, d.ref.Name, err) + } + if !exists { + log.V(logs.LogDebug).Info("skipping stale cluster entry (cluster gone)", + "classifier", classifier.Name, + "cluster", d.ref.Name, + "namespace", d.ref.Namespace) + continue } if err := upsertMigrationClassifierReport(ctx, c, classifier.Name, &d.ref, d.hash, d.deploymentStatus, d.failureMessage, @@ -194,3 +195,26 @@ func migrationDeriveClusterType(ref *corev1.ObjectReference) libsveltosv1beta1.C } return libsveltosv1beta1.ClusterTypeCapi } + +// migrationClusterExists reports whether the cluster a deprecated status entry refers to +// still exists. Checking the cluster itself (rather than its namespace) reuses RBAC the +// controller already has on sveltosclusters/clusters, instead of requiring a new grant on +// the cluster-scoped namespaces resource. +func migrationClusterExists(ctx context.Context, c client.Client, ref *corev1.ObjectReference) (bool, error) { + key := types.NamespacedName{Namespace: ref.Namespace, Name: ref.Name} + + var obj client.Object + if migrationDeriveClusterType(ref) == libsveltosv1beta1.ClusterTypeSveltos { + obj = &libsveltosv1beta1.SveltosCluster{} + } else { + obj = &clusterv1.Cluster{} + } + + if err := c.Get(ctx, key, obj); err != nil { + if apierrors.IsNotFound(err) { + return false, nil + } + return false, err + } + return true, nil +} diff --git a/migration_test.go b/migration_test.go index 7c7e9cf..01c0a81 100644 --- a/migration_test.go +++ b/migration_test.go @@ -35,7 +35,10 @@ import ( libsveltosv1beta1 "github.com/projectsveltos/libsveltos/api/v1beta1" ) -const testClassifierName = "default-classifier" +const ( + testClassifierName = "default-classifier" + testExistingClusterNamespace = "existing-ns" +) func TestMigration(t *testing.T) { RegisterFailHandler(Fail) @@ -61,8 +64,8 @@ var _ = Describe("migrateOneClassifier", func() { return s } - It("creates ClassifierReport and clears deprecated fields when namespace exists", func() { - clusterNamespace := "existing-ns" + It("creates ClassifierReport and clears deprecated fields when cluster exists", func() { + clusterNamespace := testExistingClusterNamespace clusterName := "my-cluster" classifierName := testClassifierName status := libsveltosv1beta1.SveltosStatusProvisioned @@ -88,13 +91,15 @@ var _ = Describe("migrateOneClassifier", func() { }, } - ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: clusterNamespace}} + sveltosCluster := &libsveltosv1beta1.SveltosCluster{ + ObjectMeta: metav1.ObjectMeta{Namespace: clusterNamespace, Name: clusterName}, + } scheme := buildScheme() c := fake.NewClientBuilder(). WithScheme(scheme). WithStatusSubresource(classifier, &libsveltosv1beta1.ClassifierReport{}). - WithObjects(classifier, ns). + WithObjects(classifier, sveltosCluster). Build() Expect(migrateOneClassifier(ctx, c, classifier, logger)).To(Succeed()) @@ -116,7 +121,51 @@ var _ = Describe("migrateOneClassifier", func() { Expect(updated.Status.MachingClusterStatuses).To(BeNil()) //nolint:staticcheck // deprecated, migration only }) - It("skips stale entry and still clears deprecated fields when namespace is gone", func() { + It("creates ClassifierReport when the cluster is a CAPI Cluster", func() { + clusterNamespace := testExistingClusterNamespace + clusterName := "my-capi-cluster" + classifierName := testClassifierName + status := libsveltosv1beta1.SveltosStatusProvisioned + + cluster := corev1.ObjectReference{ + APIVersion: clusterv1.GroupVersion.String(), + Kind: clusterv1.ClusterKind, + Namespace: clusterNamespace, + Name: clusterName, + } + + classifier := &libsveltosv1beta1.Classifier{ + ObjectMeta: metav1.ObjectMeta{Name: classifierName}, + Status: libsveltosv1beta1.ClassifierStatus{ + ClusterInfo: []libsveltosv1beta1.ClusterInfo{ + { + Cluster: cluster, + Status: status, + }, + }, + }, + } + + capiCluster := &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{Namespace: clusterNamespace, Name: clusterName}, + } + + scheme := buildScheme() + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithStatusSubresource(classifier, &libsveltosv1beta1.ClassifierReport{}). + WithObjects(classifier, capiCluster). + Build() + + Expect(migrateOneClassifier(ctx, c, classifier, logger)).To(Succeed()) + + clusterType := libsveltosv1beta1.ClusterTypeCapi + reportName := libsveltosv1beta1.GetClassifierReportName(classifierName, clusterName, &clusterType) + report := &libsveltosv1beta1.ClassifierReport{} + Expect(c.Get(ctx, types.NamespacedName{Namespace: clusterNamespace, Name: reportName}, report)).To(Succeed()) + }) + + It("skips stale entry and still clears deprecated fields when cluster is gone", func() { staleNamespace := "deleted-ns" staleClusterName := "customer-b-shoot-1" classifierName := testClassifierName @@ -143,7 +192,7 @@ var _ = Describe("migrateOneClassifier", func() { }, } - // Namespace is intentionally NOT created — it was deleted with the cluster. + // SveltosCluster is intentionally NOT created — it was deleted. scheme := buildScheme() c := fake.NewClientBuilder(). WithScheme(scheme).