Skip to content

Commit

Permalink
Multi-cluster tproxy with consul-dataplane's DNS proxy
Browse files Browse the repository at this point in the history
  • Loading branch information
ishustava committed Oct 21, 2022
1 parent 117ebca commit f986784
Show file tree
Hide file tree
Showing 18 changed files with 220 additions and 224 deletions.
2 changes: 2 additions & 0 deletions acceptance/tests/connect/connect_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,8 @@ func (c *ConnectHelper) helmValues() map[string]string {
"global.tls.enabled": strconv.FormatBool(c.Secure),
"global.tls.enableAutoEncrypt": strconv.FormatBool(c.AutoEncrypt),
"global.acls.manageSystemACLs": strconv.FormatBool(c.Secure),
"dns.enabled": "true",
"dns.enableRedirection": "true",
}

helpers.MergeMaps(helmValues, c.HelmValues)
Expand Down
2 changes: 1 addition & 1 deletion acceptance/tests/peering/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ 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")
Expand Down
4 changes: 0 additions & 4 deletions acceptance/tests/peering/peering_connect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
13 changes: 0 additions & 13 deletions charts/consul/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/}}
Expand Down
6 changes: 0 additions & 6 deletions charts/consul/templates/client-daemonset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }}
Expand Down Expand Up @@ -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:
Expand Down
7 changes: 0 additions & 7 deletions charts/consul/templates/server-statefulset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }}
Expand All @@ -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) }}
Expand Down
24 changes: 0 additions & 24 deletions charts/consul/test/unit/client-daemonset.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
22 changes: 0 additions & 22 deletions charts/consul/test/unit/server-statefulset.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
11 changes: 10 additions & 1 deletion control-plane/connect-inject/consul_dataplane_sidecar.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ import (
"k8s.io/utils/pointer"
)

const ConsulCAFile = "/consul/connect-inject/consul-ca.pem"
const (
ConsulCAFile = "/consul/connect-inject/consul-ca.pem"
ConsulDataplaneDNSBindPort = 8600
)

func (w *MeshWebhook) consulDataplaneSidecar(namespace corev1.Namespace, pod corev1.Pod, mpi multiPortInfo) (corev1.Container, error) {
resources, err := w.sidecarResources(pod)
Expand Down Expand Up @@ -190,6 +193,12 @@ func (w *MeshWebhook) getContainerSidecarCommand(namespace corev1.Namespace, mpi
cmd = append(cmd, fmt.Sprintf("-envoy-admin-bind-port=%d", 19000+mpi.serviceIndex))
}

// 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
Expand Down
20 changes: 20 additions & 0 deletions control-plane/connect-inject/consul_dataplane_sidecar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,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)
Expand Down
64 changes: 64 additions & 0 deletions control-plane/connect-inject/dns.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package connectinject

import (
"fmt"
"strconv"

"github.com/miekg/dns"
corev1 "k8s.io/api/core/v1"
"k8s.io/utils/pointer"
)

