From 561b2fe606b9f07741dac2eecd7dfb3fa282ddca Mon Sep 17 00:00:00 2001
From: "R.B. Boyer" <rb@hashicorp.com>
Date: Mon, 19 Aug 2019 13:03:03 -0500
Subject: [PATCH] connect: generate the full SNI names for discovery targets in
 the compiler rather than in the xds package (#6340)

---
 agent/cache-types/discovery_chain_test.go     |   2 +-
 agent/connect/ca/provider_consul.go           |   2 +-
 agent/connect/ca/provider_consul_test.go      |   2 +-
 agent/connect/sni.go                          |  42 ++++++
 agent/connect/sni_test.go                     | 140 ++++++++++++++++++
 agent/connect/testing_ca.go                   |   2 +
 agent/consul/connect_ca_endpoint.go           |   6 +-
 agent/consul/connect_ca_endpoint_test.go      |   2 +-
 agent/consul/discovery_chain_endpoint.go      |  24 +++
 agent/consul/discovery_chain_endpoint_test.go |  12 +-
 agent/consul/discoverychain/compile.go        |  36 +++--
 agent/consul/discoverychain/compile_test.go   |  14 +-
 agent/consul/discoverychain/testing.go        |  12 +-
 agent/consul/fsm/commands_oss_test.go         |   4 +-
 agent/consul/fsm/snapshot_oss_test.go         |   4 +-
 agent/consul/leader_connect.go                |   8 +-
 agent/consul/state/config_entry.go            |  13 +-
 agent/consul/state/connect_ca.go              |   4 +-
 agent/consul/state/connect_ca_test.go         |  10 +-
 agent/discovery_chain_endpoint_test.go        |  22 ++-
 agent/proxycfg/manager_test.go                |   5 +-
 agent/proxycfg/state_test.go                  |  10 +-
 agent/proxycfg/testing.go                     |   2 +-
 agent/structs/discovery_chain.go              |  17 ++-
 agent/xds/clusters.go                         |  35 ++---
 agent/xds/endpoints.go                        |  19 ++-
 agent/xds/listeners.go                        |  10 +-
 agent/xds/naming.go                           |  16 ++
 agent/xds/routes.go                           |   6 +-
 agent/xds/sni.go                              |  56 -------
 api/discovery_chain.go                        |   3 +-
 api/discovery_chain_test.go                   |  13 ++
 32 files changed, 390 insertions(+), 163 deletions(-)
 create mode 100644 agent/connect/sni.go
 create mode 100644 agent/connect/sni_test.go
 create mode 100644 agent/xds/naming.go
 delete mode 100644 agent/xds/sni.go

diff --git a/agent/cache-types/discovery_chain_test.go b/agent/cache-types/discovery_chain_test.go
index 535f2e2a7a..f94469c795 100644
--- a/agent/cache-types/discovery_chain_test.go
+++ b/agent/cache-types/discovery_chain_test.go
@@ -16,7 +16,7 @@ func TestCompiledDiscoveryChain(t *testing.T) {
 	typ := &CompiledDiscoveryChain{RPC: rpc}
 
 	// just do the default chain
-	chain := discoverychain.TestCompileConfigEntries(t, "web", "default", "dc1", "dc1", nil)
+	chain := discoverychain.TestCompileConfigEntries(t, "web", "default", "dc1", "trustdomain.consul", "dc1", nil)
 
 	// Expect the proper RPC call. This also sets the expected value
 	// since that is return-by-pointer in the arguments.
diff --git a/agent/connect/ca/provider_consul.go b/agent/connect/ca/provider_consul.go
index 1b0d10d48f..730a06e690 100644
--- a/agent/connect/ca/provider_consul.go
+++ b/agent/connect/ca/provider_consul.go
@@ -607,7 +607,7 @@ func (c *ConsulProvider) incrementProviderIndex(providerState *structs.CAConsulP
 // generateCA makes a new root CA using the current private key
 func (c *ConsulProvider) generateCA(privateKey string, sn uint64) (string, error) {
 	state := c.Delegate.State()
-	_, config, err := state.CAConfig()
+	_, config, err := state.CAConfig(nil)
 	if err != nil {
 		return "", err
 	}
diff --git a/agent/connect/ca/provider_consul_test.go b/agent/connect/ca/provider_consul_test.go
index 6ec9b56752..774dcc2649 100644
--- a/agent/connect/ca/provider_consul_test.go
+++ b/agent/connect/ca/provider_consul_test.go
@@ -21,7 +21,7 @@ func (c *consulCAMockDelegate) State() *state.Store {
 }
 
 func (c *consulCAMockDelegate) ApplyCARequest(req *structs.CARequest) error {
-	idx, _, err := c.state.CAConfig()
+	idx, _, err := c.state.CAConfig(nil)
 	if err != nil {
 		return err
 	}
diff --git a/agent/connect/sni.go b/agent/connect/sni.go
new file mode 100644
index 0000000000..efe89ec160
--- /dev/null
+++ b/agent/connect/sni.go
@@ -0,0 +1,42 @@
+package connect
+
+import (
+	"fmt"
+
+	"github.com/hashicorp/consul/agent/structs"
+)
+
+func UpstreamSNI(u *structs.Upstream, subset string, dc string, trustDomain string) string {
+	if u.Datacenter != "" {
+		dc = u.Datacenter
+	}
+
+	if u.DestinationType == structs.UpstreamDestTypePreparedQuery {
+		return QuerySNI(u.DestinationName, dc, trustDomain)
+	}
+	return ServiceSNI(u.DestinationName, subset, u.DestinationNamespace, dc, trustDomain)
+}
+
+func DatacenterSNI(dc string, trustDomain string) string {
+	return fmt.Sprintf("%s.internal.%s", dc, trustDomain)
+}
+
+func ServiceSNI(service string, subset string, namespace string, datacenter string, trustDomain string) string {
+	if namespace == "" {
+		namespace = "default"
+	}
+
+	if subset == "" {
+		return fmt.Sprintf("%s.%s.%s.internal.%s", service, namespace, datacenter, trustDomain)
+	} else {
+		return fmt.Sprintf("%s.%s.%s.%s.internal.%s", subset, service, namespace, datacenter, trustDomain)
+	}
+}
+
+func QuerySNI(service string, datacenter string, trustDomain string) string {
+	return fmt.Sprintf("%s.default.%s.query.%s", service, datacenter, trustDomain)
+}
+
+func TargetSNI(target *structs.DiscoveryTarget, trustDomain string) string {
+	return ServiceSNI(target.Service, target.ServiceSubset, target.Namespace, target.Datacenter, trustDomain)
+}
diff --git a/agent/connect/sni_test.go b/agent/connect/sni_test.go
new file mode 100644
index 0000000000..61715cbae5
--- /dev/null
+++ b/agent/connect/sni_test.go
@@ -0,0 +1,140 @@
+package connect
+
+import (
+	"testing"
+
+	"github.com/hashicorp/consul/agent/structs"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	testTrustDomain1 = "5fcd4b81-a2ca-405a-ac62-0fac602c1949.consul"
+	testTrustDomain2 = "d2e1a32e-5733-47f2-a9dd-6cf271aab5b7.consul"
+
+	testTrustDomainSuffix1 = "internal.5fcd4b81-a2ca-405a-ac62-0fac602c1949.consul"
+	testTrustDomainSuffix2 = "internal.d2e1a32e-5733-47f2-a9dd-6cf271aab5b7.consul"
+)
+
+func TestUpstreamSNI(t *testing.T) {
+	newup := func(typ, name, ns, dc string) *structs.Upstream {
+		u := &structs.Upstream{
+			DestinationType:      typ,
+			DestinationNamespace: ns,
+			DestinationName:      name,
+			Datacenter:           dc,
+			LocalBindPort:        9999, // required
+		}
+		require.NoError(t, u.Validate())
+		return u
+	}
+
+	t.Run("service", func(t *testing.T) {
+		// empty namespace, empty subset, empty dc
+		require.Equal(t, "api.default.foo."+testTrustDomainSuffix1,
+			UpstreamSNI(newup(structs.UpstreamDestTypeService,
+				"api", "", "",
+			), "", "foo", testTrustDomain1))
+
+		// empty namespace, empty subset, set dc
+		require.Equal(t, "api.default.bar."+testTrustDomainSuffix1,
+			UpstreamSNI(newup(structs.UpstreamDestTypeService,
+				"api", "", "bar",
+			), "", "foo", testTrustDomain1))
+
+		// set namespace, empty subset, empty dc
+		require.Equal(t, "api.neighbor.foo."+testTrustDomainSuffix2,
+			UpstreamSNI(newup(structs.UpstreamDestTypeService,
+				"api", "neighbor", "",
+			), "", "foo", testTrustDomain2))
+
+		// set namespace, empty subset, set dc
+		require.Equal(t, "api.neighbor.bar."+testTrustDomainSuffix2,
+			UpstreamSNI(newup(structs.UpstreamDestTypeService,
+				"api", "neighbor", "bar",
+			), "", "foo", testTrustDomain2))
+
+		// empty namespace, set subset, empty dc
+		require.Equal(t, "v2.api.default.foo."+testTrustDomainSuffix1,
+			UpstreamSNI(newup(structs.UpstreamDestTypeService,
+				"api", "", "",
+			), "v2", "foo", testTrustDomain1))
+
+		// empty namespace, set subset, set dc
+		require.Equal(t, "v2.api.default.bar."+testTrustDomainSuffix1,
+			UpstreamSNI(newup(structs.UpstreamDestTypeService,
+				"api", "", "bar",
+			), "v2", "foo", testTrustDomain1))
+
+		// set namespace, set subset, empty dc
+		require.Equal(t, "canary.api.neighbor.foo."+testTrustDomainSuffix2,
+			UpstreamSNI(newup(structs.UpstreamDestTypeService,
+				"api", "neighbor", "",
+			), "canary", "foo", testTrustDomain2))
+
+		// set namespace, set subset, set dc
+		require.Equal(t, "canary.api.neighbor.bar."+testTrustDomainSuffix2,
+			UpstreamSNI(newup(structs.UpstreamDestTypeService,
+				"api", "neighbor", "bar",
+			), "canary", "foo", testTrustDomain2))
+	})
+
+	t.Run("prepared query", func(t *testing.T) {
+		// empty dc
+		require.Equal(t, "magicquery.default.foo.query."+testTrustDomain1,
+			UpstreamSNI(newup(structs.UpstreamDestTypePreparedQuery,
+				"magicquery", "", "",
+			), "", "foo", testTrustDomain1))
+
+		// set dc
+		require.Equal(t, "magicquery.default.bar.query."+testTrustDomain2,
+			UpstreamSNI(newup(structs.UpstreamDestTypePreparedQuery,
+				"magicquery", "", "bar",
+			), "", "foo", testTrustDomain2))
+	})
+}
+
+func TestDatacenterSNI(t *testing.T) {
+	require.Equal(t, "foo."+testTrustDomainSuffix1, DatacenterSNI("foo", testTrustDomain1))
+	require.Equal(t, "bar."+testTrustDomainSuffix2, DatacenterSNI("bar", testTrustDomain2))
+}
+
+func TestServiceSNI(t *testing.T) {
+	// empty namespace, empty subset
+	require.Equal(t, "api.default.foo."+testTrustDomainSuffix1,
+		ServiceSNI("api", "", "", "foo", testTrustDomain1))
+
+	// set namespace, empty subset
+	require.Equal(t, "api.neighbor.foo."+testTrustDomainSuffix2,
+		ServiceSNI("api", "", "neighbor", "foo", testTrustDomain2))
+
+	// empty namespace, set subset
+	require.Equal(t, "v2.api.default.foo."+testTrustDomainSuffix1,
+		ServiceSNI("api", "v2", "", "foo", testTrustDomain1))
+
+	// set namespace, set subset
+	require.Equal(t, "canary.api.neighbor.foo."+testTrustDomainSuffix2,
+		ServiceSNI("api", "canary", "neighbor", "foo", testTrustDomain2))
+}
+
+func TestQuerySNI(t *testing.T) {
+	require.Equal(t, "magicquery.default.foo.query."+testTrustDomain1,
+		QuerySNI("magicquery", "foo", testTrustDomain1))
+}
+
+func TestTargetSNI(t *testing.T) {
+	// empty namespace, empty subset
+	require.Equal(t, "api.default.foo."+testTrustDomainSuffix1,
+		TargetSNI(structs.NewDiscoveryTarget("api", "", "", "foo"), testTrustDomain1))
+
+	// set namespace, empty subset
+	require.Equal(t, "api.neighbor.foo."+testTrustDomainSuffix2,
+		TargetSNI(structs.NewDiscoveryTarget("api", "", "neighbor", "foo"), testTrustDomain2))
+
+	// empty namespace, set subset
+	require.Equal(t, "v2.api.default.foo."+testTrustDomainSuffix1,
+		TargetSNI(structs.NewDiscoveryTarget("api", "v2", "", "foo"), testTrustDomain1))
+
+	// set namespace, set subset
+	require.Equal(t, "canary.api.neighbor.foo."+testTrustDomainSuffix2,
+		TargetSNI(structs.NewDiscoveryTarget("api", "canary", "neighbor", "foo"), testTrustDomain2))
+}
diff --git a/agent/connect/testing_ca.go b/agent/connect/testing_ca.go
index 1f06ca8096..a1deeeedbf 100644
--- a/agent/connect/testing_ca.go
+++ b/agent/connect/testing_ca.go
@@ -19,6 +19,8 @@ import (
 )
 
 // TestClusterID is the Consul cluster ID for testing.
+//
+// NOTE: this is duplicated in the api package as testClusterID
 const TestClusterID = "11111111-2222-3333-4444-555555555555"
 
 // testCACounter is just an atomically incremented counter for creating
diff --git a/agent/consul/connect_ca_endpoint.go b/agent/consul/connect_ca_endpoint.go
index 980f9351ba..fbcdc403cb 100644
--- a/agent/consul/connect_ca_endpoint.go
+++ b/agent/consul/connect_ca_endpoint.go
@@ -124,7 +124,7 @@ func (s *ConnectCA) ConfigurationGet(
 	}
 
 	state := s.srv.fsm.State()
-	_, config, err := state.CAConfig()
+	_, config, err := state.CAConfig(nil)
 	if err != nil {
 		return err
 	}
@@ -157,7 +157,7 @@ func (s *ConnectCA) ConfigurationSet(
 
 	// Exit early if it's a no-op change
 	state := s.srv.fsm.State()
-	confIdx, config, err := state.CAConfig()
+	confIdx, config, err := state.CAConfig(nil)
 	if err != nil {
 		return err
 	}
@@ -407,7 +407,7 @@ func (s *ConnectCA) Sign(
 
 	// Verify that the CSR entity is in the cluster's trust domain
 	state := s.srv.fsm.State()
-	_, config, err := state.CAConfig()
+	_, config, err := state.CAConfig(nil)
 	if err != nil {
 		return err
 	}
diff --git a/agent/consul/connect_ca_endpoint_test.go b/agent/consul/connect_ca_endpoint_test.go
index 63114930c8..d11de89b77 100644
--- a/agent/consul/connect_ca_endpoint_test.go
+++ b/agent/consul/connect_ca_endpoint_test.go
@@ -52,7 +52,7 @@ func TestConnectCARoots(t *testing.T) {
 	ok, err := state.CARootSetCAS(idx, idx, []*structs.CARoot{ca1, ca2})
 	assert.True(ok)
 	require.NoError(err)
-	_, caCfg, err := state.CAConfig()
+	_, caCfg, err := state.CAConfig(nil)
 	require.NoError(err)
 
 	// Request
diff --git a/agent/consul/discovery_chain_endpoint.go b/agent/consul/discovery_chain_endpoint.go
index d6e8ab1f39..514427bd3c 100644
--- a/agent/consul/discovery_chain_endpoint.go
+++ b/agent/consul/discovery_chain_endpoint.go
@@ -1,11 +1,13 @@
 package consul
 
 import (
+	"errors"
 	"fmt"
 	"time"
 
 	metrics "github.com/armon/go-metrics"
 	"github.com/hashicorp/consul/acl"
+	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/consul/discoverychain"
 	"github.com/hashicorp/consul/agent/consul/state"
 	"github.com/hashicorp/consul/agent/structs"
@@ -17,6 +19,11 @@ type DiscoveryChain struct {
 }
 
 func (c *DiscoveryChain) Get(args *structs.DiscoveryChainRequest, reply *structs.DiscoveryChainResponse) error {
+	// Exit early if Connect hasn't been enabled.
+	if !c.srv.config.ConnectEnabled {
+		return ErrConnectNotEnabled
+	}
+
 	if done, err := c.srv.forward("DiscoveryChain.Get", args, args, reply); done {
 		return err
 	}
@@ -55,11 +62,28 @@ func (c *DiscoveryChain) Get(args *structs.DiscoveryChainRequest, reply *structs
 				return err
 			}
 
+			_, config, err := state.CAConfig(ws)
+			if err != nil {
+				return err
+			} else if config == nil {
+				return errors.New("no cluster ca config setup")
+			}
+
+			// Build TrustDomain based on the ClusterID stored.
+			signingID := connect.SpiffeIDSigningForCluster(config)
+			if signingID == nil {
+				// If CA is bootstrapped at all then this should never happen but be
+				// defensive.
+				return errors.New("no cluster trust domain setup")
+			}
+			currentTrustDomain := signingID.Host()
+
 			// Then we compile it into something useful.
 			chain, err := discoverychain.Compile(discoverychain.CompileRequest{
 				ServiceName:            args.Name,
 				EvaluateInNamespace:    evalNS,
 				EvaluateInDatacenter:   evalDC,
+				EvaluateInTrustDomain:  currentTrustDomain,
 				UseInDatacenter:        c.srv.config.Datacenter,
 				OverrideMeshGateway:    args.OverrideMeshGateway,
 				OverrideProtocol:       args.OverrideProtocol,
diff --git a/agent/consul/discovery_chain_endpoint_test.go b/agent/consul/discovery_chain_endpoint_test.go
index a69ce2972d..17fc1c2f91 100644
--- a/agent/consul/discovery_chain_endpoint_test.go
+++ b/agent/consul/discovery_chain_endpoint_test.go
@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/hashicorp/consul/acl"
+	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/testrpc"
 	msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc"
@@ -48,6 +49,13 @@ func TestDiscoveryChainEndpoint_Get(t *testing.T) {
 		return &resp, nil
 	}
 
+	newTarget := func(service, serviceSubset, namespace, datacenter string) *structs.DiscoveryTarget {
+		t := structs.NewDiscoveryTarget(service, serviceSubset, namespace, datacenter)
+		t.SNI = connect.TargetSNI(t, connect.TestClusterID+".consul")
+		t.Name = t.SNI
+		return t
+	}
+
 	// ==== compiling the default chain (no config entries)
 
 	{ // no token
@@ -94,7 +102,7 @@ func TestDiscoveryChainEndpoint_Get(t *testing.T) {
 				},
 			},
 			Targets: map[string]*structs.DiscoveryTarget{
-				"web.default.dc1": structs.NewDiscoveryTarget("web", "", "default", "dc1"),
+				"web.default.dc1": newTarget("web", "", "default", "dc1"),
 			},
 		},
 	}
@@ -187,7 +195,7 @@ func TestDiscoveryChainEndpoint_Get(t *testing.T) {
 					},
 				},
 				Targets: map[string]*structs.DiscoveryTarget{
-					"web.default.dc1": structs.NewDiscoveryTarget("web", "", "default", "dc1"),
+					"web.default.dc1": newTarget("web", "", "default", "dc1"),
 				},
 			},
 		}
diff --git a/agent/consul/discoverychain/compile.go b/agent/consul/discoverychain/compile.go
index 0e94a87e56..730684e1e5 100644
--- a/agent/consul/discoverychain/compile.go
+++ b/agent/consul/discoverychain/compile.go
@@ -5,16 +5,18 @@ import (
 	"strings"
 	"time"
 
+	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/mitchellh/hashstructure"
 	"github.com/mitchellh/mapstructure"
 )
 
 type CompileRequest struct {
-	ServiceName          string
-	EvaluateInNamespace  string
-	EvaluateInDatacenter string
-	UseInDatacenter      string // where the results will be used from
+	ServiceName           string
+	EvaluateInNamespace   string
+	EvaluateInDatacenter  string
+	EvaluateInTrustDomain string
+	UseInDatacenter       string // where the results will be used from
 
 	// OverrideMeshGateway allows for the setting to be overridden for any
 	// resolver in the compiled chain.
@@ -53,11 +55,12 @@ type CompileRequest struct {
 // valid.
 func Compile(req CompileRequest) (*structs.CompiledDiscoveryChain, error) {
 	var (
-		serviceName          = req.ServiceName
-		evaluateInNamespace  = req.EvaluateInNamespace
-		evaluateInDatacenter = req.EvaluateInDatacenter
-		useInDatacenter      = req.UseInDatacenter
-		entries              = req.Entries
+		serviceName           = req.ServiceName
+		evaluateInNamespace   = req.EvaluateInNamespace
+		evaluateInDatacenter  = req.EvaluateInDatacenter
+		evaluateInTrustDomain = req.EvaluateInTrustDomain
+		useInDatacenter       = req.UseInDatacenter
+		entries               = req.Entries
 	)
 	if serviceName == "" {
 		return nil, fmt.Errorf("serviceName is required")
@@ -68,6 +71,9 @@ func Compile(req CompileRequest) (*structs.CompiledDiscoveryChain, error) {
 	if evaluateInDatacenter == "" {
 		return nil, fmt.Errorf("evaluateInDatacenter is required")
 	}
+	if evaluateInTrustDomain == "" {
+		return nil, fmt.Errorf("evaluateInTrustDomain is required")
+	}
 	if useInDatacenter == "" {
 		return nil, fmt.Errorf("useInDatacenter is required")
 	}
@@ -79,6 +85,7 @@ func Compile(req CompileRequest) (*structs.CompiledDiscoveryChain, error) {
 		serviceName:            serviceName,
 		evaluateInNamespace:    evaluateInNamespace,
 		evaluateInDatacenter:   evaluateInDatacenter,
+		evaluateInTrustDomain:  evaluateInTrustDomain,
 		useInDatacenter:        useInDatacenter,
 		overrideMeshGateway:    req.OverrideMeshGateway,
 		overrideProtocol:       req.OverrideProtocol,
@@ -114,6 +121,7 @@ type compiler struct {
 	serviceName            string
 	evaluateInNamespace    string
 	evaluateInDatacenter   string
+	evaluateInTrustDomain  string
 	useInDatacenter        string
 	overrideMeshGateway    structs.MeshGatewayConfig
 	overrideProtocol       string
@@ -601,6 +609,14 @@ func (c *compiler) newTarget(service, serviceSubset, namespace, datacenter strin
 		defaultIfEmpty(datacenter, c.evaluateInDatacenter),
 	)
 
+	// Set default connect SNI. This will be overridden later if the service
+	// has an explicit SNI value configured in service-defaults.
+	t.SNI = connect.TargetSNI(t, c.evaluateInTrustDomain)
+
+	// Use the same representation for the name. This will NOT be overridden
+	// later.
+	t.Name = t.SNI
+
 	prev, ok := c.loadedTargets[t.ID]
 	if ok {
 		return prev
@@ -814,7 +830,7 @@ RESOLVE_AGAIN:
 	target.Subset = resolver.Subsets[target.ServiceSubset]
 
 	if serviceDefault := c.entries.GetService(target.Service); serviceDefault != nil && serviceDefault.ExternalSNI != "" {
-		// Explicitly set the SNI value.
+		// Override the default SNI value.
 		target.SNI = serviceDefault.ExternalSNI
 		target.External = true
 	}
diff --git a/agent/consul/discoverychain/compile_test.go b/agent/consul/discoverychain/compile_test.go
index ee68c405c1..df38b1ec4a 100644
--- a/agent/consul/discoverychain/compile_test.go
+++ b/agent/consul/discoverychain/compile_test.go
@@ -4,6 +4,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/stretchr/testify/require"
 )
@@ -96,11 +97,12 @@ func TestCompile(t *testing.T) {
 			}
 
 			req := CompileRequest{
-				ServiceName:          "main",
-				EvaluateInNamespace:  "default",
-				EvaluateInDatacenter: "dc1",
-				UseInDatacenter:      "dc1",
-				Entries:              tc.entries,
+				ServiceName:           "main",
+				EvaluateInNamespace:   "default",
+				EvaluateInDatacenter:  "dc1",
+				EvaluateInTrustDomain: "trustdomain.consul",
+				UseInDatacenter:       "dc1",
+				Entries:               tc.entries,
 			}
 			if tc.setup != nil {
 				tc.setup(&req)
@@ -2197,6 +2199,8 @@ func newEntries() *structs.DiscoveryChainConfigEntries {
 
 func newTarget(service, serviceSubset, namespace, datacenter string, modFn func(t *structs.DiscoveryTarget)) *structs.DiscoveryTarget {
 	t := structs.NewDiscoveryTarget(service, serviceSubset, namespace, datacenter)
+	t.SNI = connect.TargetSNI(t, "trustdomain.consul")
+	t.Name = t.SNI
 	if modFn != nil {
 		modFn(t)
 	}
diff --git a/agent/consul/discoverychain/testing.go b/agent/consul/discoverychain/testing.go
index 3520c7bd3c..863fa9f87e 100644
--- a/agent/consul/discoverychain/testing.go
+++ b/agent/consul/discoverychain/testing.go
@@ -11,6 +11,7 @@ func TestCompileConfigEntries(
 	serviceName string,
 	evaluateInNamespace string,
 	evaluateInDatacenter string,
+	evaluateInTrustDomain string,
 	useInDatacenter string,
 	setup func(req *CompileRequest),
 	entries ...structs.ConfigEntry,
@@ -20,11 +21,12 @@ func TestCompileConfigEntries(
 	set.AddEntries(entries...)
 
 	req := CompileRequest{
-		ServiceName:          serviceName,
-		EvaluateInNamespace:  evaluateInNamespace,
-		EvaluateInDatacenter: evaluateInDatacenter,
-		UseInDatacenter:      useInDatacenter,
-		Entries:              set,
+		ServiceName:           serviceName,
+		EvaluateInNamespace:   evaluateInNamespace,
+		EvaluateInDatacenter:  evaluateInDatacenter,
+		EvaluateInTrustDomain: evaluateInTrustDomain,
+		UseInDatacenter:       useInDatacenter,
+		Entries:               set,
 	}
 	if setup != nil {
 		setup(&req)
diff --git a/agent/consul/fsm/commands_oss_test.go b/agent/consul/fsm/commands_oss_test.go
index d147601316..d6cba2d9b1 100644
--- a/agent/consul/fsm/commands_oss_test.go
+++ b/agent/consul/fsm/commands_oss_test.go
@@ -1252,7 +1252,7 @@ func TestFSM_CAConfig(t *testing.T) {
 	}
 
 	// Verify key is set directly in the state store.
-	_, config, err := fsm.state.CAConfig()
+	_, config, err := fsm.state.CAConfig(nil)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -1285,7 +1285,7 @@ func TestFSM_CAConfig(t *testing.T) {
 		t.Fatalf("bad: %v", resp)
 	}
 
-	_, config, err = fsm.state.CAConfig()
+	_, config, err = fsm.state.CAConfig(nil)
 	assert.Nil(err)
 	if config.Provider != "static" {
 		t.Fatalf("bad: %v", config.Provider)
diff --git a/agent/consul/fsm/snapshot_oss_test.go b/agent/consul/fsm/snapshot_oss_test.go
index 09c0dd5464..c22aff816f 100644
--- a/agent/consul/fsm/snapshot_oss_test.go
+++ b/agent/consul/fsm/snapshot_oss_test.go
@@ -473,7 +473,7 @@ func TestFSM_SnapshotRestore_OSS(t *testing.T) {
 	assert.Equal("bar", state.RootCert)
 
 	// Verify CA configuration is restored.
-	_, caConf, err := fsm2.state.CAConfig()
+	_, caConf, err := fsm2.state.CAConfig(nil)
 	require.NoError(err)
 	assert.Equal(caConfig, caConf)
 
@@ -594,7 +594,7 @@ func TestFSM_BadSnapshot_NilCAConfig(t *testing.T) {
 
 	// Make sure there's no entry in the CA config table.
 	state := fsm2.State()
-	idx, config, err := state.CAConfig()
+	idx, config, err := state.CAConfig(nil)
 	require.NoError(err)
 	require.Equal(uint64(0), idx)
 	if config != nil {
diff --git a/agent/consul/leader_connect.go b/agent/consul/leader_connect.go
index 2b2118fc7e..e22ed31a03 100644
--- a/agent/consul/leader_connect.go
+++ b/agent/consul/leader_connect.go
@@ -43,7 +43,7 @@ var (
 // when setting up the CA during establishLeadership
 func (s *Server) initializeCAConfig() (*structs.CAConfiguration, error) {
 	state := s.fsm.State()
-	_, config, err := state.CAConfig()
+	_, config, err := state.CAConfig(nil)
 	if err != nil {
 		return nil, err
 	}
@@ -326,7 +326,7 @@ func (s *Server) initializeSecondaryCA(provider ca.Provider, roots structs.Index
 			return err
 		}
 
-		_, config, err := state.CAConfig()
+		_, config, err := state.CAConfig(nil)
 		if err != nil {
 			return err
 		}
@@ -475,7 +475,7 @@ func (s *Server) pruneCARoots() error {
 		return err
 	}
 
-	_, caConf, err := state.CAConfig()
+	_, caConf, err := state.CAConfig(nil)
 	if err != nil {
 		return err
 	}
@@ -743,7 +743,7 @@ func (s *Server) initializeSecondaryProvider(provider ca.Provider, roots structs
 	}
 
 	clusterID := strings.Split(roots.TrustDomain, ".")[0]
-	_, conf, err := s.fsm.State().CAConfig()
+	_, conf, err := s.fsm.State().CAConfig(nil)
 	if err != nil {
 		return err
 	}
diff --git a/agent/consul/state/config_entry.go b/agent/consul/state/config_entry.go
index 1f9fcc2b3f..83d99660b6 100644
--- a/agent/consul/state/config_entry.go
+++ b/agent/consul/state/config_entry.go
@@ -445,13 +445,14 @@ func (s *Store) testCompileDiscoveryChain(
 	// Note we use an arbitrary namespace and datacenter as those would not
 	// currently affect the graph compilation in ways that matter here.
 	//
-	// TODO(rb): we should thread a better value than "dc1" down here as that is going to sometimes show up in user facing errors
+	// TODO(rb): we should thread a better value than "dc1" and the throwaway trust domain down here as that is going to sometimes show up in user facing errors
 	req := discoverychain.CompileRequest{
-		ServiceName:          chainName,
-		EvaluateInNamespace:  "default",
-		EvaluateInDatacenter: "dc1",
-		UseInDatacenter:      "dc1",
-		Entries:              speculativeEntries,
+		ServiceName:           chainName,
+		EvaluateInNamespace:   "default",
+		EvaluateInDatacenter:  "dc1",
+		EvaluateInTrustDomain: "b6fc9da3-03d4-4b5a-9134-c045e9b20152.consul",
+		UseInDatacenter:       "dc1",
+		Entries:               speculativeEntries,
 	}
 	_, err = discoverychain.Compile(req)
 	return err
diff --git a/agent/consul/state/connect_ca.go b/agent/consul/state/connect_ca.go
index 6fa01abff3..2034d3f508 100644
--- a/agent/consul/state/connect_ca.go
+++ b/agent/consul/state/connect_ca.go
@@ -108,11 +108,11 @@ func (s *Restore) CAConfig(config *structs.CAConfiguration) error {
 }
 
 // CAConfig is used to get the current CA configuration.
-func (s *Store) CAConfig() (uint64, *structs.CAConfiguration, error) {
+func (s *Store) CAConfig(ws memdb.WatchSet) (uint64, *structs.CAConfiguration, error) {
 	tx := s.db.Txn(false)
 	defer tx.Abort()
 
-	return s.caConfigTxn(tx, nil)
+	return s.caConfigTxn(tx, ws)
 }
 
 func (s *Store) caConfigTxn(tx *memdb.Txn, ws memdb.WatchSet) (uint64, *structs.CAConfiguration, error) {
diff --git a/agent/consul/state/connect_ca_test.go b/agent/consul/state/connect_ca_test.go
index 2b35bcd2be..5a8c25591e 100644
--- a/agent/consul/state/connect_ca_test.go
+++ b/agent/consul/state/connect_ca_test.go
@@ -28,7 +28,7 @@ func TestStore_CAConfig(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	idx, config, err := s.CAConfig()
+	idx, config, err := s.CAConfig(nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -66,7 +66,7 @@ func TestStore_CAConfigCAS(t *testing.T) {
 
 	// Check that the index is untouched and the entry
 	// has not been updated.
-	idx, config, err := s.CAConfig()
+	idx, config, err := s.CAConfig(nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -86,7 +86,7 @@ func TestStore_CAConfigCAS(t *testing.T) {
 	}
 
 	// Make sure the config was updated
-	idx, config, err = s.CAConfig()
+	idx, config, err = s.CAConfig(nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -136,7 +136,7 @@ func TestStore_CAConfig_Snapshot_Restore(t *testing.T) {
 	}
 	restore.Commit()
 
-	idx, res, err := s2.CAConfig()
+	idx, res, err := s2.CAConfig(nil)
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
@@ -171,7 +171,7 @@ func TestStore_CAConfig_Snapshot_Restore_BlankConfig(t *testing.T) {
 	}
 	restore.Commit()
 
-	idx, result, err := s2.CAConfig()
+	idx, result, err := s2.CAConfig(nil)
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
diff --git a/agent/discovery_chain_endpoint_test.go b/agent/discovery_chain_endpoint_test.go
index 2b5f98c6dd..4001cdae86 100644
--- a/agent/discovery_chain_endpoint_test.go
+++ b/agent/discovery_chain_endpoint_test.go
@@ -8,6 +8,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/sdk/testutil/retry"
 	"github.com/hashicorp/consul/testrpc"
@@ -21,6 +22,13 @@ func TestDiscoveryChainRead(t *testing.T) {
 	defer a.Shutdown()
 	testrpc.WaitForTestAgent(t, a.RPC, "dc1")
 
+	newTarget := func(service, serviceSubset, namespace, datacenter string) *structs.DiscoveryTarget {
+		t := structs.NewDiscoveryTarget(service, serviceSubset, namespace, datacenter)
+		t.SNI = connect.TargetSNI(t, connect.TestClusterID+".consul")
+		t.Name = t.SNI
+		return t
+	}
+
 	for _, method := range []string{"GET", "POST"} {
 		require.True(t, t.Run(method+": error on no service name", func(t *testing.T) {
 			var (
@@ -79,7 +87,7 @@ func TestDiscoveryChainRead(t *testing.T) {
 					},
 				},
 				Targets: map[string]*structs.DiscoveryTarget{
-					"web.default.dc1": structs.NewDiscoveryTarget("web", "", "default", "dc1"),
+					"web.default.dc1": newTarget("web", "", "default", "dc1"),
 				},
 			}
 			require.Equal(t, expect, value.Chain)
@@ -122,7 +130,7 @@ func TestDiscoveryChainRead(t *testing.T) {
 					},
 				},
 				Targets: map[string]*structs.DiscoveryTarget{
-					"web.default.dc2": structs.NewDiscoveryTarget("web", "", "default", "dc2"),
+					"web.default.dc2": newTarget("web", "", "default", "dc2"),
 				},
 			}
 			require.Equal(t, expect, value.Chain)
@@ -174,7 +182,7 @@ func TestDiscoveryChainRead(t *testing.T) {
 					},
 				},
 				Targets: map[string]*structs.DiscoveryTarget{
-					"web.default.dc1": structs.NewDiscoveryTarget("web", "", "default", "dc1"),
+					"web.default.dc1": newTarget("web", "", "default", "dc1"),
 				},
 			}
 			require.Equal(t, expect, value.Chain)
@@ -238,8 +246,8 @@ func TestDiscoveryChainRead(t *testing.T) {
 					},
 				},
 				Targets: map[string]*structs.DiscoveryTarget{
-					"web.default.dc1": structs.NewDiscoveryTarget("web", "", "default", "dc1"),
-					"web.default.dc2": structs.NewDiscoveryTarget("web", "", "default", "dc2"),
+					"web.default.dc1": newTarget("web", "", "default", "dc1"),
+					"web.default.dc2": newTarget("web", "", "default", "dc2"),
 				},
 			}
 			if !reflect.DeepEqual(expect, value.Chain) {
@@ -250,7 +258,7 @@ func TestDiscoveryChainRead(t *testing.T) {
 
 	// TODO(namespaces): add a test
 
-	expectTarget_DC2 := structs.NewDiscoveryTarget("web", "", "default", "dc2")
+	expectTarget_DC2 := newTarget("web", "", "default", "dc2")
 	expectTarget_DC2.MeshGateway = structs.MeshGatewayConfig{
 		Mode: structs.MeshGatewayModeLocal,
 	}
@@ -276,7 +284,7 @@ func TestDiscoveryChainRead(t *testing.T) {
 			},
 		},
 		Targets: map[string]*structs.DiscoveryTarget{
-			"web.default.dc1":   structs.NewDiscoveryTarget("web", "", "default", "dc1"),
+			"web.default.dc1":   newTarget("web", "", "default", "dc1"),
 			expectTarget_DC2.ID: expectTarget_DC2,
 		},
 	}
diff --git a/agent/proxycfg/manager_test.go b/agent/proxycfg/manager_test.go
index 2dff6f48a2..f244acdec0 100644
--- a/agent/proxycfg/manager_test.go
+++ b/agent/proxycfg/manager_test.go
@@ -12,6 +12,7 @@ import (
 
 	"github.com/hashicorp/consul/agent/cache"
 	cachetype "github.com/hashicorp/consul/agent/cache-types"
+	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/consul/discoverychain"
 	"github.com/hashicorp/consul/agent/local"
 	"github.com/hashicorp/consul/agent/structs"
@@ -46,7 +47,7 @@ func TestManager_BasicLifecycle(t *testing.T) {
 	roots, leaf := TestCerts(t)
 
 	dbDefaultChain := func() *structs.CompiledDiscoveryChain {
-		return discoverychain.TestCompileConfigEntries(t, "db", "default", "dc1", "dc1",
+		return discoverychain.TestCompileConfigEntries(t, "db", "default", "dc1", connect.TestClusterID+".consul", "dc1",
 			func(req *discoverychain.CompileRequest) {
 				// This is because structs.TestUpstreams uses an opaque config
 				// to override connect timeouts.
@@ -59,7 +60,7 @@ func TestManager_BasicLifecycle(t *testing.T) {
 		)
 	}
 	dbSplitChain := func() *structs.CompiledDiscoveryChain {
-		return discoverychain.TestCompileConfigEntries(t, "db", "default", "dc1", "dc1",
+		return discoverychain.TestCompileConfigEntries(t, "db", "default", "dc1", "trustdomain.consul", "dc1",
 			func(req *discoverychain.CompileRequest) {
 				// This is because structs.TestUpstreams uses an opaque config
 				// to override connect timeouts.
diff --git a/agent/proxycfg/state_test.go b/agent/proxycfg/state_test.go
index 65da26e69e..591e81ff90 100644
--- a/agent/proxycfg/state_test.go
+++ b/agent/proxycfg/state_test.go
@@ -420,7 +420,7 @@ func TestState_WatchesAndUpdates(t *testing.T) {
 				cache.UpdateEvent{
 					CorrelationID: "discovery-chain:api",
 					Result: &structs.DiscoveryChainResponse{
-						Chain: discoverychain.TestCompileConfigEntries(t, "api", "default", "dc1", "dc1",
+						Chain: discoverychain.TestCompileConfigEntries(t, "api", "default", "dc1", "trustdomain.consul", "dc1",
 							func(req *discoverychain.CompileRequest) {
 								req.OverrideMeshGateway.Mode = meshGatewayProxyConfigValue
 							}),
@@ -430,7 +430,7 @@ func TestState_WatchesAndUpdates(t *testing.T) {
 				cache.UpdateEvent{
 					CorrelationID: "discovery-chain:api-failover-remote?dc=dc2",
 					Result: &structs.DiscoveryChainResponse{
-						Chain: discoverychain.TestCompileConfigEntries(t, "api-failover-remote", "default", "dc2", "dc1",
+						Chain: discoverychain.TestCompileConfigEntries(t, "api-failover-remote", "default", "dc2", "trustdomain.consul", "dc1",
 							func(req *discoverychain.CompileRequest) {
 								req.OverrideMeshGateway.Mode = structs.MeshGatewayModeRemote
 							}),
@@ -440,7 +440,7 @@ func TestState_WatchesAndUpdates(t *testing.T) {
 				cache.UpdateEvent{
 					CorrelationID: "discovery-chain:api-failover-local?dc=dc2",
 					Result: &structs.DiscoveryChainResponse{
-						Chain: discoverychain.TestCompileConfigEntries(t, "api-failover-local", "default", "dc2", "dc1",
+						Chain: discoverychain.TestCompileConfigEntries(t, "api-failover-local", "default", "dc2", "trustdomain.consul", "dc1",
 							func(req *discoverychain.CompileRequest) {
 								req.OverrideMeshGateway.Mode = structs.MeshGatewayModeLocal
 							}),
@@ -450,7 +450,7 @@ func TestState_WatchesAndUpdates(t *testing.T) {
 				cache.UpdateEvent{
 					CorrelationID: "discovery-chain:api-failover-direct?dc=dc2",
 					Result: &structs.DiscoveryChainResponse{
-						Chain: discoverychain.TestCompileConfigEntries(t, "api-failover-direct", "default", "dc2", "dc1",
+						Chain: discoverychain.TestCompileConfigEntries(t, "api-failover-direct", "default", "dc2", "trustdomain.consul", "dc1",
 							func(req *discoverychain.CompileRequest) {
 								req.OverrideMeshGateway.Mode = structs.MeshGatewayModeNone
 							}),
@@ -460,7 +460,7 @@ func TestState_WatchesAndUpdates(t *testing.T) {
 				cache.UpdateEvent{
 					CorrelationID: "discovery-chain:api-dc2",
 					Result: &structs.DiscoveryChainResponse{
-						Chain: discoverychain.TestCompileConfigEntries(t, "api-dc2", "default", "dc1", "dc1",
+						Chain: discoverychain.TestCompileConfigEntries(t, "api-dc2", "default", "dc1", "trustdomain.consul", "dc1",
 							func(req *discoverychain.CompileRequest) {
 								req.OverrideMeshGateway.Mode = meshGatewayProxyConfigValue
 							},
diff --git a/agent/proxycfg/testing.go b/agent/proxycfg/testing.go
index b383ffe060..c2d66b61e5 100644
--- a/agent/proxycfg/testing.go
+++ b/agent/proxycfg/testing.go
@@ -839,7 +839,7 @@ func testConfigSnapshotDiscoveryChain(t testing.T, variation string, additionalE
 		entries = append(entries, additionalEntries...)
 	}
 
-	dbChain := discoverychain.TestCompileConfigEntries(t, "db", "default", "dc1", "dc1", compileSetup, entries...)
+	dbChain := discoverychain.TestCompileConfigEntries(t, "db", "default", "dc1", connect.TestClusterID+".consul", "dc1", compileSetup, entries...)
 
 	snap := &ConfigSnapshot{
 		Kind:    structs.ServiceKindConnectProxy,
diff --git a/agent/structs/discovery_chain.go b/agent/structs/discovery_chain.go
index 4534105fd7..289539b85b 100644
--- a/agent/structs/discovery_chain.go
+++ b/agent/structs/discovery_chain.go
@@ -151,6 +151,8 @@ type DiscoveryFailover struct {
 // config entry to execute a catalog query to generate a list of service
 // instances during discovery.
 type DiscoveryTarget struct {
+	// ID is a unique identifier for referring to this target in a compiled
+	// chain. It should be treated as a per-compile opaque string.
 	ID string `json:",omitempty"`
 
 	Service       string `json:",omitempty"`
@@ -161,10 +163,17 @@ type DiscoveryTarget struct {
 	MeshGateway MeshGatewayConfig     `json:",omitempty"`
 	Subset      ServiceResolverSubset `json:",omitempty"`
 
-	// SNI if set is the sni field to use when addressing this set of
-	// endpoints. If not configured then the Connect default should be used.
-	SNI      string `json:",omitempty"`
-	External bool   `json:",omitempty"`
+	// External is true if this target is outside of this consul cluster.
+	External bool `json:",omitempty"`
+
+	// SNI is the sni field to use when connecting to this set of endpoints
+	// over TLS.
+	SNI string `json:",omitempty"`
+
+	// Name is the unique name for this target for use when generating load
+	// balancer objects.  This has a structure similar to SNI, but will not be
+	// affected by SNI customizations.
+	Name string `json:",omitempty"`
 }
 
 func NewDiscoveryTarget(service, serviceSubset, namespace, datacenter string) *DiscoveryTarget {
diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go
index 343e7e19b2..24d2a7248e 100644
--- a/agent/xds/clusters.go
+++ b/agent/xds/clusters.go
@@ -16,6 +16,7 @@ import (
 	"github.com/gogo/protobuf/proto"
 	"github.com/gogo/protobuf/types"
 
+	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/proxycfg"
 	"github.com/hashicorp/consul/agent/structs"
 )
@@ -88,7 +89,7 @@ func (s *Server) clustersFromSnapshotMeshGateway(cfgSnap *proxycfg.ConfigSnapsho
 
 	// generate the remote dc clusters
 	for dc, _ := range cfgSnap.MeshGateway.GatewayGroups {
-		clusterName := DatacenterSNI(dc, cfgSnap)
+		clusterName := connect.DatacenterSNI(dc, cfgSnap.Roots.TrustDomain)
 
 		cluster, err := s.makeMeshGatewayCluster(clusterName, cfgSnap)
 		if err != nil {
@@ -99,7 +100,7 @@ func (s *Server) clustersFromSnapshotMeshGateway(cfgSnap *proxycfg.ConfigSnapsho
 
 	// generate the per-service clusters
 	for svc, _ := range cfgSnap.MeshGateway.ServiceGroups {
-		clusterName := ServiceSNI(svc, "", "default", cfgSnap.Datacenter, cfgSnap)
+		clusterName := connect.ServiceSNI(svc, "", "default", cfgSnap.Datacenter, cfgSnap.Roots.TrustDomain)
 
 		cluster, err := s.makeMeshGatewayCluster(clusterName, cfgSnap)
 		if err != nil {
@@ -111,7 +112,7 @@ func (s *Server) clustersFromSnapshotMeshGateway(cfgSnap *proxycfg.ConfigSnapsho
 	// generate the service subset clusters
 	for svc, resolver := range cfgSnap.MeshGateway.ServiceResolvers {
 		for subsetName, _ := range resolver.Subsets {
-			clusterName := ServiceSNI(svc, subsetName, "default", cfgSnap.Datacenter, cfgSnap)
+			clusterName := connect.ServiceSNI(svc, subsetName, "default", cfgSnap.Datacenter, cfgSnap.Roots.TrustDomain)
 
 			cluster, err := s.makeMeshGatewayCluster(clusterName, cfgSnap)
 			if err != nil {
@@ -172,19 +173,12 @@ func (s *Server) makeUpstreamCluster(upstream structs.Upstream, cfgSnap *proxycf
 	var c *envoy.Cluster
 	var err error
 
-	ns := "default"
-	if upstream.DestinationNamespace != "" {
-		ns = upstream.DestinationNamespace
-	}
-	dc := cfgSnap.Datacenter
-	if upstream.Datacenter != "" {
-		dc = upstream.Datacenter
+	dc := upstream.Datacenter
+	if dc == "" {
+		dc = cfgSnap.Datacenter
 	}
+	sni := connect.UpstreamSNI(&upstream, "", dc, cfgSnap.Roots.TrustDomain)
 
-	sni := ServiceSNI(upstream.DestinationName, "", ns, dc, cfgSnap)
-	if upstream.DestinationType == "prepared_query" {
-		sni = QuerySNI(upstream.DestinationName, dc, cfgSnap)
-	}
 	cfg, err := ParseUpstreamConfig(upstream.Config)
 	if err != nil {
 		// Don't hard fail on a config typo, just warn. The parse func returns
@@ -269,8 +263,8 @@ func (s *Server) makeUpstreamClustersForDiscoveryChain(
 		// Determine if we have to generate the entire cluster differently.
 		failoverThroughMeshGateway := chain.WillFailoverThroughMeshGateway(node)
 
-		sni := TargetSNI(target, cfgSnap)
-		clusterName := CustomizeClusterName(sni, chain)
+		sni := target.SNI
+		clusterName := CustomizeClusterName(target.Name, chain)
 
 		if failoverThroughMeshGateway {
 			actualTargetID := firstHealthyTarget(
@@ -282,7 +276,7 @@ func (s *Server) makeUpstreamClustersForDiscoveryChain(
 
 			if actualTargetID != targetID {
 				actualTarget := chain.Targets[actualTargetID]
-				sni = TargetSNI(actualTarget, cfgSnap)
+				sni = actualTarget.SNI
 			}
 		}
 
@@ -321,15 +315,10 @@ func (s *Server) makeUpstreamClustersForDiscoveryChain(
 			c.Http2ProtocolOptions = &envoycore.Http2ProtocolOptions{}
 		}
 
-		finalSNI := sni
-		if target.SNI != "" {
-			finalSNI = target.SNI
-		}
-
 		// Enable TLS upstream with the configured client certificate.
 		c.TlsContext = &envoyauth.UpstreamTlsContext{
 			CommonTlsContext: makeCommonTLSContext(cfgSnap),
-			Sni:              finalSNI,
+			Sni:              sni,
 		}
 
 		out = append(out, c)
diff --git a/agent/xds/endpoints.go b/agent/xds/endpoints.go
index 3635de7c98..a8f897a99b 100644
--- a/agent/xds/endpoints.go
+++ b/agent/xds/endpoints.go
@@ -9,6 +9,7 @@ import (
 	envoyendpoint "github.com/envoyproxy/go-control-plane/envoy/api/v2/endpoint"
 	"github.com/gogo/protobuf/proto"
 
+	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/proxycfg"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/api"
@@ -38,8 +39,6 @@ func (s *Server) endpointsFromSnapshotConnectProxy(cfgSnap *proxycfg.ConfigSnaps
 	resources := make([]proto.Message, 0,
 		len(cfgSnap.ConnectProxy.UpstreamEndpoints)+len(cfgSnap.ConnectProxy.WatchedUpstreamEndpoints))
 
-	// TODO(rb): should naming from 1.5 -> 1.6 for clusters remain unchanged?
-
 	for _, u := range cfgSnap.Proxy.Upstreams {
 		id := u.Identifier()
 
@@ -51,7 +50,12 @@ func (s *Server) endpointsFromSnapshotConnectProxy(cfgSnap *proxycfg.ConfigSnaps
 		if chain == nil {
 			// We ONLY want this branch for prepared queries.
 
-			clusterName := UpstreamSNI(&u, "", cfgSnap)
+			dc := u.Datacenter
+			if dc == "" {
+				dc = cfgSnap.Datacenter
+			}
+			clusterName := connect.UpstreamSNI(&u, "", dc, cfgSnap.Roots.TrustDomain)
+
 			endpoints, ok := cfgSnap.ConnectProxy.UpstreamEndpoints[id]
 			if ok {
 				la := makeLoadAssignment(
@@ -77,8 +81,7 @@ func (s *Server) endpointsFromSnapshotConnectProxy(cfgSnap *proxycfg.ConfigSnaps
 
 				target := chain.Targets[targetID]
 
-				sni := TargetSNI(target, cfgSnap)
-				clusterName := CustomizeClusterName(sni, chain)
+				clusterName := CustomizeClusterName(target.Name, chain)
 
 				// Determine if we have to generate the entire cluster differently.
 				failoverThroughMeshGateway := chain.WillFailoverThroughMeshGateway(node)
@@ -151,7 +154,7 @@ func (s *Server) endpointsFromSnapshotMeshGateway(cfgSnap *proxycfg.ConfigSnapsh
 
 	// generate the endpoints for the gateways in the remote datacenters
 	for dc, endpoints := range cfgSnap.MeshGateway.GatewayGroups {
-		clusterName := DatacenterSNI(dc, cfgSnap)
+		clusterName := connect.DatacenterSNI(dc, cfgSnap.Roots.TrustDomain)
 		la := makeLoadAssignment(
 			clusterName,
 			[]loadAssignmentEndpointGroup{
@@ -164,7 +167,7 @@ func (s *Server) endpointsFromSnapshotMeshGateway(cfgSnap *proxycfg.ConfigSnapsh
 
 	// generate the endpoints for the local service groups
 	for svc, endpoints := range cfgSnap.MeshGateway.ServiceGroups {
-		clusterName := ServiceSNI(svc, "", "default", cfgSnap.Datacenter, cfgSnap)
+		clusterName := connect.ServiceSNI(svc, "", "default", cfgSnap.Datacenter, cfgSnap.Roots.TrustDomain)
 		la := makeLoadAssignment(
 			clusterName,
 			[]loadAssignmentEndpointGroup{
@@ -178,7 +181,7 @@ func (s *Server) endpointsFromSnapshotMeshGateway(cfgSnap *proxycfg.ConfigSnapsh
 	// generate the endpoints for the service subsets
 	for svc, resolver := range cfgSnap.MeshGateway.ServiceResolvers {
 		for subsetName, subset := range resolver.Subsets {
-			clusterName := ServiceSNI(svc, subsetName, "default", cfgSnap.Datacenter, cfgSnap)
+			clusterName := connect.ServiceSNI(svc, subsetName, "default", cfgSnap.Datacenter, cfgSnap.Roots.TrustDomain)
 
 			endpoints := cfgSnap.MeshGateway.ServiceGroups[svc]
 
diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go
index 3db8a4cdba..35650dd745 100644
--- a/agent/xds/listeners.go
+++ b/agent/xds/listeners.go
@@ -20,6 +20,7 @@ import (
 	"github.com/gogo/protobuf/proto"
 	"github.com/gogo/protobuf/types"
 
+	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/proxycfg"
 	"github.com/hashicorp/consul/agent/structs"
 )
@@ -293,7 +294,12 @@ func (s *Server) makeUpstreamListenerIgnoreDiscoveryChain(
 
 	upstreamID := u.Identifier()
 
-	sni := UpstreamSNI(u, "", cfgSnap)
+	dc := u.Datacenter
+	if dc == "" {
+		dc = cfgSnap.Datacenter
+	}
+	sni := connect.UpstreamSNI(u, "", dc, cfgSnap.Roots.TrustDomain)
+
 	clusterName := CustomizeClusterName(sni, chain)
 
 	l := makeListener(upstreamID, addr, u.LocalBindPort)
@@ -343,7 +349,7 @@ func (s *Server) makeGatewayListener(name, addr string, port int, cfgSnap *proxy
 	// TODO (mesh-gateway) - Do we need to create clusters for all the old trust domains as well?
 	// We need 1 Filter Chain per datacenter
 	for dc := range cfgSnap.MeshGateway.GatewayGroups {
-		clusterName := DatacenterSNI(dc, cfgSnap)
+		clusterName := connect.DatacenterSNI(dc, cfgSnap.Roots.TrustDomain)
 		filterName := fmt.Sprintf("%s_%s", name, dc)
 		dcTCPProxy, err := makeTCPProxyFilter(filterName, clusterName, "mesh_gateway_remote_")
 		if err != nil {
diff --git a/agent/xds/naming.go b/agent/xds/naming.go
new file mode 100644
index 0000000000..620032a422
--- /dev/null
+++ b/agent/xds/naming.go
@@ -0,0 +1,16 @@
+package xds
+
+import (
+	"fmt"
+
+	"github.com/hashicorp/consul/agent/structs"
+)
+
+func CustomizeClusterName(clusterName string, chain *structs.CompiledDiscoveryChain) string {
+	if chain == nil || chain.CustomizationHash == "" {
+		return clusterName
+	}
+	// Use a colon to delimit this prefix instead of a dot to avoid a
+	// theoretical collision problem with subsets.
+	return fmt.Sprintf("%s~%s", chain.CustomizationHash, clusterName)
+}
diff --git a/agent/xds/routes.go b/agent/xds/routes.go
index 6469c65b43..520545bdc3 100644
--- a/agent/xds/routes.go
+++ b/agent/xds/routes.go
@@ -310,8 +310,7 @@ func makeDefaultRouteMatch() envoyroute.RouteMatch {
 func makeRouteActionForSingleCluster(targetID string, chain *structs.CompiledDiscoveryChain, cfgSnap *proxycfg.ConfigSnapshot) *envoyroute.Route_Route {
 	target := chain.Targets[targetID]
 
-	sni := TargetSNI(target, cfgSnap)
-	clusterName := CustomizeClusterName(sni, chain)
+	clusterName := CustomizeClusterName(target.Name, chain)
 
 	return &envoyroute.Route_Route{
 		Route: &envoyroute.RouteAction{
@@ -334,8 +333,7 @@ func makeRouteActionForSplitter(splits []*structs.DiscoverySplit, chain *structs
 
 		target := chain.Targets[targetID]
 
-		sni := TargetSNI(target, cfgSnap)
-		clusterName := CustomizeClusterName(sni, chain)
+		clusterName := CustomizeClusterName(target.Name, chain)
 
 		// The smallest representable weight is 1/10000 or .01% but envoy
 		// deals with integers so scale everything up by 100x.
diff --git a/agent/xds/sni.go b/agent/xds/sni.go
deleted file mode 100644
index 1730797d88..0000000000
--- a/agent/xds/sni.go
+++ /dev/null
@@ -1,56 +0,0 @@
-package xds
-
-import (
-	"fmt"
-
-	"github.com/hashicorp/consul/agent/proxycfg"
-	"github.com/hashicorp/consul/agent/structs"
-)
-
-func UpstreamSNI(u *structs.Upstream, subset string, cfgSnap *proxycfg.ConfigSnapshot) string {
-	if u.DestinationType == "prepared_query" {
-		return QuerySNI(u.DestinationName, u.Datacenter, cfgSnap)
-	}
-	return ServiceSNI(u.DestinationName, subset, u.DestinationNamespace, u.Datacenter, cfgSnap)
-}
-
-func DatacenterSNI(dc string, cfgSnap *proxycfg.ConfigSnapshot) string {
-	return fmt.Sprintf("%s.internal.%s", dc, cfgSnap.Roots.TrustDomain)
-}
-
-func ServiceSNI(service string, subset string, namespace string, datacenter string, cfgSnap *proxycfg.ConfigSnapshot) string {
-	if namespace == "" {
-		namespace = "default"
-	}
-
-	if datacenter == "" {
-		datacenter = cfgSnap.Datacenter
-	}
-
-	if subset == "" {
-		return fmt.Sprintf("%s.%s.%s.internal.%s", service, namespace, datacenter, cfgSnap.Roots.TrustDomain)
-	} else {
-		return fmt.Sprintf("%s.%s.%s.%s.internal.%s", subset, service, namespace, datacenter, cfgSnap.Roots.TrustDomain)
-	}
-}
-
-func QuerySNI(service string, datacenter string, cfgSnap *proxycfg.ConfigSnapshot) string {
-	if datacenter == "" {
-		datacenter = cfgSnap.Datacenter
-	}
-
-	return fmt.Sprintf("%s.default.%s.query.%s", service, datacenter, cfgSnap.Roots.TrustDomain)
-}
-
-func TargetSNI(target *structs.DiscoveryTarget, cfgSnap *proxycfg.ConfigSnapshot) string {
-	return ServiceSNI(target.Service, target.ServiceSubset, target.Namespace, target.Datacenter, cfgSnap)
-}
-
-func CustomizeClusterName(sni string, chain *structs.CompiledDiscoveryChain) string {
-	if chain == nil || chain.CustomizationHash == "" {
-		return sni
-	}
-	// Use a tilde to delimit this prefix instead of a dot to avoid a
-	// theoretical collision problem with subsets.
-	return fmt.Sprintf("%s~%s", chain.CustomizationHash, sni)
-}
diff --git a/api/discovery_chain.go b/api/discovery_chain.go
index 976d179d87..4d7b112c84 100644
--- a/api/discovery_chain.go
+++ b/api/discovery_chain.go
@@ -187,6 +187,7 @@ type DiscoveryTarget struct {
 
 	MeshGateway MeshGatewayConfig
 	Subset      ServiceResolverSubset
-	SNI         string
 	External    bool
+	SNI         string
+	Name        string
 }
diff --git a/api/discovery_chain_test.go b/api/discovery_chain_test.go
index 5433f60d44..d38bcb76e7 100644
--- a/api/discovery_chain_test.go
+++ b/api/discovery_chain_test.go
@@ -7,6 +7,11 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// testClusterID is the Consul cluster ID for testing.
+//
+// NOTE: this is explicitly duplicated from agent/connect:TestClusterID
+const testClusterID = "11111111-2222-3333-4444-555555555555"
+
 func TestAPI_DiscoveryChain_Get(t *testing.T) {
 	t.Parallel()
 	c, s := makeClient(t)
@@ -43,6 +48,8 @@ func TestAPI_DiscoveryChain_Get(t *testing.T) {
 						Service:    "web",
 						Namespace:  "default",
 						Datacenter: "dc1",
+						SNI:        "web.default.dc1.internal." + testClusterID + ".consul",
+						Name:       "web.default.dc1.internal." + testClusterID + ".consul",
 					},
 				},
 			},
@@ -81,6 +88,8 @@ func TestAPI_DiscoveryChain_Get(t *testing.T) {
 						Service:    "web",
 						Namespace:  "default",
 						Datacenter: "dc2",
+						SNI:        "web.default.dc2.internal." + testClusterID + ".consul",
+						Name:       "web.default.dc2.internal." + testClusterID + ".consul",
 					},
 				},
 			},
@@ -125,6 +134,8 @@ func TestAPI_DiscoveryChain_Get(t *testing.T) {
 						Service:    "web",
 						Namespace:  "default",
 						Datacenter: "dc1",
+						SNI:        "web.default.dc1.internal." + testClusterID + ".consul",
+						Name:       "web.default.dc1.internal." + testClusterID + ".consul",
 					},
 				},
 			},
@@ -171,6 +182,8 @@ func TestAPI_DiscoveryChain_Get(t *testing.T) {
 						MeshGateway: MeshGatewayConfig{
 							Mode: MeshGatewayModeLocal,
 						},
+						SNI:  "web.default.dc2.internal." + testClusterID + ".consul",
+						Name: "web.default.dc2.internal." + testClusterID + ".consul",
 					},
 				},
 			},