RoleBinding SSA (#3190)

Switch to SSA for RoleBindings during install (as we do now for Services and ClusterRoleBindings) to avoid issues with race conditions and/or failures to retrieve resources due to missing labels.

Signed-off-by: Daniel Franz <[email protected]>
Upstream-repository: operator-lifecycle-manager
Upstream-commit: 6b2b933f7fb5dd1fd6fe7da967f3f22a27c4253c

Author: dtfranz

staging/operator-lifecycle-manager/pkg/controller/install/certresources.go

@@ -463,53 +463,24 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo
 		return nil, nil, err
 	}
 
-	// Create RoleBinding to extension-apiserver-authentication-reader Role in the kube-system namespace.
-	authReaderRoleBinding := &rbacv1.RoleBinding{
-		Subjects: []rbacv1.Subject{
-			{
-				Kind:      "ServiceAccount",
-				APIGroup:  "",
-				Name:      depSpec.Template.Spec.ServiceAccountName,
-				Namespace: i.owner.GetNamespace(),
-			},
-		},
-		RoleRef: rbacv1.RoleRef{
-			APIGroup: "rbac.authorization.k8s.io",
-			Kind:     "Role",
-			Name:     "extension-apiserver-authentication-reader",
-		},
-	}
-	authReaderRoleBinding.SetName(AuthReaderRoleBindingName(serviceName))
-	authReaderRoleBinding.SetNamespace(KubeSystem)
-	authReaderRoleBinding.SetLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue})
+	// Apply RoleBinding to extension-apiserver-authentication-reader Role in the kube-system namespace.
+	authReaderRoleBindingApplyConfig := rbacv1ac.RoleBinding(AuthReaderRoleBindingName(serviceName), KubeSystem).
+		WithLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue}).
+		WithSubjects(rbacv1ac.Subject().
+			WithKind("ServiceAccount").
+			WithAPIGroup("").
+			WithName(depSpec.Template.Spec.ServiceAccountName).
+			WithNamespace(i.owner.GetNamespace())).
+		WithRoleRef(rbacv1ac.RoleRef().
+			WithAPIGroup("rbac.authorization.k8s.io").
+			WithKind("Role").
+			WithName("extension-apiserver-authentication-reader"))
 
-	existingAuthReaderRoleBinding, err := i.strategyClient.GetOpLister().RbacV1().RoleBindingLister().RoleBindings(KubeSystem).Get(authReaderRoleBinding.GetName())
-	if err == nil {
-		// Check if the only owners are this CSV or in this CSV's replacement chain.
-		if ownerutil.AdoptableLabels(existingAuthReaderRoleBinding.GetLabels(), true, i.owner) {
-			logger.WithFields(log.Fields{"obj": "existingAuthReaderRB", "labels": existingAuthReaderRoleBinding.GetLabels()}).Debug("adopting")
-			if err := ownerutil.AddOwnerLabels(authReaderRoleBinding, i.owner); err != nil {
-				return nil, nil, err
-			}
-		}
-		// Attempt an update.
-		if _, err := i.strategyClient.GetOpClient().UpdateRoleBinding(authReaderRoleBinding); err != nil {
-			logger.Warnf("could not update auth reader role binding %s", authReaderRoleBinding.GetName())
-			return nil, nil, err
-		}
-	} else if apierrors.IsNotFound(err) {
-		// Create the role.
-		if err := ownerutil.AddOwnerLabels(authReaderRoleBinding, i.owner); err != nil {
-			return nil, nil, err
-		}
-		_, err = i.strategyClient.GetOpClient().CreateRoleBinding(authReaderRoleBinding)
-		if err != nil {
-			log.Warnf("could not create auth reader role binding %s", authReaderRoleBinding.GetName())
-			return nil, nil, err
-		}
-	} else {
+	if _, err = i.strategyClient.GetOpClient().ApplyRoleBinding(authReaderRoleBindingApplyConfig, metav1.ApplyOptions{Force: true, FieldManager: "olm.install"}); err != nil {
+		log.Errorf("could not apply auth reader rolebinding %s: %s", *authReaderRoleBindingApplyConfig.Name, err.Error())
 		return nil, nil, err
 	}
+
 	AddDefaultCertVolumeAndVolumeMounts(&depSpec, secret.GetName())
 
 	// Setting the olm hash label forces a rollout and ensures that the new secret

staging/operator-lifecycle-manager/pkg/controller/install/certresources_test.go

@@ -309,7 +309,19 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
 				authReaderRoleBinding.SetNamespace(KubeSystem)
 				authReaderRoleBinding.SetLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue})
 
-				mockOpClient.EXPECT().UpdateRoleBinding(authReaderRoleBinding).Return(authReaderRoleBinding, nil)
+				authReaderRoleBindingApplyConfig := rbacv1ac.RoleBinding(AuthReaderRoleBindingName(service.GetName()), KubeSystem).
+					WithLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue}).
+					WithSubjects(rbacv1ac.Subject().
+						WithKind("ServiceAccount").
+						WithAPIGroup("").
+						WithName(args.depSpec.Template.Spec.ServiceAccountName).
+						WithNamespace(namespace)).
+					WithRoleRef(rbacv1ac.RoleRef().
+						WithAPIGroup("rbac.authorization.k8s.io").
+						WithKind("Role").
+						WithName("extension-apiserver-authentication-reader"))
+
+				mockOpClient.EXPECT().ApplyRoleBinding(authReaderRoleBindingApplyConfig, metav1.ApplyOptions{Force: true, FieldManager: "olm.install"}).Return(authReaderRoleBinding, nil)
 			},
 			state: fakeState{
 				existingService: &corev1.Service{
@@ -569,7 +581,19 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
 				authReaderRoleBinding.SetNamespace(KubeSystem)
 				authReaderRoleBinding.SetLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue})
 
