Fix mesh gateways incorrectly matching peer locality. (#16257)

Fix mesh gateways incorrectly matching peer locality.

This fixes an issue where local mesh gateways use an
incorrect address when attempting to forward traffic to a
peered datacenter. Prior to this change it would use the
lan address instead of the wan if the locality matched. This
should never be done for peering, since we must route all
traffic through the remote mesh gateway.

.changelog/16257.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+peering: Fix issue where mesh gateways would use the wrong address when contacting a remote peer with the same datacenter name.
+```

agent/proxycfg/testing_mesh_gateway.go

@@ -659,8 +659,8 @@ func TestConfigSnapshotPeeredMeshGateway(t testing.T, variant string, nsFn func(
 				CorrelationID: "peering-connect-service:peer-a:db",
 				Result: &structs.IndexedCheckServiceNodes{
 					Nodes: structs.CheckServiceNodes{
-						structs.TestCheckNodeServiceWithNameInPeer(t, "db", "peer-a", "10.40.1.1", false),
-						structs.TestCheckNodeServiceWithNameInPeer(t, "db", "peer-a", "10.40.1.2", false),
+						structs.TestCheckNodeServiceWithNameInPeer(t, "db", "dc1", "peer-a", "10.40.1.1", false),
+						structs.TestCheckNodeServiceWithNameInPeer(t, "db", "dc1", "peer-a", "10.40.1.2", false),
 					},
 				},
 			},
@@ -668,8 +668,8 @@ func TestConfigSnapshotPeeredMeshGateway(t testing.T, variant string, nsFn func(
 				CorrelationID: "peering-connect-service:peer-b:alt",
 				Result: &structs.IndexedCheckServiceNodes{
 					Nodes: structs.CheckServiceNodes{
-						structs.TestCheckNodeServiceWithNameInPeer(t, "alt", "peer-b", "10.40.2.1", false),
-						structs.TestCheckNodeServiceWithNameInPeer(t, "alt", "peer-b", "10.40.2.2", true),
+						structs.TestCheckNodeServiceWithNameInPeer(t, "alt", "remote-dc", "peer-b", "10.40.2.1", false),
+						structs.TestCheckNodeServiceWithNameInPeer(t, "alt", "remote-dc", "peer-b", "10.40.2.2", true),
 					},
 				},
 			},

agent/proxycfg/testing_upstreams.go

@@ -94,7 +94,7 @@ func setupTestVariationConfigEntriesAndSnapshot(
 		events = append(events, UpdateEvent{
 			CorrelationID: "upstream-peer:db?peer=cluster-01",
 			Result: &structs.IndexedCheckServiceNodes{
-				Nodes: structs.CheckServiceNodes{structs.TestCheckNodeServiceWithNameInPeer(t, "db", "cluster-01", "10.40.1.1", false)},
+				Nodes: structs.CheckServiceNodes{structs.TestCheckNodeServiceWithNameInPeer(t, "db", "dc1", "cluster-01", "10.40.1.1", false)},
 			},
 		})
 	case "redirect-to-cluster-peer":
@@ -112,7 +112,7 @@ func setupTestVariationConfigEntriesAndSnapshot(
 		events = append(events, UpdateEvent{
 			CorrelationID: "upstream-peer:db?peer=cluster-01",
 			Result: &structs.IndexedCheckServiceNodes{
-				Nodes: structs.CheckServiceNodes{structs.TestCheckNodeServiceWithNameInPeer(t, "db", "cluster-01", "10.40.1.1", false)},
+				Nodes: structs.CheckServiceNodes{structs.TestCheckNodeServiceWithNameInPeer(t, "db", "dc2", "cluster-01", "10.40.1.1", false)},
 			},
 		})
 	case "failover-through-double-remote-gateway-triggered":

agent/structs/testing_catalog.go

@@ -55,11 +55,13 @@ func TestNodeServiceWithName(t testing.T, name string) *NodeService {
 
 const peerTrustDomain = "1c053652-8512-4373-90cf-5a7f6263a994.consul"
 
-func TestCheckNodeServiceWithNameInPeer(t testing.T, name, peer, ip string, useHostname bool) CheckServiceNode {
+func TestCheckNodeServiceWithNameInPeer(t testing.T, name, dc, peer, ip string, useHostname bool) CheckServiceNode {
 	service := &NodeService{
-		Kind:     ServiceKindTypical,
-		Service:  name,
-		Port:     8080,
+		Kind:    ServiceKindTypical,
+		Service: name,
+		// We should not see this port number appear in most xds golden tests,
+		// because the WAN addr should typically be used.
+		Port:     9090,
 		PeerName: peer,
 		Connect: ServiceConnect{
 			PeerMeta: &PeeringServiceMeta{
@@ -72,6 +74,13 @@ func TestCheckNodeServiceWithNameInPeer(t testing.T, name, peer, ip string, useH
 				Protocol: "tcp",
 			},
 		},
+		// This value should typically be seen in golden file output, since this is a peered service.
+		TaggedAddresses: map[string]ServiceAddress{
+			TaggedAddressWAN: {
+				Address: ip,
+				Port:    8080,
+			},
+		},
 	}
 
 	if useHostname {
@@ -89,10 +98,12 @@ func TestCheckNodeServiceWithNameInPeer(t testing.T, name, peer, ip string, useH
 
 	return CheckServiceNode{
 		Node: &Node{
-			ID:         "test1",
-			Node:       "test1",
-			Address:    ip,
-			Datacenter: "cloud-dc",
+			ID:   "test1",
+			Node: "test1",
+			// We should not see this address appear in most xds golden tests,
+			// because the WAN addr should typically be used.
+			Address:    "1.23.45.67",
+			Datacenter: dc,
 		},
 		Service: service,
 	}

agent/xds/endpoints.go

@@ -449,7 +449,9 @@ func (s *ResourceGenerator) makeEndpointsForOutgoingPeeredServices(
 			la := makeLoadAssignment(
 				clusterName,
 				groups,
-				cfgSnap.Locality,
+				// Use an empty key here so that it never matches. This will force the mesh gateway to always
+				// reference the remote mesh gateway's wan addr.
+				proxycfg.GatewayKey{},
 			)
 			resources = append(resources, la)
 		}