feat: opt out of cluster owner rbac (#834)

* feat: apply nocreatorrbac annotation to rancher clusters Signed-off-by: Carlos Salas <[email protected]> * test: update import v3 controller tests Signed-off-by: Carlos Salas <[email protected]> * test: update e2e test to validate rbac annotation Signed-off-by: Carlos Salas <[email protected]> --------- Signed-off-by: Carlos Salas <[email protected]>
rancher · Nov 14, 2024 · a56b877 · a56b877
1 parent 98357e0
commit a56b877
Show file tree

Hide file tree

Showing 7 changed files with 55 additions and 3 deletions.
diff --git a/internal/controllers/helpers.go b/internal/controllers/helpers.go
@@ -53,6 +53,7 @@ const (
 	v1ClusterMigrated         = "cluster-api.cattle.io/migrated"
 
 	defaultRequeueDuration = 1 * time.Minute
+	trueAnnotationValue    = "true"
 )
 
 func getClusterRegistrationManifest(ctx context.Context, clusterName, namespace string, cl client.Client,

diff --git a/internal/controllers/import_controller.go b/internal/controllers/import_controller.go
@@ -355,7 +355,7 @@ func (r *CAPIImportReconciler) reconcileDelete(ctx context.Context, capiCluster
 		annotations = map[string]string{}
 	}
 
-	annotations[turtlesannotations.ClusterImportedAnnotation] = "true"
+	annotations[turtlesannotations.ClusterImportedAnnotation] = trueAnnotationValue
 	capiCluster.SetAnnotations(annotations)
 
 	return ctrl.Result{}, nil

diff --git a/internal/controllers/import_controller_v3.go b/internal/controllers/import_controller_v3.go
@@ -276,6 +276,9 @@ func (r *CAPIImportManagementV3Reconciler) reconcileNormal(ctx context.Context,
 					capiClusterOwnerNamespace: capiCluster.Namespace,
 					ownedLabelName:            "",
 				},
+				Annotations: map[string]string{
+					turtlesannotations.NoCreatorRBACAnnotation: trueAnnotationValue,
+				},
 				Finalizers: []string{
 					managementv3.CapiClusterFinalizer,
 				},
@@ -305,6 +308,10 @@ func (r *CAPIImportManagementV3Reconciler) reconcileNormal(ctx context.Context,
 		return ctrl.Result{}, err
 	}
 
+	if err := r.optOutOfClusterOwner(ctx, rancherCluster); err != nil {
+		return ctrl.Result{}, fmt.Errorf("error annotating rancher cluster %s to opt out of cluster owner: %w", rancherCluster.Name, err)
+	}
+
 	patchBase := client.MergeFromWithOptions(rancherCluster.DeepCopy(), client.MergeFromWithOptimisticLock{})
 	needsFinalizer := controllerutil.AddFinalizer(rancherCluster, managementv3.CapiClusterFinalizer)
 
@@ -479,7 +486,7 @@ func (r *CAPIImportManagementV3Reconciler) reconcileDelete(ctx context.Context,
 		annotations = map[string]string{}
 	}
 
-	annotations[turtlesannotations.ClusterImportedAnnotation] = "true"
+	annotations[turtlesannotations.ClusterImportedAnnotation] = trueAnnotationValue
 	capiCluster.SetAnnotations(annotations)
 	controllerutil.RemoveFinalizer(capiCluster, managementv3.CapiClusterFinalizer)
 
@@ -538,3 +545,31 @@ func (r *CAPIImportManagementV3Reconciler) verifyV1ClusterMigration(ctx context.
 
 	return true, nil
 }
+
+// optOutOfClusterOwner annotates the cluster with the opt-out annotation.
+// Rancher will detect this annotation and it won't create ProjectOwner or ClusterOwner roles.
+func (r *CAPIImportManagementV3Reconciler) optOutOfClusterOwner(ctx context.Context, rancherCluster *managementv3.Cluster) error {
+	log := log.FromContext(ctx)
+
+	annotations := rancherCluster.GetAnnotations()
+	if annotations == nil {
+		annotations = map[string]string{}
+	}
+
+	if _, ok := annotations[turtlesannotations.NoCreatorRBACAnnotation]; !ok {
+		log.Info(fmt.Sprintf("Rancher cluster '%s' does not have the NoCreatorRBACAnnotation annotation: '%s', annotating it",
+			rancherCluster.Name,
+			turtlesannotations.ClusterImportedAnnotation))
+
+		patchBase := client.MergeFromWithOptions(rancherCluster.DeepCopy(), client.MergeFromWithOptimisticLock{})
+
+		annotations[turtlesannotations.NoCreatorRBACAnnotation] = trueAnnotationValue
+		rancherCluster.SetAnnotations(annotations)
+
+		if err := r.Client.Patch(ctx, rancherCluster, patchBase); err != nil {
+			return fmt.Errorf("error patching rancher cluster: %w", err)
+		}
+	}
+
+	return nil
+}
diff --git a/internal/controllers/import_controller_v3_test.go b/internal/controllers/import_controller_v3_test.go
@@ -29,6 +29,8 @@ import (
 	provisioningv1 "github.com/rancher/turtles/api/rancher/provisioning/v1"
 	"github.com/rancher/turtles/internal/controllers/testdata"
 	"github.com/rancher/turtles/internal/test"
+
+	turtlesannotations "github.com/rancher/turtles/util/annotations"
 	turtlesnaming "github.com/rancher/turtles/util/naming"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -198,6 +200,7 @@ var _ = Describe("reconcile CAPI Cluster", func() {
 			g.Expect(rancherClusters.Items).To(HaveLen(1))
 			g.Expect(rancherClusters.Items[0].Name).To(ContainSubstring("c-"))
 			g.Expect(rancherClusters.Items[0].Labels).To(HaveKeyWithValue(testLabelName, testLabelVal))
+			g.Expect(rancherClusters.Items[0].Annotations).To(HaveKey(turtlesannotations.NoCreatorRBACAnnotation))
 			g.Expect(rancherClusters.Items[0].Finalizers).To(ContainElement(managementv3.CapiClusterFinalizer))
 		}).Should(Succeed())
 
@@ -547,5 +550,4 @@ var _ = Describe("reconcile CAPI Cluster", func() {
 			Expect(rancherClusters.Items).To(HaveLen(0))
 		}).Should(Succeed())
 	})
-
 })
