Maintain pre-existing Mutating Webhook default values for Kubernetes …

…1.22 (hashicorp#692) * Prepare default values for MutatingWebhookConfiguration hashicorp#691 * Add values.yaml values to injector-mutating-webhook.yaml hashicorp#691 * Duplicate and deprecate top-level webhook settings and put them in a webhook object * Made the new values default with the fallback to the old values.yaml * Fix _helpers.tpl to support both old and new webhook annotations * Add new tests and deprecate old ones for injector webhook configuration * Old tests now work with old values.yaml * Add all new fields showing that they have priority over old ones * Add deprecation note to injector.failurePolicy hashicorp#691
Iristyle · Mar 18, 2022 · 56a253b · 56a253b
1 parent 549d9b8
commit 56a253b
Show file tree

Hide file tree

Showing 5 changed files with 293 additions and 47 deletions.
diff --git a/templates/_helpers.tpl b/templates/_helpers.tpl
@@ -372,13 +372,13 @@ Sets extra injector service annotations
 Sets extra injector webhook annotations
 */}}
 {{- define "injector.webhookAnnotations" -}}
-  {{- if .Values.injector.webhookAnnotations }}
+  {{- if or (((.Values.injector.webhook)).annotations) (.Values.injector.webhookAnnotations)  }}
   annotations:
-    {{- $tp := typeOf .Values.injector.webhookAnnotations }}
+    {{- $tp := typeOf (or (((.Values.injector.webhook)).annotations) (.Values.injector.webhookAnnotations)) }}
     {{- if eq $tp "string" }}
-      {{- tpl .Values.injector.webhookAnnotations . | nindent 4 }}
+      {{- tpl (((.Values.injector.webhook)).annotations | default .Values.injector.webhookAnnotations) . | nindent 4 }}
     {{- else }}
-      {{- toYaml .Values.injector.webhookAnnotations | nindent 4 }}
+      {{- toYaml (((.Values.injector.webhook)).annotations | default .Values.injector.webhookAnnotations) | nindent 4 }}
     {{- end }}
   {{- end }}
 {{- end -}}

diff --git a/templates/injector-mutating-webhook.yaml b/templates/injector-mutating-webhook.yaml
@@ -14,10 +14,11 @@ metadata:
   {{- template "injector.webhookAnnotations" . }}
 webhooks:
   - name: vault.hashicorp.com
+    failurePolicy: {{ ((.Values.injector.webhook)).failurePolicy | default .Values.injector.failurePolicy }}
+    matchPolicy: {{ ((.Values.injector.webhook)).matchPolicy | default "Exact" }}
     sideEffects: None
-    admissionReviewVersions:
-    - "v1beta1"
-    - "v1"
+    timeoutSeconds: {{ ((.Values.injector.webhook)).timeoutSeconds | default "30" }}
+    admissionReviewVersions: ["v1", "v1beta1"]
     clientConfig:
       service:
         name: {{ template "vault.fullname" . }}-agent-injector-svc
@@ -29,15 +30,12 @@ webhooks:
         apiGroups: [""]
         apiVersions: ["v1"]
         resources: ["pods"]
-{{- if .Values.injector.namespaceSelector }}
+{{- if or (.Values.injector.namespaceSelector) (((.Values.injector.webhook)).namespaceSelector) }}
     namespaceSelector:
-{{ toYaml .Values.injector.namespaceSelector | indent 6}}
+{{ toYaml (((.Values.injector.webhook)).namespaceSelector | default .Values.injector.namespaceSelector) | indent 6}}
 {{ end }}
-{{- if .Values.injector.objectSelector }}
+{{- if or (((.Values.injector.webhook)).objectSelector) (.Values.injector.objectSelector) }}
     objectSelector:
-{{ toYaml .Values.injector.objectSelector | indent 6}}
-{{ end }}
-{{- with .Values.injector.failurePolicy }}
-    failurePolicy: {{.}}
+{{ toYaml (((.Values.injector.webhook)).objectSelector | default .Values.injector.objectSelector) | indent 6}}
 {{ end }}
 {{ end }}
diff --git a/test/unit/injector-mutating-webhook.bats b/test/unit/injector-mutating-webhook.bats
@@ -53,103 +53,274 @@ load _helpers
   [ "${actual}" = "\"\"" ]
 }
 
