Skip to content

Commit cce0b4b

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #706 from openshift-bot/synchronize-upstream
NO-ISSUE: Synchronize From Upstream Repositories
2 parents b3bfaa4 + 978852d commit cce0b4b

File tree

18 files changed

+588
-437
lines changed

18 files changed

+588
-437
lines changed

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

Lines changed: 48 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import (
1212
apierrors "k8s.io/apimachinery/pkg/api/errors"
1313
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1414
"k8s.io/apimachinery/pkg/util/intstr"
15+
corev1ac "k8s.io/client-go/applyconfigurations/core/v1"
16+
rbacv1ac "k8s.io/client-go/applyconfigurations/rbac/v1"
1517

1618
"github.com/operator-framework/api/pkg/operators/v1alpha1"
1719
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/certs"
@@ -245,43 +247,33 @@ func CalculateCertRotatesAt(certExpirationTime time.Time) time.Time {
245247
func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deploymentName string, ca *certs.KeyPair, expiration time.Time, depSpec appsv1.DeploymentSpec, ports []corev1.ServicePort) (*appsv1.DeploymentSpec, []byte, error) {
246248
logger := log.WithFields(log.Fields{})
247249

248-
// Create a service for the deployment
249-
service := &corev1.Service{
250-
Spec: corev1.ServiceSpec{
251-
Ports: ports,
252-
Selector: depSpec.Selector.MatchLabels,
253-
},
254-
}
255-
service.SetName(ServiceName(deploymentName))
256-
service.SetNamespace(i.owner.GetNamespace())
257-
ownerutil.AddNonBlockingOwner(service, i.owner)
258-
service.SetLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue})
259-
260-
existingService, err := i.strategyClient.GetOpLister().CoreV1().ServiceLister().Services(i.owner.GetNamespace()).Get(service.GetName())
261-
if err == nil {
262-
if !ownerutil.Adoptable(i.owner, existingService.GetOwnerReferences()) {
263-
return nil, nil, fmt.Errorf("service %s not safe to replace: extraneous ownerreferences found", service.GetName())
264-
}
265-
service.SetOwnerReferences(existingService.GetOwnerReferences())
266-
267-
// Delete the Service to replace
268-
deleteErr := i.strategyClient.GetOpClient().DeleteService(service.GetNamespace(), service.GetName(), &metav1.DeleteOptions{})
269-
if deleteErr != nil && !apierrors.IsNotFound(deleteErr) {
270-
return nil, nil, fmt.Errorf("could not delete existing service %s", service.GetName())
271-
}
272-
}
273-
274-
// Attempt to create the Service
275-
_, err = i.strategyClient.GetOpClient().CreateService(service)
276-
if err != nil {
277-
logger.Warnf("could not create service %s", service.GetName())
278-
return nil, nil, fmt.Errorf("could not create service %s: %s", service.GetName(), err.Error())
250+
// apply Service
251+
serviceName := ServiceName(deploymentName)
252+
portsApplyConfig := []*corev1ac.ServicePortApplyConfiguration{}
253+
for _, p := range ports {
254+
ac := corev1ac.ServicePort().
255+
WithName(p.Name).
256+
WithPort(p.Port).
257+
WithTargetPort(p.TargetPort)
258+
portsApplyConfig = append(portsApplyConfig, ac)
259+
}
260+
261+
svcApplyConfig := corev1ac.Service(serviceName, i.owner.GetNamespace()).
262+
WithSpec(corev1ac.ServiceSpec().
263+
WithPorts(portsApplyConfig...).
264+
WithSelector(depSpec.Selector.MatchLabels)).
265+
WithOwnerReferences(ownerutil.NonBlockingOwnerApplyConfiguration(i.owner)).
266+
WithLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue})
267+
268+
if _, err := i.strategyClient.GetOpClient().ApplyService(svcApplyConfig, metav1.ApplyOptions{Force: true, FieldManager: "olm.install"}); err != nil {
269+
log.Errorf("could not apply service %s: %s", *svcApplyConfig.Name, err.Error())
270+
return nil, nil, err
279271
}
280272

