fix: delete existing RBAC resources while switching scope of Rollouts

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")
 	}

controllers/resources_test.go

@@ -1139,6 +1139,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)).ToNot(Succeed())
+		})
+
+		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)).ToNot(Succeed())
+		})
+
+		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)).ToNot(Succeed())
+		})
+
+		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)).ToNot(Succeed())
+		})
+	})
 })
 
 func serviceMonitor() *monitoringv1.ServiceMonitor {

tests/e2e/rollout_tests_all.go

@@ -689,5 +689,100 @@ func RunRolloutsTests(namespaceScopedParam bool) {
 			Expect(newPods.Items[0].Name).NotTo(Equal(oldPods.Items[0].Name)) // Ensure the Pod names are different
 		})
 
+		When("a namespace-scoped RolloutManager is installed into a namespace that previously contained a cluster-scoped RolloutManager, or vice versa", func() {
+
+			It("should cleanup any cluster/role/rolebinding resources that are present in the namespace, that do not match the current .spec.namespaceScoped value of the RolloutManager CR", func() {
+
+				var fakeRole rbacv1.Role
+				var fakeRoleBinding rbacv1.RoleBinding
+
+				var fakeClusterRole rbacv1.ClusterRole
+				var fakeClusterRoleBinding rbacv1.ClusterRoleBinding
+
+				By("creating ClusterRole/Binding in the namespace-scoped case, and Role/Binding in the cluster-scoped case")
+
+				if namespaceScopedParam {
+
+					fakeClusterRole = rbacv1.ClusterRole{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      controllers.DefaultArgoRolloutsResourceName,
+							Namespace: rolloutManager.Namespace,
+						},
+					}
+					Expect(k8sClient.Create(ctx, &fakeClusterRole)).To(Succeed())
+
+					fakeClusterRoleBinding = rbacv1.ClusterRoleBinding{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      controllers.DefaultArgoRolloutsResourceName,
+							Namespace: rolloutManager.Namespace,
+						},
+						RoleRef: rbacv1.RoleRef{
+							APIGroup: rbacv1.GroupName,
+							Kind:     "ClusterRole",
+							Name:     fakeClusterRole.Name,
+						},
+						Subjects: []rbacv1.Subject{
+							{
+								Kind:      rbacv1.ServiceAccountKind,
+								Name:      controllers.DefaultArgoRolloutsResourceName,
+								Namespace: rolloutManager.Namespace,
+							},
+						},
+					}
+					Expect(k8sClient.Create(ctx, &fakeClusterRoleBinding)).To(Succeed())
+
+				} else {
+
+					fakeRole = rbacv1.Role{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      controllers.DefaultArgoRolloutsResourceName,
+							Namespace: rolloutManager.Namespace,
+						},
+					}
+					Expect(k8sClient.Create(ctx, &fakeRole)).To(Succeed())
+
+					fakeRoleBinding = rbacv1.RoleBinding{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      controllers.DefaultArgoRolloutsResourceName,
+							Namespace: rolloutManager.Namespace,
+						},
+						RoleRef: rbacv1.RoleRef{
+							APIGroup: rbacv1.GroupName,
+							Kind:     "Role",
+							Name:     fakeRole.Name,
+						},
+						Subjects: []rbacv1.Subject{
+							{
+								Kind:      rbacv1.ServiceAccountKind,
+								Name:      controllers.DefaultArgoRolloutsResourceName,
+								Namespace: rolloutManager.Namespace,
+							},
+						},
+					}
+					Expect(k8sClient.Create(ctx, &fakeRoleBinding)).To(Succeed())
+
+				}
+
+				By("creating RolloutManager and waiting for it to be available")
+				Expect(k8sClient.Create(ctx, &rolloutManager)).To(Succeed())
+				Eventually(rolloutManager, "1m", "1s").Should(rolloutManagerFixture.HavePhase(rolloutsmanagerv1alpha1.PhaseAvailable))
+
+				if namespaceScopedParam {
+
+					By("verifying that in the namespace-scoped case, the cluster-scoped resources are deleted after reconciliation")
+					Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&fakeClusterRole), &fakeClusterRole)).ToNot(Succeed())
+					Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&fakeClusterRoleBinding), &fakeClusterRoleBinding)).ToNot(Succeed())
+
+				} else {
+
+					By("verifying that in the cluster-scoped case, the namespace-scoped resources are deleted after reconciliation")
+					Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&fakeRole), &fakeRole)).ToNot(Succeed())
+					Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&fakeRoleBinding), &fakeRoleBinding)).ToNot(Succeed())
+
+				}
+
+			})
+		})
+
 	})
 }