Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

agentless: Multi-cluster tproxy #1632

Merged
merged 14 commits into from
Oct 25, 2022
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ 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
CONSUL_VERSION: 1.14.0-beta1 # Consul's OSS version to use in tests
CONSUL_ENT_VERSION: 1.14.0-beta1+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.

jobs:
Expand Down Expand Up @@ -158,11 +158,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 hashicorppreview/consul:1.14-dev)
docker cp "$container_id:/bin/consul" $HOME/bin/consul
docker rm "$container_id"
- name: Run go tests
working-directory: control-plane
run: |
Expand Down Expand Up @@ -207,10 +205,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 hashicorppreview/consul-enterprise:1.14-dev)
docker cp "$container_id:/bin/consul" $HOME/bin/consul
docker rm "$container_id"

- name: Run go tests
working-directory: control-plane
Expand Down
2 changes: 2 additions & 0 deletions acceptance/framework/connhelper/connect_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 3 additions & 4 deletions acceptance/tests/consul-dns/consul_dns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -59,16 +57,17 @@ func TestConsulDNS(t *testing.T) {
serverIPs = append(serverIPs, serverPod.Status.PodIP)
}

dnsPodName := fmt.Sprintf("%s-dns-pod", releaseName)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test fails sometimes because a pod with this name already exists, so making the names unique per test.

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) {
Expand Down
3 changes: 1 addition & 2 deletions acceptance/tests/partitions/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
3 changes: 1 addition & 2 deletions acceptance/tests/peering/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
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")
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omg so this works???

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup! it works! 🎉

const staticServerPeer = "server"
const staticClientPeer = "client"
cases := []struct {
Expand Down
3 changes: 1 addition & 2 deletions acceptance/tests/wan-federation/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
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 -}}

Comment on lines -151 to -163
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YAYYYYY!!!!

{{/*
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
2 changes: 1 addition & 1 deletion control-plane/cni/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions control-plane/cni/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
12 changes: 11 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,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)
Expand Down Expand Up @@ -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
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 @@ -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)
Expand Down
1 change: 0 additions & 1 deletion control-plane/connect-inject/container_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ const (
sidecarUserAndGroupID = 5995
initContainersUserAndGroupID = 5996
netAdminCapability = "NET_ADMIN"
dnsServiceHostEnvSuffix = "DNS_SERVICE_HOST"
)

type initContainerCommandData struct {
Expand Down
Loading