From 1ab04a8b6ae4e122eada1d4d59bd704e2e1da70e Mon Sep 17 00:00:00 2001 From: "R.B. Boyer" Date: Thu, 17 Oct 2019 16:44:59 -0500 Subject: [PATCH] xds: tcp services using the discovery chain should not assume RDS during LDS (#6623) Previously the logic for configuring RDS during LDS for L7 upstreams was overapplied to TCP proxies resulting in a cluster name of being used incorrectly. Fixes #6621 --- agent/xds/listeners.go | 22 ++++++++- ...nnect-proxy-with-chain-external-sni.golden | 2 +- ...hain-failover-through-local-gateway.golden | 2 +- ...ain-failover-through-remote-gateway.golden | 2 +- .../connect-proxy-with-tcp-chain.golden | 2 +- .../config_entries.hcl | 0 .../s3.hcl | 0 .../setup.sh | 0 .../vars.sh | 0 .../verify.bats | 0 .../config_entries.hcl | 21 +++++++++ .../case-cfg-resolver-svc-redirect-tcp/s3.hcl | 5 ++ .../setup.sh | 11 +++++ .../vars.sh | 3 ++ .../verify.bats | 46 +++++++++++++++++++ 15 files changed, 111 insertions(+), 5 deletions(-) rename test/integration/connect/envoy/{case-cfg-resolver-svc-redirect => case-cfg-resolver-svc-redirect-http}/config_entries.hcl (100%) rename test/integration/connect/envoy/{case-cfg-resolver-svc-redirect => case-cfg-resolver-svc-redirect-http}/s3.hcl (100%) rename test/integration/connect/envoy/{case-cfg-resolver-svc-redirect => case-cfg-resolver-svc-redirect-http}/setup.sh (100%) rename test/integration/connect/envoy/{case-cfg-resolver-svc-redirect => case-cfg-resolver-svc-redirect-http}/vars.sh (100%) rename test/integration/connect/envoy/{case-cfg-resolver-svc-redirect => case-cfg-resolver-svc-redirect-http}/verify.bats (100%) create mode 100644 test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/config_entries.hcl create mode 100644 test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/s3.hcl create mode 100644 test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/setup.sh create mode 100644 test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/vars.sh create mode 100644 test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/verify.bats diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index d7b1dee97..b44cd996c 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -577,8 +577,23 @@ func (s *Server) makeUpstreamListenerForDiscoveryChain( proto = "tcp" } + useRDS := true + clusterName := "" + if proto == "tcp" { + startNode := chain.Nodes[chain.StartNode] + if startNode == nil { + panic("missing first node in compiled discovery chain for: " + chain.ServiceName) + } else if startNode.Type != structs.DiscoveryGraphNodeTypeResolver { + panic(fmt.Sprintf("unexpected first node in discovery chain using protocol=%q: %s", proto, startNode.Type)) + } + targetID := startNode.Resolver.Target + target := chain.Targets[targetID] + clusterName = CustomizeClusterName(target.Name, chain) + useRDS = false + } + filter, err := makeListenerFilter( - true, proto, upstreamID, "", "upstream_", "", false) + useRDS, proto, upstreamID, clusterName, "upstream_", "", false) if err != nil { return nil, err } @@ -607,6 +622,11 @@ func makeListenerFilter( case "tcp": fallthrough default: + if useRDS { + return envoylistener.Filter{}, fmt.Errorf("RDS is not compatible with the tcp proxy filter") + } else if cluster == "" { + return envoylistener.Filter{}, fmt.Errorf("cluster name is required for a tcp proxy filter") + } return makeTCPProxyFilter(filterName, cluster, statPrefix) } } diff --git a/agent/xds/testdata/listeners/connect-proxy-with-chain-external-sni.golden b/agent/xds/testdata/listeners/connect-proxy-with-chain-external-sni.golden index e83b237df..8908c8c0e 100644 --- a/agent/xds/testdata/listeners/connect-proxy-with-chain-external-sni.golden +++ b/agent/xds/testdata/listeners/connect-proxy-with-chain-external-sni.golden @@ -16,7 +16,7 @@ { "name": "envoy.tcp_proxy", "config": { - "cluster": "", + "cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", "stat_prefix": "upstream_db_tcp" } } diff --git a/agent/xds/testdata/listeners/connect-proxy-with-tcp-chain-failover-through-local-gateway.golden b/agent/xds/testdata/listeners/connect-proxy-with-tcp-chain-failover-through-local-gateway.golden index e83b237df..8908c8c0e 100644 --- a/agent/xds/testdata/listeners/connect-proxy-with-tcp-chain-failover-through-local-gateway.golden +++ b/agent/xds/testdata/listeners/connect-proxy-with-tcp-chain-failover-through-local-gateway.golden @@ -16,7 +16,7 @@ { "name": "envoy.tcp_proxy", "config": { - "cluster": "", + "cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", "stat_prefix": "upstream_db_tcp" } } diff --git a/agent/xds/testdata/listeners/connect-proxy-with-tcp-chain-failover-through-remote-gateway.golden b/agent/xds/testdata/listeners/connect-proxy-with-tcp-chain-failover-through-remote-gateway.golden index e83b237df..8908c8c0e 100644 --- a/agent/xds/testdata/listeners/connect-proxy-with-tcp-chain-failover-through-remote-gateway.golden +++ b/agent/xds/testdata/listeners/connect-proxy-with-tcp-chain-failover-through-remote-gateway.golden @@ -16,7 +16,7 @@ { "name": "envoy.tcp_proxy", "config": { - "cluster": "", + "cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", "stat_prefix": "upstream_db_tcp" } } diff --git a/agent/xds/testdata/listeners/connect-proxy-with-tcp-chain.golden b/agent/xds/testdata/listeners/connect-proxy-with-tcp-chain.golden index e83b237df..8908c8c0e 100644 --- a/agent/xds/testdata/listeners/connect-proxy-with-tcp-chain.golden +++ b/agent/xds/testdata/listeners/connect-proxy-with-tcp-chain.golden @@ -16,7 +16,7 @@ { "name": "envoy.tcp_proxy", "config": { - "cluster": "", + "cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", "stat_prefix": "upstream_db_tcp" } } diff --git a/test/integration/connect/envoy/case-cfg-resolver-svc-redirect/config_entries.hcl b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-http/config_entries.hcl similarity index 100% rename from test/integration/connect/envoy/case-cfg-resolver-svc-redirect/config_entries.hcl rename to test/integration/connect/envoy/case-cfg-resolver-svc-redirect-http/config_entries.hcl diff --git a/test/integration/connect/envoy/case-cfg-resolver-svc-redirect/s3.hcl b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-http/s3.hcl similarity index 100% rename from test/integration/connect/envoy/case-cfg-resolver-svc-redirect/s3.hcl rename to test/integration/connect/envoy/case-cfg-resolver-svc-redirect-http/s3.hcl diff --git a/test/integration/connect/envoy/case-cfg-resolver-svc-redirect/setup.sh b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-http/setup.sh similarity index 100% rename from test/integration/connect/envoy/case-cfg-resolver-svc-redirect/setup.sh rename to test/integration/connect/envoy/case-cfg-resolver-svc-redirect-http/setup.sh diff --git a/test/integration/connect/envoy/case-cfg-resolver-svc-redirect/vars.sh b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-http/vars.sh similarity index 100% rename from test/integration/connect/envoy/case-cfg-resolver-svc-redirect/vars.sh rename to test/integration/connect/envoy/case-cfg-resolver-svc-redirect-http/vars.sh diff --git a/test/integration/connect/envoy/case-cfg-resolver-svc-redirect/verify.bats b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-http/verify.bats similarity index 100% rename from test/integration/connect/envoy/case-cfg-resolver-svc-redirect/verify.bats rename to test/integration/connect/envoy/case-cfg-resolver-svc-redirect-http/verify.bats diff --git a/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/config_entries.hcl b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/config_entries.hcl new file mode 100644 index 000000000..f98df5e44 --- /dev/null +++ b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/config_entries.hcl @@ -0,0 +1,21 @@ +enable_central_service_config = true + +config_entries { + bootstrap { + kind = "proxy-defaults" + name = "global" + + config { + protocol = "tcp" + } + } + + bootstrap { + kind = "service-resolver" + name = "s2" + + redirect { + service = "s3" + } + } +} diff --git a/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/s3.hcl b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/s3.hcl new file mode 100644 index 000000000..c8761365b --- /dev/null +++ b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/s3.hcl @@ -0,0 +1,5 @@ +services { + name = "s3" + port = 8282 + connect { sidecar_service {} } +} \ No newline at end of file diff --git a/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/setup.sh b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/setup.sh new file mode 100644 index 000000000..582377619 --- /dev/null +++ b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/setup.sh @@ -0,0 +1,11 @@ +#!/bin/bash + +set -eEuo pipefail + +# wait for bootstrap to apply config entries +wait_for_config_entry proxy-defaults global +wait_for_config_entry service-resolver s2 + +gen_envoy_bootstrap s1 19000 primary +gen_envoy_bootstrap s2 19001 primary +gen_envoy_bootstrap s3 19002 primary \ No newline at end of file diff --git a/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/vars.sh b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/vars.sh new file mode 100644 index 000000000..fd3a5ae7d --- /dev/null +++ b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/vars.sh @@ -0,0 +1,3 @@ +#!/bin/bash + +export REQUIRED_SERVICES="$DEFAULT_REQUIRED_SERVICES s3 s3-sidecar-proxy" diff --git a/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/verify.bats b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/verify.bats new file mode 100644 index 000000000..39462d6e1 --- /dev/null +++ b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/verify.bats @@ -0,0 +1,46 @@ +#!/usr/bin/env bats + +load helpers + +@test "s1 proxy admin is up on :19000" { + retry_default curl -f -s localhost:19000/stats -o /dev/null +} + +@test "s2 proxy admin is up on :19001" { + retry_default curl -f -s localhost:19001/stats -o /dev/null +} + +@test "s3 proxy admin is up on :19002" { + retry_default curl -f -s localhost:19002/stats -o /dev/null +} + +@test "s1 proxy listener should be up and have right cert" { + assert_proxy_presents_cert_uri localhost:21000 s1 +} + +@test "s2 proxy listener should be up and have right cert" { + assert_proxy_presents_cert_uri localhost:21001 s2 +} + +@test "s3 proxy listener should be up and have right cert" { + assert_proxy_presents_cert_uri localhost:21002 s3 +} + +@test "s3 proxy should be healthy" { + assert_service_has_healthy_instances s3 1 +} + +@test "s1 upstream should have healthy endpoints for s3" { + assert_upstream_has_endpoints_in_status 127.0.0.1:19000 s3.default.primary HEALTHY 1 +} + +@test "s1 upstream should be able to connect to its upstream simply" { + run retry_default curl -s -f -d hello localhost:5000 + [ "$status" -eq 0 ] + [ "$output" = "hello" ] +} + +@test "s1 upstream should be able to connect to s3 via upstream s2" { + assert_expected_fortio_name s3 +} +