xds: tcp services using the discovery chain should not assume RDS dur…

…ing LDS (#6623)

Previously the logic for configuring RDS during LDS for L7 upstreams was
overapplied to TCP proxies resulting in a cluster name of <emptystring>
being used incorrectly.

Fixes #6621

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)
 	}
 }

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"
                 }
             }

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"
                 }
             }

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"
                 }
             }

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"
                 }
             }

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"
+    }
+  }
+}

test/integration/connect/envoy/case-cfg-resolver-svc-redirect-tcp/s3.hcl

@@ -0,0 +1,5 @@
+services {
+  name = "s3"
+  port = 8282
+  connect { sidecar_service {} }
+}

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

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"

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
+}
+