Set default object selector for webhooks to exclude injector itself (h…

…ashicorp#736) Set default object selector for webhooks to exclude injector itself If `injector.failurePolicy` is set to `Fail`, there is a race condition where if the mutating webhook config is setup before the injector, then the injector can fail to start because it tries to inject itself. We can work around this by ignoring the injector pod in in the webhook by default. Thanks to @joeyslalom for the object selector to exclude the pod. Fixes hashicorp/vault-k8s#258
ryuseongryong · May 24, 2022 · daa2b3b · daa2b3b
1 parent 830761a
commit daa2b3b
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,7 +1,8 @@
 ## Unreleased
 
 CHANGES:
-* `vault` service account is now created even if the server is set to disabled, as per before 0.20.0
+* `vault` service account is now created even if the server is set to disabled, as per before 0.20.0 [GH-737](https://github.com/hashicorp/vault-helm/pull/737)
+* Mutating webhook will no longer target the agent injector pod [GH-736](https://github.com/hashicorp/vault-helm/pull/736)
 
 ## 0.20.0 (May 16th, 2022)
 

diff --git a/templates/_helpers.tpl b/templates/_helpers.tpl
@@ -485,6 +485,22 @@ Sets extra injector webhook annotations
   {{- end }}
 {{- end -}}
 
+{{/*
+Set's the injector webhook objectSelector
+*/}}
+{{- define "injector.objectSelector" -}}
+  {{- $v := or (((.Values.injector.webhook)).objectSelector) (.Values.injector.objectSelector) -}}
+  {{ if $v }}
+    objectSelector:
+    {{- $tp := typeOf $v -}}
+    {{ if eq $tp "string" }}
+      {{ tpl $v . | indent 6 | trim }}
+    {{ else }}
+      {{ toYaml $v | indent 6 | trim }}
+    {{ end }}
+  {{ end }}
+{{ end }}
+
 {{/*
 Sets extra ui service annotations
 */}}

diff --git a/templates/injector-mutating-webhook.yaml b/templates/injector-mutating-webhook.yaml
@@ -35,8 +35,5 @@ webhooks:
     namespaceSelector:
 {{ toYaml (((.Values.injector.webhook)).namespaceSelector | default .Values.injector.namespaceSelector) | indent 6}}
 {{ end }}
-{{- if or (((.Values.injector.webhook)).objectSelector) (.Values.injector.objectSelector) }}
-    objectSelector:
-{{ toYaml (((.Values.injector.webhook)).objectSelector | default .Values.injector.objectSelector) | indent 6}}
-{{ end }}
+{{- template "injector.objectSelector" . -}}
 {{ end }}
diff --git a/test/unit/injector-mutating-webhook.bats b/test/unit/injector-mutating-webhook.bats
@@ -301,15 +301,15 @@ load _helpers
   [ "${actual}" = "true" ]
 }
 
-@test "injector/MutatingWebhookConfiguration: webhook.objectSelector empty by default" {
+@test "injector/MutatingWebhookConfiguration: webhook.objectSelector not 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" ]
+      yq -r '.webhooks[0].objectSelector.matchExpressions[0].key' | tee /dev/stderr)
+  [ "${actual}" = "app.kubernetes.io/name" ]
 }
 
 @test "injector/MutatingWebhookConfiguration: can set webhook.objectSelector" {

diff --git a/values.schema.json b/values.schema.json
@@ -336,7 +336,10 @@
                     ]
                 },
                 "objectSelector": {
-                    "type": "object"
+                    "type": [
+                        "object",
+                        "string"
+                    ]
                 },
                 "podDisruptionBudget": {
                     "type": "object"
@@ -406,7 +409,10 @@
                             "type": "object"
                         },
                         "objectSelector": {
-                            "type": "object"
+                            "type": [
+                                "object",
+                                "string"
+                            ]
                         },
                         "timeoutSeconds": {
                             "type": "integer"

diff --git a/values.yaml b/values.yaml
@@ -91,15 +91,15 @@ injector:
   # Configures all Vault Agent sidecars to revoke their token when shutting down
   revokeOnShutdown: false
 
-  webhook: 
+  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 
+    # 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.
@@ -113,7 +113,7 @@ injector:
     # 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
@@ -132,7 +132,12 @@ injector:
     # objectSelector:
     #    matchLabels:
     #      vault-sidecar-injector: enabled
-    objectSelector: {}
+    objectSelector: |
+      matchExpressions:
+      - key: app.kubernetes.io/name
+        operator: NotIn
+        values:
+        - {{ template "vault.name" . }}-agent-injector
 
     # Extra annotations to attach to the webhook
     annotations: {}