agentless: Multi-cluster tproxy (#1632)

* Redirect traffic to the consul-dataplane's DNS proxy when DNS redirection is enabled. This means we no longer need to configure recursors for the clients and servers * To fall back to kube dns when consul cannot resolve names, we define our own DNSConfig on the pod that uses Consul first, but falls back to the config from /etc/resolv.conf in the cluster.
hashicorp · Oct 25, 2022 · 933cc5e · 933cc5e
1 parent 9597c82
commit 933cc5e
Show file tree

Hide file tree

Showing 28 changed files with 301 additions and 267 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -4,9 +4,10 @@ on:
 
 env:
   TEST_RESULTS: /tmp/test-results # path to where test results are saved
-  CONSUL_VERSION: 1.13.1 # Consul's OSS version to use in tests
-  CONSUL_ENT_VERSION: 1.13.1+ent # Consul's enterprise version to use in tests
   GOTESTSUM_VERSION: 1.8.2 # You cannot use environment variables with workflows. The gotestsum version is hardcoded in the reusable workflows too.
+  # We use docker images to copy the consul binary for unit tests.
+  CONSUL_OSS_DOCKER_IMAGE: hashicorppreview/consul:1.14-dev # Consul's OSS version to use in tests
+  CONSUL_ENT_DOCKER_IMAGE: hashicorppreview/consul-enterprise:1.14-dev # Consul's enterprise version to use in tests
 
 jobs:
   get-go-version:
@@ -158,11 +159,9 @@ jobs:
         working-directory: control-plane
         run: |
           mkdir -p $HOME/bin
-          wget https://releases.hashicorp.com/consul/${{env.CONSUL_VERSION}}/consul_${{env.CONSUL_VERSION}}_linux_amd64.zip && \
-            unzip consul_${{env.CONSUL_VERSION}}_linux_amd64.zip -d $HOME/bin && \
-            rm consul_${{env.CONSUL_VERSION}}_linux_amd64.zip
-          chmod +x $HOME/bin/consul
-
+          container_id=$(docker create ${{env.CONSUL_OSS_DOCKER_IMAGE}})
+          docker cp "$container_id:/bin/consul" $HOME/bin/consul
+          docker rm "$container_id"
       - name: Run go tests
         working-directory: control-plane
         run: |
@@ -207,10 +206,9 @@ jobs:
         working-directory: control-plane
         run: |
           mkdir -p $HOME/bin
-          wget https://releases.hashicorp.com/consul/${{env.CONSUL_ENT_VERSION}}/consul_${{env.CONSUL_ENT_VERSION}}_linux_amd64.zip && \
-            unzip consul_${{env.CONSUL_ENT_VERSION}}_linux_amd64.zip -d $HOME/bin && \
-            rm consul_${{env.CONSUL_ENT_VERSION}}_linux_amd64.zip
-          chmod +x $HOME/bin/consul
+          container_id=$(docker create ${{env.CONSUL_ENT_DOCKER_IMAGE}})
+          docker cp "$container_id:/bin/consul" $HOME/bin/consul
+          docker rm "$container_id"
 
       - name: Run go tests
         working-directory: control-plane

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,14 @@
 ## UNRELEASED
 
+BREAKING_CHANGES:
+* Helm:
+  * Remove `global.consulSidecarContainer` from values file as there is no longer a consul sidecar. [[GH-1635](https://github.com/hashicorp/consul-k8s/pull/1635)]
+
+FEATURES:
+* Consul-dataplane:
+  * Support merged metrics with consul-dataplane. [[GH-1635](https://github.com/hashicorp/consul-k8s/pull/1635)]
+  * Support transparent proxying when using consul-dataplane. [[GH-1625](https://github.com/hashicorp/consul-k8s/pull/1478),[GH-1632](https://github.com/hashicorp/consul-k8s/pull/1632)]
+
 IMPROVEMENTS:
 * CLI
   * Update minimum go version for project to 1.19 [[GH-1633](https://github.com/hashicorp/consul-k8s/pull/1633)]
@@ -9,13 +18,6 @@ IMPROVEMENTS:
   * Remove deprecated annotation `service.alpha.kubernetes.io/tolerate-unready-endpoints: "true"` in the `server-service` template. [[GH-1619](https://github.com/hashicorp/consul-k8s/pull/1619)]
   * Support `minAvailable` on connect injector `PodDisruptionBudget`. [[GH-1557](https://github.com/hashicorp/consul-k8s/pull/1557)]
   * Add `tolerations` and `nodeSelector` to Server ACL init jobs and `nodeSelector` to Webhook cert manager. [[GH-1581](https://github.com/hashicorp/consul-k8s/pull/1581)]
-* Control plane
-  * Support merged metrics with consul-dataplane. [[GH-1635](https://github.com/hashicorp/consul-k8s/pull/1635)]
-
-BREAKING_CHANGES:
-
-* Helm:
-  * Removal of `global.consulSidecarContainer` from values file as there is no longer a consul sidecar. [[GH-1635](https://github.com/hashicorp/consul-k8s/pull/1635)]
 
 ## 1.0.0-beta3 (October 12, 2022)
 

diff --git a/acceptance/framework/connhelper/connect_helper.go b/acceptance/framework/connhelper/connect_helper.go
@@ -208,6 +208,8 @@ func (c *ConnectHelper) helmValues() map[string]string {
 		"connectInject.enabled":        "true",
 		"global.tls.enabled":           strconv.FormatBool(c.Secure),
 		"global.acls.manageSystemACLs": strconv.FormatBool(c.Secure),
+		"dns.enabled":                  "true",
+		"dns.enableRedirection":        "true",
 	}
 
 	helpers.MergeMaps(helmValues, c.HelmValues)

diff --git a/acceptance/tests/consul-dns/consul_dns_test.go b/acceptance/tests/consul-dns/consul_dns_test.go
@@ -14,8 +14,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-const podName = "dns-pod"
-
 func TestConsulDNS(t *testing.T) {
 	cfg := suite.Config()
 	if cfg.EnableCNI {
@@ -59,16 +57,17 @@ func TestConsulDNS(t *testing.T) {
 				serverIPs = append(serverIPs, serverPod.Status.PodIP)
 			}
 
+			dnsPodName := fmt.Sprintf("%s-dns-pod", releaseName)
 			dnsTestPodArgs := []string{
-				"run", "-i", podName, "--restart", "Never", "--image", "anubhavmishra/tiny-tools", "--", "dig", fmt.Sprintf("@%s-consul-dns", releaseName), "consul.service.consul",
+				"run", "-i", dnsPodName, "--restart", "Never", "--image", "anubhavmishra/tiny-tools", "--", "dig", fmt.Sprintf("@%s-consul-dns", releaseName), "consul.service.consul",
 			}
 
 			helpers.Cleanup(t, suite.Config().NoCleanupOnFailure, func() {
 				// Note: this delete command won't wait for pods to be fully terminated.
 				// This shouldn't cause any test pollution because the underlying
 				// objects are deployments, and so when other tests create these
 				// they should have different pod names.
-				k8s.RunKubectl(t, ctx.KubectlOptions(t), "delete", "pod", podName)
+				k8s.RunKubectl(t, ctx.KubectlOptions(t), "delete", "pod", dnsPodName)
 			})
 
 			retry.Run(t, func(r *retry.R) {

diff --git a/acceptance/tests/partitions/main_test.go b/acceptance/tests/partitions/main_test.go
@@ -13,8 +13,7 @@ var suite testsuite.Suite
 func TestMain(m *testing.M) {
 	suite = testsuite.NewSuite(m)
 
-	// todo(agentless): Re-enable tproxy tests once we support it for multi-cluster.
-	if suite.Config().EnableMultiCluster && !suite.Config().EnableTransparentProxy {
+	if suite.Config().EnableMultiCluster {
 		os.Exit(suite.Run())
 	} else {
 		fmt.Println("Skipping partitions tests because -enable-multi-cluster is not set")

diff --git a/acceptance/tests/peering/main_test.go b/acceptance/tests/peering/main_test.go
@@ -13,8 +13,7 @@ var suite testsuite.Suite
 func TestMain(m *testing.M) {
 	suite = testsuite.NewSuite(m)
 
-	// todo(agentless): Re-enable tproxy tests once we support it for multi-cluster.
-	if suite.Config().EnableMultiCluster && !suite.Config().DisablePeering && !suite.Config().EnableTransparentProxy {
+	if suite.Config().EnableMultiCluster && !suite.Config().DisablePeering {
 		os.Exit(suite.Run())
 	} else {
 		fmt.Println("Skipping peering tests because either -enable-multi-cluster is not set or -disable-peering is set")

diff --git a/acceptance/tests/peering/peering_connect_test.go b/acceptance/tests/peering/peering_connect_test.go
@@ -31,10 +31,6 @@ func TestPeering_Connect(t *testing.T) {
 		t.Skipf("skipping this test because peering is not supported in version %v", cfg.ConsulVersion.String())
 	}
 
-	if cfg.EnableTransparentProxy {
-		t.Skipf("skipping because no t-proxy support")
-	}
-
 	const staticServerPeer = "server"
 	const staticClientPeer = "client"
 	cases := []struct {

diff --git a/acceptance/tests/wan-federation/main_test.go b/acceptance/tests/wan-federation/main_test.go
@@ -13,8 +13,7 @@ var suite testsuite.Suite
 func TestMain(m *testing.M) {
 	suite = testsuite.NewSuite(m)
 
-	// todo(agentless): Re-enable tproxy tests once we support it for multi-cluster.
-	if suite.Config().EnableMultiCluster && !suite.Config().EnableTransparentProxy {
+	if suite.Config().EnableMultiCluster {
 		os.Exit(suite.Run())
 	} else {
 		fmt.Println("Skipping wan federation tests because -enable-multi-cluster is not set")

diff --git a/charts/consul/templates/_helpers.tpl b/charts/consul/templates/_helpers.tpl
@@ -148,19 +148,6 @@ is passed to consul as a -config-file param on command line.
               [ -n "${HOSTNAME}" ] && sed -Ei "s|HOSTNAME|${HOSTNAME?}|g" /consul/extra-config/extra-from-values.json
 {{- end -}}
 
-{{/*
-Sets up a list of recusor flags for Consul agents by iterating over the IPs of every nameserver
-in /etc/resolv.conf and concatenating them into a string of arguments that can be passed directly
-to the consul agent command.
-*/}}
-{{- define "consul.recursors" -}}
-              recursor_flags=""
-              for ip in $(cat /etc/resolv.conf | grep nameserver | cut -d' ' -f2)
-              do
-                 recursor_flags="$recursor_flags -recursor=$ip"
-              done
-{{- end -}}
-
 {{/*
 Create chart name and version as used by the chart label.
 */}}

diff --git a/charts/consul/templates/client-daemonset.yaml b/charts/consul/templates/client-daemonset.yaml
@@ -271,9 +271,6 @@ spec:
               {{- if and .Values.global.secretsBackend.vault.enabled .Values.global.gossipEncryption.secretName }}
               GOSSIP_KEY=`cat /vault/secrets/gossip.txt`
               {{- end }}
-              {{- if (and .Values.dns.enabled .Values.dns.enableRedirection) }}
-              {{ template "consul.recursors" }}
-              {{- end }}
 
               {{ template "consul.extraconfig" }}
 
@@ -379,9 +376,6 @@ spec:
                 {{- range $value := .Values.global.recursors }}
                 -recursor={{ quote $value }} \
                 {{- end }}
-                {{- if (and .Values.dns.enabled .Values.dns.enableRedirection) }}
-                $recursor_flags \
-                {{- end }}
                 -config-file=/consul/extra-config/extra-from-values.json \
                 -domain={{ .Values.global.domain }}
           volumeMounts:

diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml
@@ -312,10 +312,6 @@ spec:
               {{- if and .Values.global.secretsBackend.vault.enabled .Values.global.gossipEncryption.secretName }}
               GOSSIP_KEY=`cat /vault/secrets/gossip.txt`
               {{- end }}
-              
-              {{- if (and .Values.dns.enabled .Values.dns.enableRedirection) }}
-              {{ template "consul.recursors" }}
-              {{- end }}
 
               {{ template "consul.extraconfig" }}
 
@@ -332,9 +328,6 @@ spec:
                 -hcl="acl { tokens { agent = \"${ACL_REPLICATION_TOKEN}\", replication = \"${ACL_REPLICATION_TOKEN}\" } }" \
                 {{- end }}
                 {{- end }}
-                {{- if (and .Values.dns.enabled .Values.dns.enableRedirection) }}
-                $recursor_flags \
-                {{- end }}
                 {{- if and .Values.global.secretsBackend.vault.enabled .Values.global.acls.bootstrapToken.secretName }}
                 -config-file=/vault/secrets/bootstrap-token-config.hcl \
                 {{- else if (and (not .Values.global.secretsBackend.vault.enabled) .Values.global.acls.bootstrapToken.secretName) }}

diff --git a/charts/consul/test/unit/client-daemonset.bats b/charts/consul/test/unit/client-daemonset.bats
@@ -1706,30 +1706,6 @@ local actual=$(echo $object |
   [ "${actual}" = "true" ]
 }
 
-#--------------------------------------------------------------------
-# DNS
-
-@test "client/DaemonSet: recursor flags is not set by default" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/client-daemonset.yaml \
-      --set 'client.enabled=true' \
-      . | tee /dev/stderr |
-      yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("$recursor_flags")' | tee /dev/stderr)
-  [ "${actual}" = "false" ]
-}
-
-@test "client/DaemonSet: add recursor flags if dns.enableRedirection is true" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/client-daemonset.yaml \
-      --set 'client.enabled=true' \
-      --set 'dns.enableRedirection=true' \
-      . | tee /dev/stderr |
-      yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("$recursor_flags")' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
 #--------------------------------------------------------------------
 # hostNetwork
 

diff --git a/charts/consul/test/unit/server-statefulset.bats b/charts/consul/test/unit/server-statefulset.bats
@@ -590,28 +590,6 @@ load _helpers
   [ "${actualBaz}" = "qux" ]
 }
 
-#--------------------------------------------------------------------
-# DNS
-
-@test "server/StatefulSet: recursor flags unset by default" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/server-statefulset.yaml \
-      . | tee /dev/stderr |
-      yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("$recursor_flags")' | tee /dev/stderr)
-  [ "${actual}" = "false" ]
-}
-
-@test "server/StatefulSet: add recursor flags if dns.enableRedirection is true" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/server-statefulset.yaml \
-      --set 'dns.enableRedirection=true' \
-      . | tee /dev/stderr |
-      yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("$recursor_flags")' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
 #--------------------------------------------------------------------
 # annotations
 

diff --git a/control-plane/cni/go.mod b/control-plane/cni/go.mod
@@ -48,6 +48,6 @@ require (
 	sigs.k8s.io/yaml v1.2.0 // indirect
 )
 
-replace github.com/hashicorp/consul/sdk v0.9.0 => github.com/hashicorp/consul/sdk v0.4.1-0.20220531155537-364758ef2f50
+replace github.com/hashicorp/consul/sdk => github.com/hashicorp/consul/sdk v0.4.1-0.20221021205723-cc843c4be892
 
 go 1.19
diff --git a/control-plane/cni/go.sum b/control-plane/cni/go.sum
@@ -131,8 +131,8 @@ github.com/googleapis/gnostic v0.5.5 h1:9fHAtK0uDfpveeqqo1hkEZJcFvYXAiCN3UutL8F9
 github.com/googleapis/gnostic v0.5.5/go.mod h1:7+EbHbldMins07ALC74bsA81Ovc97DwqyJO1AENw9kA=
 github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
 github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7/go.mod h1:FecbI9+v66THATjSRHfNgh1IVFe/9kFxbXtjV0ctIMA=
-github.com/hashicorp/consul/sdk v0.4.1-0.20220531155537-364758ef2f50 h1:GwbRRT+QxMRbYI608FGwTfcZ0iOVLX69B2ePjpQoyXw=
-github.com/hashicorp/consul/sdk v0.4.1-0.20220531155537-364758ef2f50/go.mod h1:yPkX5Q6CsxTFMjQQDJwzeNmUUF5NUGGbrDsv9wTb8cw=
+github.com/hashicorp/consul/sdk v0.4.1-0.20221021205723-cc843c4be892 h1:jw0NwPmNPr5CxAU04hACdj61JSaJBKZ0FdBo+kwfNp4=
+github.com/hashicorp/consul/sdk v0.4.1-0.20221021205723-cc843c4be892/go.mod h1:yPkX5Q6CsxTFMjQQDJwzeNmUUF5NUGGbrDsv9wTb8cw=
 github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
 github.com/hashicorp/go-hclog v0.12.0/go.mod h1:whpDNt7SSdeAju8AWKIWsul05p54N/39EeqMAyrmvFQ=
 github.com/hashicorp/go-hclog v0.16.1 h1:IVQwpTGNRRIHafnTs2dQLIk4ENtneRIEEJWOVDqz99o=

diff --git a/control-plane/connect-inject/consul_dataplane_sidecar.go b/control-plane/connect-inject/consul_dataplane_sidecar.go
@@ -13,7 +13,11 @@ import (
 	"k8s.io/utils/pointer"
 )
 
-const ConsulCAFile = "/consul/connect-inject/consul-ca.pem"
+const (
+	ConsulCAFile               = "/consul/connect-inject/consul-ca.pem"
+	ConsulDataplaneDNSBindHost = "127.0.0.1"
+	ConsulDataplaneDNSBindPort = 8600
+)
 
 func (w *MeshWebhook) consulDataplaneSidecar(namespace corev1.Namespace, pod corev1.Pod, mpi multiPortInfo) (corev1.Container, error) {
 	resources, err := w.sidecarResources(pod)
@@ -253,6 +257,12 @@ func (w *MeshWebhook) getContainerSidecarCommand(namespace corev1.Namespace, mpi
 		}
 	}
 
+	// If Consul DNS is enabled, we want to configure consul-dataplane to be the DNS proxy
+	// for Consul DNS in the pod.
+	if w.EnableConsulDNS {
+		cmd = append(cmd, "-consul-dns-bind-port="+strconv.Itoa(ConsulDataplaneDNSBindPort))
+	}
+
 	var envoyExtraArgs []string
 	extraArgs, annotationSet := pod.Annotations[annotationEnvoyExtraArgs]
 	// --base-id is an envoy arg rather than consul-dataplane, and so we need to make sure we're passing it

diff --git a/control-plane/connect-inject/consul_dataplane_sidecar_test.go b/control-plane/connect-inject/consul_dataplane_sidecar_test.go
@@ -272,6 +272,26 @@ func TestHandlerConsulDataplaneSidecar_Concurrency(t *testing.T) {
 	}
 }
 
+func TestHandlerConsulDataplaneSidecar_DNSProxy(t *testing.T) {
+	h := MeshWebhook{
+		ConsulConfig:    &consul.Config{HTTPPort: 8500, GRPCPort: 8502},
+		EnableConsulDNS: true,
+	}
+	pod := corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{},
+		Spec: corev1.PodSpec{
+			Containers: []corev1.Container{
+				{
+					Name: "web",
+				},
+			},
+		},
+	}
+	container, err := h.consulDataplaneSidecar(testNS, pod, multiPortInfo{})
+	require.NoError(t, err)
+	require.Contains(t, container.Command[2], "-consul-dns-bind-port=8600")
+}
+
 func TestHandlerConsulDataplaneSidecar_Multiport(t *testing.T) {
 	for _, aclsEnabled := range []bool{false, true} {
 		name := fmt.Sprintf("acls enabled: %t", aclsEnabled)

diff --git a/control-plane/connect-inject/container_init.go b/control-plane/connect-inject/container_init.go
@@ -17,7 +17,6 @@ const (
 	sidecarUserAndGroupID        = 5995
 	initContainersUserAndGroupID = 5996
 	netAdminCapability           = "NET_ADMIN"
-	dnsServiceHostEnvSuffix      = "DNS_SERVICE_HOST"
 )
 
 type initContainerCommandData struct {