Deprecate connectInject.centralConfig

CHANGELOG.md

@@ -1,5 +1,22 @@
 ## Unreleased
 
+BREAKING CHANGES:
+* The following Helm settings are no longer supported and will cause errors on `helm upgrade`.
+  See [Upgrade to CRDs](https://www.consul.io/docs/k8s/crds/upgrade-to-crds)
+  for more information on how to upgrade. [[GH-763](https://github.com/hashicorp/consul-helm/pull/763)]
+  * `connectInject.centralConfig.defaultProtocol`
+  * `connectInject.centralConfig.proxyDefaults`
+  * `connectInject.centralConfig.enabled`
+  * `meshGateway.globalMode`
+* The `consul.hashicorp.com/connect-service-protocol` annotation on Connect pods is
+  no longer supported with this version of `consul-k8s` (0.23.0).
+
+  Current deployments that have the annotation should remove it, otherwise they
+  will get an error if a pod from that deployment is rescheduled.
+
+  See [Upgrade to CRDs](https://www.consul.io/docs/k8s/crds/upgrade-to-crds)
+  for more information on how to upgrade.
+
 ## 0.29.0 (Jan 22, 2021)
 
 IMPROVEMENTS:

templates/client-config-configmap.yaml

@@ -14,12 +14,10 @@ metadata:
 data:
   extra-from-values.json: |-
 {{ tpl .Values.client.extraConfig . | trimAll "\"" | indent 4 }}
-  {{- if (and .Values.connectInject.enabled .Values.connectInject.centralConfig.enabled) }}
   central-config.json: |-
     {
       "enable_central_service_config": true
     }
-  {{- end }}
 
   {{- if (and .Values.connectInject.enabled .Values.connectInject.healthChecks.enabled) }}
   {{/* We set check_update_interval to 0s so that check output is immediately viewable

templates/connect-inject-deployment.yaml

@@ -2,6 +2,9 @@
 {{- if not (or (and (ne (.Values.client.enabled | toString) "-") .Values.client.enabled) (and (eq (.Values.client.enabled | toString) "-") .Values.global.enabled)) }}{{ fail "clients must be enabled for connect injection" }}{{ end }}
 {{- if not .Values.client.grpc }}{{ fail "client.grpc must be true for connect injection" }}{{ end }}
 {{- if and .Values.connectInject.consulNamespaces.mirroringK8S (not .Values.global.enableConsulNamespaces) }}{{ fail "global.enableConsulNamespaces must be true if mirroringK8S=true" }}{{ end }}
+{{- if .Values.connectInject.centralConfig }}{{- if eq (toString .Values.connectInject.centralConfig.enabled) "false" }}{{ fail "connectInject.centralConfig.enabled cannot be set to false; to disable, set enable_central_service_config to false in server.extraConfig and client.extraConfig" }}{{ end -}}{{ end -}}
+{{- if .Values.connectInject.centralConfig }}{{- if .Values.connectInject.centralConfig.defaultProtocol }}{{ fail "connectInject.centralConfig.defaultProtocol is no longer supported; instead you must migrate to CRDs (see www.consul.io/docs/k8s/crds/upgrade-to-crds)" }}{{ end }}{{ end -}}
+{{- if .Values.connectInject.centralConfig }}{{ if .Values.connectInject.centralConfig.proxyDefaults }}{{- if ne (trim .Values.connectInject.centralConfig.proxyDefaults) `{}` }}{{ fail "connectInject.centralConfig.proxyDefaults is no longer supported; instead you must migrate to CRDs (see www.consul.io/docs/k8s/crds/upgrade-to-crds)" }}{{ end }}{{ end }}{{ end -}}
 {{- if .Values.connectInject.imageEnvoy }}{{ fail "connectInject.imageEnvoy must be specified in global.imageEnvoy" }}{{ end }}
 # The deployment for running the Connect sidecar injector
 apiVersion: apps/v1
@@ -101,12 +104,6 @@ spec:
                 {{- else if .Values.global.acls.manageSystemACLs }}
                 -acl-auth-method="{{ template "consul.fullname" . }}-k8s-auth-method" \
                 {{- end }}
-                {{- if .Values.connectInject.centralConfig.enabled }}
-                -enable-central-config=true \
-                {{- end }}
-                {{- if (and .Values.connectInject.centralConfig.enabled .Values.connectInject.centralConfig.defaultProtocol) }}
-                -default-protocol="{{ .Values.connectInject.centralConfig.defaultProtocol }}" \
-                {{- end }}
                 {{- range $value := .Values.connectInject.k8sAllowNamespaces }}
                 -allow-k8s-namespace="{{ $value }}" \
                 {{- end }}

templates/mesh-gateway-deployment.yaml

@@ -3,6 +3,7 @@
 {{- if not .Values.client.grpc }}{{ fail "client.grpc must be true" }}{{ end -}}
 {{- if and .Values.global.acls.manageSystemACLs (ne .Values.meshGateway.consulServiceName "") (ne .Values.meshGateway.consulServiceName "mesh-gateway") }}{{ fail "if global.acls.manageSystemACLs is true, meshGateway.consulServiceName cannot be set" }}{{ end -}}
 {{- if .Values.meshGateway.imageEnvoy }}{{ fail "meshGateway.imageEnvoy must be specified in global.imageEnvoy" }}{{ end -}}
+{{- if .Values.meshGateway.globalMode }}{{ fail "meshGateway.globalMode is no longer supported; instead, you must migrate to CRDs (see www.consul.io/docs/k8s/crds/upgrade-to-crds)" }}{{ end -}}
 {{- /* The below test checks if clients are disabled (and if so, fails). We use the conditional from other client files and prepend 'not' */ -}}
 {{- if not (or (and (ne (.Values.client.enabled | toString) "-") .Values.client.enabled) (and (eq (.Values.client.enabled | toString) "-") .Values.global.enabled)) }}{{ fail "clients must be enabled" }}{{ end -}}
 apiVersion: apps/v1

templates/server-config-configmap.yaml

@@ -27,45 +27,8 @@ data:
       }
     }
   {{- end }}
-  {{- if and .Values.connectInject.enabled .Values.connectInject.centralConfig.enabled }}
   central-config.json: |-
     {
       "enable_central_service_config": true
     }
-  {{- if gt (len .Values.connectInject.centralConfig.proxyDefaults) 3 }}
-  proxy-defaults-config.json: |-
-    {
-      "config_entries": {
-        "bootstrap": [
-          {
-            "kind": "proxy-defaults",
-            "name": "global",
-            {{- if and .Values.meshGateway.enabled .Values.meshGateway.globalMode }}
-            "mesh_gateway": {
-              "mode": {{ .Values.meshGateway.globalMode | quote }}
-            },
-            {{- end }}
-            "config":
-{{ tpl .Values.connectInject.centralConfig.proxyDefaults . | trimAll "\"" | indent 14 }}
-          }
-        ]
-      }
-    }
-  {{- else if and .Values.meshGateway.enabled .Values.meshGateway.globalMode }}
-  proxy-defaults-config.json: |-
-    {
-      "config_entries": {
-        "bootstrap": [
-          {
-            "kind": "proxy-defaults",
-            "name": "global",
-            "mesh_gateway": {
-              "mode": {{ .Values.meshGateway.globalMode | quote }}
-            }
-          }
-        ]
-      }
-    }
-  {{- end }}
-  {{- end }}
 {{- end }}

test/acceptance/tests/fixtures/bases/mesh-gateway/kustomization.yaml

@@ -0,0 +1,2 @@
+resources:
+  - proxydefaults.yaml

test/acceptance/tests/fixtures/bases/mesh-gateway/proxydefaults.yaml

@@ -0,0 +1,7 @@
+apiVersion: consul.hashicorp.com/v1alpha1
+kind: ProxyDefaults
+metadata:
+  name: global
+spec:
+  meshGateway:
+    mode: local

test/acceptance/tests/mesh-gateway/mesh_gateway_test.go

@@ -36,6 +36,7 @@ func TestMeshGatewayDefault(t *testing.T) {
 		"global.federation.createFederationSecret": "true",
 
 		"connectInject.enabled": "true",
+		"controller.enabled":    "true",
 
 		"meshGateway.enabled":  "true",
 		"meshGateway.replicas": "1",
@@ -107,6 +108,15 @@ func TestMeshGatewayDefault(t *testing.T) {
 	logger.Log(t, "verifying federation was successful")
 	verifyFederation(t, primaryClient, secondaryClient, releaseName, false)
 
+	// Create a ProxyDefaults resource to configure services to use the mesh
+	// gateways.
+	logger.Log(t, "creating proxy-defaults config")
+	kustomizeDir := "../fixtures/bases/mesh-gateway"
+	k8s.KubectlApplyK(t, primaryContext.KubectlOptions(t), kustomizeDir)
+	helpers.Cleanup(t, cfg.NoCleanupOnFailure, func() {
+		k8s.KubectlDeleteK(t, primaryContext.KubectlOptions(t), kustomizeDir)
+	})
+
 	// Log services in DC2 that DC1 is aware of before exiting this test
 	// TODO: remove this code once issue has been debugged
 	defer func() {
@@ -166,6 +176,7 @@ func TestMeshGatewaySecure(t *testing.T) {
 				"global.federation.createFederationSecret": "true",
 
 				"connectInject.enabled": "true",
+				"controller.enabled":    "true",
 
 				"meshGateway.enabled":  "true",
 				"meshGateway.replicas": "1",
@@ -242,6 +253,15 @@ func TestMeshGatewaySecure(t *testing.T) {
 			logger.Log(t, "verifying federation was successful")
 			verifyFederation(t, primaryClient, secondaryClient, releaseName, true)
 
+			// Create a ProxyDefaults resource to configure services to use the mesh
+			// gateways.
+			logger.Log(t, "creating proxy-defaults config")
+			kustomizeDir := "../fixtures/bases/mesh-gateway"
+			k8s.KubectlApplyK(t, primaryContext.KubectlOptions(t), kustomizeDir)
+			helpers.Cleanup(t, cfg.NoCleanupOnFailure, func() {
+				k8s.KubectlDeleteK(t, primaryContext.KubectlOptions(t), kustomizeDir)
+			})
+
 			// Check that we can connect services over the mesh gateways
 			logger.Log(t, "creating static-server in dc2")
 			k8s.DeployKustomize(t, secondaryContext.KubectlOptions(t), cfg.NoCleanupOnFailure, cfg.DebugDirectory, "../fixtures/cases/static-server-inject")

test/unit/client-config-configmap.bats

@@ -49,9 +49,9 @@ load _helpers
 }
 
 #--------------------------------------------------------------------
-# connectInject.centralConfig
+# connectInject.centralConfig [DEPRECATED]
 
-@test "client/ConfigMap: centralConfig is enabled by default" {
+@test "client/ConfigMap: centralConfig is enabled" {
   cd `chart_dir`
   local actual=$(helm template \
       -s templates/client-config-configmap.yaml  \
@@ -61,17 +61,6 @@ load _helpers
   [ "${actual}" = "true" ]
 }
 
-@test "client/ConfigMap: centralConfig can be disabled" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/client-config-configmap.yaml  \
-      --set 'connectInject.enabled=true' \
-      --set 'connectInject.centralConfig.enabled=false' \
-      . | tee /dev/stderr |
-      yq '.data["central-config.json"] | length > 0' | tee /dev/stderr)
-  [ "${actual}" = "false" ]
-}
-
 #--------------------------------------------------------------------
 # connectInject.healthChecks
 

test/unit/client-daemonset.bats

@@ -398,9 +398,9 @@ load _helpers
       . | tee /dev/stderr |
       yq -r '.spec.template.metadata.annotations."consul.hashicorp.com/config-checksum"' | tee /dev/stderr)
   if [[ $(v2) ]]; then
-    [ "${actual}" = c8c40738a067bbeb4392ec038ad3b1a94d3cd747118b62d72825e44246814951 ]
+    [ "${actual}" = 79e3ac58b3bbfec6ef27d39e3e0f25e7dab63b5cc76d15f4935f308c94a5ff11 ]
   else
-    [ "${actual}" = 1cfdb3a2989f349021d0a4beb7d854a1de35b89d45184caf5290c49460955a33 ]
+    [ "${actual}" = db1cb14f20d2a2f9fe0b3a1f5a65446a32126faeeadf3813f9fe610ba8ee549b ]
   fi
 }
 
@@ -412,9 +412,9 @@ load _helpers
       . | tee /dev/stderr |
       yq -r '.spec.template.metadata.annotations."consul.hashicorp.com/config-checksum"' | tee /dev/stderr)
   if [[ $(v2) ]]; then
-    [ "${actual}" = 7e0bc6cb882996b13c2aad2295ea0a3573185456ba8e1770519b86779075da47 ]
+    [ "${actual}" = f3d2e7d13e5ef853ae79b8c528b263022e2e2f7689aa4b490393f14128a389eb ]
   else
-    [ "${actual}" = d864eda42c5c072921663de14fd9e0593a9ebce192da82a3239e6ded98ff2e8d ]
+    [ "${actual}" = 4e3576ef3ecf806b27906771411df25ff9fe5cc30ea20ea02f7890de944ecd32 ]
   fi
 }
 

test/unit/connect-inject-deployment.bats

@@ -79,6 +79,82 @@ load _helpers
   [[ "$output" =~ "client.grpc must be true for connect injection" ]]
 }
 
+
+#--------------------------------------------------------------------
+# connectInject.centralConfig [DEPRECATED]
+
+@test "connectInject/Deployment: fails if connectInject.centralConfig.enabled is set to false" {
+  cd `chart_dir`
+  run helm template \
+      -s templates/connect-inject-deployment.yaml \
+      --set 'connectInject.enabled=true' \
+      --set 'connectInject.centralConfig.enabled=false' .
+  [ "$status" -eq 1 ]
+  [[ "$output" =~ "connectInject.centralConfig.enabled cannot be set to false; to disable, set enable_central_service_config to false in server.extraConfig and client.extraConfig" ]]
+}
+
+@test "connectInject/Deployment: fails if connectInject.centralConfig.defaultProtocol is set" {
+  cd `chart_dir`
+  run helm template \
+      -s templates/connect-inject-deployment.yaml \
+      --set 'connectInject.enabled=true' \
+      --set 'connectInject.centralConfig.defaultProtocol=http' .
+  [ "$status" -eq 1 ]
+  [[ "$output" =~ "connectInject.centralConfig.defaultProtocol is no longer supported; instead you must migrate to CRDs (see www.consul.io/docs/k8s/crds/upgrade-to-crds)" ]]
+}
+
+@test "connectInject/Deployment: fails if connectInject.centralConfig.proxyDefaults is used" {
+  cd `chart_dir`
+  run helm template \
+      -s templates/connect-inject-deployment.yaml \
+      --set 'connectInject.enabled=true' \
+      --set 'connectInject.centralConfig.proxyDefaults="{\"key\":\"value\"}"' .
+  [ "$status" -eq 1 ]
+  [[ "$output" =~ "connectInject.centralConfig.proxyDefaults is no longer supported; instead you must migrate to CRDs (see www.consul.io/docs/k8s/crds/upgrade-to-crds)" ]]
+}
+
+@test "connectInject/Deployment: does not fail if connectInject.centralConfig.enabled is set to true" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/connect-inject-deployment.yaml  \
+      --set 'connectInject.enabled=true' \
+      --set 'connectInject.centralConfig.enabled=true' \
+      . | tee /dev/stderr |
+      yq 'length > 0' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+@test "connectInject/Deployment: does not fail if connectInject.centralConfig.proxyDefaults is set to {}" {
+  cd `chart_dir`
+
+  # We have to actually create a values file for this test because the
+  # --set and --set-string flags were passing {} as a YAML object rather
+  # than a string.
+  # Previously this was the default in the values.yaml so this test is testing
+  # that if someone had copied this into their values.yaml then nothing would
+  # break. We no longer use this value, but that's okay because the default
+  # empty object had no effect.
+  temp_file=$(mktemp)
+  cat <<EOF > "$temp_file"
+connectInject:
+  enabled: true
+  centralConfig:
+    proxyDefaults: |
+      {}
+EOF
+
+  local actual=$(helm template \
+      -s templates/connect-inject-deployment.yaml  \
+      -f "$temp_file" \
+      . | tee /dev/stderr |
+      yq 'length > 0' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+  rm -f "$temp_file"
+}
+
+#--------------------------------------------------------------------
+# healthChecks
+
 @test "connectInject/Deployment: health checks enabled by default" {
   cd `chart_dir`
   local cmd=$(helm template \
@@ -395,53 +471,6 @@ load _helpers
   [ "${actual}" = "true" ]
 }
 
-#--------------------------------------------------------------------
-# centralConfig
-
-@test "connectInject/Deployment: centralConfig is enabled by default" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/connect-inject-deployment.yaml  \
-      --set 'connectInject.enabled=true' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | any(contains("-enable-central-config"))' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
-@test "connectInject/Deployment: centralConfig can be disabled" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/connect-inject-deployment.yaml  \
-      --set 'connectInject.enabled=true' \
-      --set 'connectInject.centralConfig.enabled=false' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | any(contains("-enable-central-config"))' | tee /dev/stderr)
-  [ "${actual}" = "false" ]
-}
-
-@test "connectInject/Deployment: defaultProtocol is disabled by default with centralConfig enabled" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/connect-inject-deployment.yaml  \
-      --set 'connectInject.enabled=true' \
-      --set 'connectInject.centralConfig.enabled=true' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | any(contains("-default-protocol"))' | tee /dev/stderr)
-  [ "${actual}" = "false" ]
-}
-
-@test "connectInject/Deployment: defaultProtocol can be enabled with centralConfig enabled" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/connect-inject-deployment.yaml  \
-      --set 'connectInject.enabled=true' \
-      --set 'connectInject.centralConfig.enabled=true' \
-      --set 'connectInject.centralConfig.defaultProtocol=grpc' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | any(contains("-default-protocol=\"grpc\""))' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
 #--------------------------------------------------------------------
 # authMethod
 

test/unit/mesh-gateway-deployment.bats

@@ -1245,6 +1245,7 @@ EOF
 
   [ "${actual}" = "${exp}" ]
 }
+
 @test "meshGateway/Deployment: service-init init container consulServiceName can be changed" {
   cd `chart_dir`
   local actual=$(helm template \
@@ -1294,3 +1295,17 @@ EOF
 
   [ "${actual}" = "${exp}" ]
 }
+
+#--------------------------------------------------------------------
+# meshGateway.globalMode [DEPRECATED]
+
+@test "meshGateway/Deployment: fails if meshGateway.globalMode is set" {
+  cd `chart_dir`
+  run helm template \
+      -s templates/mesh-gateway-deployment.yaml  \
+      --set 'meshGateway.enabled=true' \
+      --set 'connectInject.enabled=true' \
+      --set 'meshGateway.globalMode=something' .
+  [ "$status" -eq 1 ]
+  [[ "$output" =~ "meshGateway.globalMode is no longer supported; instead, you must migrate to CRDs (see www.consul.io/docs/k8s/crds/upgrade-to-crds)" ]]
+}