-@test "injector/MutatingWebhookConfiguration: namespaceSelector empty by default" {
+@test "injector/MutatingWebhookConfiguration: failurePolicy 'Ignore' by default (deprecated)" {
   cd `chart_dir`
   local actual=$(helm template \
       --show-only templates/injector-mutating-webhook.yaml  \
       --set 'injector.enabled=true' \
+      --set 'injector.webhook=null' \
       --namespace foo \
       . | tee /dev/stderr |
-      yq '.webhooks[0].namespaceSelector' | tee /dev/stderr)
-  [ "${actual}" = "null" ]
+      yq '.webhooks[0].failurePolicy' | tee /dev/stderr)
+  [ "${actual}" = "\"Ignore\"" ]
 }
 
-@test "injector/MutatingWebhookConfiguration: can set namespaceSelector" {
+@test "injector/MutatingWebhookConfiguration: can set failurePolicy (deprecated)" {
   cd `chart_dir`
   local actual=$(helm template \
       --show-only templates/injector-mutating-webhook.yaml  \
       --set 'injector.enabled=true' \
-      --set 'injector.namespaceSelector.matchLabels.injector=true' \
+      --set 'injector.webhook=null' \
+      --set 'injector.failurePolicy=Fail' \
       . | tee /dev/stderr |
-      yq '.webhooks[0].namespaceSelector.matchLabels.injector' | tee /dev/stderr)
+      yq '.webhooks[0].failurePolicy' | tee /dev/stderr)
 
-  [ "${actual}" = "true" ]
+  [ "${actual}" = "\"Fail\"" ]
 }
 
-@test "injector/MutatingWebhookConfiguration: objectSelector empty by default" {
+@test "injector/MutatingWebhookConfiguration: webhook.failurePolicy 'Ignore' by default" {
   cd `chart_dir`
   local actual=$(helm template \
       --show-only templates/injector-mutating-webhook.yaml  \
       --set 'injector.enabled=true' \
-      --namespace foo \
+      --set 'injector.failurePolicy=Invalid' \
       . | tee /dev/stderr |
-      yq '.webhooks[0].objectSelector' | tee /dev/stderr)
-  [ "${actual}" = "null" ]
+      yq '.webhooks[0].failurePolicy' | tee /dev/stderr)
+
+  [ "${actual}" = "\"Ignore\"" ]
 }
 
-@test "injector/MutatingWebhookConfiguration: can set objectSelector" {
+@test "injector/MutatingWebhookConfiguration: can set webhook.failurePolicy" {
   cd `chart_dir`
   local actual=$(helm template \
       --show-only templates/injector-mutating-webhook.yaml  \
       --set 'injector.enabled=true' \
-      --set 'injector.objectSelector.matchLabels.injector=true' \
+      --set 'injector.webhook.failurePolicy=Fail' \
+      --set 'injector.failurePolicy=Invalid' \
       . | tee /dev/stderr |
-      yq '.webhooks[0].objectSelector.matchLabels.injector' | tee /dev/stderr)
+      yq '.webhooks[0].failurePolicy' | tee /dev/stderr)
 
-  [ "${actual}" = "true" ]
+  [ "${actual}" = "\"Fail\"" ]
 }
 
-@test "injector/MutatingWebhookConfiguration: failurePolicy 'Ignore' by default" {
+@test "injector/MutatingWebhookConfiguration: webhook.matchPolicy 'Exact' by default" {
   cd `chart_dir`
   local actual=$(helm template \
       --show-only templates/injector-mutating-webhook.yaml  \
       --set 'injector.enabled=true' \
-      --namespace foo \
       . | tee /dev/stderr |
-      yq '.webhooks[0].failurePolicy' | tee /dev/stderr)
-  [ "${actual}" = "\"Ignore\"" ]
+      yq '.webhooks[0].matchPolicy' | tee /dev/stderr)
+
+  [ "${actual}" = "\"Exact\"" ]
 }
 