diff --git a/test/e2e/specs/import_gitops_mgmtv3.go b/test/e2e/specs/import_gitops_mgmtv3.go
@@ -44,6 +44,7 @@ import (
 	"github.com/rancher/turtles/test/e2e"
 	turtlesframework "github.com/rancher/turtles/test/framework"
 	"github.com/rancher/turtles/test/testenv"
+	turtlesannotations "github.com/rancher/turtles/util/annotations"
 )
 
 type CreateMgmtV3UsingGitOpsSpecInput struct {
@@ -136,6 +137,9 @@ func CreateMgmtV3UsingGitOpsSpec(ctx context.Context, inputGetter func() CreateM
 			return conditions.IsTrue(rancherCluster, managementv3.ClusterConditionReady)
 		}, input.E2EConfig.GetIntervals(input.BootstrapClusterProxy.GetName(), "wait-rancher")...).Should(BeTrue())
 
+		By("Rancher cluster should have the 'NoCreatorRBAC' annotation")
+		Expect(rancherCluster.Annotations).To(HaveKey(turtlesannotations.NoCreatorRBACAnnotation))
+
 		By("Waiting for the CAPI cluster to be connectable using Rancher kubeconfig")
 		turtlesframework.RancherGetClusterKubeconfig(ctx, turtlesframework.RancherGetClusterKubeconfigInput{
 			Getter:           input.BootstrapClusterProxy.GetClient(),

diff --git a/test/e2e/specs/migrate_gitops_provv1_mgmtv3.go b/test/e2e/specs/migrate_gitops_provv1_mgmtv3.go
@@ -46,6 +46,7 @@ import (
 	"github.com/rancher/turtles/test/e2e"
 	turtlesframework "github.com/rancher/turtles/test/framework"
 	"github.com/rancher/turtles/test/testenv"
+	turtlesannotations "github.com/rancher/turtles/util/annotations"
 	turtlesnaming "github.com/rancher/turtles/util/naming"
 )
 
@@ -176,6 +177,9 @@ func MigrateToV3UsingGitOpsSpec(ctx context.Context, inputGetter func() MigrateT
 			return conditions.IsTrue(rancherCluster, managementv3.ClusterConditionReady)
 		}, input.E2EConfig.GetIntervals(input.BootstrapClusterProxy.GetName(), "wait-rancher")...).Should(BeTrue())
 
+		By("Rancher cluster should have the 'NoCreatorRBAC' annotation")
+		Expect(rancherCluster.Annotations).To(HaveKey(turtlesannotations.NoCreatorRBACAnnotation))
+
 		By("Waiting for the CAPI cluster to be connectable using Rancher kubeconfig")
 		turtlesframework.RancherGetClusterKubeconfig(ctx, turtlesframework.RancherGetClusterKubeconfigInput{
 			Getter:           input.BootstrapClusterProxy.GetClient(),

diff --git a/util/annotations/helpers.go b/util/annotations/helpers.go
@@ -23,6 +23,12 @@ import (
 const (
 	// ClusterImportedAnnotation represents cluster imported annotation.
 	ClusterImportedAnnotation = "imported"
+	// NoCreatorRBACAnnotation represents no creator rbac annotation.
+	// When it is set the webhook does not set the `field.cattle.io/creatorId` annotation
+	// to clusters because the controllers will no longer be creating RBAC for the creator of the cluster.
+	// This is required so that logs don't get flooded with errors when project handler and cluster handler
+	// attempt to create `ClusterOwner` and `ProjectOwner` roles.
+	NoCreatorRBACAnnotation = "field.cattle.io/no-creator-rbac"
 	// EtcdAutomaticSnapshot represents automatically generated etcd snapshot.
 	EtcdAutomaticSnapshot = "etcd.turtles.cattle.io/automatic-snapshot"
 )