Repository: scylladb/scylla-operator
Pull request #1143 (2023-01-13 08:39:24)

Fix ApplyRoleBinding updates for immutable roleRef
**Description of your changes:**
Fixes `ApplyRoleBinding` to recreate the object on roleRef changes  because `rolebinding.spec.roleRef` is immutable.
Ref https://github.com/kubernetes/kubernetes/blob/0843c4dfcab93dd242044e51d6a2a21dc73d233e/pkg/apis/rbac/validation/validation.go#L165-L167

**Which issue is resolved by this Pull Request:**
Needed by #1111



Patch:
diff --git a/pkg/resourceapply/rbac.go b/pkg/resourceapply/rbac.go
index f092d8d8e..c24c16fac 100644
--- a/pkg/resourceapply/rbac.go
+++ b/pkg/resourceapply/rbac.go
@@ -49,7 +49,20 @@ func ApplyRoleBindingWithControl(
 	required *rbacv1.RoleBinding,
 	options ApplyOptions,
 ) (*rbacv1.RoleBinding, bool, error) {
-	return ApplyGeneric[*rbacv1.RoleBinding](ctx, control, recorder, required, options)
+	return ApplyGenericWithHandlers[*rbacv1.RoleBinding](
+		ctx,
+		control,
+		recorder,
+		required,
+		options,
+		nil,
+		func(required *rbacv1.RoleBinding, existing *rbacv1.RoleBinding) string {
+			if !equality.Semantic.DeepEqual(existing.RoleRef, (*required).RoleRef) {
+				return "roleRef is immutable"
+			}
+			return ""
+		},
+	)
 }
 
 func ApplyRoleBinding(


Test patch:
diff --git a/pkg/resourceapply/rbac_test.go b/pkg/resourceapply/rbac_test.go
index de3f2828c..4e7070c58 100644
--- a/pkg/resourceapply/rbac_test.go
+++ b/pkg/resourceapply/rbac_test.go
@@ -1016,6 +1016,9 @@ func TestApplyRoleBinding(t *testing.T) {
 				ResourceVersion: "42",
 				Labels:          map[string]string{},
 			},
+			RoleRef: rbacv1.RoleRef{
+				Name: "test",
+			},
 		}
 	}
 	newRBWithControllerRef := func() *rbacv1.RoleBinding {
@@ -1112,25 +1115,28 @@ func TestApplyRoleBinding(t *testing.T) {
 			expectedEvents:  nil,
 		},
 		{
-			name: "updates the RB when AutomountRoleBindingToken differ",
+			name: "recreates the RB when roleRef changes",
 			existing: []runtime.Object{
 				newRB(),
 			},
 			allowMissingControllerRef: true,
 			required: func() *rbacv1.RoleBinding {
 				rb := newRB()
-				rb.RoleRef.Name = "foo"
+				rb.RoleRef.Name = "changed"
 				return rb
 			}(),
 			expectedRB: func() *rbacv1.RoleBinding {
 				rb := newRB()
-				rb.RoleRef.Name = "foo"
+				rb.RoleRef.Name = "changed"
 				utilruntime.Must(SetHashAnnotation(rb))
 				return rb
 			}(),
 			expectedChanged: true,
 			expectedErr:     nil,
-			expectedEvents:  []string{"Normal RoleBindingUpdated RoleBinding default/test updated"},
+			expectedEvents: []string{
+				"Normal RoleBindingDeleted RoleBinding default/test deleted",
+				"Normal RoleBindingCreated RoleBinding default/test created",
+			},
 		},
 		{
 			name: "updates the RB if labels differ",
@@ -1159,7 +1165,11 @@ func TestApplyRoleBinding(t *testing.T) {
 				func() *rbacv1.RoleBinding {
 					rb := newRBWithHash()
 					// Simulate admission by changing a value after the hash is computed.
-					rb.RoleRef.Name = "foo"
+					rb.Subjects = []rbacv1.Subject{
+						{
+							Name: "changed",
+						},
+					}
 					return rb
 				}(),
 			},
@@ -1168,7 +1178,11 @@ func TestApplyRoleBinding(t *testing.T) {
 			expectedRB: func() *rbacv1.RoleBinding {
 				rb := newRBWithHash()
 				// Simulate admission by changing a value after the hash is computed.
-				rb.RoleRef.Name = "foo"
+				rb.Subjects = []rbacv1.Subject{
+					{
+						Name: "changed",
+					},
+				}
 				return rb
 			}(),
 			expectedChanged: false,
@@ -1189,13 +1203,13 @@ func TestApplyRoleBinding(t *testing.T) {
 			required: func() *rbacv1.RoleBinding {
 				rb := newRB()
 				rb.ResourceVersion = ""
-				rb.RoleRef.Name = "foo"
+				rb.Labels = map[string]string{"changed": "changed"}
 				return rb
 			}(),
 			expectedRB: func() *rbacv1.RoleBinding {
 				rb := newRB()
 				rb.ResourceVersion = "21"
-				rb.RoleRef.Name = "foo"
+				rb.Labels = map[string]string{"changed": "changed"}
 				utilruntime.Must(SetHashAnnotation(rb))
 				return rb
 			}(),
@@ -1212,7 +1226,7 @@ func TestApplyRoleBinding(t *testing.T) {
 			allowMissingControllerRef: true,
 			required: func() *rbacv1.RoleBinding {
 				rb := newRB()
-				rb.RoleRef.Name = "foo"
+				rb.Labels = map[string]string{"changed": "changed"}
 				return rb
 			}(),
 			expectedRB:      nil,