-@test "injector/MutatingWebhookConfiguration: can set failurePolicy" {
+@test "injector/MutatingWebhookConfiguration: can set webhook.matchPolicy" {
   cd `chart_dir`
   local actual=$(helm template \
       --show-only templates/injector-mutating-webhook.yaml  \
       --set 'injector.enabled=true' \
-      --set 'injector.failurePolicy=Fail' \
+      --set 'injector.webhook.matchPolicy=Equivalent' \
       . | tee /dev/stderr |
-      yq '.webhooks[0].failurePolicy' | tee /dev/stderr)
+      yq '.webhooks[0].matchPolicy' | tee /dev/stderr)
 
-  [ "${actual}" = "\"Fail\"" ]
+  [ "${actual}" = "\"Equivalent\"" ]
+}
+
+@test "injector/MutatingWebhookConfiguration: timeoutSeconds by default 30" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/injector-mutating-webhook.yaml  \
+      --set 'injector.enabled=true' \
+      --set 'injector.webhook=null' \
+      . | tee /dev/stderr |
+      yq '.webhooks[0].timeoutSeconds' | tee /dev/stderr)
+
+  [ "${actual}" = "30" ]
+}
+
+@test "injector/MutatingWebhookConfiguration: can set webhook.timeoutSeconds" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/injector-mutating-webhook.yaml  \
+      --set 'injector.enabled=true' \
+      --set 'injector.webhook.timeoutSeconds=50' \
+      . | tee /dev/stderr |
+      yq '.webhooks[0].timeoutSeconds' | tee /dev/stderr)
+
+  [ "${actual}" = "50" ]
 }
 
 #--------------------------------------------------------------------
 # annotations
 
