From 30112288c893a43e545ff7d446f05be15c037093 Mon Sep 17 00:00:00 2001 From: Derek Menteer <105233703+hashi-derek@users.noreply.github.com> Date: Thu, 16 Feb 2023 09:22:41 -0600 Subject: [PATCH] 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 | 3 +++ agent/proxycfg/testing_mesh_gateway.go | 8 ++++---- agent/proxycfg/testing_upstreams.go | 4 ++-- agent/structs/testing_catalog.go | 27 ++++++++++++++++++-------- agent/xds/endpoints.go | 4 +++- 5 files changed, 31 insertions(+), 15 deletions(-) create mode 100644 .changelog/16257.txt diff --git a/.changelog/16257.txt b/.changelog/16257.txt new file mode 100644 index 0000000000..8e98530c42 --- /dev/null +++ b/.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. +``` diff --git a/agent/proxycfg/testing_mesh_gateway.go b/agent/proxycfg/testing_mesh_gateway.go index 039807ed61..f6b463f9d3 100644 --- a/agent/proxycfg/testing_mesh_gateway.go +++ b/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), }, }, }, diff --git a/agent/proxycfg/testing_upstreams.go b/agent/proxycfg/testing_upstreams.go index ed2e65d8d6..cb38a3de16 100644 --- a/agent/proxycfg/testing_upstreams.go +++ b/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": diff --git a/agent/structs/testing_catalog.go b/agent/structs/testing_catalog.go index 1131690511..f7f2a7e710 100644 --- a/agent/structs/testing_catalog.go +++ b/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, } diff --git a/agent/xds/endpoints.go b/agent/xds/endpoints.go index dcdbb4d2f5..d117f8dd82 100644 --- a/agent/xds/endpoints.go +++ b/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) }