From 56a253ba97369ee96cf4da4a20fe8fb6342e27f6 Mon Sep 17 00:00:00 2001 From: Remco Buddelmeijer Date: Fri, 18 Mar 2022 15:15:43 +0100 Subject: [PATCH] Maintain pre-existing Mutating Webhook default values for Kubernetes 1.22 (#692) * Prepare default values for MutatingWebhookConfiguration #691 * Add values.yaml values to injector-mutating-webhook.yaml #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 #691 --- templates/_helpers.tpl | 8 +- templates/injector-mutating-webhook.yaml | 18 +- test/unit/injector-mutating-webhook.bats | 293 ++++++++++++++++++----- values.schema.json | 26 ++ values.yaml | 65 ++++- 5 files changed, 328 insertions(+), 82 deletions(-) diff --git a/templates/_helpers.tpl b/templates/_helpers.tpl index 12afeab..9452698 100644 --- 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 index de7dd56..b0a6ac0 100644 --- 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 index 1e6e150..ef9bf83 100755 --- a/test/unit/injector-mutating-webhook.bats +++ b/test/unit/injector-mutating-webhook.bats @@ -53,18 +53,191 @@ 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].failurePolicy' | tee /dev/stderr) + [ "${actual}" = "\"Ignore\"" ] +} + +@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.webhook=null' \ + --set 'injector.failurePolicy=Fail' \ + . | tee /dev/stderr | + yq '.webhooks[0].failurePolicy' | tee /dev/stderr) + + [ "${actual}" = "\"Fail\"" ] +} + +@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' \ + --set 'injector.failurePolicy=Invalid' \ + . | tee /dev/stderr | + yq '.webhooks[0].failurePolicy' | tee /dev/stderr) + + [ "${actual}" = "\"Ignore\"" ] +} + +@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.webhook.failurePolicy=Fail' \ + --set 'injector.failurePolicy=Invalid' \ + . | tee /dev/stderr | + yq '.webhooks[0].failurePolicy' | tee /dev/stderr) + + [ "${actual}" = "\"Fail\"" ] +} + +@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' \ + . | tee /dev/stderr | + yq '.webhooks[0].matchPolicy' | tee /dev/stderr) + + [ "${actual}" = "\"Exact\"" ] +} + +@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.webhook.matchPolicy=Equivalent' \ + . | tee /dev/stderr | + yq '.webhooks[0].matchPolicy' | tee /dev/stderr) + + [ "${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 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 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 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" { +@test "injector/MutatingWebhookConfiguration: can set namespaceSelector (deprecated)" { cd `chart_dir` local actual=$(helm template \ --show-only templates/injector-mutating-webhook.yaml \ @@ -76,7 +249,59 @@ load _helpers [ "${actual}" = "true" ] } -@test "injector/MutatingWebhookConfiguration: objectSelector empty by default" { +@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 \ @@ -87,69 +312,15 @@ load _helpers [ "${actual}" = "null" ] } -@test "injector/MutatingWebhookConfiguration: can set objectSelector" { +@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.objectSelector.matchLabels.injector=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" ] -} - -@test "injector/MutatingWebhookConfiguration: failurePolicy 'Ignore' 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\"" ] -} - -@test "injector/MutatingWebhookConfiguration: can set failurePolicy" { - cd `chart_dir` - local actual=$(helm template \ - --show-only templates/injector-mutating-webhook.yaml \ - --set 'injector.enabled=true' \ - --set 'injector.failurePolicy=Fail' \ - . | tee /dev/stderr | - yq '.webhooks[0].failurePolicy' | tee /dev/stderr) - - [ "${actual}" = "\"Fail\"" ] -} - -#-------------------------------------------------------------------- -# annotations - -@test "injector/MutatingWebhookConfiguration: default annotations" { - cd `chart_dir` - local actual=$(helm template \ - --show-only templates/injector-mutating-webhook.yaml \ - . | tee /dev/stderr | - yq -r '.metadata.annotations' | tee /dev/stderr) - [ "${actual}" = "null" ] -} - -@test "injector/MutatingWebhookConfiguration: specify annotations yaml" { - cd `chart_dir` - local actual=$(helm template \ - --show-only templates/injector-mutating-webhook.yaml \ - --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" { - cd `chart_dir` - local actual=$(helm template \ - --show-only templates/injector-mutating-webhook.yaml \ - --set 'injector.webhookAnnotations=foo: bar' \ - . | tee /dev/stderr | - yq -r '.metadata.annotations.foo' | tee /dev/stderr) - [ "${actual}" = "bar" ] -} +} \ No newline at end of file diff --git a/values.schema.json b/values.schema.json index 40e3dd8..981bb7c 100644 --- 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", diff --git a/values.yaml b/values.yaml index db7c484..20d85d5 100644 --- a/values.yaml +++ b/values.yaml @@ -90,6 +90,61 @@ injector: # Configures all Vault Agent sidecars to revoke their token when shutting down revokeOnShutdown: false + webhook: + # Configures failurePolicy of the webhook. The "unspecified" default behaviour depends on the + # API Version of the WebHook. + # To block pod creation while webhook is unavailable, set the policy to `Fail` below. + # See https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#failure-policy + # + failurePolicy: Ignore + + # matchPolicy specifies the approach to accepting changes based on the rules of + # the MutatingWebhookConfiguration. + # See https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-matchpolicy + # for more details. + # + matchPolicy: Exact + + # timeoutSeconds is the amount of seconds before the webhook request will be ignored + # or fails. + # If it is ignored or fails depends on the failurePolicy + # See https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#timeouts + # for more details. + # + timeoutSeconds: 30 + + # namespaceSelector is the selector for restricting the webhook to only + # specific namespaces. + # See https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-namespaceselector + # for more details. + # Example: + # namespaceSelector: + # matchLabels: + # sidecar-injector: enabled + namespaceSelector: {} + + # objectSelector is the selector for restricting the webhook to only + # specific labels. + # See https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-objectselector + # for more details. + # Example: + # objectSelector: + # matchLabels: + # vault-sidecar-injector: enabled + objectSelector: {} + + # Extra annotations to attach to the webhook + annotations: {} + + # Deprecated: please use 'webhook.failurePolicy' instead + # Configures failurePolicy of the webhook. The "unspecified" default behaviour depends on the + # API Version of the WebHook. + # To block pod creation while webhook is unavailable, set the policy to `Fail` below. + # See https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#failure-policy + # + failurePolicy: Ignore + + # Deprecated: please use 'webhook.namespaceSelector' instead # namespaceSelector is the selector for restricting the webhook to only # specific namespaces. # See https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-namespaceselector @@ -99,6 +154,8 @@ injector: # matchLabels: # sidecar-injector: enabled namespaceSelector: {} + + # Deprecated: please use 'webhook.objectSelector' instead # objectSelector is the selector for restricting the webhook to only # specific labels. # See https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-objectselector @@ -109,13 +166,7 @@ injector: # vault-sidecar-injector: enabled objectSelector: {} - # Configures failurePolicy of the webhook. The "unspecified" default behaviour deoends on the - # API Version of the WebHook. - # To block pod creation while webhook is unavailable, set the policy to `Fail` below. - # See https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#failure-policy - # - failurePolicy: Ignore - + # Deprecated: please use 'webhook.annotations' instead # Extra annotations to attach to the webhook webhookAnnotations: {}