[webhook] Add targetPort while scaffolding csv (#4178)

* [webhook] Specify ports while scaffolding csv

* update samples

* update csv tests

Author: varshaprasad96

internal/generate/clusterserviceversion/clusterserviceversion_updaters.go

@@ -269,23 +269,36 @@ func applyCustomResourceDefinitions(c *collector.Manifests, csv *operatorsv1alph
 // applyWebhooks updates csv's webhookDefinitions with any mutating and validating webhooks in the collector.
 func applyWebhooks(c *collector.Manifests, csv *operatorsv1alpha1.ClusterServiceVersion) {
 	webhookDescriptions := []operatorsv1alpha1.WebhookDescription{}
+
 	for _, webhook := range c.ValidatingWebhooks {
-		depName, serviceName := findMatchingDeploymentAndServiceForWebhook(c, webhook.ClientConfig)
-		if serviceName == "" && depName == "" {
+		var validatingServiceName string
+		depName, svc := findMatchingDeploymentAndServiceForWebhook(c, webhook.ClientConfig)
+
+		if svc != nil {
+			validatingServiceName = svc.GetName()
+		}
+
+		if validatingServiceName == "" && depName == "" {
 			log.Infof("No service found for validating webhook %q", webhook.Name)
 		} else if depName == "" {
-			log.Infof("No deployment is selected by service %q for validating webhook %q", serviceName, webhook.Name)
+			log.Infof("No deployment is selected by service %q for validating webhook %q", validatingServiceName, webhook.Name)
 		}
-		webhookDescriptions = append(webhookDescriptions, validatingToWebhookDescription(webhook, depName))
+		webhookDescriptions = append(webhookDescriptions, validatingToWebhookDescription(webhook, depName, svc))
 	}
 	for _, webhook := range c.MutatingWebhooks {
-		depName, serviceName := findMatchingDeploymentAndServiceForWebhook(c, webhook.ClientConfig)
-		if serviceName == "" && depName == "" {
+		var mutatingServiceName string
+		depName, svc := findMatchingDeploymentAndServiceForWebhook(c, webhook.ClientConfig)
+
+		if svc != nil {
+			mutatingServiceName = svc.GetName()
+		}
+
+		if mutatingServiceName == "" && depName == "" {
 			log.Infof("No service found for mutating webhook %q", webhook.Name)
 		} else if depName == "" {
-			log.Infof("No deployment is selected by service %q for mutating webhook %q", serviceName, webhook.Name)
+			log.Infof("No deployment is selected by service %q for mutating webhook %q", mutatingServiceName, webhook.Name)
 		}
-		webhookDescriptions = append(webhookDescriptions, mutatingToWebhookDescription(webhook, depName))
+		webhookDescriptions = append(webhookDescriptions, mutatingToWebhookDescription(webhook, depName, svc))
 	}
 	csv.Spec.WebhookDefinitions = webhookDescriptions
 }
@@ -294,7 +307,7 @@ func applyWebhooks(c *collector.Manifests, csv *operatorsv1alpha1.ClusterService
 var defaultAdmissionReviewVersions = []string{"v1beta1"}
 
 // validatingToWebhookDescription transforms webhook into a WebhookDescription.
-func validatingToWebhookDescription(webhook admissionregv1.ValidatingWebhook, depName string) operatorsv1alpha1.WebhookDescription {
+func validatingToWebhookDescription(webhook admissionregv1.ValidatingWebhook, depName string, ws *corev1.Service) operatorsv1alpha1.WebhookDescription {
 	description := operatorsv1alpha1.WebhookDescription{
 		Type:                    operatorsv1alpha1.ValidatingAdmissionWebhook,
 		GenerateName:            webhook.Name,
@@ -315,8 +328,18 @@ func validatingToWebhookDescription(webhook admissionregv1.ValidatingWebhook, de
 	}
 
 	if serviceRef := webhook.ClientConfig.Service; serviceRef != nil {
+		var webhookServiceRefPort int32 = 443
 		if serviceRef.Port != nil {
-			description.ContainerPort = *serviceRef.Port
+			webhookServiceRefPort = *serviceRef.Port
+		}
+		if ws != nil {
+			for _, port := range ws.Spec.Ports {
+				if webhookServiceRefPort == port.Port {
+					description.ContainerPort = port.Port
+					description.TargetPort = &port.TargetPort
+					break
+				}
+			}
 		}
 		description.DeploymentName = depName
 		if description.DeploymentName == "" {
@@ -328,7 +351,7 @@ func validatingToWebhookDescription(webhook admissionregv1.ValidatingWebhook, de
 }
 
 // mutatingToWebhookDescription transforms webhook into a WebhookDescription.
-func mutatingToWebhookDescription(webhook admissionregv1.MutatingWebhook, depName string) operatorsv1alpha1.WebhookDescription {
+func mutatingToWebhookDescription(webhook admissionregv1.MutatingWebhook, depName string, ws *corev1.Service) operatorsv1alpha1.WebhookDescription {
 	description := operatorsv1alpha1.WebhookDescription{
 		Type:                    operatorsv1alpha1.MutatingAdmissionWebhook,
 		GenerateName:            webhook.Name,
@@ -350,8 +373,18 @@ func mutatingToWebhookDescription(webhook admissionregv1.MutatingWebhook, depNam
 	}
 
 	if serviceRef := webhook.ClientConfig.Service; serviceRef != nil {
+		var webhookServiceRefPort int32 = 443
 		if serviceRef.Port != nil {
-			description.ContainerPort = *serviceRef.Port
+			webhookServiceRefPort = *serviceRef.Port
+		}
+		if ws != nil {
+			for _, port := range ws.Spec.Ports {
+				if webhookServiceRefPort == port.Port {
+					description.ContainerPort = port.Port
+					description.TargetPort = &port.TargetPort
+					break
+				}
+			}
 		}
 		description.DeploymentName = depName
 		if description.DeploymentName == "" {
@@ -365,15 +398,14 @@ func mutatingToWebhookDescription(webhook admissionregv1.MutatingWebhook, depNam
 // findMatchingDeploymentAndServiceForWebhook matches a Service to a webhook's client config (if it uses a service)
 // then matches that Service to a Deployment by comparing label selectors (if the Service uses label selectors).
 // The names of both Service and Deployment are returned if found.
-func findMatchingDeploymentAndServiceForWebhook(c *collector.Manifests, wcc admissionregv1.WebhookClientConfig) (depName, serviceName string) {
+func findMatchingDeploymentAndServiceForWebhook(c *collector.Manifests, wcc admissionregv1.WebhookClientConfig) (depName string, ws *corev1.Service) {
 	// Return if a service reference is not specified, since a URL will be in that case.
 	if wcc.Service == nil {
 		return
 	}
 
 	// Find the matching service, if any. The webhook server may be externally managed
 	// if no service is created by the operator.
-	var ws *corev1.Service
 	for i, service := range c.Services {
 		if service.GetName() == wcc.Service.Name {
 			ws = &c.Services[i]
@@ -383,7 +415,6 @@ func findMatchingDeploymentAndServiceForWebhook(c *collector.Manifests, wcc admi
 	if ws == nil {
 		return
 	}
-	serviceName = ws.GetName()
 
 	// Only ExternalName-type services cannot have selectors.
 	if ws.Spec.Type == corev1.ServiceTypeExternalName {
@@ -416,7 +447,7 @@ func findMatchingDeploymentAndServiceForWebhook(c *collector.Manifests, wcc admi
 		}
 	}
 
-	return depName, serviceName
+	return depName, ws
 }
 
 // applyCustomResources updates csv's "alm-examples" annotation with the

internal/generate/clusterserviceversion/clusterserviceversion_updaters_test.go

@@ -49,9 +49,9 @@ var _ = Describe("findMatchingDeploymentAndServiceForWebhook", func() {
 			c.Deployments = []appsv1.Deployment{newDeployment(depName1, labels)}
 			c.Services = []corev1.Service{newService(serviceName1, labels)}
 			wcc.Service.Name = serviceName1
-			depName, serviceName := findMatchingDeploymentAndServiceForWebhook(c, wcc)
+			depName, service := findMatchingDeploymentAndServiceForWebhook(c, wcc)
 			Expect(depName).To(Equal(depName1))
-			Expect(serviceName).To(Equal(serviceName1))
+			Expect(service.GetName()).To(Equal(serviceName1))
 		})
 
 		By("parsing two deployments and two services with non-intersecting labels")
@@ -67,9 +67,9 @@ var _ = Describe("findMatchingDeploymentAndServiceForWebhook", func() {
 				newService(serviceName2, labels2),
 			}
 			wcc.Service.Name = serviceName1
-			depName, serviceName := findMatchingDeploymentAndServiceForWebhook(c, wcc)
+			depName, service := findMatchingDeploymentAndServiceForWebhook(c, wcc)
 			Expect(depName).To(Equal(depName1))
-			Expect(serviceName).To(Equal(serviceName1))
+			Expect(service.GetName()).To(Equal(serviceName1))
 		})
 
 		By("parsing two deployments and two services with a label subset")
@@ -82,9 +82,9 @@ var _ = Describe("findMatchingDeploymentAndServiceForWebhook", func() {
 			}
 			c.Services = []corev1.Service{newService(serviceName1, labels1)}
 			wcc.Service.Name = serviceName1
-			depName, serviceName := findMatchingDeploymentAndServiceForWebhook(c, wcc)
+			depName, service := findMatchingDeploymentAndServiceForWebhook(c, wcc)
 			Expect(depName).To(Equal(depName2))
-			Expect(serviceName).To(Equal(serviceName1))
+			Expect(service.GetName()).To(Equal(serviceName1))
 		})
 	})
 
@@ -95,9 +95,9 @@ var _ = Describe("findMatchingDeploymentAndServiceForWebhook", func() {
 			c.Deployments = []appsv1.Deployment{newDeployment(depName1, labels)}
 			c.Services = []corev1.Service{newService(serviceName1, labels)}
 			wcc.Service.Name = serviceName2
-			depName, serviceName := findMatchingDeploymentAndServiceForWebhook(c, wcc)
+			depName, service := findMatchingDeploymentAndServiceForWebhook(c, wcc)
 			Expect(depName).To(BeEmpty())
-			Expect(serviceName).To(BeEmpty())
+			Expect(service).To(BeNil())
 		})
 	})
 
@@ -109,9 +109,9 @@ var _ = Describe("findMatchingDeploymentAndServiceForWebhook", func() {
 			c.Deployments = []appsv1.Deployment{newDeployment(depName1, labels1)}
 			c.Services = []corev1.Service{newService(serviceName1, labels2)}
 			wcc.Service.Name = serviceName1
-			depName, serviceName := findMatchingDeploymentAndServiceForWebhook(c, wcc)
+			depName, service := findMatchingDeploymentAndServiceForWebhook(c, wcc)
 			Expect(depName).To(BeEmpty())
-			Expect(serviceName).To(Equal(serviceName1))
+			Expect(service.GetName()).To(Equal(serviceName1))
 		})
 
 		By("parsing one deployment and one service with two intersecting labels")
@@ -121,9 +121,9 @@ var _ = Describe("findMatchingDeploymentAndServiceForWebhook", func() {
 			c.Deployments = []appsv1.Deployment{newDeployment(depName1, labels1)}
 			c.Services = []corev1.Service{newService(serviceName1, labels2)}
 			wcc.Service.Name = serviceName1
-			depName, serviceName := findMatchingDeploymentAndServiceForWebhook(c, wcc)
+			depName, service := findMatchingDeploymentAndServiceForWebhook(c, wcc)
 			Expect(depName).To(BeEmpty())
-			Expect(serviceName).To(Equal(serviceName1))
+			Expect(service.GetName()).To(Equal(serviceName1))
 		})
 	})
 })

internal/generate/testdata/clusterserviceversions/output/memcached-operator.clusterserviceversion.yaml

@@ -215,6 +215,7 @@ spec:
   webhookdefinitions:
   - admissionReviewVersions:
     - v1beta1
+    containerPort: 443
     deploymentName: memcached-operator-controller-manager
     failurePolicy: Fail
     generateName: vmemcached.kb.io
@@ -229,10 +230,12 @@ spec:
       resources:
       - memcacheds
     sideEffects: None
+    targetPort: 9443
     type: ValidatingAdmissionWebhook
     webhookPath: /validate-cache-my-domain-v1alpha1-memcached
   - admissionReviewVersions:
     - v1beta1
+    containerPort: 443
     deploymentName: memcached-operator-controller-manager
     failurePolicy: Fail
     generateName: mmemcached.kb.io
@@ -247,5 +250,6 @@ spec:
       resources:
       - memcacheds
     sideEffects: None
+    targetPort: 9443
     type: MutatingAdmissionWebhook
     webhookPath: /mutate-cache-my-domain-v1alpha1-memcached

testdata/go/memcached-operator/bundle/manifests/memcached-operator.clusterserviceversion.yaml

@@ -205,6 +205,7 @@ spec:
   webhookdefinitions:
   - admissionReviewVersions:
     - v1beta1
+    containerPort: 443
     deploymentName: memcached-operator-controller-manager
     failurePolicy: Fail
     generateName: vmemcached.kb.io
@@ -219,10 +220,12 @@ spec:
       resources:
       - memcacheds
     sideEffects: None
+    targetPort: 9443
     type: ValidatingAdmissionWebhook
     webhookPath: /validate-cache-example-com-v1alpha1-memcached
   - admissionReviewVersions:
     - v1beta1
+    containerPort: 443
     deploymentName: memcached-operator-controller-manager
     failurePolicy: Fail
     generateName: mmemcached.kb.io
@@ -237,5 +240,6 @@ spec:
       resources:
       - memcacheds
     sideEffects: None
+    targetPort: 9443
     type: MutatingAdmissionWebhook
     webhookPath: /mutate-cache-example-com-v1alpha1-memcached