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