From a56b877a918625a206c6c1a1c83e072e82c8aa46 Mon Sep 17 00:00:00 2001 From: Carlos Salas Date: Thu, 14 Nov 2024 13:03:34 +0100 Subject: [PATCH] feat: opt out of cluster owner rbac (#834) * feat: apply nocreatorrbac annotation to rancher clusters Signed-off-by: Carlos Salas * test: update import v3 controller tests Signed-off-by: Carlos Salas * test: update e2e test to validate rbac annotation Signed-off-by: Carlos Salas --------- Signed-off-by: Carlos Salas --- internal/controllers/helpers.go | 1 + internal/controllers/import_controller.go | 2 +- internal/controllers/import_controller_v3.go | 37 ++++++++++++++++++- .../controllers/import_controller_v3_test.go | 4 +- test/e2e/specs/import_gitops_mgmtv3.go | 4 ++ .../e2e/specs/migrate_gitops_provv1_mgmtv3.go | 4 ++ util/annotations/helpers.go | 6 +++ 7 files changed, 55 insertions(+), 3 deletions(-) diff --git a/internal/controllers/helpers.go b/internal/controllers/helpers.go index edc90081..46cee3fa 100644 --- 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 index 376e7983..ffe71c55 100644 --- 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 index c186fcd7..984dd2f1 100644 --- 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 index d20ced91..7f6cdbc0 100644 --- 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 index 72efedbd..169413c0 100644 --- 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 index b54e428c..161a9427 100644 --- 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 index 0d77e1c0..fcbbbafd 100644 --- 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" )