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

…ing LDS 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
hashicorp · Oct 17, 2019 · 0bdf26b · 0bdf26b
1 parent 7a13850
commit 0bdf26b
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 5 deletions.
diff --git 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
@@ -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/...xds/testdata/listeners/connect-proxy-with-tcp-chain-failover-through-local-gateway.golden b/...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/...ds/testdata/listeners/connect-proxy-with-tcp-chain-failover-through-remote-gateway.golden b/...ds/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
@@ -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"
                 }
             }