NET-10763 - Allow DNS Proxy to configured with an ACL token when mana…

…geSystemACLs is false (#4300) * test passing * fix test by cleaning up secret * added comments * add changelog * bats tests * bats tests for credential-type * update values.yml docstring for acl token to describe minimum permissions needed. * remove acceptance dependency on control-plane * fix lint error * Update charts/consul/values.yaml Co-authored-by: Jeff Boruszak <[email protected]> --------- Co-authored-by: Jeff Boruszak <[email protected]>
hashicorp · Sep 4, 2024 · fad1df0 · fad1df0
1 parent 4389f85
commit fad1df0
Show file tree

Hide file tree

Showing 6 changed files with 226 additions and 16 deletions.
diff --git a/.changelog/4300.txt b/.changelog/4300.txt
@@ -0,0 +1,3 @@
+```release-note:enhancement
+dns-proxy: add the ability to deploy a DNS proxy within the kubernetes cluster that forwards DNS requests to the consul server and can be configured with an ACL token and make partition aware DNS requests.
+```
diff --git a/.gitignore b/.gitignore
@@ -11,3 +11,4 @@ pkg/
 .vscode
 .bob/
 control-plane/cni/cni
+acceptance/tests/consul-dns/coredns-custom.yaml
diff --git a/acceptance/tests/consul-dns/consul_dns_test.go b/acceptance/tests/consul-dns/consul_dns_test.go
@@ -6,14 +6,16 @@ package consuldns
 import (
 	"context"
 	"fmt"
-	"github.com/hashicorp/consul-k8s/acceptance/framework/environment"
+	"github.com/hashicorp/consul/api"
+	corev1 "k8s.io/api/core/v1"
 	"os"
 	"strconv"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/hashicorp/consul-k8s/acceptance/framework/consul"
+	"github.com/hashicorp/consul-k8s/acceptance/framework/environment"
 	"github.com/hashicorp/consul-k8s/acceptance/framework/helpers"
 	"github.com/hashicorp/consul-k8s/acceptance/framework/k8s"
 	"github.com/hashicorp/consul-k8s/acceptance/framework/logger"
@@ -37,38 +39,132 @@ func TestConsulDNS(t *testing.T) {
 	}
 
 	cases := []struct {
-		secure         bool
-		enableDNSProxy bool
+		tlsEnabled           bool
+		connectInjectEnabled bool
+		enableDNSProxy       bool
+		aclsEnabled          bool
+		manageSystemACLs     bool
 	}{
-		{secure: false, enableDNSProxy: false},
-		{secure: false, enableDNSProxy: true},
-		{secure: true, enableDNSProxy: false},
-		{secure: true, enableDNSProxy: true},
+		{tlsEnabled: false, connectInjectEnabled: true, aclsEnabled: false, manageSystemACLs: false, enableDNSProxy: false},
+		{tlsEnabled: false, connectInjectEnabled: true, aclsEnabled: false, manageSystemACLs: false, enableDNSProxy: true},
+		{tlsEnabled: true, connectInjectEnabled: true, aclsEnabled: true, manageSystemACLs: true, enableDNSProxy: false},
+		{tlsEnabled: true, connectInjectEnabled: true, aclsEnabled: true, manageSystemACLs: true, enableDNSProxy: true},
+		{tlsEnabled: true, connectInjectEnabled: false, aclsEnabled: true, manageSystemACLs: false, enableDNSProxy: true},
 	}
 
 	for _, c := range cases {
-		name := fmt.Sprintf("secure: %t / enableDNSProxy: %t", c.secure, c.enableDNSProxy)
+		name := fmt.Sprintf("tlsEnabled: %t / aclsEnabled: %t / manageSystemACLs: %t, enableDNSProxy: %t",
+			c.tlsEnabled, c.aclsEnabled, c.manageSystemACLs, c.enableDNSProxy)
 		t.Run(name, func(t *testing.T) {
 			env := suite.Environment()
 			ctx := env.DefaultContext(t)
 			releaseName := helpers.RandomName()
 			helmValues := map[string]string{
+				"connectInject.enabled":        strconv.FormatBool(c.connectInjectEnabled),
 				"dns.enabled":                  "true",
-				"global.tls.enabled":           strconv.FormatBool(c.secure),
-				"global.acls.manageSystemACLs": strconv.FormatBool(c.secure),
-				"dns.proxy.enabled":            strconv.FormatBool(c.enableDNSProxy),
+				"global.tls.enabled":           strconv.FormatBool(c.tlsEnabled),
+				"global.acls.manageSystemACLs": strconv.FormatBool(c.manageSystemACLs),
+				"global.logLevel":              "debug",
+			}
+
+			// If ACLs are enabled and we are not managing system ACLs, we need to
+			// set the initial management token in the server.extraConfig.
+			const initialManagementToken = "b1gs33cr3t"
+			if c.aclsEnabled && !c.manageSystemACLs {
+				helmValues["server.extraConfig"] = fmt.Sprintf(`"{\"acl\": {\"enabled\": true\, \"default_policy\": \"deny\"\, \"tokens\": {\"initial_management\": \"%s\"}}}"`,
+					initialManagementToken)
 			}
 
 			cluster := consul.NewHelmCluster(t, helmValues, ctx, suite.Config(), releaseName)
+
+			// attach the initial management token to the cluster so it is tied to the client requests when minting a dns proxy ACL token.
+			if c.aclsEnabled && !c.manageSystemACLs {
+				cluster.ACLToken = initialManagementToken
+			}
 			cluster.Create(t)
 
+			// If ACLs are enabled and we are not managing system ACLs, we need to
+			// create a policy and token for the DNS proxy that need to be in
+			// place before the DNS proxy is started.
+			if c.aclsEnabled && c.enableDNSProxy && !c.manageSystemACLs {
+				secretName := "consul-dns-proxy-token"
+
+				consulClient, configAddress := cluster.SetupConsulClient(t, c.tlsEnabled)
+				dnsProxyPolicy := `
+					node_prefix "" {
+					  policy = "read"
+					}
+					service_prefix "" {
+					  policy = "read"
+					}
+				`
+				err, dnsProxyToken := createACLTokenWithGivenPolicy(t, consulClient, dnsProxyPolicy, initialManagementToken, configAddress)
+				require.NoError(t, err)
+
+				// Create a secret with the token to be used by the DNS proxy.
+				_, err = ctx.KubernetesClient(t).CoreV1().Secrets(ctx.KubectlOptions(t).Namespace).Create(context.Background(), &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: secretName,
+					},
+					StringData: map[string]string{
+						"token": dnsProxyToken.SecretID,
+					},
+					Type: corev1.SecretTypeOpaque,
+				}, metav1.CreateOptions{})
+				require.NoError(t, err)
+
+				t.Cleanup(func() {
+					require.NoError(t, ctx.KubernetesClient(t).CoreV1().Secrets(ctx.KubectlOptions(t).Namespace).Delete(context.Background(), secretName, metav1.DeleteOptions{}))
+				})
+
+				// Update the helm values to include the secret name and key.
+				helmValues["dns.proxy.aclToken.secretName"] = secretName
+				helmValues["dns.proxy.aclToken.secretKey"] = "token"
+			}
+
+			// If DNS proxy is enabled, we need to set the enableDNSProxy flag in the helm values.
+			if c.enableDNSProxy {
+				helmValues["dns.proxy.enabled"] = strconv.FormatBool(c.enableDNSProxy)
+			}
+			// Upgrade the cluster to apply the changes created above.  This will
+			// also start the DNS proxy if it is enabled and it will pick up the ACL token
+			// saved in the secret.
+			cluster.Upgrade(t, helmValues)
+
 			updateCoreDNSWithConsulDomain(t, ctx, releaseName, c.enableDNSProxy)
 			verifyDNS(t, releaseName, ctx.KubectlOptions(t).Namespace, ctx, ctx, "app=consul,component=server",
 				"consul.service.consul", true, 0)
 		})
 	}
 }
 
