internal/generate: set webhook deployment name by searching for deplo…

…yments selected by webhook services

internal/generate/clusterserviceversion/clusterserviceversion_updaters.go

@@ -262,21 +262,34 @@ func applyCustomResourceDefinitions(c *collector.Manifests, csv *operatorsv1alph
 	csv.Spec.CustomResourceDefinitions.Owned = ownedDescs
 }
 
-// applyWebhooks updates csv's webhookDefinitions with any
-// mutating and validating webhooks in the collector.
+// 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 {
-		webhookDescriptions = append(webhookDescriptions, validatingToWebhookDescription(webhook))
+		depName, serviceName := findMatchingDeploymentAndServiceForWebhook(c, webhook.ClientConfig)
+		if serviceName == "" && 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)
+		}
+		webhookDescriptions = append(webhookDescriptions, validatingToWebhookDescription(webhook, depName))
 	}
 	for _, webhook := range c.MutatingWebhooks {
-		webhookDescriptions = append(webhookDescriptions, mutatingToWebhookDescription(webhook))
+		depName, serviceName := findMatchingDeploymentAndServiceForWebhook(c, webhook.ClientConfig)
+		if serviceName == "" && 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)
+		}
+		webhookDescriptions = append(webhookDescriptions, mutatingToWebhookDescription(webhook, depName))
 	}
 	csv.Spec.WebhookDefinitions = webhookDescriptions
 }
 
+var defaultAdmissionReviewVersions = []string{"v1beta1"}
+
 // validatingToWebhookDescription transforms webhook into a WebhookDescription.
