Skip to content

Commit 2b5cc29

Browse files
awgreeneawgreene
committed
(fix) Admission Webhook names must be unique
Problem: OLM currently creates all ValidatingWebhookConfigurations and MutatingWebhookConfigurations using the name provided in the WebhookDescription. This causes an issue when an operator that defines an admission webhook in the CSV is installed in multiple namespaces, causes the two operators to fight over the admission webhook. Solution: Generate a unique name for each webhook using the GenerateName field in the Object Metadata.
1 parent 280a2a6 commit 2b5cc29

File tree

5 files changed

+677
-881
lines changed

5 files changed

+677
-881
lines changed

‎pkg/controller/install/webhook.go

Lines changed: 65 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@ package install
33
import (
44
"context"
55
"fmt"
6+
"strings"
67

78
"github.com/operator-framework/api/pkg/operators/v1alpha1"
89
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil"
910

1011
log "github.com/sirupsen/logrus"
1112
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
12-
k8serrors "k8s.io/apimachinery/pkg/api/errors"
1313
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1414
"k8s.io/apimachinery/pkg/labels"
1515
)
@@ -69,80 +69,105 @@ func (i *StrategyDeploymentInstaller) createOrUpdateWebhook(caPEM []byte, desc v
6969
}
7070

7171
func (i *StrategyDeploymentInstaller) createOrUpdateMutatingWebhook(ogNamespacelabelSelector *metav1.LabelSelector, caPEM []byte, desc v1alpha1.WebhookDescription) error {
72-
webhooks := []admissionregistrationv1.MutatingWebhook{
73-
desc.GetMutatingWebhook(i.owner.GetNamespace(), ogNamespacelabelSelector, caPEM),
72+
webhookSelector := labels.SelectorFromSet(ownerutil.OwnerLabel(i.owner, i.owner.GetObjectKind().GroupVersionKind().Kind)).String()
73+
existingWebhooks, err := i.strategyClient.GetOpClient().KubernetesInterface().AdmissionregistrationV1().MutatingWebhookConfigurations().List(context.TODO(), metav1.ListOptions{LabelSelector: webhookSelector})
74+
if err != nil {
75+
return err
76+
}
77+
78+
var webhook *admissionregistrationv1.MutatingWebhookConfiguration
79+
for _, item := range existingWebhooks.Items {
80+
if strings.HasPrefix(item.Name, desc.Name) {
81+
webhook = &item
82+
break
83+
}
7484
}
75-
existingHook, err := i.strategyClient.GetOpClient().KubernetesInterface().AdmissionregistrationV1().MutatingWebhookConfigurations().Get(context.TODO(), desc.Name, metav1.GetOptions{})
76-
if err == nil {
85+
86+
if webhook != nil {
7787
// Check if the only owners are this CSV or in this CSV's replacement chain
78-
if ownerutil.Adoptable(i.owner, existingHook.GetOwnerReferences()) {
79-
ownerutil.AddNonBlockingOwner(existingHook, i.owner)
88+
if ownerutil.AdoptableLabels(webhook.GetLabels(), false, i.owner) {
89+
ownerutil.AddOwnerLabels(webhook, i.owner)
8090
}
8191

8292
// Update the list of webhooks
83-
existingHook.Webhooks = webhooks
93+
webhook.Webhooks = []admissionregistrationv1.MutatingWebhook{
94+
desc.GetMutatingWebhook(i.owner.GetNamespace(), ogNamespacelabelSelector, caPEM),
95+
}
8496

8597
// Attempt an update
86-
if _, err := i.strategyClient.GetOpClient().KubernetesInterface().AdmissionregistrationV1().MutatingWebhookConfigurations().Update(context.TODO(), existingHook, metav1.UpdateOptions{}); err != nil {
87-
log.Warnf("could not update MutatingWebhookConfiguration %s", existingHook.GetName())
98+
if _, err := i.strategyClient.GetOpClient().KubernetesInterface().AdmissionregistrationV1().MutatingWebhookConfigurations().Update(context.TODO(), webhook, metav1.UpdateOptions{}); err != nil {
99+
log.Warnf("could not update MutatingWebhookConfiguration %s", webhook.GetName())
88100
return err
89101
}
90-
} else if k8serrors.IsNotFound(err) {
91-
hook := admissionregistrationv1.MutatingWebhookConfiguration{
92-
ObjectMeta: metav1.ObjectMeta{Name: desc.Name,
93-
Namespace: i.owner.GetNamespace(),
102+
} else {
103+
// Create a ValidatingWebhookConfiguration
104+
hook := admissionregistrationv1.ValidatingWebhookConfiguration{
105+
ObjectMeta: metav1.ObjectMeta{
106+
GenerateName: desc.Name + "-",
107+
Namespace: i.owner.GetNamespace(),
108+
Labels: ownerutil.OwnerLabel(i.owner, i.owner.GetObjectKind().GroupVersionKind().Kind),
109+
},
110+
Webhooks: []admissionregistrationv1.ValidatingWebhook{
111+
desc.GetValidatingWebhook(i.owner.GetNamespace(), ogNamespacelabelSelector, caPEM),
94112
},
95-
Webhooks: webhooks,
96113
}
97-
// Add an owner
98-
ownerutil.AddNonBlockingOwner(&hook, i.owner)
99-
if _, err := i.strategyClient.GetOpClient().KubernetesInterface().AdmissionregistrationV1().MutatingWebhookConfigurations().Create(context.TODO(), &hook, metav1.CreateOptions{}); err != nil {
100-
log.Errorf("Webhooks: Error creating mutating MutatingVebhookConfiguration: %v", err)
114+
115+
if _, err := i.strategyClient.GetOpClient().KubernetesInterface().AdmissionregistrationV1().ValidatingWebhookConfigurations().Create(context.TODO(), &hook, metav1.CreateOptions{}); err != nil {
116+
log.Errorf("Webhooks: Error creating ValidationWebhookConfiguration: %v", err)
101117
return err
102118
}
103-
} else {
104-
return err
105119
}
106-
107120
return nil
108121
}
109122

110123
func (i *StrategyDeploymentInstaller) createOrUpdateValidatingWebhook(ogNamespacelabelSelector *metav1.LabelSelector, caPEM []byte, desc v1alpha1.WebhookDescription) error {
111-
webhooks := []admissionregistrationv1.ValidatingWebhook{
112-
desc.GetValidatingWebhook(i.owner.GetNamespace(), ogNamespacelabelSelector, caPEM),
124+
webhookSelector := labels.SelectorFromSet(ownerutil.OwnerLabel(i.owner, i.owner.GetObjectKind().GroupVersionKind().Kind)).String()
125+
existingWebhooks, err := i.strategyClient.GetOpClient().KubernetesInterface().AdmissionregistrationV1().ValidatingWebhookConfigurations().List(context.TODO(), metav1.ListOptions{LabelSelector: webhookSelector})
126+
if err != nil {
127+
return err
128+
}
129+
130+
var webhook *admissionregistrationv1.ValidatingWebhookConfiguration
131+
for _, item := range existingWebhooks.Items {
132+
if strings.HasPrefix(item.Name, desc.Name) {
133+
webhook = &item
134+
break
135+
}
113136
}
114-
existingHook, err := i.strategyClient.GetOpClient().KubernetesInterface().AdmissionregistrationV1().ValidatingWebhookConfigurations().Get(context.TODO(), desc.Name, metav1.GetOptions{})
115-
if err == nil {
137+
138+
if webhook != nil {
116139
// Check if the only owners are this CSV or in this CSV's replacement chain
117-
if ownerutil.Adoptable(i.owner, existingHook.GetOwnerReferences()) {
118-
ownerutil.AddNonBlockingOwner(existingHook, i.owner)
140+
if ownerutil.AdoptableLabels(webhook.GetLabels(), false, i.owner) {
141+
ownerutil.AddOwnerLabels(webhook, i.owner)
119142
}
120143

121144
// Update the list of webhooks
122-
existingHook.Webhooks = webhooks
145+
webhook.Webhooks = []admissionregistrationv1.ValidatingWebhook{
146+
desc.GetValidatingWebhook(i.owner.GetNamespace(), ogNamespacelabelSelector, caPEM),
147+
}
123148

124149
// Attempt an update
125-
if _, err := i.strategyClient.GetOpClient().KubernetesInterface().AdmissionregistrationV1().ValidatingWebhookConfigurations().Update(context.TODO(), existingHook, metav1.UpdateOptions{}); err != nil {
126-
log.Warnf("could not update ValidatingWebhookConfiguration %s", existingHook.GetName())
150+
if _, err := i.strategyClient.GetOpClient().KubernetesInterface().AdmissionregistrationV1().ValidatingWebhookConfigurations().Update(context.TODO(), webhook, metav1.UpdateOptions{}); err != nil {
151+
log.Warnf("could not update ValidatingWebhookConfiguration %s", webhook.GetName())
127152
return err
128153
}
129-
} else if k8serrors.IsNotFound(err) {
154+
} else {
130155
// Create a ValidatingWebhookConfiguration
131156
hook := admissionregistrationv1.ValidatingWebhookConfiguration{
132-
ObjectMeta: metav1.ObjectMeta{Name: desc.Name,
133-
Namespace: i.owner.GetNamespace(),
157+
ObjectMeta: metav1.ObjectMeta{
158+
GenerateName: desc.Name + "-",
159+
Namespace: i.owner.GetNamespace(),
160+
Labels: ownerutil.OwnerLabel(i.owner, i.owner.GetObjectKind().GroupVersionKind().Kind),
161+
},
162+
Webhooks: []admissionregistrationv1.ValidatingWebhook{
163+
desc.GetValidatingWebhook(i.owner.GetNamespace(), ogNamespacelabelSelector, caPEM),
134164
},
135-
Webhooks: webhooks,
136165
}
137166

138-
// Add an owner
139-
ownerutil.AddNonBlockingOwner(&hook, i.owner)
140167
if _, err := i.strategyClient.GetOpClient().KubernetesInterface().AdmissionregistrationV1().ValidatingWebhookConfigurations().Create(context.TODO(), &hook, metav1.CreateOptions{}); err != nil {
141-
log.Errorf("Webhooks: Error create creating ValidationVebhookConfiguration: %v", err)
168+
log.Errorf("Webhooks: Error creating ValidationWebhookConfiguration: %v", err)
142169
return err
143170
}
144-
} else {
145-
return err
146171
}
147172
return nil
148173
}

‎pkg/controller/operators/olm/apiservices.go

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,22 @@ package olm
33
import (
44
"context"
55
"fmt"
6+
"strings"
67

78
log "github.com/sirupsen/logrus"
89
appsv1 "k8s.io/api/apps/v1"
910
corev1 "k8s.io/api/core/v1"
1011
rbacv1 "k8s.io/api/rbac/v1"
1112
k8serrors "k8s.io/apimachinery/pkg/api/errors"
1213
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
14+
"k8s.io/apimachinery/pkg/labels"
1315
utilerrors "k8s.io/apimachinery/pkg/util/errors"
1416

1517
"github.com/operator-framework/api/pkg/operators/v1alpha1"
1618
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/certs"
1719
olmerrors "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/errors"
1820
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install"
21+
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil"
1922
)
2023

2124
const (
@@ -271,22 +274,33 @@ func (a *Operator) getCaBundle(csv *v1alpha1.ClusterServiceVersion) ([]byte, err
271274
}
272275

273276
for _, desc := range csv.Spec.WebhookDefinitions {
274-
webhookName := desc.Name
275-
if desc.Type == "ValidatingAdmissionWebhook" {
276-
webhook, err := a.opClient.KubernetesInterface().AdmissionregistrationV1().ValidatingWebhookConfigurations().Get(context.TODO(), webhookName, metav1.GetOptions{})
277+
if desc.Type == v1alpha1.MutatingAdmissionWebhook {
278+
ownerLabels := ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind)
279+
ownerLabelStrings := labels.SelectorFromSet(ownerLabels).String()
280+
281+
existingHooks, err := a.opClient.KubernetesInterface().AdmissionregistrationV1().MutatingWebhookConfigurations().List(context.TODO(), metav1.ListOptions{LabelSelector: ownerLabelStrings})
277282
if err != nil {
278-
return nil, fmt.Errorf("could not retrieve generated APIService: %v", err)
283+
return nil, fmt.Errorf("could not retrieve generated MutatingWebhookConfiguration: %v", err)
279284
}
280-
if len(webhook.Webhooks[0].ClientConfig.CABundle) > 0 {
281-
return webhook.Webhooks[0].ClientConfig.CABundle, nil
285+
286+
for _, item := range existingHooks.Items {
287+
if strings.HasPrefix(item.Name, desc.Name) {
288+
return item.Webhooks[0].ClientConfig.CABundle, nil
289+
}
282290
}
283-
} else {
284-
webhook, err := a.opClient.KubernetesInterface().AdmissionregistrationV1().MutatingWebhookConfigurations().Get(context.TODO(), webhookName, metav1.GetOptions{})
291+
} else if desc.Type == v1alpha1.ValidatingAdmissionWebhook {
292+
ownerLabels := ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind)
293+
ownerLabelStrings := labels.SelectorFromSet(ownerLabels).String()
294+
295+
existingHooks, err := a.opClient.KubernetesInterface().AdmissionregistrationV1().ValidatingWebhookConfigurations().List(context.TODO(), metav1.ListOptions{LabelSelector: ownerLabelStrings})
285296
if err != nil {
286-
return nil, fmt.Errorf("could not retrieve generated APIService: %v", err)
297+
return nil, fmt.Errorf("could not retrieve generated ValidatingWebhookConfiguration: %v", err)
287298
}
288-
if len(webhook.Webhooks[0].ClientConfig.CABundle) > 0 {
289-
return webhook.Webhooks[0].ClientConfig.CABundle, nil
299+
300+
for _, item := range existingHooks.Items {
301+
if strings.HasPrefix(item.Name, desc.Name) {
302+
return item.Webhooks[0].ClientConfig.CABundle, nil
303+
}
290304
}
291305
}
292306
}

‎pkg/controller/operators/olm/operator.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/kubestate"
1212
"github.com/sirupsen/logrus"
1313
log "github.com/sirupsen/logrus"
14+
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
1415
corev1 "k8s.io/api/core/v1"
1516
rbacv1 "k8s.io/api/rbac/v1"
1617
extinf "k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions"
@@ -632,6 +633,44 @@ func (a *Operator) syncGCObject(obj interface{}) (syncError error) {
632633
break
633634
}
634635
logger.Debugf("Deleted cluster role binding %v due to no owning CSV", metaObj.GetName())
636+
case *admissionregistrationv1.MutatingWebhookConfiguration:
637+
if name, ns, ok := ownerutil.GetOwnerByKindLabel(metaObj, v1alpha1.ClusterServiceVersionKind); ok {
638+
_, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(ns).Get(name)
639+
if err == nil {
640+
logger.Debugf("CSV still present, must wait until it is deleted (owners=%v)", name)
641+
syncError = fmt.Errorf("cleanup must wait")
642+
return
643+
} else if !k8serrors.IsNotFound(err) {
644+
logger.Infof("error CSV retrieval error")
645+
syncError = err
646+
return
647+
}
648+
}
649+
650+
if err := a.opClient.KubernetesInterface().AdmissionregistrationV1().MutatingWebhookConfigurations().Delete(context.TODO(), metaObj.GetName(), metav1.DeleteOptions{}); err != nil {
651+
logger.WithError(err).Warn("cannot delete MutatingWebhookConfiguration")
652+
break
653+
}
654+
logger.Debugf("Deleted MutatingWebhookConfiguration %v due to no owning CSV", metaObj.GetName())
655+
case *admissionregistrationv1.ValidatingWebhookConfiguration:
656+
if name, ns, ok := ownerutil.GetOwnerByKindLabel(metaObj, v1alpha1.ClusterServiceVersionKind); ok {
657+
_, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(ns).Get(name)
658+
if err == nil {
659+
logger.Debugf("CSV still present, must wait until it is deleted (owners=%v)", name)
660+
syncError = fmt.Errorf("cleanup must wait")
661+
return
662+
} else if !k8serrors.IsNotFound(err) {
663+
logger.Infof("Error CSV retrieval error")
664+
syncError = err
665+
return
666+
}
667+
}
668+
669+
if err := a.opClient.KubernetesInterface().AdmissionregistrationV1().ValidatingWebhookConfigurations().Delete(context.TODO(), metaObj.GetName(), metav1.DeleteOptions{}); err != nil {
670+
logger.WithError(err).Warn("cannot delete ValidatingWebhookConfiguration")
671+
break
672+
}
673+
logger.Debugf("Deleted ValidatingWebhookConfiguration %v due to no owning CSV", metaObj.GetName())
635674
}
636675

637676
return
@@ -920,6 +959,25 @@ func (a *Operator) handleClusterServiceVersionDeletion(obj interface{}) {
920959
syncError := a.objGCQueueSet.RequeueEvent("", kubestate.NewResourceEvent(kubestate.ResourceUpdated, cr))
921960
logger.Debugf("handleCSVdeletion - requeued update event for %v, res=%v", cr, syncError)
922961
}
962+
963+
webhookSelector := labels.SelectorFromSet(ownerutil.OwnerLabel(clusterServiceVersion, v1alpha1.ClusterServiceVersionKind)).String()
964+
mWebhooks, err := a.opClient.KubernetesInterface().AdmissionregistrationV1().MutatingWebhookConfigurations().List(context.TODO(), metav1.ListOptions{LabelSelector: webhookSelector})
965+
if err != nil {
966+
logger.WithError(err).Warn("cannot list MutatingWebhookConfigurations")
967+
}
968+
for _, webhook := range mWebhooks.Items {
969+
syncError := a.objGCQueueSet.RequeueEvent("", kubestate.NewResourceEvent(kubestate.ResourceUpdated, &webhook))
970+
logger.Debugf("handleCSVdeletion - requeued update event for %v, res=%v", webhook, syncError)
971+
}
972+
973+
vWebhooks, err := a.opClient.KubernetesInterface().AdmissionregistrationV1().ValidatingWebhookConfigurations().List(context.TODO(), metav1.ListOptions{LabelSelector: webhookSelector})
974+
if err != nil {
975+
logger.WithError(err).Warn("cannot list MutatingWebhookConfigurations")
976+
}
977+
for _, webhook := range vWebhooks.Items {
978+
syncError := a.objGCQueueSet.RequeueEvent("", kubestate.NewResourceEvent(kubestate.ResourceUpdated, &webhook))
979+
logger.Debugf("handleCSVdeletion - requeued update event for %v, res=%v", webhook, syncError)
980+
}
923981
}
924982

925983
func (a *Operator) removeDanglingChildCSVs(csv *v1alpha1.ClusterServiceVersion) error {

0 commit comments

Comments
 (0)