281273
// Create signed serving cert
282274
hosts := []string{
283-
fmt.Sprintf("%s.%s", service.GetName(), i.owner.GetNamespace()),
284-
fmt.Sprintf("%s.%s.svc", service.GetName(), i.owner.GetNamespace()),
275+
fmt.Sprintf("%s.%s", serviceName, i.owner.GetNamespace()),
276+
fmt.Sprintf("%s.%s.svc", serviceName, i.owner.GetNamespace()),
285277
}
286278
servingPair, err := certGenerator.Generate(expiration, Organization, ca, hosts)
287279
if err != nil {
@@ -292,14 +284,14 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo
292284
// Create Secret for serving cert
293285
certPEM, privPEM, err := servingPair.ToPEM()
294286
if err != nil {
295-
logger.Warnf("unable to convert serving certificate and private key to PEM format for Service %s", service.GetName())
287+
logger.Warnf("unable to convert serving certificate and private key to PEM format for Service %s", serviceName)
296288
return nil, nil, err
297289
}
298290

299291
// Add olmcahash as a label to the caPEM
300292
caPEM, _, err := ca.ToPEM()
301293
if err != nil {
302-
logger.Warnf("unable to convert CA certificate to PEM format for Service %s", service)
294+
logger.Warnf("unable to convert CA certificate to PEM format for Service %s", serviceName)
303295
return nil, nil, err
304296
}
305297
caHash := certs.PEMSHA256(caPEM)
@@ -317,7 +309,7 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo
317309
},
318310
Type: corev1.SecretTypeTLS,
319311
}
320-
secret.SetName(SecretName(service.GetName()))
312+
secret.SetName(SecretName(serviceName))
321313
secret.SetNamespace(i.owner.GetNamespace())
322314
secret.SetLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue})
323315
secret.SetAnnotations(annotations)
@@ -449,51 +441,25 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo
449441
return nil, nil, err
450442
}
451443

452-
// create ClusterRoleBinding to system:auth-delegator Role
453-
authDelegatorClusterRoleBinding := &rbacv1.ClusterRoleBinding{
454-
Subjects: []rbacv1.Subject{
455-
{
456-
Kind: "ServiceAccount",
457-
APIGroup: "",
458-
Name: depSpec.Template.Spec.ServiceAccountName,
459-
Namespace: i.owner.GetNamespace(),
460-
},
461-
},
462-
RoleRef: rbacv1.RoleRef{
463-
APIGroup: "rbac.authorization.k8s.io",
464-
Kind: "ClusterRole",
465-
Name: "system:auth-delegator",
466-
},
467-
}
468-
authDelegatorClusterRoleBinding.SetName(AuthDelegatorClusterRoleBindingName(service.GetName()))
469-
authDelegatorClusterRoleBinding.SetLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue})
470-
471-
existingAuthDelegatorClusterRoleBinding, err := i.strategyClient.GetOpLister().RbacV1().ClusterRoleBindingLister().Get(authDelegatorClusterRoleBinding.GetName())
472-
if err == nil {
473-
// Check if the only owners are this CSV or in this CSV's replacement chain.
474-
if ownerutil.AdoptableLabels(existingAuthDelegatorClusterRoleBinding.GetLabels(), true, i.owner) {
475-
logger.WithFields(log.Fields{"obj": "authDelegatorCRB", "labels": existingAuthDelegatorClusterRoleBinding.GetLabels()}).Debug("adopting")
476-
if err := ownerutil.AddOwnerLabels(authDelegatorClusterRoleBinding, i.owner); err != nil {
477-
return nil, nil, err
478-
}
479-
}
480-
481-
// Attempt an update.
482-
if _, err := i.strategyClient.GetOpClient().UpdateClusterRoleBinding(authDelegatorClusterRoleBinding); err != nil {
483-
logger.Warnf("could not update auth delegator clusterrolebinding %s", authDelegatorClusterRoleBinding.GetName())
484-
return nil, nil, err
485-
}
486-
} else if apierrors.IsNotFound(err) {
487-
// Create the role.
488-
if err := ownerutil.AddOwnerLabels(authDelegatorClusterRoleBinding, i.owner); err != nil {
489-
return nil, nil, err
490-
}
491-
_, err = i.strategyClient.GetOpClient().CreateClusterRoleBinding(authDelegatorClusterRoleBinding)
492-
if err != nil {
493-
log.Warnf("could not create auth delegator clusterrolebinding %s", authDelegatorClusterRoleBinding.GetName())
494-
return nil, nil, err
495-
}
496-
} else {
444+
// apply ClusterRoleBinding to system:auth-delegator Role
445+
crbLabels := map[string]string{OLMManagedLabelKey: OLMManagedLabelValue}
446+
for key, val := range ownerutil.OwnerLabel(i.owner, i.owner.GetObjectKind().GroupVersionKind().Kind) {
447+
crbLabels[key] = val
448+
}
449+
crbApplyConfig := rbacv1ac.ClusterRoleBinding(AuthDelegatorClusterRoleBindingName(serviceName)).
450+
WithSubjects(rbacv1ac.Subject().
451+
WithKind("ServiceAccount").
452+
WithAPIGroup("").
453+
WithName(depSpec.Template.Spec.ServiceAccountName).
454+
WithNamespace(i.owner.GetNamespace())).
455+
WithRoleRef(rbacv1ac.RoleRef().
456+
WithAPIGroup("rbac.authorization.k8s.io").
457+
WithKind("ClusterRole").
458+
WithName("system:auth-delegator")).
459+
WithLabels(crbLabels)
460+
461+
if _, err = i.strategyClient.GetOpClient().ApplyClusterRoleBinding(crbApplyConfig, metav1.ApplyOptions{Force: true, FieldManager: "olm.install"}); err != nil {
462+
log.Errorf("could not apply auth delegator clusterrolebinding %s: %s", *crbApplyConfig.Name, err.Error())
497463
return nil, nil, err
498464
}
499465

@@ -513,7 +479,7 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo
513479
Name: "extension-apiserver-authentication-reader",
514480
},
515481
}
516-
authReaderRoleBinding.SetName(AuthReaderRoleBindingName(service.GetName()))
482+
authReaderRoleBinding.SetName(AuthReaderRoleBindingName(serviceName))
517483
authReaderRoleBinding.SetNamespace(KubeSystem)
518484
authReaderRoleBinding.SetLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue})
519485

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