-func validatingToWebhookDescription(webhook admissionregv1.ValidatingWebhook) operatorsv1alpha1.WebhookDescription {
+func validatingToWebhookDescription(webhook admissionregv1.ValidatingWebhook, depName string) operatorsv1alpha1.WebhookDescription {
 	description := operatorsv1alpha1.WebhookDescription{
 		Type:                    operatorsv1alpha1.ValidatingAdmissionWebhook,
 		GenerateName:            webhook.Name,
@@ -288,18 +301,25 @@ func validatingToWebhookDescription(webhook admissionregv1.ValidatingWebhook) op
 		TimeoutSeconds:          webhook.TimeoutSeconds,
 		AdmissionReviewVersions: webhook.AdmissionReviewVersions,
 	}
+	if len(description.AdmissionReviewVersions) == 0 {
+		description.AdmissionReviewVersions = defaultAdmissionReviewVersions
+	}
+
 	if serviceRef := webhook.ClientConfig.Service; serviceRef != nil {
 		if serviceRef.Port != nil {
 			description.ContainerPort = *serviceRef.Port
 		}
-		description.DeploymentName = strings.TrimSuffix(serviceRef.Name, "-service")
+		description.DeploymentName = depName
+		if description.DeploymentName == "" {
+			description.DeploymentName = strings.TrimSuffix(serviceRef.Name, "-service")
+		}
 		description.WebhookPath = serviceRef.Path
 	}
 	return description
 }
 
 // mutatingToWebhookDescription transforms webhook into a WebhookDescription.
-func mutatingToWebhookDescription(webhook admissionregv1.MutatingWebhook) operatorsv1alpha1.WebhookDescription {
+func mutatingToWebhookDescription(webhook admissionregv1.MutatingWebhook, depName string) operatorsv1alpha1.WebhookDescription {
 	description := operatorsv1alpha1.WebhookDescription{
 		Type:                    operatorsv1alpha1.MutatingAdmissionWebhook,
 		GenerateName:            webhook.Name,
@@ -312,16 +332,81 @@ func mutatingToWebhookDescription(webhook admissionregv1.MutatingWebhook) operat
 		AdmissionReviewVersions: webhook.AdmissionReviewVersions,
 		ReinvocationPolicy:      webhook.ReinvocationPolicy,
 	}
+	if len(description.AdmissionReviewVersions) == 0 {
+		description.AdmissionReviewVersions = defaultAdmissionReviewVersions
+	}
+
 	if serviceRef := webhook.ClientConfig.Service; serviceRef != nil {
 		if serviceRef.Port != nil {
 			description.ContainerPort = *serviceRef.Port
 		}
-		description.DeploymentName = strings.TrimSuffix(serviceRef.Name, "-service")
+		description.DeploymentName = depName
+		if description.DeploymentName == "" {
+			description.DeploymentName = strings.TrimSuffix(serviceRef.Name, "-service")
+		}
 		description.WebhookPath = serviceRef.Path
 	}
 	return description
 }
 
+// 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) {
+	// 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]
+			break
+		}
+	}
+	if ws == nil {
+		return
+	}
+	serviceName = ws.GetName()
+
+	// Only ExternalName-type services cannot have selectors.
+	if ws.Spec.Type == corev1.ServiceTypeExternalName {
+		return
+	}
+
+	// If a selector does not exist, there is either an Endpoint or EndpointSlice object accompanying
+	// the service so it should not be added to the CSV.
+	if len(ws.Spec.Selector) == 0 {
+		return
+	}
+
+	// Match service against pod labels, in which the webhook server will be running.
+	for _, dep := range c.Deployments {
+		sel := dep.Spec.Template.GetLabels()
+		// A null label selector matches no objects.
+		if sel == nil {
+			continue
+		}
+		// An empty label selector matches all objects.
+		if len(sel) == 0 {
+			depName = dep.GetName()
+			break
+		}
+
+		// Check that any label matches.
+		for dkey, dvalue := range sel {
+			if svalue, hasKey := ws.Spec.Selector[dkey]; hasKey && svalue == dvalue {
+				return dep.GetName(), serviceName
+			}
+		}
+	}
+
+	return depName, serviceName
+}
+
 // applyCustomResources updates csv's "alm-examples" annotation with the
 // Custom Resources in the collector.
 func applyCustomResources(c *collector.Manifests, csv *operatorsv1alpha1.ClusterServiceVersion) error {

internal/generate/collector/manifests.go

@@ -45,6 +45,7 @@ type Manifests struct {
 	ClusterRoleBindings              []rbacv1.ClusterRoleBinding
 	Deployments                      []appsv1.Deployment
 	ServiceAccounts                  []corev1.ServiceAccount
+	Services                         []corev1.Service
 	V1CustomResourceDefinitions      []apiextv1.CustomResourceDefinition
 	V1beta1CustomResourceDefinitions []apiextv1beta1.CustomResourceDefinition
 	ValidatingWebhooks               []admissionregv1.ValidatingWebhook
@@ -61,6 +62,7 @@ var (
 	roleBindingGK          = rbacv1.SchemeGroupVersion.WithKind("RoleBinding").GroupKind()
 	clusterRoleBindingGK   = rbacv1.SchemeGroupVersion.WithKind("ClusterRoleBinding").GroupKind()
 	serviceAccountGK       = corev1.SchemeGroupVersion.WithKind("ServiceAccount").GroupKind()
+	serviceGK              = corev1.SchemeGroupVersion.WithKind("Service").GroupKind()
 	deploymentGK           = appsv1.SchemeGroupVersion.WithKind("Deployment").GroupKind()
 	crdGK                  = apiextv1.SchemeGroupVersion.WithKind("CustomResourceDefinition").GroupKind()
 	validatingWebhookCfgGK = admissionregv1.SchemeGroupVersion.WithKind("ValidatingWebhookConfiguration").GroupKind()
@@ -104,6 +106,8 @@ func (c *Manifests) UpdateFromDirs(deployDir, crdsDir string) error {
 				err = c.addClusterRoleBindings(manifest)
 			case serviceAccountGK:
 				err = c.addServiceAccounts(manifest)
+			case serviceGK:
+				err = c.addServices(manifest)
 			case deploymentGK:
 				err = c.addDeployments(manifest)
 			case crdGK:
@@ -172,6 +176,8 @@ func (c *Manifests) UpdateFromReader(r io.Reader) error {
 			err = c.addClusterRoleBindings(manifest)
 		case serviceAccountGK:
 			err = c.addServiceAccounts(manifest)
+		case serviceGK:
+			err = c.addServices(manifest)
 		case deploymentGK:
 			err = c.addDeployments(manifest)
 		case crdGK:
@@ -266,6 +272,18 @@ func (c *Manifests) addServiceAccounts(rawManifests ...[]byte) error {
 	return nil
 }
 
+// addServices assumes all manifest data in rawManifests are Services and adds them to the collector.
+func (c *Manifests) addServices(rawManifests ...[]byte) error {
+	for _, rawManifest := range rawManifests {
+		s := corev1.Service{}
+		if err := yaml.Unmarshal(rawManifest, &s); err != nil {
+			return err
+		}
+		c.Services = append(c.Services, s)
+	}
+	return nil
+}
+
 // addDeployments assumes all manifest data in rawManifests are Deployments
 // and adds them to the collector.
 func (c *Manifests) addDeployments(rawManifests ...[]byte) error {

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

@@ -109,14 +109,27 @@ spec:
                 - /manager
                 image: controller:latest
                 name: manager
+                ports:
+                - containerPort: 9443
+                  name: webhook-server
+                  protocol: TCP
                 resources:
                   limits:
                     cpu: 100m
                     memory: 30Mi
                   requests:
                     cpu: 100m
                     memory: 20Mi
+                volumeMounts:
+                - mountPath: /tmp/k8s-webhook-server/serving-certs
+                  name: cert
+                  readOnly: true
               terminationGracePeriodSeconds: 10
+              volumes:
+              - name: cert
+                secret:
+                  defaultMode: 420
+                  secretName: webhook-server-cert
       permissions:
       - rules:
         - apiGroups:
@@ -170,3 +183,40 @@ spec:
     name: Provider Name
     url: https://your.domain
   version: 0.0.1
+  webhookdefinitions:
+  - admissionReviewVersions:
+    - v1beta1
+    deploymentName: memcached-operator-controller-manager
+    failurePolicy: Fail
+    generateName: vmemcached.kb.io
+    rules:
+    - apiGroups:
+      - cache.example.com
+      apiVersions:
+      - v1alpha1
+      operations:
+      - CREATE
+      - UPDATE
+      resources:
+      - memcacheds
+    sideEffects: null
+    type: ValidatingAdmissionWebhook
+    webhookPath: /validate-cache-my-domain-v1alpha1-memcached
+  - admissionReviewVersions:
+    - v1beta1
+    deploymentName: memcached-operator-controller-manager
+    failurePolicy: Fail
+    generateName: mmemcached.kb.io
+    rules:
+    - apiGroups:
+      - cache.example.com
+      apiVersions:
+      - v1alpha1
+      operations:
+      - CREATE
+      - UPDATE
+      resources:
+      - memcacheds
+    sideEffects: null
+    type: MutatingAdmissionWebhook
+    webhookPath: /mutate-cache-my-domain-v1alpha1-memcached

internal/generate/testdata/go/static/basic.operator.yaml

@@ -62,6 +62,31 @@ status:
   conditions: []
   storedVersions: []
 ---
+apiVersion: admissionregistration.k8s.io/v1beta1
+kind: MutatingWebhookConfiguration
+metadata:
+  creationTimestamp: null
+  name: memcached-operator-mutating-webhook-configuration
+webhooks:
+- clientConfig:
+    caBundle: Cg==
+    service:
+      name: memcached-operator-webhook-service
+      namespace: memcached-operator-system
+      path: /mutate-cache-my-domain-v1alpha1-memcached
+  failurePolicy: Fail
+  name: mmemcached.kb.io
+  rules:
+  - apiGroups:
+    - cache.example.com
+    apiVersions:
+    - v1alpha1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - memcacheds
+---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
 metadata:
@@ -206,6 +231,18 @@ spec:
   selector:
     control-plane: controller-manager
 ---
+apiVersion: v1
+kind: Service
+metadata:
+  name: memcached-operator-webhook-service
+  namespace: memcached-operator-system
+spec:
+  ports:
+  - port: 443
+    targetPort: 9443
+  selector:
+    control-plane: controller-manager
+---
 apiVersion: apps/v1
 kind: Deployment
 metadata:
@@ -241,18 +278,56 @@ spec:
         - /manager
         image: controller:latest
         name: manager
+        ports:
+        - containerPort: 9443
+          name: webhook-server
+          protocol: TCP
         resources:
           limits:
             cpu: 100m
             memory: 30Mi
           requests:
             cpu: 100m
             memory: 20Mi
+        volumeMounts:
+        - mountPath: /tmp/k8s-webhook-server/serving-certs
+          name: cert
+          readOnly: true
       terminationGracePeriodSeconds: 10
+      volumes:
+      - name: cert
+        secret:
+          defaultMode: 420
+          secretName: webhook-server-cert
 ---
 apiVersion: cache.example.com/v1alpha1
 kind: Memcached
 metadata:
   name: memcached-sample
 spec:
   foo: bar
+---
+apiVersion: admissionregistration.k8s.io/v1beta1
+kind: ValidatingWebhookConfiguration
+metadata:
+  creationTimestamp: null
+  name: memcached-operator-validating-webhook-configuration
+webhooks:
+- clientConfig:
+    caBundle: Cg==
+    service:
+      name: memcached-operator-webhook-service
+      namespace: memcached-operator-system
+      path: /validate-cache-my-domain-v1alpha1-memcached
+  failurePolicy: Fail
+  name: vmemcached.kb.io
+  rules:
+  - apiGroups:
+    - cache.example.com
+    apiVersions:
+    - v1alpha1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - memcacheds