fix: delete existing RBAC resources while switching scope of Rollouts

Signed-off-by: Jayendra Parsai <[email protected]>
argoproj-labs · Oct 19, 2024 · acd04d8 · acd04d8
1 parent aea86d7
commit acd04d8
Show file tree

Hide file tree

Showing 2 changed files with 139 additions and 3 deletions.
diff --git a/controllers/resources.go b/controllers/resources.go
@@ -64,8 +64,16 @@ func (r *RolloutManagerReconciler) reconcileRolloutsServiceAccount(ctx context.C
 
 // Reconciles Rollouts Role.
 func (r *RolloutManagerReconciler) reconcileRolloutsRole(ctx context.Context, cr rolloutsmanagerv1alpha1.RolloutManager) (*rbacv1.Role, error) {
-	expectedPolicyRules := GetPolicyRules()
 
+	// Delete existing ClusterRole
+	liveClusterRole := &rbacv1.ClusterRole{ObjectMeta: metav1.ObjectMeta{Name: DefaultArgoRolloutsResourceName}}
+	if err := fetchObject(ctx, r.Client, "", liveClusterRole.Name, liveClusterRole); err == nil {
+		if err := r.Client.Delete(ctx, liveClusterRole); err != nil {
+			return nil, fmt.Errorf("failed to delete existing ClusterRole %s: %w", liveClusterRole.Name, err)
+		}
+	}
+
+	expectedPolicyRules := GetPolicyRules()
 	expectedRole := &rbacv1.Role{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      DefaultArgoRolloutsResourceName,
@@ -121,8 +129,15 @@ func (r *RolloutManagerReconciler) reconcileRolloutsRole(ctx context.Context, cr
 
 // Reconciles Rollouts ClusterRole.
 func (r *RolloutManagerReconciler) reconcileRolloutsClusterRole(ctx context.Context, cr rolloutsmanagerv1alpha1.RolloutManager) (*rbacv1.ClusterRole, error) {
-	expectedPolicyRules := GetPolicyRules()
 
+	// Delete existing Role
+	liveRole := &rbacv1.Role{ObjectMeta: metav1.ObjectMeta{Name: DefaultArgoRolloutsResourceName, Namespace: cr.Namespace}}
+	if err := fetchObject(ctx, r.Client, cr.Namespace, liveRole.Name, liveRole); err == nil {
+		if err := r.Client.Delete(ctx, liveRole); err != nil {
+			return nil, fmt.Errorf("failed to delete existing Role %s for Namespace %s: %w", liveRole.Name, liveRole.Namespace, err)
+		}
+	}
+	expectedPolicyRules := GetPolicyRules()
 	expectedClusterRole := &rbacv1.ClusterRole{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: DefaultArgoRolloutsResourceName,
@@ -134,7 +149,6 @@ func (r *RolloutManagerReconciler) reconcileRolloutsClusterRole(ctx context.Cont
 		if !apierrors.IsNotFound(err) {
 			return nil, fmt.Errorf("failed to Reconcile the ClusterRole for the ServiceAccount associated with %s: %w", liveClusterRole.Name, err)
 		}
-
 		log.Info(fmt.Sprintf("Creating ClusterRole %s", liveClusterRole.Name))
 		expectedClusterRole.Rules = expectedPolicyRules
 		return expectedClusterRole, r.Client.Create(ctx, expectedClusterRole)
@@ -169,6 +183,14 @@ func (r *RolloutManagerReconciler) reconcileRolloutsClusterRole(ctx context.Cont
 // Reconcile Rollouts RoleBinding.
 func (r *RolloutManagerReconciler) reconcileRolloutsRoleBinding(ctx context.Context, cr rolloutsmanagerv1alpha1.RolloutManager, role *rbacv1.Role, sa *corev1.ServiceAccount) error {
 
+	// Delete existing ClusterRoleBinding
+	liveClusterRoleBinding := &rbacv1.ClusterRoleBinding{ObjectMeta: metav1.ObjectMeta{Name: DefaultArgoRolloutsResourceName}}
+	if err := fetchObject(ctx, r.Client, "", liveClusterRoleBinding.Name, liveClusterRoleBinding); err == nil {
+		if err := r.Client.Delete(ctx, liveClusterRoleBinding); err != nil {
+			return fmt.Errorf("failed to delete existing ClusterRoleBinding %s: %w", liveClusterRoleBinding.Name, err)
+		}
+	}
+
 	if role == nil {
 		return fmt.Errorf("received Role is nil while reconciling RoleBinding")
 	}
@@ -247,6 +269,14 @@ func (r *RolloutManagerReconciler) reconcileRolloutsRoleBinding(ctx context.Cont
 // Reconcile Rollouts ClusterRoleBinding.
 func (r *RolloutManagerReconciler) reconcileRolloutsClusterRoleBinding(ctx context.Context, clusterRole *rbacv1.ClusterRole, sa *corev1.ServiceAccount, cr rolloutsmanagerv1alpha1.RolloutManager) error {
 
+	// Delete existing RoleBinding
+	liveRoleBinding := &rbacv1.RoleBinding{ObjectMeta: metav1.ObjectMeta{Name: DefaultArgoRolloutsResourceName, Namespace: cr.Namespace}}
+	if err := fetchObject(ctx, r.Client, cr.Namespace, liveRoleBinding.Name, liveRoleBinding); err == nil {
+		if err := r.Client.Delete(ctx, liveRoleBinding); err != nil {
+			return fmt.Errorf("failed to delete existing RoleBinding %s for Namespace %s: %w", liveRoleBinding.Name, liveRoleBinding.Namespace, err)
+		}
+	}
+
 	if clusterRole == nil {
 		return fmt.Errorf("received ClusterRole is nil while reconciling ClusterRoleBinding")
 	}

diff --git a/controllers/resources_test.go b/controllers/resources_test.go
@@ -1110,6 +1110,112 @@ var _ = Describe("Resource creation and cleanup tests", func() {
 		})
 	})
 
+	Context("Verify correct RBAC permissions are assigned while switching between namespace and cluster scoped Rollouts", func() {
+		var (
+			ctx context.Context
+			a   v1alpha1.RolloutManager
+			r   *RolloutManagerReconciler
+		)
+
+		BeforeEach(func() {
+			ctx = context.Background()
+			a = *makeTestRolloutManager()
+			r = makeTestReconciler(&a)
+			err := createNamespace(r, a.Namespace)
+			Expect(err).ToNot(HaveOccurred())
+		})
+
+		It("Should delete existing Role when ClusterRole is reconciled", func() {
+			By("Reconcile Role.")
+			role, err := r.reconcileRolloutsRole(ctx, a)
+			Expect(err).ToNot(HaveOccurred())
+
+			By("Verify Role is created")
+			Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(role), role)).To(Succeed())
+
+			By("Reconcile ClusterRole")
+			clusterRole, err := r.reconcileRolloutsClusterRole(ctx, a)
+			Expect(err).ToNot(HaveOccurred())
+
+			By("Verify ClusterRole is created")
+			Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(clusterRole), clusterRole)).To(Succeed())
+
+			By("Verify existing Role is deleted")
+			Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(role), role)).To(HaveOccurred())
+		})
+
+		It("Should delete existing ClusterRole when Role is reconciled", func() {
+
+			By("Reconcile ClusterRole")
+			clusterRole, err := r.reconcileRolloutsClusterRole(ctx, a)
+			Expect(err).ToNot(HaveOccurred())
+
+			By("Verify ClusterRole is created")
+			Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(clusterRole), clusterRole)).To(Succeed())
+
+			By("Reconcile Role.")
+			role, err := r.reconcileRolloutsRole(ctx, a)
+			Expect(err).ToNot(HaveOccurred())
+
+			By("Verify Role is created")
+			Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(role), role)).To(Succeed())
+
+			By("Verify existing ClusterRole is deleted")
+			Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(clusterRole), clusterRole)).To(HaveOccurred())
+		})
+
+		It("Should delete existing RoleBinding when ClusterRoleBinding is reconciled", func() {
+
+			By("Reconcile RoleBinding")
+			sa, err := r.reconcileRolloutsServiceAccount(ctx, a)
+			Expect(err).ToNot(HaveOccurred())
+			role, err := r.reconcileRolloutsRole(ctx, a)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(r.reconcileRolloutsRoleBinding(ctx, a, role, sa)).To(Succeed())
+
+			By("Verify RoleBinding is created")
+			roleBinding := &rbacv1.RoleBinding{ObjectMeta: metav1.ObjectMeta{Name: DefaultArgoRolloutsResourceName, Namespace: a.Namespace}}
+			Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(roleBinding), roleBinding)).To(Succeed())
+
+			By("Reconcile ClusterRoleBinding")
+			clusterRole, err := r.reconcileRolloutsClusterRole(ctx, a)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(r.reconcileRolloutsClusterRoleBinding(ctx, clusterRole, sa, a)).To(Succeed())
+
+			By("Verify ClusterRoleBinding is created")
+			clusterRoleBinding := &rbacv1.ClusterRoleBinding{ObjectMeta: metav1.ObjectMeta{Name: DefaultArgoRolloutsResourceName}}
+			Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(clusterRoleBinding), clusterRoleBinding)).To(Succeed())
+
+			By("Verify RoleBinding is deleted")
+			Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(roleBinding), roleBinding)).To(HaveOccurred())
+		})
+
+		It("Should delete existing ClusterRoleBinding when RoleBinding is reconciled", func() {
+
+			By("Reconcile ClusterRoleBinding")
+			sa, err := r.reconcileRolloutsServiceAccount(ctx, a)
+			Expect(err).ToNot(HaveOccurred())
+			clusterRole, err := r.reconcileRolloutsClusterRole(ctx, a)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(r.reconcileRolloutsClusterRoleBinding(ctx, clusterRole, sa, a)).To(Succeed())
+
+			By("Verify ClusterRoleBinding is created")
+			clusterRoleBinding := &rbacv1.ClusterRoleBinding{ObjectMeta: metav1.ObjectMeta{Name: DefaultArgoRolloutsResourceName}}
+			Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(clusterRoleBinding), clusterRoleBinding)).To(Succeed())
+
+			By("Reconcile RoleBinding")
+			role, err := r.reconcileRolloutsRole(ctx, a)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(r.reconcileRolloutsRoleBinding(ctx, a, role, sa)).To(Succeed())
+
+			By("Verify RoleBinding is created")
+			roleBinding := &rbacv1.RoleBinding{ObjectMeta: metav1.ObjectMeta{Name: DefaultArgoRolloutsResourceName, Namespace: a.Namespace}}
+			Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(roleBinding), roleBinding)).To(Succeed())
+
+			By("Verify ClusterRoleBinding is deleted")
+			Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(clusterRole), clusterRole)).To(HaveOccurred())
+		})
+	})
 })
 
 func serviceMonitor() *monitoringv1.ServiceMonitor {