Lines changed: 107 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import (
1616
"k8s.io/apimachinery/pkg/api/errors"
1717
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1818
"k8s.io/apimachinery/pkg/runtime/schema"
19+
corev1ac "k8s.io/client-go/applyconfigurations/core/v1"
20+
rbacv1ac "k8s.io/client-go/applyconfigurations/rbac/v1"
1921

2022
"github.com/operator-framework/api/pkg/operators/v1alpha1"
2123
"github.com/operator-framework/operator-lifecycle-manager/pkg/api/wrappers"
@@ -153,7 +155,6 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
153155
{
154156
name: "adds certs to deployment spec",
155157
mockExternal: func(mockOpClient *operatorclientmocks.MockClientInterface, fakeLister *operatorlisterfakes.FakeOperatorLister, namespace string, args args) {
156-
mockOpClient.EXPECT().DeleteService(namespace, "test-service", &metav1.DeleteOptions{}).Return(nil)
157158
service := corev1.Service{
158159
ObjectMeta: metav1.ObjectMeta{
159160
Name: "test-service",
@@ -167,7 +168,24 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
167168
Selector: selector(t, "test=label").MatchLabels,
168169
},
169170
}
170-
mockOpClient.EXPECT().CreateService(&service).Return(&service, nil)
171+
172+
portsApplyConfig := []*corev1ac.ServicePortApplyConfiguration{}
173+
for _, p := range args.ports {
174+
ac := corev1ac.ServicePort().
175+
WithName(p.Name).
176+
WithPort(p.Port).
177+
WithTargetPort(p.TargetPort)
178+
portsApplyConfig = append(portsApplyConfig, ac)
179+
}
180+
181+
svcApplyConfig := corev1ac.Service(service.Name, service.Namespace).
182+
WithSpec(corev1ac.ServiceSpec().
183+
WithPorts(portsApplyConfig...).
184+
WithSelector(selector(t, "test=label").MatchLabels)).
185+
WithOwnerReferences(ownerutil.NonBlockingOwnerApplyConfiguration(&v1alpha1.ClusterServiceVersion{})).
186+
WithLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue})
187+
188+
mockOpClient.EXPECT().ApplyService(svcApplyConfig, metav1.ApplyOptions{Force: true, FieldManager: "olm.install"}).Return(&service, nil)
171189

172190
hosts := []string{
173191
fmt.Sprintf("%s.%s", service.GetName(), namespace),
@@ -255,7 +273,22 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
255273
},
256274
}
257275