-@test "injector/MutatingWebhookConfiguration: default annotations" {
+@test "injector/MutatingWebhookConfiguration: default webhookAnnotations (deprecated)" {
   cd `chart_dir`
   local actual=$(helm template \
       --show-only templates/injector-mutating-webhook.yaml \
+      --set 'injector.enabled=true' \
+      --set 'injector.webhook=null' \
       . | tee /dev/stderr |
       yq -r '.metadata.annotations' | tee /dev/stderr)
   [ "${actual}" = "null" ]
 }
 
-@test "injector/MutatingWebhookConfiguration: specify annotations yaml" {
+@test "injector/MutatingWebhookConfiguration: specify webhookAnnotations yaml (deprecated)" {
   cd `chart_dir`
   local actual=$(helm template \
       --show-only templates/injector-mutating-webhook.yaml \
+      --set 'injector.enabled=true' \
+      --set 'injector.webhook=null' \
       --set 'injector.webhookAnnotations.foo=bar' \
       . | tee /dev/stderr |
       yq -r '.metadata.annotations.foo' | tee /dev/stderr)
   [ "${actual}" = "bar" ]
 }
 
-@test "injector/MutatingWebhookConfiguration: specify annotations yaml string" {
+@test "injector/MutatingWebhookConfiguration: specify webhookAnnotations yaml string (deprecated)" {
   cd `chart_dir`
   local actual=$(helm template \
       --show-only templates/injector-mutating-webhook.yaml \
+      --set 'injector.enabled=true' \
+      --set 'injector.webhook=null' \
       --set 'injector.webhookAnnotations=foo: bar' \
       . | tee /dev/stderr |
       yq -r '.metadata.annotations.foo' | tee /dev/stderr)
   [ "${actual}" = "bar" ]
 }
+
+@test "injector/MutatingWebhookConfiguration: default webhook.annotations" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/injector-mutating-webhook.yaml \
+      --set 'injector.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.metadata.annotations' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "injector/MutatingWebhookConfiguration: specify webhook.annotations yaml" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/injector-mutating-webhook.yaml \
+      --set 'injector.enabled=true' \
+      --set 'injector.webhook.annotations.foo=bar' \
+      --set 'injector.webhookAnnotations.invalid=invalid' \
+      . | tee /dev/stderr |
+      yq -r '.metadata.annotations.foo' | tee /dev/stderr)
+  [ "${actual}" = "bar" ]
+}
+
+@test "injector/MutatingWebhookConfiguration: specify webhook.annotations yaml string" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/injector-mutating-webhook.yaml \
+      --set 'injector.enabled=true' \
+      --set 'injector.webhook.annotations=foo: bar' \
+      --set 'injector.webhookAnnotations=invalid: invalid' \
+      . | tee /dev/stderr |
+      yq -r '.metadata.annotations.foo' | tee /dev/stderr)
+  [ "${actual}" = "bar" ]
+}
+
+#--------------------------------------------------------------------
+# namespaceSelector
+
+@test "injector/MutatingWebhookConfiguration: namespaceSelector empty by default (deprecated)" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/injector-mutating-webhook.yaml  \
+      --set 'injector.enabled=true' \
+      --set 'injector.webhook=null' \
+      --namespace foo \
+      . | tee /dev/stderr |
+      yq '.webhooks[0].namespaceSelector' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "injector/MutatingWebhookConfiguration: can set namespaceSelector (deprecated)" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/injector-mutating-webhook.yaml  \
+      --set 'injector.enabled=true' \
+      --set 'injector.namespaceSelector.matchLabels.injector=true' \
+      . | tee /dev/stderr |
+      yq '.webhooks[0].namespaceSelector.matchLabels.injector' | tee /dev/stderr)
+
+  [ "${actual}" = "true" ]
+}
+
+@test "injector/MutatingWebhookConfiguration: webhook.namespaceSelector empty by default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/injector-mutating-webhook.yaml  \
+      --set 'injector.enabled=true' \
+      --namespace foo \
+      . | tee /dev/stderr |
+      yq '.webhooks[0].namespaceSelector' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "injector/MutatingWebhookConfiguration: can set set webhook.namespaceSelector" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/injector-mutating-webhook.yaml  \
+      --set 'injector.enabled=true' \
+      --set 'injector.webhook.namespaceSelector.matchLabels.injector=true' \
+      --set 'injector.namespaceSelector.matchLabels.injector=false' \
+      . | tee /dev/stderr |
+      yq '.webhooks[0].namespaceSelector.matchLabels.injector' | tee /dev/stderr)
+
+  [ "${actual}" = "true" ]
+}
+
+#--------------------------------------------------------------------
+# objectSelector
+
+@test "injector/MutatingWebhookConfiguration: objectSelector empty by default (deprecated)" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/injector-mutating-webhook.yaml  \
+      --set 'injector.enabled=true' \
+      --set 'injector.webhook=null' \
+      --namespace foo \
+      . | tee /dev/stderr |
+      yq '.webhooks[0].objectSelector' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "injector/MutatingWebhookConfiguration: can set objectSelector (deprecated)" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/injector-mutating-webhook.yaml  \
+      --set 'injector.enabled=true' \
+      --set 'injector.webhook=null' \
+      --set 'injector.objectSelector.matchLabels.injector=true' \
+      . | tee /dev/stderr |
+      yq '.webhooks[0].objectSelector.matchLabels.injector' | tee /dev/stderr)
+
+  [ "${actual}" = "true" ]
+}
+
+@test "injector/MutatingWebhookConfiguration: webhook.objectSelector empty by default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/injector-mutating-webhook.yaml  \
+      --set 'injector.enabled=true' \
+      --namespace foo \
+      . | tee /dev/stderr |
+      yq '.webhooks[0].objectSelector' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "injector/MutatingWebhookConfiguration: can set webhook.objectSelector" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/injector-mutating-webhook.yaml  \
+      --set 'injector.enabled=true' \
+      --set 'injector.webhook.objectSelector.matchLabels.injector=true' \
+      --set 'injector.objectSelector.matchLabels.injector=false' \
+      . | tee /dev/stderr |
+      yq '.webhooks[0].objectSelector.matchLabels.injector' | tee /dev/stderr)
+
+  [ "${actual}" = "true" ]
+}
diff --git a/values.schema.json b/values.schema.json
@@ -374,6 +374,32 @@
                         "string"
                     ]
                 },
+                "webhook": {
+                    "type": "object",
+                    "properties": {
+                        "annotations": {
+                            "type": [
+                                "object",
+                                "string"
+                            ]
+                        },
+                        "failurePolicy": {
+                            "type": "string"
+                        },
+                        "matchPolicy": {
+                            "type": "string"
+                        },
+                        "namespaceSelector": {
+                            "type": "object"
+                        },
+                        "objectSelector": {
+                            "type": "object"
+                        },
+                        "timeoutSeconds": {
+                            "type": "integer"
+                        }
+                    }
+                },
                 "webhookAnnotations": {
                     "type": [
                         "object",