Support transparent proxy in the Consul Helm chart (hashicorp#905)

Support transparent proxy in the consul helm chart * Add new connectInject.transparentProxy.defaultEnabled value (default to true) that will allow users to enable or disable tproxy for each helm installation. * Add acceptance tests for connect-inject to test with tproxy * Acceptance tests default to tproxy not enabled since we don't fully support it for all features yet.
rrondeau · Apr 16, 2021 · dc4b400 · dc4b400
1 parent 34cd398
commit dc4b400
Show file tree

Hide file tree

Showing 12 changed files with 326 additions and 177 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -137,13 +137,15 @@ jobs:
             echo "Running $pkgs"
             for pkg in $pkgs
             do
-              if ! gotestsum --no-summary=all --jsonfile=jsonfile-${pkg////-} -- $pkg -p 1 -timeout 30m -failfast \
+              if ! gotestsum --no-summary=all --jsonfile=jsonfile-${pkg////-} -- $pkg -p 1 -timeout 50m -failfast \
                     -use-kind \
+                    -enable-enterprise \
                     -enable-multi-cluster \
                     -kubecontext="kind-dc1" \
                     -secondary-kubecontext="kind-dc2" \
                     -debug-directory="$TEST_RESULTS/debug" \
-                    -consul-k8s-image=ashwinvenkatesh/consul-k8s@sha256:f66106414f9e6b05271bd2944bf0a01308a6b9405221d73e89f2d00159189d6e # TODO: change once feature-tproxy consul-k8s branch is merged
+                    -consul-image="ishustava/consul-enterprise:tproxy-test" \
+                    -consul-k8s-image="ishustava/consul-k8s-dev:04-13-2021-8f91d97" # TODO: change once feature-tproxy consul-k8s branch is merged
               then
                 echo "Tests in ${pkg} failed, aborting early"
                 exit_code=1
@@ -212,7 +214,7 @@ jobs:
             # The license expires 15-Oct-2025.
             KUBECONFIG=$primary_kubeconfig kubectl create secret generic ent-license --from-literal=key="${CONSUL_ENT_LICENSE}"
 
-            gotestsum --junitfile "$TEST_RESULTS/gotestsum-report.xml" -- ./... -p 1 -timeout 40m -failfast \
+            gotestsum --junitfile "$TEST_RESULTS/gotestsum-report.xml" -- ./... -p 1 -timeout 50m -failfast \
               -enable-enterprise \
               -enterprise-license-secret-name=ent-license \
               -enterprise-license-secret-key=key \
@@ -350,7 +352,7 @@ jobs:
             chmod 600 "$primary_kubeconfig"
             chmod 600 "$secondary_kubeconfig"
 
-            gotestsum --junitfile "$TEST_RESULTS/gotestsum-report.xml" -- ./... -p 1 -timeout 40m -failfast \
+            gotestsum --junitfile "$TEST_RESULTS/gotestsum-report.xml" -- ./... -p 1 -timeout 50m -failfast \
               -enable-enterprise \
               -enable-multi-cluster \
               -kubeconfig="$primary_kubeconfig" \

diff --git a/templates/connect-inject-clusterrole.yaml b/templates/connect-inject-clusterrole.yaml
@@ -11,7 +11,7 @@ metadata:
     release: {{ .Release.Name }}
 rules:
 - apiGroups: [""]
-  resources: ["pods", "nodes", "endpoints"]
+  resources: ["pods", "endpoints", "services"]
   verbs:
   - "get"
   - "list"

diff --git a/templates/connect-inject-deployment.yaml b/templates/connect-inject-deployment.yaml
@@ -90,6 +90,9 @@ spec:
                 -release-name="{{ .Release.Name }}" \
                 -release-namespace="{{ .Release.Namespace }}" \
                 -listen=:8080 \
+                {{- if .Values.connectInject.transparentProxy.defaultEnabled }}
+                -enable-transparent-proxy \
+                {{- end }}
                 {{- if .Values.connectInject.logLevel }}
                 -log-level={{ .Values.connectInject.logLevel }} \
                 {{- end }}

diff --git a/test/acceptance/framework/consul/consul_cluster.go b/test/acceptance/framework/consul/consul_cluster.go
@@ -68,6 +68,9 @@ func NewHelmCluster(
 		"server.bootstrapExpect":       "1",
 		"connectInject.envoyExtraArgs": "--log-level debug",
 		"connectInject.logLevel":       "debug",
+		// Disable default tproxy mode for tests because we instead selectively choose which
+		// tests should have it enabled.
+		"connectInject.transparentProxy.defaultEnabled": "false",
 	}
 	valuesFromConfig, err := cfg.HelmValuesFromConfig()
 	require.NoError(t, err)

diff --git a/test/acceptance/framework/consul/consul_cluster_test.go b/test/acceptance/framework/consul/consul_cluster_test.go
@@ -23,30 +23,33 @@ func TestNewHelmCluster(t *testing.T) {
 			name:       "defaults are used when no helmValues are set",
 			helmValues: map[string]string{},
 			want: map[string]string{
-				"global.image":                 "test-config-image",
-				"server.bootstrapExpect":       "1",
-				"server.replicas":              "1",
-				"connectInject.envoyExtraArgs": "--log-level debug",
-				"connectInject.logLevel":       "debug",
+				"global.image":                                  "test-config-image",
+				"server.bootstrapExpect":                        "1",
+				"server.replicas":                               "1",
+				"connectInject.envoyExtraArgs":                  "--log-level debug",
+				"connectInject.logLevel":                        "debug",
+				"connectInject.transparentProxy.defaultEnabled": "false",
 			},
 		},
 		{
 			name: "when using helmValues, defaults are overridden",
 			helmValues: map[string]string{
-				"global.image":                 "test-image",
-				"server.bootstrapExpect":       "3",
-				"server.replicas":              "3",
-				"connectInject.envoyExtraArgs": "--foo",
-				"connectInject.logLevel":       "debug",
-				"feature.enabled":              "true",
+				"global.image":                                  "test-image",
+				"server.bootstrapExpect":                        "3",
+				"server.replicas":                               "3",
+				"connectInject.envoyExtraArgs":                  "--foo",
+				"connectInject.logLevel":                        "debug",
+				"connectInject.transparentProxy.defaultEnabled": "true",
+				"feature.enabled":                               "true",
 			},
 			want: map[string]string{
-				"global.image":                 "test-image",
-				"server.bootstrapExpect":       "3",
-				"server.replicas":              "3",
-				"connectInject.envoyExtraArgs": "--foo",
-				"connectInject.logLevel":       "debug",
-				"feature.enabled":              "true",
+				"global.image":                                  "test-image",
+				"server.bootstrapExpect":                        "3",
+				"server.replicas":                               "3",
+				"connectInject.envoyExtraArgs":                  "--foo",
+				"connectInject.logLevel":                        "debug",
+				"connectInject.transparentProxy.defaultEnabled": "true",
+				"feature.enabled":                               "true",
 			},
 		},
 	}

diff --git a/test/acceptance/framework/k8s/deploy.go b/test/acceptance/framework/k8s/deploy.go
@@ -80,12 +80,12 @@ func CheckStaticServerConnection(
 	options *k8s.KubectlOptions,
 	expectSuccess bool,
 	deploymentName string,
-	failureMessage string,
+	failureMessages []string,
 	curlArgs ...string,
 ) {
 	t.Helper()
 
-	CheckStaticServerConnectionMultipleFailureMessages(t, options, expectSuccess, deploymentName, []string{failureMessage}, curlArgs...)
+	CheckStaticServerConnectionMultipleFailureMessages(t, options, expectSuccess, deploymentName, failureMessages, curlArgs...)
 }
 
 // CheckStaticServerConnectionMultipleFailureMessages execs into a pod of the deployment given by deploymentName
@@ -133,15 +133,26 @@ func CheckStaticServerConnectionMultipleFailureMessages(
 // CheckStaticServerConnectionSuccessful is just like CheckStaticServerConnection
 // but it always expects a successful connection.
 func CheckStaticServerConnectionSuccessful(t *testing.T, options *k8s.KubectlOptions, deploymentName string, curlArgs ...string) {
+	t.Helper()
 	start := time.Now()
-	CheckStaticServerConnection(t, options, true, deploymentName, "", curlArgs...)
+	CheckStaticServerConnection(t, options, true, deploymentName, nil, curlArgs...)
 	logger.Logf(t, "Took %s to check if static server connection was successful", time.Since(start))
 }
 
 // CheckStaticServerConnectionSuccessful is just like CheckStaticServerConnection
 // but it always expects a failing connection with error "Empty reply from server."
 func CheckStaticServerConnectionFailing(t *testing.T, options *k8s.KubectlOptions, deploymentName string, curlArgs ...string) {
-	CheckStaticServerConnection(t, options, false, deploymentName, "curl: (52) Empty reply from server", curlArgs...)
+	t.Helper()
+	CheckStaticServerConnection(t,
+		options,
+		false,
+		deploymentName,
+		[]string{
+			"curl: (52) Empty reply from server",
+			"curl: (7) Failed to connect to static-server port 80: Connection refused",
+			"curl: (7) Failed to connect to static-server.ns1 port 80: Connection refused",
+		},
+		curlArgs...)
 }
 
 // labelMapToString takes a label map[string]string