generate: default CSV admissionReviewVersions and sideEffects (o…

…perator-framework#3903)
estroz · Oct 12, 2020 · e852b7d · e852b7d
1 parent ec8d227
commit e852b7d
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 4 deletions.
diff --git a/changelog/fragments/csv-whdescs-defaults.yaml b/changelog/fragments/csv-whdescs-defaults.yaml
@@ -0,0 +1,11 @@
+entries:
+  - description: >
+      `generate <bundle|packagemanifests>` now defaults a CSV's `spec.webhookDefinition[].admissionReviewVersions`
+      to []string{"v1beta1"}, as an empty or null value is invalid.
+    kind: bugfix
+    pull_request_override: 3903
+  - description: >
+      `generate <bundle|packagemanifests>` now defaults a CSV's `spec.webhookDefinition[].sideEffects`
+      to "None", as an empty or null value is invalid.
+    kind: bugfix
+    pull_request_override: 3903
diff --git a/internal/generate/clusterserviceversion/clusterserviceversion_updaters.go b/internal/generate/clusterserviceversion/clusterserviceversion_updaters.go
@@ -286,6 +286,9 @@ func applyWebhooks(c *collector.Manifests, csv *operatorsv1alpha1.ClusterService
 	csv.Spec.WebhookDefinitions = webhookDescriptions
 }
 
+// The default AdmissionReviewVersions set in a CSV if not set in the source webhook.
+var defaultAdmissionReviewVersions = []string{"v1beta1"}
+
 // validatingToWebhookDescription transforms webhook into a WebhookDescription.
 func validatingToWebhookDescription(webhook admissionregv1.ValidatingWebhook, depName string) operatorsv1alpha1.WebhookDescription {
 	description := operatorsv1alpha1.WebhookDescription{
@@ -299,6 +302,13 @@ func validatingToWebhookDescription(webhook admissionregv1.ValidatingWebhook, de
 		TimeoutSeconds:          webhook.TimeoutSeconds,
 		AdmissionReviewVersions: webhook.AdmissionReviewVersions,
 	}
+	if len(description.AdmissionReviewVersions) == 0 {
+		description.AdmissionReviewVersions = defaultAdmissionReviewVersions
+	}
+	if description.SideEffects == nil {
+		seNone := admissionregv1.SideEffectClassNone
+		description.SideEffects = &seNone
+	}
 
 	if serviceRef := webhook.ClientConfig.Service; serviceRef != nil {
 		if serviceRef.Port != nil {
@@ -327,6 +337,13 @@ func mutatingToWebhookDescription(webhook admissionregv1.MutatingWebhook, depNam
 		AdmissionReviewVersions: webhook.AdmissionReviewVersions,
 		ReinvocationPolicy:      webhook.ReinvocationPolicy,
 	}
+	if len(description.AdmissionReviewVersions) == 0 {
+		description.AdmissionReviewVersions = defaultAdmissionReviewVersions
+	}
+	if description.SideEffects == nil {
+		seNone := admissionregv1.SideEffectClassNone
+		description.SideEffects = &seNone
+	}
 
 	if serviceRef := webhook.ClientConfig.Service; serviceRef != nil {
 		if serviceRef.Port != nil {

diff --git a/...rate/testdata/clusterserviceversions/output/memcached-operator.clusterserviceversion.yaml b/...rate/testdata/clusterserviceversions/output/memcached-operator.clusterserviceversion.yaml
@@ -184,7 +184,8 @@ spec:
     url: https://your.domain
   version: 0.0.1
   webhookdefinitions:
-  - admissionReviewVersions: null
+  - admissionReviewVersions:
+    - v1beta1
     deploymentName: memcached-operator-controller-manager
     failurePolicy: Fail
     generateName: vmemcached.kb.io
@@ -198,10 +199,11 @@ spec:
       - UPDATE
       resources:
       - memcacheds
-    sideEffects: null
+    sideEffects: None
     type: ValidatingAdmissionWebhook
     webhookPath: /validate-cache-my-domain-v1alpha1-memcached
-  - admissionReviewVersions: null
+  - admissionReviewVersions:
+    - v1beta1
     deploymentName: memcached-operator-controller-manager
     failurePolicy: Fail
     generateName: mmemcached.kb.io
@@ -215,6 +217,6 @@ spec:
       - UPDATE
       resources:
       - memcacheds
-    sideEffects: null
+    sideEffects: None
     type: MutatingAdmissionWebhook
     webhookPath: /mutate-cache-my-domain-v1alpha1-memcached