diff --git a/controllers/resources.go b/controllers/resources.go index 0713a27..f50572a 100644 --- 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 index df30a4e..e6c50de 100644 --- a/controllers/resources_test.go +++ b/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 { diff --git a/tests/e2e/rollout_tests_all.go b/tests/e2e/rollout_tests_all.go index 3116c3d..782ebce 100644 --- a/tests/e2e/rollout_tests_all.go +++ b/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()) + + } + + }) + }) + }) }