+func createACLTokenWithGivenPolicy(t *testing.T, consulClient *api.Client, policyRules string, initialManagementToken string, configAddress string) (error, *api.ACLToken) {
+	_, _, err := consulClient.ACL().TokenCreate(&api.ACLToken{}, &api.WriteOptions{
+		Token: initialManagementToken,
+	})
+	require.NoError(t, err)
+
+	// Create the policy and token _before_ we enable dns proxy and upgrade the cluster.
+	require.NoError(t, err)
+	policy, _, err := consulClient.ACL().PolicyCreate(&api.ACLPolicy{
+		Name:        "dns-proxy-token",
+		Description: "DNS Proxy Policy",
+		Rules:       policyRules,
+	}, nil)
+	require.NoError(t, err)
+	dnsProxyToken, _, err := consulClient.ACL().TokenCreate(&api.ACLToken{
+		Description: fmt.Sprintf("DNS Proxy Token for %s", strings.Split(configAddress, ":")[0]),
+		Policies: []*api.ACLTokenPolicyLink{
+			{
+				Name: policy.Name,
+			},
+		},
+	}, nil)
+	require.NoError(t, err)
+	logger.Log(t, "created DNS Proxy token", "token", dnsProxyToken)
+	return err, dnsProxyToken
+}
+
 func updateCoreDNSWithConsulDomain(t *testing.T, ctx environment.TestContext, releaseName string, enableDNSProxy bool) {
 	updateCoreDNSFile(t, ctx, releaseName, enableDNSProxy, "coredns-custom.yaml")
 	updateCoreDNS(t, ctx, "coredns-custom.yaml")

diff --git a/charts/consul/templates/dns-proxy-deployment.yaml b/charts/consul/templates/dns-proxy-deployment.yaml
@@ -1,5 +1,4 @@
 {{- if (or (and (ne (.Values.dns.proxy.enabled | toString) "-") .Values.dns.proxy.enabled) (and (eq (.Values.dns.proxy.enabled | toString) "-") .Values.global.enabled)) }}
-{{- if not .Values.connectInject.enabled }}{{ fail "connectInject.enabled must be true" }}{{ end -}}
 {{ template "consul.validateRequiredCloudSecretsExist" . }}
 {{ template "consul.validateCloudSecretKeys" . }}
 
@@ -110,12 +109,20 @@ spec:
               valueFrom:
                 fieldRef:
                   fieldPath: spec.nodeName
+            - name: DP_SERVICE_NODE_NAME
+              value: $(NODE_NAME)-virtual
+      {{- if .Values.global.acls.manageSystemACLs }}
             - name: DP_CREDENTIAL_LOGIN_META1
               value: pod=$(NAMESPACE)/$(POD_NAME)
             - name: DP_CREDENTIAL_LOGIN_META2
               value: component=dns-proxy
-            - name: DP_SERVICE_NODE_NAME
-              value: $(NODE_NAME)-virtual
+      {{- else if (and .Values.dns.proxy.aclToken.secretName .Values.dns.proxy.aclToken.secretKey) }}
+            - name: DP_CREDENTIAL_STATIC_TOKEN
+              valueFrom:
+                secretKeyRef:
+                  name: {{ .Values.dns.proxy.aclToken.secretName }}
+                  key: {{ .Values.dns.proxy.aclToken.secretKey }}
+      {{- end }}
           command:
             - consul-dataplane
           args:
@@ -159,6 +166,8 @@ spec:
         {{- if .Values.global.adminPartitions.enabled }}
             - -login-partition={{ .Values.global.adminPartitions.name }}
         {{- end }}
+        {{- else }}
+            - -credential-type=static
         {{- end }}
         {{- if .Values.global.adminPartitions.enabled }}
             - -service-partition={{ .Values.global.adminPartitions.name }}

diff --git a/charts/consul/test/unit/dns-proxy-deployment.bats b/charts/consul/test/unit/dns-proxy-deployment.bats
@@ -82,7 +82,30 @@ load _helpers
 
 
 #--------------------------------------------------------------------
-# authMethod
+# credentials
+
+@test "dnsProxy/Deployment: -credential-type is login by default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/dns-proxy-deployment.yaml  \
+      --set 'dns.proxy.enabled=true' \
+      --set 'global.acls.manageSystemACLs=true' \
+      . | tee /dev/stderr |
+      yq '.spec.template.spec.containers[0] | any(.args[]; . == "-credential-type=login")' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+@test "dnsProxy/Deployment: -credential-type is static when manageSystemACLs is false and dns.proxy.aclToken.secretName and dns.proxy.aclToken.secretKey are set" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/dns-proxy-deployment.yaml  \
+      --set 'dns.proxy.enabled=true' \
+      --set 'dns.proxy.aclToken.secretName=foo' \
+      --set 'dns.proxy.aclToken.secretKey=bar' \
+      . | tee /dev/stderr |
+      yq '.spec.template.spec.containers[0] | any(.args[]; . == "-credential-type=static")' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
 
 @test "dnsProxy/Deployment: -login-auth-method is not set by default" {
   cd `chart_dir`
@@ -235,6 +258,67 @@ load _helpers
 
 #--------------------------------------------------------------------
 # global.acls.manageSystemACLs
+
+@test "dnsProxy/Deployment: sets DP_CREDENTIAL_LOGIN_META1 and DP_CREDENTIAL_LOGIN_META2 when managedSystemACLs is true" {
+  cd `chart_dir`
+  local env=$(helm template \
+      -s templates/dns-proxy-deployment.yaml \
+      --set 'dns.proxy.enabled=true' \
+      --set 'global.acls.manageSystemACLs=true' \
+      . | tee /dev/stderr |
+      yq '.spec.template.spec.containers[0].env[]' | tee /dev/stderr)
+
+  local actual=$(echo $env | jq -r '. | select(.name == "DP_CREDENTIAL_LOGIN_META1") | .value' | tee /dev/stderr)
+    [ "${actual}" = 'pod=$(NAMESPACE)/$(POD_NAME)' ]
+
+  local actual=$(echo $env | jq -r '. | select(.name == "DP_CREDENTIAL_LOGIN_META2") | .value' | tee /dev/stderr)
+    [ "${actual}" = 'component=dns-proxy' ]
+}
+
+@test "dnsProxy/Deployment: does not set DP_CREDENTIAL_LOGIN_META1 and DP_CREDENTIAL_LOGIN_META2 when managedSystemACLs is false" {
+  cd `chart_dir`
+  local env=$(helm template \
+      -s templates/dns-proxy-deployment.yaml \
+      --set 'dns.proxy.enabled=true' \
+      --set 'global.acls.manageSystemACLs=false' \
+      . | tee /dev/stderr |
+      yq '.spec.template.spec.containers[0].env[]' | tee /dev/stderr)
+
+  local actual=$(echo $env | jq -r '. | select(.name == "DP_CREDENTIAL_LOGIN_META1")' | tee /dev/stderr)
+    [ "${actual}" = '' ]
+
+  local actual=$(echo $env | jq -r '. | select(.name == "DP_CREDENTIAL_LOGIN_META2")' | tee /dev/stderr)
+    [ "${actual}" = '' ]
+}
+
+@test "dnsProxy/Deployment: sets DP_CREDENTIAL_STATIC_TOKEN when managedSystemACLs is false and dns.proxy.aclToken.secretName and dns.proxy.aclToken.secretKey are set" {
+  cd `chart_dir`
+  local env=$(helm template \
+      -s templates/dns-proxy-deployment.yaml \
+      --set 'dns.proxy.enabled=true' \
+      --set 'global.acls.manageSystemACLs=false' \
+      --set 'dns.proxy.aclToken.secretName=foo' \
+      --set 'dns.proxy.aclToken.secretKey=bar' \
+      . | tee /dev/stderr |
+      yq '.spec.template.spec.containers[0].env[]' | tee /dev/stderr)
+
+  local actual=$(echo $env | jq -r '. | select(.name == "DP_CREDENTIAL_STATIC_TOKEN") | .valueFrom.secretKeyRef.name' | tee /dev/stderr)
+    [ "${actual}" = 'foo' ]
+}
+
+@test "dnsProxy/Deployment: does not set DP_CREDENTIAL_STATIC_TOKEN when managedSystemACLs is true or dns.proxy.aclToken.secretName or dns.proxy.aclToken.secretKey are not set" {
+  cd `chart_dir`
+  local env=$(helm template \
+      -s templates/dns-proxy-deployment.yaml \
+      --set 'dns.proxy.enabled=true' \
+      --set 'global.acls.manageSystemACLs=true' \
+      . | tee /dev/stderr |
+      yq '.spec.template.spec.containers[0].env[]' | tee /dev/stderr)
+
+  local actual=$(echo $env | jq -r '. | select(.name == "DP_CREDENTIAL_STATIC_TOKEN")' | tee /dev/stderr)
+    [ "${actual}" = '' ]
+}
+
 @test "dnsProxy/Deployment: sets global auth method and primary datacenter when federation and acls" {
   cd `chart_dir`
   local actual=$(helm template \

diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml
@@ -1922,9 +1922,26 @@ dns:
     # The number of deployment replicas.
     replicas: 1
 
-    # Port number to be used by DNS proxy
     port: 53
 
+    # Refers to an existing Kubernetes secret that contains an ACL token
+    # for your Consul cluster. This token provides permissions for the DNS
+    # proxy. This field is required when `global.acls.manageSystemACLs` 
+    # is set to `false` to enable manual ACL management in a Consul cluster.
+    # node_prefix "" {
+    #   policy = "read"
+    # }
+    # service_prefix "" {
+    #   policy = "read"
+    # }
+    aclToken:
+      # The name of the Kubernetes secret that holds the ACL token.
+      # @type: string
+      secretName: null
+      # The key within the Kubernetes secret that holds the ACL token.
+      # @type: string
+      secretKey: null
+
 # Values that configure the Consul UI.
 ui:
   # If true, the UI will be enabled. This will
-Original file line number
+Diff line change
@@ Expand Up / @@ -11,3 +11,4 @@ pkg/ @@
     .vscode
     .bob/
     control-plane/cni/cni
+    acceptance/tests/consul-dns/coredns-custom.yaml