From 7d5c3733c685206da28bb6250376722d0e685f4e Mon Sep 17 00:00:00 2001 From: Vitalii Parfonov Date: Wed, 1 Jul 2026 18:38:54 +0300 Subject: [PATCH] fix(rbac): handle immutable roleRef in RoleBinding reconciliation RoleBinding.roleRef is immutable in Kubernetes. When the referenced Role name changes (e.g. during upgrade from v6.5 to v6.6), delete and recreate the RoleBinding instead of attempting to update. Signed-off-by: Vitalii Parfonov --- internal/reconcile/rbac.go | 32 +++++++----- internal/reconcile/rbac_test.go | 86 +++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 11 deletions(-) create mode 100644 internal/reconcile/rbac_test.go diff --git a/internal/reconcile/rbac.go b/internal/reconcile/rbac.go index be7962e89..7ab9371d5 100644 --- a/internal/reconcile/rbac.go +++ b/internal/reconcile/rbac.go @@ -28,19 +28,29 @@ func Role(k8Client client.Client, desired *rbacv1.Role) error { } func RoleBinding(k8Client client.Client, desired *rbacv1.RoleBinding) error { - roleBinding := runtime.NewRoleBinding(desired.Namespace, desired.Name, desired.RoleRef) - op, err := controllerutil.CreateOrUpdate(context.TODO(), k8Client, roleBinding, func() error { - // Update the rolebinding with our desired state - roleBinding.RoleRef = desired.RoleRef - roleBinding.Subjects = desired.Subjects - roleBinding.OwnerReferences = desired.OwnerReferences - return nil - }) + existing := runtime.NewRoleBinding(desired.Namespace, desired.Name, rbacv1.RoleRef{}) + err := k8Client.Get(context.TODO(), client.ObjectKeyFromObject(existing), existing) + if apierrors.IsNotFound(err) { + log.V(3).Info("Creating roleBinding", "name", desired.Name, "namespace", desired.Namespace) + return k8Client.Create(context.TODO(), desired) + } + if err != nil { + return err + } - if err == nil { - log.V(3).Info(fmt.Sprintf("reconciled roleBinding - operation: %s", op)) + if existing.RoleRef != desired.RoleRef { + log.V(3).Info("Deleting roleBinding due to roleRef change", "name", desired.Name, "namespace", desired.Namespace) + if err := k8Client.Delete(context.TODO(), existing); err != nil { + return err + } + log.V(3).Info("Recreating roleBinding", "name", desired.Name, "namespace", desired.Namespace) + return k8Client.Create(context.TODO(), desired) } - return err + + existing.Subjects = desired.Subjects + existing.OwnerReferences = desired.OwnerReferences + log.V(3).Info("Updating roleBinding", "name", desired.Name, "namespace", desired.Namespace) + return k8Client.Update(context.TODO(), existing) } func ClusterRoleBinding(k8sClient client.Client, name string, generator func() *rbacv1.ClusterRoleBinding) error { diff --git a/internal/reconcile/rbac_test.go b/internal/reconcile/rbac_test.go new file mode 100644 index 000000000..52f39b3c3 --- /dev/null +++ b/internal/reconcile/rbac_test.go @@ -0,0 +1,86 @@ +package reconcile_test + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/openshift/cluster-logging-operator/internal/reconcile" + "github.com/openshift/cluster-logging-operator/internal/runtime" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8sruntime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +var _ = Describe("reconciling RoleBinding", func() { + + var ( + namespace = "test-namespace" + name = "test-rolebinding" + oldRef = rbacv1.RoleRef{APIGroup: rbacv1.GroupName, Kind: "Role", Name: "old-role"} + newRef = rbacv1.RoleRef{APIGroup: rbacv1.GroupName, Kind: "Role", Name: "new-role"} + subject = rbacv1.Subject{Kind: "ServiceAccount", Name: "test-sa", Namespace: namespace} + ownerRef = metav1.OwnerReference{APIVersion: "v1", Kind: "ConfigMap", Name: "owner", UID: "12345"} + ) + + newClient := func(objs ...client.Object) client.Client { + globalScheme := k8sruntime.NewScheme() + Expect(scheme.AddToScheme(globalScheme)).To(Succeed()) + return fake.NewClientBuilder().WithScheme(globalScheme).WithObjects(objs...).Build() + } + + getRoleBinding := func(k8sClient client.Client) *rbacv1.RoleBinding { + result := &rbacv1.RoleBinding{} + key := client.ObjectKey{Namespace: namespace, Name: name} + Expect(k8sClient.Get(context.TODO(), key, result)).To(Succeed()) + return result + } + + It("should create the RoleBinding when it does not exist", func() { + k8sClient := newClient() + desired := runtime.NewRoleBinding(namespace, name, newRef, subject) + desired.OwnerReferences = []metav1.OwnerReference{ownerRef} + + Expect(reconcile.RoleBinding(k8sClient, desired)).To(Succeed()) + + result := getRoleBinding(k8sClient) + Expect(result.RoleRef).To(Equal(newRef)) + Expect(result.Subjects).To(Equal([]rbacv1.Subject{subject})) + Expect(result.OwnerReferences).To(Equal([]metav1.OwnerReference{ownerRef})) + }) + + It("should update Subjects and OwnerReferences when roleRef is unchanged", func() { + existing := runtime.NewRoleBinding(namespace, name, newRef, + rbacv1.Subject{Kind: "ServiceAccount", Name: "old-sa", Namespace: namespace}) + k8sClient := newClient(existing) + + desired := runtime.NewRoleBinding(namespace, name, newRef, subject) + desired.OwnerReferences = []metav1.OwnerReference{ownerRef} + + Expect(reconcile.RoleBinding(k8sClient, desired)).To(Succeed()) + + result := getRoleBinding(k8sClient) + Expect(result.RoleRef).To(Equal(newRef)) + Expect(result.Subjects).To(Equal([]rbacv1.Subject{subject})) + Expect(result.OwnerReferences).To(Equal([]metav1.OwnerReference{ownerRef})) + }) + + It("should delete and recreate the RoleBinding when roleRef changes", func() { + existing := runtime.NewRoleBinding(namespace, name, oldRef, + rbacv1.Subject{Kind: "ServiceAccount", Name: "old-sa", Namespace: namespace}) + k8sClient := newClient(existing) + + desired := runtime.NewRoleBinding(namespace, name, newRef, subject) + desired.OwnerReferences = []metav1.OwnerReference{ownerRef} + + Expect(reconcile.RoleBinding(k8sClient, desired)).To(Succeed()) + + result := getRoleBinding(k8sClient) + Expect(result.RoleRef).To(Equal(newRef)) + Expect(result.Subjects).To(Equal([]rbacv1.Subject{subject})) + Expect(result.OwnerReferences).To(Equal([]metav1.OwnerReference{ownerRef})) + }) +})