Skip to content

Commit 56020ad

Browse files
committed
RoleBinding SSA
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]>
1 parent 0f1e945 commit 56020ad

File tree

5 files changed

+91
-47
lines changed

5 files changed

+91
-47
lines changed

Diff for: pkg/controller/install/certresources.go

+15-44
Original file line numberDiff line numberDiff line change
@@ -454,53 +454,24 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo
454454
return nil, nil, err
455455
}
456456

457-
// Create RoleBinding to extension-apiserver-authentication-reader Role in the kube-system namespace.
458-
authReaderRoleBinding := &rbacv1.RoleBinding{
459-
Subjects: []rbacv1.Subject{
460-
{
461-
Kind: "ServiceAccount",
462-
APIGroup: "",
463-
Name: depSpec.Template.Spec.ServiceAccountName,
464-
Namespace: i.owner.GetNamespace(),
465-
},
466-
},
467-
RoleRef: rbacv1.RoleRef{
468-
APIGroup: "rbac.authorization.k8s.io",
469-
Kind: "Role",
470-
Name: "extension-apiserver-authentication-reader",
471-
},
472-
}
473-
authReaderRoleBinding.SetName(AuthReaderRoleBindingName(serviceName))
474-
authReaderRoleBinding.SetNamespace(KubeSystem)
475-
authReaderRoleBinding.SetLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue})
457+
// Apply RoleBinding to extension-apiserver-authentication-reader Role in the kube-system namespace.
458+
authReaderRoleBindingApplyConfig := rbacv1ac.RoleBinding(AuthReaderRoleBindingName(serviceName), KubeSystem).
459+
WithLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue}).
460+
WithSubjects(rbacv1ac.Subject().
461+
WithKind("ServiceAccount").
462+
WithAPIGroup("").
463+
WithName(depSpec.Template.Spec.ServiceAccountName).
464+
WithNamespace(i.owner.GetNamespace())).
465+
WithRoleRef(rbacv1ac.RoleRef().
466+
WithAPIGroup("rbac.authorization.k8s.io").
467+
WithKind("Role").
468+
WithName("extension-apiserver-authentication-reader"))
476469

477-
existingAuthReaderRoleBinding, err := i.strategyClient.GetOpLister().RbacV1().RoleBindingLister().RoleBindings(KubeSystem).Get(authReaderRoleBinding.GetName())
478-
if err == nil {
479-
// Check if the only owners are this CSV or in this CSV's replacement chain.
480-
if ownerutil.AdoptableLabels(existingAuthReaderRoleBinding.GetLabels(), true, i.owner) {
481-
logger.WithFields(log.Fields{"obj": "existingAuthReaderRB", "labels": existingAuthReaderRoleBinding.GetLabels()}).Debug("adopting")
482-
if err := ownerutil.AddOwnerLabels(authReaderRoleBinding, i.owner); err != nil {
483-
return nil, nil, err
484-
}
485-
}
486-
// Attempt an update.
487-
if _, err := i.strategyClient.GetOpClient().UpdateRoleBinding(authReaderRoleBinding); err != nil {
488-
logger.Warnf("could not update auth reader role binding %s", authReaderRoleBinding.GetName())
489-
return nil, nil, err
490-
}
491-
} else if apierrors.IsNotFound(err) {
492-
// Create the role.
493-
if err := ownerutil.AddOwnerLabels(authReaderRoleBinding, i.owner); err != nil {
494-
return nil, nil, err
495-
}
496-
_, err = i.strategyClient.GetOpClient().CreateRoleBinding(authReaderRoleBinding)
497-
if err != nil {
498-
log.Warnf("could not create auth reader role binding %s", authReaderRoleBinding.GetName())
499-
return nil, nil, err
500-
}
501-
} else {
470+
if _, err = i.strategyClient.GetOpClient().ApplyRoleBinding(authReaderRoleBindingApplyConfig, metav1.ApplyOptions{Force: true, FieldManager: "olm.install"}); err != nil {
471+
log.Errorf("could not apply auth reader rolebinding %s: %s", *authReaderRoleBindingApplyConfig.Name, err.Error())
502472
return nil, nil, err
503473
}
474+
504475
AddDefaultCertVolumeAndVolumeMounts(&depSpec, secret.GetName())
505476

506477
// Setting the olm hash label forces a rollout and ensures that the new secret

Diff for: pkg/controller/install/certresources_test.go

+39-3
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,19 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
309309
authReaderRoleBinding.SetNamespace(KubeSystem)
310310
authReaderRoleBinding.SetLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue})
311311

