Skip to content

Commit 11ad4ee

Browse files
[v1.0.x] generate: correctly set CSV webhookDefinition deployment names (#3904)
Co-authored-by: Eric Stroczynski <[email protected]>
1 parent c48d05c commit 11ad4ee

File tree

6 files changed

+374
-8
lines changed

6 files changed

+374
-8
lines changed

changelog/fragments/3761.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
entries:
2+
- description: >
3+
`generate <bundle|packagemanifests>` will populate a CSV's `webhookDefinition[].deploymentName`
4+
by selecting an input Deployment via its PodTemplate labels using a webhook Service's label selectors,
5+
defaulting to "<service.metadata.name>-service" if none is selected.
6+
kind: bugfix

internal/generate/clusterserviceversion/clusterserviceversion_updaters.go

Lines changed: 84 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -262,21 +262,32 @@ func applyCustomResourceDefinitions(c *collector.Manifests, csv *operatorsv1alph
262262
csv.Spec.CustomResourceDefinitions.Owned = ownedDescs
263263
}
264264

265-
// applyWebhooks updates csv's webhookDefinitions with any
266-
// mutating and validating webhooks in the collector.
265+
// applyWebhooks updates csv's webhookDefinitions with any mutating and validating webhooks in the collector.
267266
func applyWebhooks(c *collector.Manifests, csv *operatorsv1alpha1.ClusterServiceVersion) {
268267
webhookDescriptions := []operatorsv1alpha1.WebhookDescription{}
269268
for _, webhook := range c.ValidatingWebhooks {
270-
webhookDescriptions = append(webhookDescriptions, validatingToWebhookDescription(webhook))
269+
depName, serviceName := findMatchingDeploymentAndServiceForWebhook(c, webhook.ClientConfig)
270+
if serviceName == "" && depName == "" {
271+
log.Infof("No service found for validating webhook %q", webhook.Name)
272+
} else if depName == "" {
273+
log.Infof("No deployment is selected by service %q for validating webhook %q", serviceName, webhook.Name)
274+
}
275+
webhookDescriptions = append(webhookDescriptions, validatingToWebhookDescription(webhook, depName))
271276
}
272277
for _, webhook := range c.MutatingWebhooks {
273-
webhookDescriptions = append(webhookDescriptions, mutatingToWebhookDescription(webhook))
278+
depName, serviceName := findMatchingDeploymentAndServiceForWebhook(c, webhook.ClientConfig)
279+
if serviceName == "" && depName == "" {
280+
log.Infof("No service found for mutating webhook %q", webhook.Name)
281+
} else if depName == "" {
282+
log.Infof("No deployment is selected by service %q for mutating webhook %q", serviceName, webhook.Name)
283+
}
284+
webhookDescriptions = append(webhookDescriptions, mutatingToWebhookDescription(webhook, depName))
274285
}
275286
csv.Spec.WebhookDefinitions = webhookDescriptions
276287
}
277288

278289
// validatingToWebhookDescription transforms webhook into a WebhookDescription.
279-
func validatingToWebhookDescription(webhook admissionregv1.ValidatingWebhook) operatorsv1alpha1.WebhookDescription {
290+
func validatingToWebhookDescription(webhook admissionregv1.ValidatingWebhook, depName string) operatorsv1alpha1.WebhookDescription {
280291
description := operatorsv1alpha1.WebhookDescription{
281292
Type: operatorsv1alpha1.ValidatingAdmissionWebhook,
282293
GenerateName: webhook.Name,
@@ -288,18 +299,22 @@ func validatingToWebhookDescription(webhook admissionregv1.ValidatingWebhook) op
288299
TimeoutSeconds: webhook.TimeoutSeconds,
289300
AdmissionReviewVersions: webhook.AdmissionReviewVersions,
290301
}
302+
291303
if serviceRef := webhook.ClientConfig.Service; serviceRef != nil {
292304
if serviceRef.Port != nil {
293305
description.ContainerPort = *serviceRef.Port
294306
}
295-
description.DeploymentName = strings.TrimSuffix(serviceRef.Name, "-service")
307+
description.DeploymentName = depName
308+
if description.DeploymentName == "" {
309+
description.DeploymentName = strings.TrimSuffix(serviceRef.Name, "-service")
310+
}
296311
description.WebhookPath = serviceRef.Path
297312
}
298313
return description
299314
}
300315

301316
// mutatingToWebhookDescription transforms webhook into a WebhookDescription.
302-
func mutatingToWebhookDescription(webhook admissionregv1.MutatingWebhook) operatorsv1alpha1.WebhookDescription {
317+
func mutatingToWebhookDescription(webhook admissionregv1.MutatingWebhook, depName string) operatorsv1alpha1.WebhookDescription {
303318
description := operatorsv1alpha1.WebhookDescription{
304319
Type: operatorsv1alpha1.MutatingAdmissionWebhook,
305320
GenerateName: webhook.Name,
@@ -312,16 +327,77 @@ func mutatingToWebhookDescription(webhook admissionregv1.MutatingWebhook) operat
312327
AdmissionReviewVersions: webhook.AdmissionReviewVersions,
313328
ReinvocationPolicy: webhook.ReinvocationPolicy,
314329
}
330+
315331
if serviceRef := webhook.ClientConfig.Service; serviceRef != nil {
316332
if serviceRef.Port != nil {
317333
description.ContainerPort = *serviceRef.Port
318334
}
319-
description.DeploymentName = strings.TrimSuffix(serviceRef.Name, "-service")
335+
description.DeploymentName = depName
336+
if description.DeploymentName == "" {
337+
description.DeploymentName = strings.TrimSuffix(serviceRef.Name, "-service")
338+
}
320339
description.WebhookPath = serviceRef.Path
321340
}
322341
return description
323342
}
324343

344+
// findMatchingDeploymentAndServiceForWebhook matches a Service to a webhook's client config (if it uses a service)
345+
// then matches that Service to a Deployment by comparing label selectors (if the Service uses label selectors).
346+
// The names of both Service and Deployment are returned if found.
347+
func findMatchingDeploymentAndServiceForWebhook(c *collector.Manifests, wcc admissionregv1.WebhookClientConfig) (depName, serviceName string) {
348+
// Return if a service reference is not specified, since a URL will be in that case.
349+
if wcc.Service == nil {
350+
return
351+
}
352+
353+
// Find the matching service, if any. The webhook server may be externally managed
354+
// if no service is created by the operator.
355+
var ws *corev1.Service
356+
for i, service := range c.Services {
357+
if service.GetName() == wcc.Service.Name {
358+
ws = &c.Services[i]
359+
break
360+
}
361+
}
362+
if ws == nil {
363+
return
364+
}
365+
serviceName = ws.GetName()
366+
367+
// Only ExternalName-type services cannot have selectors.
368+
if ws.Spec.Type == corev1.ServiceTypeExternalName {
369+
return
370+
}
371+
372+
// If a selector does not exist, there is either an Endpoint or EndpointSlice object accompanying
373+
// the service so it should not be added to the CSV.
374+
if len(ws.Spec.Selector) == 0 {
375+
return
376+
}
377+
378+
// Match service against pod labels, in which the webhook server will be running.
379+
for _, dep := range c.Deployments {
380+
podTemplateLabels := dep.Spec.Template.GetLabels()
381+
if len(podTemplateLabels) == 0 {
382+
continue
383+
}
384+
385+
depName = dep.GetName()
386+
// Check that all labels match.
387+
for key, serviceValue := range ws.Spec.Selector {
388+
if podTemplateValue, hasKey := podTemplateLabels[key]; !hasKey || podTemplateValue != serviceValue {
389+
depName = ""
390+
break
391+
}
392+
}
393+
if depName != "" {
394+
break
395+
}
396+
}
397+
398+
return depName, serviceName
399+
}
400+
325401
// applyCustomResources updates csv's "alm-examples" annotation with the
326402
// Custom Resources in the collector.
327403
func applyCustomResources(c *collector.Manifests, csv *operatorsv1alpha1.ClusterServiceVersion) error {
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
// Copyright 2020 The Operator-SDK Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package clusterserviceversion
16+
17+
import (
18+
. "github.com/onsi/ginkgo"
19+
. "github.com/onsi/gomega"
20+
admissionregv1 "k8s.io/api/admissionregistration/v1"
21+
appsv1 "k8s.io/api/apps/v1"
22+
corev1 "k8s.io/api/core/v1"
23+
24+
"github.com/operator-framework/operator-sdk/internal/generate/collector"
25+
)
26+
27+
var _ = Describe("findMatchingDeploymentAndServiceForWebhook", func() {
28+
29+
var (
30+
c *collector.Manifests
31+
wcc admissionregv1.WebhookClientConfig
32+
33+
depName1 = "dep-name-1"
34+
depName2 = "dep-name-2"
35+
serviceName1 = "service-name-1"
36+
serviceName2 = "service-name-2"
37+
)
38+
39+
BeforeEach(func() {
40+
c = &collector.Manifests{}
41+
wcc = admissionregv1.WebhookClientConfig{}
42+
wcc.Service = &admissionregv1.ServiceReference{}
43+
})
44+
45+
Context("webhook config has a matching service name", func() {
46+
By("parsing one deployment and one service with one label")
47+
It("returns the first service and deployment", func() {
48+
labels := map[string]string{"operator-name": "test-operator"}
49+
c.Deployments = []appsv1.Deployment{newDeployment(depName1, labels)}
50+
c.Services = []corev1.Service{newService(serviceName1, labels)}
51+
wcc.Service.Name = serviceName1
52+
depName, serviceName := findMatchingDeploymentAndServiceForWebhook(c, wcc)
53+
Expect(depName).To(Equal(depName1))
54+
Expect(serviceName).To(Equal(serviceName1))
55+
})
56+
57+
By("parsing two deployments and two services with non-intersecting labels")
58+
It("returns the first service and deployment", func() {
59+
labels1 := map[string]string{"operator-name": "test-operator"}
60+
labels2 := map[string]string{"foo": "bar"}
61+
c.Deployments = []appsv1.Deployment{
62+
newDeployment(depName1, labels1),
63+
newDeployment(depName2, labels2),
64+
}
65+
c.Services = []corev1.Service{
66+
newService(serviceName1, labels1),
67+
newService(serviceName2, labels2),
68+
}
69+
wcc.Service.Name = serviceName1
70+
depName, serviceName := findMatchingDeploymentAndServiceForWebhook(c, wcc)
71+
Expect(depName).To(Equal(depName1))
72+
Expect(serviceName).To(Equal(serviceName1))
73+
})
74+
75+
By("parsing two deployments and two services with a label subset")
76+
It("returns the first service and second deployment", func() {
77+
labels1 := map[string]string{"operator-name": "test-operator"}
78+
labels2 := map[string]string{"operator-name": "test-operator", "foo": "bar"}
79+
c.Deployments = []appsv1.Deployment{
80+
newDeployment(depName2, labels2),
81+
newDeployment(depName1, labels1),
82+
}
83+
c.Services = []corev1.Service{newService(serviceName1, labels1)}
84+
wcc.Service.Name = serviceName1
85+
depName, serviceName := findMatchingDeploymentAndServiceForWebhook(c, wcc)
86+
Expect(depName).To(Equal(depName2))
87+
Expect(serviceName).To(Equal(serviceName1))
88+
})
89+
})
90+
91+
Context("webhook config does not have a matching service", func() {
92+
By("parsing one deployment and one service with one label")
93+
It("returns neither service nor deployment name", func() {
94+
labels := map[string]string{"operator-name": "test-operator"}
95+
c.Deployments = []appsv1.Deployment{newDeployment(depName1, labels)}
96+
c.Services = []corev1.Service{newService(serviceName1, labels)}
97+
wcc.Service.Name = serviceName2
98+
depName, serviceName := findMatchingDeploymentAndServiceForWebhook(c, wcc)
99+
Expect(depName).To(BeEmpty())
100+
Expect(serviceName).To(BeEmpty())
101+
})
102+
})
103+
104+
Context("webhook config has a matching service but labels do not match", func() {
105+
By("parsing one deployment and one service with one label")
106+
It("returns the first service and no deployment", func() {
107+
labels1 := map[string]string{"operator-name": "test-operator"}
108+
labels2 := map[string]string{"foo": "bar"}
109+
c.Deployments = []appsv1.Deployment{newDeployment(depName1, labels1)}
110+
c.Services = []corev1.Service{newService(serviceName1, labels2)}
111+
wcc.Service.Name = serviceName1
112+
depName, serviceName := findMatchingDeploymentAndServiceForWebhook(c, wcc)
113+
Expect(depName).To(BeEmpty())
114+
Expect(serviceName).To(Equal(serviceName1))
115+
})
116+
117+
By("parsing one deployment and one service with two intersecting labels")
118+
It("returns the first service and no deployment", func() {
119+
labels1 := map[string]string{"operator-name": "test-operator", "foo": "bar"}
120+
labels2 := map[string]string{"foo": "bar", "baz": "bat"}
121+
c.Deployments = []appsv1.Deployment{newDeployment(depName1, labels1)}
122+
c.Services = []corev1.Service{newService(serviceName1, labels2)}
123+
wcc.Service.Name = serviceName1
124+
depName, serviceName := findMatchingDeploymentAndServiceForWebhook(c, wcc)
125+
Expect(depName).To(BeEmpty())
126+
Expect(serviceName).To(Equal(serviceName1))
127+
})
128+
})
129+
})
130+
131+
func newDeployment(name string, labels map[string]string) appsv1.Deployment {
132+
dep := appsv1.Deployment{}
133+
dep.SetName(name)
134+
dep.Spec.Template.SetLabels(labels)
135+
return dep
136+
}
137+
138+
func newService(name string, labels map[string]string) corev1.Service {
139+
s := corev1.Service{}
140+
s.SetName(name)
141+
s.Spec.Selector = labels
142+
return s
143+
}

internal/generate/collector/manifests.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ type Manifests struct {
4545
ClusterRoleBindings []rbacv1.ClusterRoleBinding
4646
Deployments []appsv1.Deployment
4747
ServiceAccounts []corev1.ServiceAccount
48+
Services []corev1.Service
4849
V1CustomResourceDefinitions []apiextv1.CustomResourceDefinition
4950
V1beta1CustomResourceDefinitions []apiextv1beta1.CustomResourceDefinition
5051
ValidatingWebhooks []admissionregv1.ValidatingWebhook
@@ -61,6 +62,7 @@ var (
6162
roleBindingGK = rbacv1.SchemeGroupVersion.WithKind("RoleBinding").GroupKind()
6263
clusterRoleBindingGK = rbacv1.SchemeGroupVersion.WithKind("ClusterRoleBinding").GroupKind()
6364
serviceAccountGK = corev1.SchemeGroupVersion.WithKind("ServiceAccount").GroupKind()
65+
serviceGK = corev1.SchemeGroupVersion.WithKind("Service").GroupKind()
6466
deploymentGK = appsv1.SchemeGroupVersion.WithKind("Deployment").GroupKind()
6567
crdGK = apiextv1.SchemeGroupVersion.WithKind("CustomResourceDefinition").GroupKind()
6668
validatingWebhookCfgGK = admissionregv1.SchemeGroupVersion.WithKind("ValidatingWebhookConfiguration").GroupKind()
@@ -104,6 +106,8 @@ func (c *Manifests) UpdateFromDirs(deployDir, crdsDir string) error {
104106
err = c.addClusterRoleBindings(manifest)
105107
case serviceAccountGK:
106108
err = c.addServiceAccounts(manifest)
109+
case serviceGK:
110+
err = c.addServices(manifest)
107111
case deploymentGK:
108112
err = c.addDeployments(manifest)
109113
case crdGK:
@@ -172,6 +176,8 @@ func (c *Manifests) UpdateFromReader(r io.Reader) error {
172176
err = c.addClusterRoleBindings(manifest)
173177
case serviceAccountGK:
174178
err = c.addServiceAccounts(manifest)
179+
case serviceGK:
180+
err = c.addServices(manifest)
175181
case deploymentGK:
176182
err = c.addDeployments(manifest)
177183
case crdGK:
@@ -266,6 +272,18 @@ func (c *Manifests) addServiceAccounts(rawManifests ...[]byte) error {
266272
return nil
267273
}
268274

275+
// addServices assumes all manifest data in rawManifests are Services and adds them to the collector.
276+
func (c *Manifests) addServices(rawManifests ...[]byte) error {
277+
for _, rawManifest := range rawManifests {
278+
s := corev1.Service{}
279+
if err := yaml.Unmarshal(rawManifest, &s); err != nil {
280+
return err
281+
}
282+
c.Services = append(c.Services, s)
283+
}
284+
return nil
285+
}
286+
269287
// addDeployments assumes all manifest data in rawManifests are Deployments
270288
// and adds them to the collector.
271289
func (c *Manifests) addDeployments(rawManifests ...[]byte) error {

0 commit comments

Comments
 (0)