From de510170f4c1bde29d5dc2b1d8b59fbc5f9f3ba8 Mon Sep 17 00:00:00 2001
From: Luke Kysow <1034429+lkysow@users.noreply.github.com>
Date: Tue, 26 Oct 2021 14:23:09 -0700
Subject: [PATCH] Default to excluding system ns's from injection (#726)

* Default to excluding system ns's from injection

kube-system is excluded because it's unlikely users will want to
provision Connect pods in that namespace and also because we don't want
to block pods being provisioned there if our webhook injector is down.

local-path-storage is excluded because this ns is used by kind to
provision PVCs and if ACLs are enabled then the install gets into a
deadlock where:
- PVC can't be provisioned because Kind needs to create a Pod
- Pod can't be created because injector webhook needs to be up
- injector webhook can't come up until its got an ACL token
- ACL token can't be provisioned because Consul server isn't up
- Consul server can't be started because it doesn't have a PVC

NOTE: This matching is only supported in Kube 1.21+ where they've added
these labels to namespaces automatically now.
---
 CHANGELOG.md                          |  5 +++++
 acceptance/framework/config/config.go |  5 -----
 charts/consul/values.yaml             | 11 ++++++++++-
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index aa4e981474..aa92e03d2b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,8 +1,12 @@
 ## UNRELEASED
 
+BREAKING CHANGES:
+* Helm Chart
+  * The `kube-system` and `local-path-storage` namespaces are now _excluded_ from connect injection by default on Kubernetes versions >= 1.21. If you wish to enable injection on those namespaces, set `connectInject.namespaceSelector` to `null`. [[GH-726](https://github.com/hashicorp/consul-k8s/pull/726)]
 IMPROVEMENTS:
 * Helm Chart
   * Automatic retry for `gossip-encryption-autogenerate-job` on failure [[GH-789](https://github.com/hashicorp/consul-k8s/pull/789)]
+  * `kube-system` and `local-path-storage` namespaces are now excluded from connect injection by default on Kubernetes versions >= 1.21. This prevents deadlock issues when `kube-system` components go down and allows Kind to work without changing the failure policy of the mutating webhook. [[GH-726](https://github.com/hashicorp/consul-k8s/pull/726)]
 * CLI
   * Add `status` command. [[GH-768](https://github.com/hashicorp/consul-k8s/pull/768)]
 
@@ -38,6 +42,7 @@ BUG FIXES:
   * Fix consul-k8s image version in values file. [[GH-732](https://github.com/hashicorp/consul-k8s/pull/732)]
 
 ## 0.34.0 (September 17, 2021)
+
 FEATURES:
 * CLI
   * The `consul-k8s` CLI enables users to deploy and operate Consul on Kubernetes.
diff --git a/acceptance/framework/config/config.go b/acceptance/framework/config/config.go
index a998587291..28803faa20 100644
--- a/acceptance/framework/config/config.go
+++ b/acceptance/framework/config/config.go
@@ -55,11 +55,6 @@ type TestConfig struct {
 func (t *TestConfig) HelmValuesFromConfig() (map[string]string, error) {
 	helmValues := map[string]string{}
 
-	// If Kind is being used they use a pod to provision the underlying PV which will hang if we
-	// use "Fail" for the webhook failurePolicy.
-	if t.UseKind {
-		setIfNotEmpty(helmValues, "connectInject.failurePolicy", "Ignore")
-	}
 	// Set the enterprise image first if enterprise tests are enabled.
 	// It can be overwritten by the -consul-image flag later.
 	if t.EnableEnterprise {
diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml
index a64c09fc7f..44b81cc327 100644
--- a/charts/consul/values.yaml
+++ b/charts/consul/values.yaml
@@ -1625,6 +1625,11 @@ connectInject:
   # See https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-namespaceselector
   # for more details.
   #
+  # By default, we exclude the kube-system namespace since usually users won't
+  # want those pods injected and also the local-path-storage namespace so that
+  # Kind (Kubernetes In Docker) can provision Pods used to create PVCs.
+  # Note that this exclusion is only supported in Kubernetes v1.21.1+.
+  #
   # Example:
   #
   # ```yaml
@@ -1633,7 +1638,11 @@ connectInject:
   #     namespace-label: label-value
   # ```
   # @type: string
-  namespaceSelector: null
+  namespaceSelector: |
+    matchExpressions:
+      - key: "kubernetes.io/metadata.name"
+        operator: "NotIn"
+        values: ["kube-system","local-path-storage"]
 
   # List of k8s namespaces to allow Connect sidecar
   # injection in. If a k8s namespace is not included or is listed in `k8sDenyNamespaces`,