312-
mockOpClient.EXPECT().UpdateRoleBinding(authReaderRoleBinding).Return(authReaderRoleBinding, nil)
312+
authReaderRoleBindingApplyConfig := rbacv1ac.RoleBinding(AuthReaderRoleBindingName(service.GetName()), KubeSystem).
313+
WithLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue}).
314+
WithSubjects(rbacv1ac.Subject().
315+
WithKind("ServiceAccount").
316+
WithAPIGroup("").
317+
WithName(args.depSpec.Template.Spec.ServiceAccountName).
318+
WithNamespace(namespace)).
319+
WithRoleRef(rbacv1ac.RoleRef().
320+
WithAPIGroup("rbac.authorization.k8s.io").
321+
WithKind("Role").
322+
WithName("extension-apiserver-authentication-reader"))
323+
324+
mockOpClient.EXPECT().ApplyRoleBinding(authReaderRoleBindingApplyConfig, metav1.ApplyOptions{Force: true, FieldManager: "olm.install"}).Return(authReaderRoleBinding, nil)
313325
},
314326
state: fakeState{
315327
existingService: &corev1.Service{
@@ -569,7 +581,19 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
569581
authReaderRoleBinding.SetNamespace(KubeSystem)
570582
authReaderRoleBinding.SetLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue})
571583

572-
mockOpClient.EXPECT().UpdateRoleBinding(authReaderRoleBinding).Return(authReaderRoleBinding, nil)
584+
authReaderRoleBindingApplyConfig := rbacv1ac.RoleBinding(AuthReaderRoleBindingName(service.GetName()), KubeSystem).
585+
WithLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue}).
586+
WithSubjects(rbacv1ac.Subject().
587+
WithKind("ServiceAccount").
588+
WithAPIGroup("").
589+
WithName(args.depSpec.Template.Spec.ServiceAccountName).
590+
WithNamespace(namespace)).
591+
WithRoleRef(rbacv1ac.RoleRef().
592+
WithAPIGroup("rbac.authorization.k8s.io").
593+
WithKind("Role").
594+
WithName("extension-apiserver-authentication-reader"))
595+
596+
mockOpClient.EXPECT().ApplyRoleBinding(authReaderRoleBindingApplyConfig, metav1.ApplyOptions{Force: true, FieldManager: "olm.install"}).Return(authReaderRoleBinding, nil)
573597
},
574598
state: fakeState{
575599
existingService: &corev1.Service{
@@ -831,7 +855,19 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
831855
authReaderRoleBinding.SetNamespace(KubeSystem)
832856
authReaderRoleBinding.SetLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue})
833857

834-
mockOpClient.EXPECT().UpdateRoleBinding(authReaderRoleBinding).Return(authReaderRoleBinding, nil)
858+
authReaderRoleBindingApplyConfig := rbacv1ac.RoleBinding(AuthReaderRoleBindingName(service.GetName()), KubeSystem).
859+
WithLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue}).
860+
WithSubjects(rbacv1ac.Subject().
861+
WithKind("ServiceAccount").
862+
WithAPIGroup("").
863+
WithName(args.depSpec.Template.Spec.ServiceAccountName).
864+
WithNamespace(namespace)).
865+
WithRoleRef(rbacv1ac.RoleRef().
866+
WithAPIGroup("rbac.authorization.k8s.io").
867+
WithKind("Role").
868+
WithName("extension-apiserver-authentication-reader"))
869+
870+
mockOpClient.EXPECT().ApplyRoleBinding(authReaderRoleBindingApplyConfig, metav1.ApplyOptions{Force: true, FieldManager: "olm.install"}).Return(authReaderRoleBinding, nil)
835871
},
836872
state: fakeState{
837873
existingService: nil,

Diff for: pkg/lib/operatorclient/client.go

+1
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ type RoleClient interface {
9494

9595
// RoleBindingClient contains methods for manipulating RoleBindings.
9696
type RoleBindingClient interface {
97+
ApplyRoleBinding(applyConfig *rbacv1ac.RoleBindingApplyConfiguration, applyOptions metav1.ApplyOptions) (*rbacv1.RoleBinding, error)
9798
CreateRoleBinding(*rbacv1.RoleBinding) (*rbacv1.RoleBinding, error)
9899
GetRoleBinding(namespace, name string) (*rbacv1.RoleBinding, error)
99100
UpdateRoleBinding(modified *rbacv1.RoleBinding) (*rbacv1.RoleBinding, error)

Diff for: pkg/lib/operatorclient/operatorclientmocks/mock_client.go

+30
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: pkg/lib/operatorclient/rolebinding.go

+6
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,15 @@ import (
77
rbacv1 "k8s.io/api/rbac/v1"
88
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
99
"k8s.io/apimachinery/pkg/types"
10+
acv1 "k8s.io/client-go/applyconfigurations/rbac/v1"
1011
"k8s.io/klog"
1112
)
1213

14+
// ApplyRoleBinding applies the roleBinding.
15+
func (c *Client) ApplyRoleBinding(applyConfig *acv1.RoleBindingApplyConfiguration, applyOptions metav1.ApplyOptions) (*rbacv1.RoleBinding, error) {
16+
return c.RbacV1().RoleBindings(*applyConfig.Namespace).Apply(context.TODO(), applyConfig, applyOptions)
17+
}
18+
1319
// CreateRoleBinding creates the roleBinding.
1420
func (c *Client) CreateRoleBinding(ig *rbacv1.RoleBinding) (*rbacv1.RoleBinding, error) {
1521
return c.RbacV1().RoleBindings(ig.GetNamespace()).Create(context.TODO(), ig, metav1.CreateOptions{})

0 commit comments

Comments
 (0)