258-
mockOpClient.EXPECT().UpdateClusterRoleBinding(authDelegatorClusterRoleBinding).Return(authDelegatorClusterRoleBinding, nil)
276+
crbLabels := map[string]string{OLMManagedLabelKey: OLMManagedLabelValue}
277+
for key, val := range ownerutil.OwnerLabel(ownerutil.Owner(&v1alpha1.ClusterServiceVersion{}), owner.GetObjectKind().GroupVersionKind().Kind) {
278+
crbLabels[key] = val
279+
}
280+
crbApplyConfig := rbacv1ac.ClusterRoleBinding(AuthDelegatorClusterRoleBindingName(service.GetName())).
281+
WithSubjects(rbacv1ac.Subject().
282+
WithKind("ServiceAccount").
283+
WithAPIGroup("").
284+
WithName(args.depSpec.Template.Spec.ServiceAccountName).
285+
WithNamespace("")). // Empty owner with no namespace
286+
WithRoleRef(rbacv1ac.RoleRef().
287+
WithAPIGroup("rbac.authorization.k8s.io").
288+
WithKind("ClusterRole").
289+
WithName("system:auth-delegator")).
290+
WithLabels(crbLabels)
291+
mockOpClient.EXPECT().ApplyClusterRoleBinding(crbApplyConfig, metav1.ApplyOptions{Force: true, FieldManager: "olm.install"}).Return(authDelegatorClusterRoleBinding, nil)
259292

260293
authReaderRoleBinding := &rbacv1.RoleBinding{
261294
Subjects: []rbacv1.Subject{
@@ -381,7 +414,6 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
381414
{
382415
name: "doesn't add duplicate service ownerrefs",
383416
mockExternal: func(mockOpClient *operatorclientmocks.MockClientInterface, fakeLister *operatorlisterfakes.FakeOperatorLister, namespace string, args args) {
384-
mockOpClient.EXPECT().DeleteService(namespace, "test-service", &metav1.DeleteOptions{}).Return(nil)
385417
service := corev1.Service{
386418
ObjectMeta: metav1.ObjectMeta{
387419
Name: "test-service",
@@ -396,7 +428,23 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
396428
Selector: selector(t, "test=label").MatchLabels,
397429
},
398430
}
399-
mockOpClient.EXPECT().CreateService(&service).Return(&service, nil)
431+
portsApplyConfig := []*corev1ac.ServicePortApplyConfiguration{}
432+
for _, p := range args.ports {
433+
ac := corev1ac.ServicePort().
434+
WithName(p.Name).
435+
WithPort(p.Port).
436+
WithTargetPort(p.TargetPort)
437+
portsApplyConfig = append(portsApplyConfig, ac)
438+
}
439+
440+
svcApplyConfig := corev1ac.Service(service.Name, service.Namespace).
441+
WithSpec(corev1ac.ServiceSpec().
442+
WithPorts(portsApplyConfig...).
443+
WithSelector(selector(t, "test=label").MatchLabels)).
444+
WithOwnerReferences(ownerutil.NonBlockingOwnerApplyConfiguration(owner)).
445+
WithLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue})
446+
447+
mockOpClient.EXPECT().ApplyService(svcApplyConfig, metav1.ApplyOptions{Force: true, FieldManager: "olm.install"}).Return(&service, nil)
400448

401449
hosts := []string{
402450
fmt.Sprintf("%s.%s", service.GetName(), namespace),
@@ -484,7 +532,23 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
484532
},
485533
}
486534

487-
mockOpClient.EXPECT().UpdateClusterRoleBinding(authDelegatorClusterRoleBinding).Return(authDelegatorClusterRoleBinding, nil)
535+
crbLabels := map[string]string{OLMManagedLabelKey: OLMManagedLabelValue}
536+
for key, val := range ownerutil.OwnerLabel(owner, owner.GetObjectKind().GroupVersionKind().Kind) {
537+
crbLabels[key] = val
538+
}
539+
crbApplyConfig := rbacv1ac.ClusterRoleBinding(service.GetName() + "-system:auth-delegator").
540+
WithSubjects(rbacv1ac.Subject().
541+
WithKind("ServiceAccount").
542+
WithAPIGroup("").
543+
WithName("test-sa").
544+
WithNamespace(namespace)).
545+
WithRoleRef(rbacv1ac.RoleRef().
546+
WithAPIGroup("rbac.authorization.k8s.io").
547+
WithKind("ClusterRole").
548+
WithName("system:auth-delegator")).
549+
WithLabels(crbLabels)
550+
551+
mockOpClient.EXPECT().ApplyClusterRoleBinding(crbApplyConfig, metav1.ApplyOptions{Force: true, FieldManager: "olm.install"}).Return(authDelegatorClusterRoleBinding, nil)
488552

489553
authReaderRoleBinding := &rbacv1.RoleBinding{
490554
Subjects: []rbacv1.Subject{
@@ -602,9 +666,8 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
602666
},
603667
},
604668
{
605-
name: "labels an unlabelled secret if present",
669+
name: "labels an unlabelled secret if present; creates Service and ClusterRoleBinding if not existing",
606670
mockExternal: func(mockOpClient *operatorclientmocks.MockClientInterface, fakeLister *operatorlisterfakes.FakeOperatorLister, namespace string, args args) {
607-
mockOpClient.EXPECT().DeleteService(namespace, "test-service", &metav1.DeleteOptions{}).Return(nil)
608671
service := corev1.Service{
609672
ObjectMeta: metav1.ObjectMeta{
610673
Name: "test-service",
@@ -618,7 +681,24 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
618681
Selector: selector(t, "test=label").MatchLabels,
619682
},
620683
}
621-
mockOpClient.EXPECT().CreateService(&service).Return(&service, nil)
684+
685+
portsApplyConfig := []*corev1ac.ServicePortApplyConfiguration{}
686+
for _, p := range args.ports {
687+
ac := corev1ac.ServicePort().
688+
WithName(p.Name).
689+
WithPort(p.Port).
690+
WithTargetPort(p.TargetPort)
691+
portsApplyConfig = append(portsApplyConfig, ac)
692+
}
693+
694+
svcApplyConfig := corev1ac.Service(service.Name, service.Namespace).
695+
WithSpec(corev1ac.ServiceSpec().
696+
WithPorts(portsApplyConfig...).
697+
WithSelector(selector(t, "test=label").MatchLabels)).
698+
WithOwnerReferences(ownerutil.NonBlockingOwnerApplyConfiguration(&v1alpha1.ClusterServiceVersion{})).
699+
WithLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue})
700+
701+
mockOpClient.EXPECT().ApplyService(svcApplyConfig, metav1.ApplyOptions{Force: true, FieldManager: "olm.install"}).Return(&service, nil)
622702

623703
hosts := []string{
624704
fmt.Sprintf("%s.%s", service.GetName(), namespace),
@@ -715,8 +795,22 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
715795
Name: "system:auth-delegator",
716796
},
717797
}
718-
719-
mockOpClient.EXPECT().UpdateClusterRoleBinding(authDelegatorClusterRoleBinding).Return(authDelegatorClusterRoleBinding, nil)
798+
crbLabels := map[string]string{OLMManagedLabelKey: OLMManagedLabelValue}
799+
for key, val := range ownerutil.OwnerLabel(ownerutil.Owner(&v1alpha1.ClusterServiceVersion{}), owner.GetObjectKind().GroupVersionKind().Kind) {
800+
crbLabels[key] = val
801+
}
802+
crbApplyConfig := rbacv1ac.ClusterRoleBinding(AuthDelegatorClusterRoleBindingName(service.GetName())).
803+
WithSubjects(rbacv1ac.Subject().WithKind("ServiceAccount").
804+
WithAPIGroup("").
805+
WithName("test-sa").
806+
WithNamespace(namespace)).
807+
WithRoleRef(rbacv1ac.RoleRef().
808+
WithAPIGroup("rbac.authorization.k8s.io").
809+
WithKind("ClusterRole").
810+
WithName("system:auth-delegator")).
811+
WithLabels(crbLabels)
812+
813+
mockOpClient.EXPECT().ApplyClusterRoleBinding(crbApplyConfig, metav1.ApplyOptions{Force: true, FieldManager: "olm.install"}).Return(authDelegatorClusterRoleBinding, nil)
720814

