Separate tproxy tests (hashicorp#957)

* Add -enable-transparent-proxy flag to the acceptance tests flags which allows you to globally enable or disable tproxy for all tests
* Update all (except a few) connect-related tests to now reach upstreams via kube DNS when transparent proxy is enabled
* Refactor circleci to add commands that we can re-use when running acceptance tests
* Add acceptance tests job with tproxy enabled in parallel to the regular job to be run on every PR
* Enable tproxy on all nightly acceptance tests
* Disable some tests when tproxy is enabled due to either bugs or flakiness (to be fixed later) mesh gateway test with explicit upstreams doesn't work right now because of a bug in consul that is not yet fixed RestartConsulClients connect test is flakey.

CONTRIBUTING.md

@@ -96,6 +96,9 @@ Below is the list of available flags:
     If true, the tests will automatically add Openshift Helm value for each Helm install.
 -enable-pod-security-policies
     If true, the test suite will run tests with pod security policies enabled.
+-enable-transparent-proxy
+    If true, the test suite will run tests with transparent proxy enabled.
+    This applies only to tests that enable connectInject.
 -enterprise-license-secret-name
     The name of the Kubernetes secret containing the enterprise license.
 -enterprise-license-secret-key

test/acceptance/framework/config/config.go

@@ -5,16 +5,17 @@ import (
 	"fmt"
 	"io/ioutil"
 	"path/filepath"
+	"strconv"
 	"strings"
 
 	"gopkg.in/yaml.v2"
 )
 
-// The path to the helm chart.
+// HelmChartPath is the path to the helm chart.
 // Note: this will need to be changed if this file is moved.
 const HelmChartPath = "../../../.."
 
-// TestConfig holds configuration for the test suite
+// TestConfig holds configuration for the test suite.
 type TestConfig struct {
 	Kubeconfig    string
 	KubeContext   string
@@ -33,6 +34,8 @@ type TestConfig struct {
 
 	EnablePodSecurityPolicies bool
 
+	EnableTransparentProxy bool
+
 	ConsulImage    string
 	ConsulK8SImage string
 
@@ -45,7 +48,7 @@ type TestConfig struct {
 }
 
 // HelmValuesFromConfig returns a map of Helm values
-// that includes any non-empty values from the TestConfig
+// that includes any non-empty values from the TestConfig.
 func (t *TestConfig) HelmValuesFromConfig() (map[string]string, error) {
 	helmValues := map[string]string{}
 
@@ -72,6 +75,8 @@ func (t *TestConfig) HelmValuesFromConfig() (map[string]string, error) {
 		setIfNotEmpty(helmValues, "global.enablePodSecurityPolicies", "true")
 	}
 
+	setIfNotEmpty(helmValues, "connectInject.transparentProxy.defaultEnabled", strconv.FormatBool(t.EnableTransparentProxy))
+
 	setIfNotEmpty(helmValues, "global.image", t.ConsulImage)
 	setIfNotEmpty(helmValues, "global.imageK8S", t.ConsulK8SImage)
 
@@ -112,7 +117,7 @@ func (t *TestConfig) entImage() (string, error) {
 	return fmt.Sprintf("hashicorp/consul-enterprise:%s-ent%s", appVersion, preRelease), nil
 }
 
-// setIfNotEmpty sets key to val in map m if value is not empty
+// setIfNotEmpty sets key to val in map m if value is not empty.
 func setIfNotEmpty(m map[string]string, key, val string) {
 	if val != "" {
 		m[key] = val

test/acceptance/framework/config/config_test.go

@@ -19,21 +19,29 @@ func TestConfig_HelmValuesFromConfig(t *testing.T) {
 		{
 			"returns empty map by default",
 			TestConfig{},
-			map[string]string{},
+			map[string]string{
+				"connectInject.transparentProxy.defaultEnabled": "false",
+			},
 		},
 		{
 			"sets consul image",
 			TestConfig{
 				ConsulImage: "consul:test-version",
 			},
-			map[string]string{"global.image": "consul:test-version"},
+			map[string]string{
+				"global.image": "consul:test-version",
+				"connectInject.transparentProxy.defaultEnabled": "false",
+			},
 		},
 		{
 			"sets consul-k8s image",
 			TestConfig{
 				ConsulK8SImage: "consul-k8s:test-version",
 			},
-			map[string]string{"global.imageK8S": "consul-k8s:test-version"},
+			map[string]string{
+				"global.imageK8S": "consul-k8s:test-version",
+				"connectInject.transparentProxy.defaultEnabled": "false",
+			},
 		},
 		{
 			"sets both images",
@@ -44,6 +52,7 @@ func TestConfig_HelmValuesFromConfig(t *testing.T) {
 			map[string]string{
 				"global.image":    "consul:test-version",
 				"global.imageK8S": "consul-k8s:test-version",
+				"connectInject.transparentProxy.defaultEnabled": "false",
 			},
 		},
 		{
@@ -53,31 +62,37 @@ func TestConfig_HelmValuesFromConfig(t *testing.T) {
 				EnterpriseLicenseSecretKey:  "key",
 			},
 			map[string]string{
-				"server.enterpriseLicense.secretName": "ent-license",
-				"server.enterpriseLicense.secretKey":  "key",
+				"server.enterpriseLicense.secretName":           "ent-license",
+				"server.enterpriseLicense.secretKey":            "key",
+				"connectInject.transparentProxy.defaultEnabled": "false",
 			},
 		},
 		{
 			"doesn't set ent license secret when only secret name is set",
 			TestConfig{
 				EnterpriseLicenseSecretName: "ent-license",
 			},
-			map[string]string{},
+			map[string]string{
+				"connectInject.transparentProxy.defaultEnabled": "false",
+			},
 		},
 		{
 			"doesn't set ent license secret when only secret key is set",
 			TestConfig{
 				EnterpriseLicenseSecretKey: "key",
 			},
-			map[string]string{},
+			map[string]string{
+				"connectInject.transparentProxy.defaultEnabled": "false",
+			},
 		},
 		{
 			"sets openshift value when EnableOpenshift is set",
 			TestConfig{
 				EnableOpenshift: true,
 			},
 			map[string]string{
-				"global.openshift.enabled": "true",
+				"global.openshift.enabled":                      "true",
+				"connectInject.transparentProxy.defaultEnabled": "false",
 			},
 		},
 		{
@@ -86,7 +101,17 @@ func TestConfig_HelmValuesFromConfig(t *testing.T) {
 				EnablePodSecurityPolicies: true,
 			},
 			map[string]string{
-				"global.enablePodSecurityPolicies": "true",
+				"global.enablePodSecurityPolicies":              "true",
+				"connectInject.transparentProxy.defaultEnabled": "false",
+			},
+		},
+		{
+			"sets transparentProxy.defaultEnabled helm value to true when -enable-transparent-proxy is set",
+			TestConfig{
+				EnableTransparentProxy: true,
+			},
+			map[string]string{
+				"connectInject.transparentProxy.defaultEnabled": "true",
 			},
 		},
 	}

test/acceptance/framework/consul/consul_cluster.go

@@ -69,9 +69,6 @@ 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)

test/acceptance/framework/flags/flags.go

@@ -26,6 +26,8 @@ type TestFlags struct {
 
 	flagEnablePodSecurityPolicies bool
 
+	flagEnableTransparentProxy bool
+
 	flagConsulImage    string
 	flagConsulK8sImage string
 
@@ -78,6 +80,10 @@ func (t *TestFlags) init() {
 	flag.BoolVar(&t.flagEnablePodSecurityPolicies, "enable-pod-security-policies", false,
 		"If true, the test suite will run tests with pod security policies enabled.")
 
+	flag.BoolVar(&t.flagEnableTransparentProxy, "enable-transparent-proxy", false,
+		"If true, the test suite will run tests with transparent proxy enabled. "+
+			"This applies only to tests that enable connectInject.")
+
 	flag.BoolVar(&t.flagNoCleanupOnFailure, "no-cleanup-on-failure", false,
 		"If true, the tests will not cleanup Kubernetes resources they create when they finish running."+
 			"Note this flag must be run with -failfast flag, otherwise subsequent tests will fail.")
@@ -126,6 +132,8 @@ func (t *TestFlags) TestConfigFromFlags() *config.TestConfig {
 
 		EnablePodSecurityPolicies: t.flagEnablePodSecurityPolicies,
 
+		EnableTransparentProxy: t.flagEnableTransparentProxy,
+
 		ConsulImage:    t.flagConsulImage,
 		ConsulK8SImage: t.flagConsulK8sImage,
 

test/acceptance/framework/k8s/deploy.go

@@ -16,6 +16,8 @@ import (
 	"k8s.io/apimachinery/pkg/util/yaml"
 )
 
+const staticClientName = "static-client"
+
 // Deploy creates a Kubernetes deployment by applying configuration stored at filepath,
 // sets up a cleanup function and waits for the deployment to become available.
 func Deploy(t *testing.T, options *k8s.KubectlOptions, noCleanupOnFailure bool, debugDirectory string, filepath string) {
@@ -75,17 +77,10 @@ func DeployKustomize(t *testing.T, options *k8s.KubectlOptions, noCleanupOnFailu
 // to be "hello world" in a case of success.
 // If expectSuccess is true, it will expect connection to succeed,
 // otherwise it will expect failure due to intentions.
-func CheckStaticServerConnection(
-	t *testing.T,
-	options *k8s.KubectlOptions,
-	expectSuccess bool,
-	deploymentName string,
-	failureMessages []string,
-	curlArgs ...string,
-) {
+func CheckStaticServerConnection(t *testing.T, options *k8s.KubectlOptions, expectSuccess bool, failureMessages []string, curlArgs ...string) {
 	t.Helper()
 
-	CheckStaticServerConnectionMultipleFailureMessages(t, options, expectSuccess, deploymentName, failureMessages, curlArgs...)
+	CheckStaticServerConnectionMultipleFailureMessages(t, options, expectSuccess, failureMessages, curlArgs...)
 }
 
 // CheckStaticServerConnectionMultipleFailureMessages execs into a pod of the deployment given by deploymentName
@@ -95,19 +90,12 @@ func CheckStaticServerConnection(
 // If expectSuccess is true, it will expect connection to succeed,
 // otherwise it will expect failure due to intentions. If multiple failureMessages are provided it will assert
 // on the existence of any of them.
-func CheckStaticServerConnectionMultipleFailureMessages(
-	t *testing.T,
-	options *k8s.KubectlOptions,
-	expectSuccess bool,
-	deploymentName string,
-	failureMessages []string,
-	curlArgs ...string,
-) {
+func CheckStaticServerConnectionMultipleFailureMessages(t *testing.T, options *k8s.KubectlOptions, expectSuccess bool, failureMessages []string, curlArgs ...string) {
 	t.Helper()
 
 	retrier := &retry.Timer{Timeout: 80 * time.Second, Wait: 2 * time.Second}
 
-	args := []string{"exec", "deploy/" + deploymentName, "-c", deploymentName, "--", "curl", "-vvvsSf"}
+	args := []string{"exec", "deploy/" + staticClientName, "-c", staticClientName, "--", "curl", "-vvvsSf"}
 	args = append(args, curlArgs...)
 
 	retry.RunWith(retrier, t, func(r *retry.R) {
@@ -132,26 +120,21 @@ 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) {
+func CheckStaticServerConnectionSuccessful(t *testing.T, options *k8s.KubectlOptions, curlArgs ...string) {
 	t.Helper()
 	start := time.Now()
-	CheckStaticServerConnection(t, options, true, deploymentName, nil, curlArgs...)
+	CheckStaticServerConnection(t, options, true, nil, curlArgs...)
 	logger.Logf(t, "Took %s to check if static server connection was successful", time.Since(start))
 }
 
 // CheckStaticServerConnectionFailing is just like CheckStaticServerConnection
 // but it always expects a failing connection with various errors.
-func CheckStaticServerConnectionFailing(t *testing.T, options *k8s.KubectlOptions, deploymentName string, curlArgs ...string) {
+func CheckStaticServerConnectionFailing(t *testing.T, options *k8s.KubectlOptions, curlArgs ...string) {
 	t.Helper()
-	CheckStaticServerConnection(t,
-		options,
-		false,
-		deploymentName,
-		[]string{
-			"curl: (52) Empty reply from server",
-			"curl: (7) Failed to connect",
-		},
-		curlArgs...)
+	CheckStaticServerConnection(t, options, false, []string{
+		"curl: (52) Empty reply from server",
+		"curl: (7) Failed to connect",
+	}, curlArgs...)
 }
 
 // labelMapToString takes a label map[string]string