Skip to content

Commit 8d74f8c

Browse files
author
Jayendra Parsai
committed
fix: delete existing RBAC resources while switching scope of Rollouts
Signed-off-by: Jayendra Parsai <[email protected]>
1 parent f36a278 commit 8d74f8c

File tree

2 files changed

+140
-4
lines changed

2 files changed

+140
-4
lines changed

‎controllers/resources.go‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,16 @@ func (r *RolloutManagerReconciler) reconcileRolloutsServiceAccount(ctx context.C
6464

6565
// Reconciles Rollouts Role.
6666
func (r *RolloutManagerReconciler) reconcileRolloutsRole(ctx context.Context, cr rolloutsmanagerv1alpha1.RolloutManager) (*rbacv1.Role, error) {
67-
expectedPolicyRules := GetPolicyRules()
6867

68+
// Delete existing ClusterRole
69+
liveClusterRole := &rbacv1.ClusterRole{ObjectMeta: metav1.ObjectMeta{Name: DefaultArgoRolloutsResourceName}}
70+
if err := fetchObject(ctx, r.Client, "", liveClusterRole.Name, liveClusterRole); err == nil {
71+
if err := r.Client.Delete(ctx, liveClusterRole); err != nil {
72+
return nil, fmt.Errorf("failed to delete existing ClusterRole %s: %w", liveClusterRole.Name, err)
73+
}
74+
}
75+
76+
expectedPolicyRules := GetPolicyRules()
6977
expectedRole := &rbacv1.Role{
7078
ObjectMeta: metav1.ObjectMeta{
7179
Name: DefaultArgoRolloutsResourceName,
@@ -121,8 +129,15 @@ func (r *RolloutManagerReconciler) reconcileRolloutsRole(ctx context.Context, cr
121129

122130
// Reconciles Rollouts ClusterRole.
123131
func (r *RolloutManagerReconciler) reconcileRolloutsClusterRole(ctx context.Context, cr rolloutsmanagerv1alpha1.RolloutManager) (*rbacv1.ClusterRole, error) {
124-
expectedPolicyRules := GetPolicyRules()
125132

133+
// Delete existing Role
134+
liveRole := &rbacv1.Role{ObjectMeta: metav1.ObjectMeta{Name: DefaultArgoRolloutsResourceName, Namespace: cr.Namespace}}
135+
if err := fetchObject(ctx, r.Client, cr.Namespace, liveRole.Name, liveRole); err == nil {
136+
if err := r.Client.Delete(ctx, liveRole); err != nil {
137+
return nil, fmt.Errorf("failed to delete existing Role %s for Namespace %s: %w", liveRole.Name, liveRole.Namespace, err)
138+
}
139+
}
140+
expectedPolicyRules := GetPolicyRules()
126141
expectedClusterRole := &rbacv1.ClusterRole{
127142
ObjectMeta: metav1.ObjectMeta{
128143
Name: DefaultArgoRolloutsResourceName,
@@ -134,12 +149,11 @@ func (r *RolloutManagerReconciler) reconcileRolloutsClusterRole(ctx context.Cont
134149
if !apierrors.IsNotFound(err) {
135150
return nil, fmt.Errorf("failed to Reconcile the ClusterRole for the ServiceAccount associated with %s: %w", liveClusterRole.Name, err)
136151
}
137-
138152
log.Info(fmt.Sprintf("Creating ClusterRole %s", liveClusterRole.Name))
139153
expectedClusterRole.Rules = expectedPolicyRules
140154
return expectedClusterRole, r.Client.Create(ctx, expectedClusterRole)
141155
}
142-
156+
fmt.Println("666")
143157
updateNeeded := false
144158

145159
if !reflect.DeepEqual(liveClusterRole.Rules, expectedPolicyRules) {
@@ -169,6 +183,14 @@ func (r *RolloutManagerReconciler) reconcileRolloutsClusterRole(ctx context.Cont
169183
// Reconcile Rollouts RoleBinding.
170184
func (r *RolloutManagerReconciler) reconcileRolloutsRoleBinding(ctx context.Context, cr rolloutsmanagerv1alpha1.RolloutManager, role *rbacv1.Role, sa *corev1.ServiceAccount) error {
171185

186+
// Delete existing ClusterRoleBinding
187+
liveClusterRoleBinding := &rbacv1.ClusterRoleBinding{ObjectMeta: metav1.ObjectMeta{Name: DefaultArgoRolloutsResourceName}}
188+
if err := fetchObject(ctx, r.Client, "", liveClusterRoleBinding.Name, liveClusterRoleBinding); err == nil {
189+
if err := r.Client.Delete(ctx, liveClusterRoleBinding); err != nil {
190+
return fmt.Errorf("failed to delete existing ClusterRoleBinding %s: %w", liveClusterRoleBinding.Name, err)
191+
}
192+
}
193+
172194
if role == nil {
173195
return fmt.Errorf("received Role is nil while reconciling RoleBinding")
174196
}
@@ -247,6 +269,14 @@ func (r *RolloutManagerReconciler) reconcileRolloutsRoleBinding(ctx context.Cont
247269
// Reconcile Rollouts ClusterRoleBinding.
248270
func (r *RolloutManagerReconciler) reconcileRolloutsClusterRoleBinding(ctx context.Context, clusterRole *rbacv1.ClusterRole, sa *corev1.ServiceAccount, cr rolloutsmanagerv1alpha1.RolloutManager) error {
249271

272+
// Delete existing RoleBinding
273+
liveRoleBinding := &rbacv1.RoleBinding{ObjectMeta: metav1.ObjectMeta{Name: DefaultArgoRolloutsResourceName, Namespace: cr.Namespace}}
274+
if err := fetchObject(ctx, r.Client, cr.Namespace, liveRoleBinding.Name, liveRoleBinding); err == nil {
275+
if err := r.Client.Delete(ctx, liveRoleBinding); err != nil {
276+
return fmt.Errorf("failed to delete existing RoleBinding %s for Namespace %s: %w", liveRoleBinding.Name, liveRoleBinding.Namespace, err)
277+
}
278+
}
279+
250280
if clusterRole == nil {
251281
return fmt.Errorf("received ClusterRole is nil while reconciling ClusterRoleBinding")
252282
}

‎controllers/resources_test.go‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1139,6 +1139,112 @@ var _ = Describe("Resource creation and cleanup tests", func() {
11391139
})
11401140
})
11411141

1142+
Context("Verify correct RBAC permissions are assigned while switching between namespace and cluster scoped Rollouts", func() {
1143+
var (
1144+
ctx context.Context
1145+
a v1alpha1.RolloutManager
1146+
r *RolloutManagerReconciler
1147+
)
1148+
1149+
BeforeEach(func() {
1150+
ctx = context.Background()
1151+
a = *makeTestRolloutManager()
1152+
r = makeTestReconciler(&a)
1153+
err := createNamespace(r, a.Namespace)
1154+
Expect(err).ToNot(HaveOccurred())
1155+
})
1156+
1157+
It("Should delete existing Role when ClusterRole is reconciled", func() {
1158+
By("Reconcile Role.")
1159+
role, err := r.reconcileRolloutsRole(ctx, a)
1160+
Expect(err).ToNot(HaveOccurred())
1161+
1162+
By("Verify Role is created")
1163+
Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(role), role)).To(Succeed())
1164+
1165+
By("Reconcile ClusterRole")
1166+
clusterRole, err := r.reconcileRolloutsClusterRole(ctx, a)
1167+
Expect(err).ToNot(HaveOccurred())
1168+
1169+
By("Verify ClusterRole is created")
1170+
Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(clusterRole), clusterRole)).To(Succeed())
1171+
1172+
By("Verify existing Role is deleted")
1173+
Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(role), role)).To(HaveOccurred())
1174+
})
1175+
1176+
It("Should delete existing ClusterRole when Role is reconciled", func() {
1177+
1178+
By("Reconcile ClusterRole")
1179+
clusterRole, err := r.reconcileRolloutsClusterRole(ctx, a)
1180+
Expect(err).ToNot(HaveOccurred())
1181+
1182+
By("Verify ClusterRole is created")
1183+
Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(clusterRole), clusterRole)).To(Succeed())
1184+
1185+
By("Reconcile Role.")
1186+
role, err := r.reconcileRolloutsRole(ctx, a)
1187+
Expect(err).ToNot(HaveOccurred())
1188+
1189+
By("Verify Role is created")
1190+
Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(role), role)).To(Succeed())
1191+
1192+
By("Verify existing ClusterRole is deleted")
1193+
Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(clusterRole), clusterRole)).To(HaveOccurred())
1194+
})
1195+
1196+
It("Should delete existing RoleBinding when ClusterRoleBinding is reconciled", func() {
1197+
1198+
By("Reconcile RoleBinding")
1199+
sa, err := r.reconcileRolloutsServiceAccount(ctx, a)
1200+
Expect(err).ToNot(HaveOccurred())
1201+
role, err := r.reconcileRolloutsRole(ctx, a)
1202+
Expect(err).ToNot(HaveOccurred())
1203+
Expect(r.reconcileRolloutsRoleBinding(ctx, a, role, sa)).To(Succeed())
1204+
1205+
By("Verify RoleBinding is created")
1206+
roleBinding := &rbacv1.RoleBinding{ObjectMeta: metav1.ObjectMeta{Name: DefaultArgoRolloutsResourceName, Namespace: a.Namespace}}
1207+
Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(roleBinding), roleBinding)).To(Succeed())
1208+
1209+
By("Reconcile ClusterRoleBinding")
1210+
clusterRole, err := r.reconcileRolloutsClusterRole(ctx, a)
1211+
Expect(err).ToNot(HaveOccurred())
1212+
Expect(r.reconcileRolloutsClusterRoleBinding(ctx, clusterRole, sa, a)).To(Succeed())
1213+
1214+
By("Verify ClusterRoleBinding is created")
1215+
clusterRoleBinding := &rbacv1.ClusterRoleBinding{ObjectMeta: metav1.ObjectMeta{Name: DefaultArgoRolloutsResourceName}}
1216+
Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(clusterRoleBinding), clusterRoleBinding)).To(Succeed())
1217+
1218+
By("Verify RoleBinding is deleted")
1219+
Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(roleBinding), roleBinding)).To(HaveOccurred())
1220+
})
1221+
1222+
It("Should delete existing ClusterRoleBinding when RoleBinding is reconciled", func() {
1223+
1224+
By("Reconcile ClusterRoleBinding")
1225+
sa, err := r.reconcileRolloutsServiceAccount(ctx, a)
1226+
Expect(err).ToNot(HaveOccurred())
1227+
clusterRole, err := r.reconcileRolloutsClusterRole(ctx, a)
1228+
Expect(err).ToNot(HaveOccurred())
1229+
Expect(r.reconcileRolloutsClusterRoleBinding(ctx, clusterRole, sa, a)).To(Succeed())
1230+
1231+
By("Verify ClusterRoleBinding is created")
1232+
clusterRoleBinding := &rbacv1.ClusterRoleBinding{ObjectMeta: metav1.ObjectMeta{Name: DefaultArgoRolloutsResourceName}}
1233+
Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(clusterRoleBinding), clusterRoleBinding)).To(Succeed())
1234+
1235+
By("Reconcile RoleBinding")
1236+
role, err := r.reconcileRolloutsRole(ctx, a)
1237+
Expect(err).ToNot(HaveOccurred())
1238+
Expect(r.reconcileRolloutsRoleBinding(ctx, a, role, sa)).To(Succeed())
1239+
1240+
By("Verify RoleBinding is created")
1241+
roleBinding := &rbacv1.RoleBinding{ObjectMeta: metav1.ObjectMeta{Name: DefaultArgoRolloutsResourceName, Namespace: a.Namespace}}
1242+
Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(roleBinding), roleBinding)).To(Succeed())
1243+
1244+
By("Verify ClusterRoleBinding is deleted")
1245+
Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(clusterRole), clusterRole)).To(HaveOccurred())
1246+
})
1247+
})
11421248
})
11431249

11441250
func serviceMonitor() *monitoringv1.ServiceMonitor {

0 commit comments

Comments
 (0)