func (w *MeshWebhook) configureDNS(pod *corev1.Pod) error {
// First, we need to determine the nameservers configured in this cluster from /etc/resolv.conf.
etcResolvConf := "/etc/resolv.conf"
if w.etcResolvFile != "" {
etcResolvConf = w.etcResolvFile
}
cfg, err := dns.ClientConfigFromFile(etcResolvConf)
if err != nil {
return err
}

// Set DNS policy on the pod to None because we want DNS to work according to the config we will provide.
pod.Spec.DNSPolicy = corev1.DNSNone

// Set the consul-dataplane's DNS server as the first server in the list (i.e. localhost).
// We want to do that so that when consul cannot resolve the record, we will fall back to the nameservers
// configured in our /etc/resolv.conf. It's important to add Consul DNS as the first nameserver because
// if we put kube DNS first, it will return NXDOMAIN response and a DNS client will not fall back to other nameservers.
if pod.Spec.DNSConfig == nil {
consulDPAddress := "127.0.0.1"
nameservers := []string{consulDPAddress}
nameservers = append(nameservers, cfg.Servers...)
var options []corev1.PodDNSConfigOption
if cfg.Ndots != 0 {
ndots := strconv.Itoa(cfg.Ndots)
options = append(options, corev1.PodDNSConfigOption{
Name: "ndots",
Value: &ndots,
})
}
if cfg.Timeout != 0 {
options = append(options, corev1.PodDNSConfigOption{
Name: "timeout",
Value: pointer.String(strconv.Itoa(cfg.Timeout)),
})
}
if cfg.Attempts != 0 {
options = append(options, corev1.PodDNSConfigOption{
Name: "attempts",
Value: pointer.String(strconv.Itoa(cfg.Attempts)),
})
}

pod.Spec.DNSConfig = &corev1.PodDNSConfig{
Nameservers: nameservers,
Searches: cfg.Search,
Options: options,
}
} else {
return fmt.Errorf("DNS redirection to Consul is not supported with an already defined DNSConfig on the pod")
}
return nil
}
98 changes: 98 additions & 0 deletions control-plane/connect-inject/dns_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package connectinject

import (
"os"
"testing"

"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
"k8s.io/utils/pointer"
)

func TestMeshWebhook_configureDNS(t *testing.T) {
cases := map[string]struct {
etcResolv string
expDNSConfig *corev1.PodDNSConfig
}{
"empty /etc/resolv.conf file": {
expDNSConfig: &corev1.PodDNSConfig{
Nameservers: []string{"127.0.0.1"},
Searches: []string{},
Options: []corev1.PodDNSConfigOption{
{
Name: "ndots",
Value: pointer.String("1"),
},
},
},
},
"one nameserver": {
etcResolv: `nameserver 1.1.1.1`,
expDNSConfig: &corev1.PodDNSConfig{
Nameservers: []string{"127.0.0.1", "1.1.1.1"},
Searches: []string{},
Options: []corev1.PodDNSConfigOption{
{
Name: "ndots",
Value: pointer.String("1"),
},
},
},
},
"mutiple nameservers, searches, and options": {
etcResolv: `
nameserver 1.1.1.1
nameserver 2.2.2.2
search foo.bar bar.baz
options ndots:5 timeout:6 attempts:3`,
expDNSConfig: &corev1.PodDNSConfig{
Nameservers: []string{"127.0.0.1", "1.1.1.1", "2.2.2.2"},
Searches: []string{"foo.bar", "bar.baz"},
Options: []corev1.PodDNSConfigOption{
{
Name: "ndots",
Value: pointer.String("5"),
},
{
Name: "timeout",
Value: pointer.String("6"),
},
{
Name: "attempts",
Value: pointer.String("3"),
},
},
},
},
}

for name, c := range cases {
t.Run(name, func(t *testing.T) {
etcResolvFile, err := os.CreateTemp("", "")
require.NoError(t, err)
t.Cleanup(func() {
_ = os.Remove(etcResolvFile.Name())
})
_, err = etcResolvFile.WriteString(c.etcResolv)
require.NoError(t, err)
w := MeshWebhook{
etcResolvFile: etcResolvFile.Name(),
}

pod := minimal()
err = w.configureDNS(pod)
require.NoError(t, err)
require.Equal(t, corev1.DNSNone, pod.Spec.DNSPolicy)
require.Equal(t, c.expDNSConfig, pod.Spec.DNSConfig)
})
}
}

func TestMeshWebhook_configureDNS_error(t *testing.T) {
w := MeshWebhook{}

pod := minimal()
pod.Spec.DNSConfig = &corev1.PodDNSConfig{Nameservers: []string{"1.1.1.1"}}
err := w.configureDNS(pod)
require.EqualError(t, err, "DNS redirection to Consul is not supported with an already defined DNSConfig on the pod")
}
Loading

0 comments on commit f986784

Please sign in to comment.