721815
authReaderRoleBinding := &rbacv1.RoleBinding{
722816
Subjects: []rbacv1.Subject{
@@ -740,13 +834,7 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
740834
mockOpClient.EXPECT().UpdateRoleBinding(authReaderRoleBinding).Return(authReaderRoleBinding, nil)
741835
},
742836
state: fakeState{
743-
existingService: &corev1.Service{
744-
ObjectMeta: metav1.ObjectMeta{
745-
OwnerReferences: []metav1.OwnerReference{
746-
ownerutil.NonBlockingOwner(&v1alpha1.ClusterServiceVersion{}),
747-
},
748-
},
749-
},
837+
existingService: nil,
750838
// unlabelled secret won't be in cache
751839
getSecretError: errors.NewNotFound(schema.GroupResource{
752840
Group: "",
@@ -758,9 +846,7 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
758846
existingRoleBinding: &rbacv1.RoleBinding{
759847
ObjectMeta: metav1.ObjectMeta{},
760848
},
761-
existingClusterRoleBinding: &rbacv1.ClusterRoleBinding{
762-
ObjectMeta: metav1.ObjectMeta{},
763-
},
849+
existingClusterRoleBinding: nil,
764850
},
765851
fields: fields{
766852
owner: &v1alpha1.ClusterServiceVersion{},

0 commit comments

Comments
 (0)