-				mockOpClient.EXPECT().UpdateRoleBinding(authReaderRoleBinding).Return(authReaderRoleBinding, nil)
+				authReaderRoleBindingApplyConfig := rbacv1ac.RoleBinding(AuthReaderRoleBindingName(service.GetName()), KubeSystem).
+					WithLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue}).
+					WithSubjects(rbacv1ac.Subject().
+						WithKind("ServiceAccount").
+						WithAPIGroup("").
+						WithName(args.depSpec.Template.Spec.ServiceAccountName).
+						WithNamespace(namespace)).
+					WithRoleRef(rbacv1ac.RoleRef().
+						WithAPIGroup("rbac.authorization.k8s.io").
+						WithKind("Role").
+						WithName("extension-apiserver-authentication-reader"))
+
+				mockOpClient.EXPECT().ApplyRoleBinding(authReaderRoleBindingApplyConfig, metav1.ApplyOptions{Force: true, FieldManager: "olm.install"}).Return(authReaderRoleBinding, nil)
 			},
 			state: fakeState{
 				existingService: &corev1.Service{
@@ -831,7 +855,19 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
 				authReaderRoleBinding.SetNamespace(KubeSystem)
 				authReaderRoleBinding.SetLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue})
 
-				mockOpClient.EXPECT().UpdateRoleBinding(authReaderRoleBinding).Return(authReaderRoleBinding, nil)
+				authReaderRoleBindingApplyConfig := rbacv1ac.RoleBinding(AuthReaderRoleBindingName(service.GetName()), KubeSystem).
+					WithLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue}).
+					WithSubjects(rbacv1ac.Subject().
+						WithKind("ServiceAccount").
+						WithAPIGroup("").
+						WithName(args.depSpec.Template.Spec.ServiceAccountName).
+						WithNamespace(namespace)).
+					WithRoleRef(rbacv1ac.RoleRef().
+						WithAPIGroup("rbac.authorization.k8s.io").
+						WithKind("Role").
+						WithName("extension-apiserver-authentication-reader"))
+
+				mockOpClient.EXPECT().ApplyRoleBinding(authReaderRoleBindingApplyConfig, metav1.ApplyOptions{Force: true, FieldManager: "olm.install"}).Return(authReaderRoleBinding, nil)
 			},
 			state: fakeState{
 				existingService: nil,

staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator_test.go

@@ -1511,6 +1511,7 @@ func TestTransitionCSV(t *testing.T) {
 					// Note: Ideally we would not pre-create these objects, but fake client does not support
 					// creation through SSA, see issue here: https://github.com/kubernetes/kubernetes/issues/115598
 					// Once resolved, these objects and others in this file may be removed.
+					roleBinding("a1-service-auth-reader", "kube-system", "extension-apiserver-authentication-reader", "sa", namespace),
 					service("a1-service", namespace, "a1", 80),
 					clusterRoleBinding("a1-service-system:auth-delegator", "system:auth-delegator", "sa", namespace),
 				},
@@ -5985,8 +5986,9 @@ func TestCARotation(t *testing.T) {
 					), defaultTemplateAnnotations), apis("a1.v1.a1Kind"), nil),
 				},
 				clientObjs: []runtime.Object{addAnnotation(defaultOperatorGroup, operatorsv1.OperatorGroupProvidedAPIsAnnotationKey, "c1.v1.g1,a1Kind.v1.a1")},
-				// The service and clusterRoleBinding have been added here as a workaround to fake client not supporting SSA
+				// The rolebinding, service, and clusterRoleBinding have been added here as a workaround to fake client not supporting SSA
 				objs: []runtime.Object{
+					roleBinding("a1-service-auth-reader", "kube-system", "extension-apiserver-authentication-reader", "sa", namespace),
 					service("a1-service", namespace, "a1", 80, ownerReference),
 					clusterRoleBinding("a1-service-system:auth-delegator", "system:auth-delegator", "sa", namespace),
 				},

staging/operator-lifecycle-manager/pkg/lib/operatorclient/client.go

@@ -94,6 +94,7 @@ type RoleClient interface {
 
 // RoleBindingClient contains methods for manipulating RoleBindings.
 type RoleBindingClient interface {
+	ApplyRoleBinding(applyConfig *rbacv1ac.RoleBindingApplyConfiguration, applyOptions metav1.ApplyOptions) (*rbacv1.RoleBinding, error)
 	CreateRoleBinding(*rbacv1.RoleBinding) (*rbacv1.RoleBinding, error)
 	GetRoleBinding(namespace, name string) (*rbacv1.RoleBinding, error)
 	UpdateRoleBinding(modified *rbacv1.RoleBinding) (*rbacv1.RoleBinding, error)

staging/operator-lifecycle-manager/pkg/lib/operatorclient/operatorclientmocks/mock_client.go

@@ -7,9 +7,15 @@ import (
 	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	acv1 "k8s.io/client-go/applyconfigurations/rbac/v1"
 	"k8s.io/klog"
 )
 
+// ApplyRoleBinding applies the roleBinding.
+func (c *Client) ApplyRoleBinding(applyConfig *acv1.RoleBindingApplyConfiguration, applyOptions metav1.ApplyOptions) (*rbacv1.RoleBinding, error) {
+	return c.RbacV1().RoleBindings(*applyConfig.Namespace).Apply(context.TODO(), applyConfig, applyOptions)
+}
+
 // CreateRoleBinding creates the roleBinding.
 func (c *Client) CreateRoleBinding(ig *rbacv1.RoleBinding) (*rbacv1.RoleBinding, error) {
 	return c.RbacV1().RoleBindings(ig.GetNamespace()).Create(context.TODO(), ig, metav1.CreateOptions{})