From 4d4ccedb3a15a3f5e2f87191b9d14e8fc33c0106 Mon Sep 17 00:00:00 2001 From: freddygv Date: Thu, 28 Oct 2021 18:41:48 -0600 Subject: [PATCH 1/5] Update locality check in proxycfg --- agent/proxycfg/mesh_gateway.go | 10 ++- agent/proxycfg/state.go | 4 +- agent/proxycfg/state_test.go | 125 ++++++++++++++++++++++++++ agent/proxycfg/terminating_gateway.go | 5 +- agent/proxycfg/upstreams.go | 2 + 5 files changed, 141 insertions(+), 5 deletions(-) diff --git a/agent/proxycfg/mesh_gateway.go b/agent/proxycfg/mesh_gateway.go index 39f8d9d109..dffca3e9a5 100644 --- a/agent/proxycfg/mesh_gateway.go +++ b/agent/proxycfg/mesh_gateway.go @@ -143,7 +143,10 @@ func (s *handlerMeshGateway) handleUpdate(ctx context.Context, u cache.UpdateEve for dc, nodes := range dcIndexedNodes.DatacenterNodes { snap.MeshGateway.HostnameDatacenters[dc] = hostnameEndpoints( - s.logger.Named(logging.MeshGateway), snap.Datacenter, nodes) + s.logger.Named(logging.MeshGateway), + GatewayKey{Partition: snap.ProxyID.PartitionOrDefault(), Datacenter: snap.Datacenter}, + nodes, + ) } for dc := range snap.MeshGateway.HostnameDatacenters { @@ -323,7 +326,10 @@ func (s *handlerMeshGateway) handleUpdate(ctx context.Context, u cache.UpdateEve if len(resp.Nodes) > 0 { snap.MeshGateway.GatewayGroups[key] = resp.Nodes snap.MeshGateway.HostnameDatacenters[key] = hostnameEndpoints( - s.logger.Named(logging.MeshGateway), snap.Datacenter, resp.Nodes) + s.logger.Named(logging.MeshGateway), + GatewayKey{Partition: snap.ProxyID.PartitionOrDefault(), Datacenter: snap.Datacenter}, + resp.Nodes, + ) } default: diff --git a/agent/proxycfg/state.go b/agent/proxycfg/state.go index 2c443f6be6..31a380b1c3 100644 --- a/agent/proxycfg/state.go +++ b/agent/proxycfg/state.go @@ -401,7 +401,7 @@ func (s *state) Changed(ns *structs.NodeService, token string) bool { // Envoy cannot resolve hostnames provided through EDS, so we exclusively use CDS for these clusters. // If there is a mix of hostnames and addresses we exclusively use the hostnames, since clusters cannot discover // services with both EDS and DNS. -func hostnameEndpoints(logger hclog.Logger, localDC string, nodes structs.CheckServiceNodes) structs.CheckServiceNodes { +func hostnameEndpoints(logger hclog.Logger, localKey GatewayKey, nodes structs.CheckServiceNodes) structs.CheckServiceNodes { var ( hasIP bool hasHostname bool @@ -409,7 +409,7 @@ func hostnameEndpoints(logger hclog.Logger, localDC string, nodes structs.CheckS ) for _, n := range nodes { - addr, _ := n.BestAddress(localDC != n.Node.Datacenter) + addr, _ := n.BestAddress(!localKey.Matches(n.Node.Datacenter, n.Node.PartitionOrDefault())) if net.ParseIP(addr) != nil { hasIP = true continue diff --git a/agent/proxycfg/state_test.go b/agent/proxycfg/state_test.go index 38c2da65b7..1378ac0a53 100644 --- a/agent/proxycfg/state_test.go +++ b/agent/proxycfg/state_test.go @@ -2313,3 +2313,128 @@ func TestState_WatchesAndUpdates(t *testing.T) { }) } } + +func Test_hostnameEndpoints(t *testing.T) { + type testCase struct { + name string + localKey GatewayKey + nodes structs.CheckServiceNodes + want structs.CheckServiceNodes + } + run := func(t *testing.T, tc testCase) { + logger := hclog.New(nil) + got := hostnameEndpoints(logger, tc.localKey, tc.nodes) + require.Equal(t, tc.want, got) + } + + cases := []testCase{ + { + name: "same locality and no LAN hostname endpoints", + localKey: GatewayKey{Datacenter: "dc1", Partition: structs.PartitionOrDefault("")}, + nodes: structs.CheckServiceNodes{ + { + Node: &structs.Node{ + Node: "mesh-gateway", + Datacenter: "dc1", + }, + Service: structs.TestNodeServiceMeshGatewayWithAddrs(t, + "10.0.1.1", 8443, + structs.ServiceAddress{}, + structs.ServiceAddress{Address: "123.us-west-1.elb.notaws.com", Port: 443}), + }, + { + Node: &structs.Node{ + Node: "mesh-gateway", + Datacenter: "dc1", + }, + Service: structs.TestNodeServiceMeshGatewayWithAddrs(t, + "10.0.2.2", 8443, + structs.ServiceAddress{}, + structs.ServiceAddress{Address: "123.us-west-2.elb.notaws.com", Port: 443}), + }, + }, + want: nil, + }, + { + name: "same locality and one LAN hostname endpoint", + localKey: GatewayKey{Datacenter: "dc1", Partition: structs.PartitionOrDefault("")}, + nodes: structs.CheckServiceNodes{ + { + Node: &structs.Node{ + Node: "mesh-gateway", + Datacenter: "dc1", + }, + Service: structs.TestNodeServiceMeshGatewayWithAddrs(t, + "gateway.mydomain", 8443, + structs.ServiceAddress{}, + structs.ServiceAddress{Address: "123.us-west-1.elb.notaws.com", Port: 443}), + }, + { + Node: &structs.Node{ + Node: "mesh-gateway", + Datacenter: "dc1", + }, + Service: structs.TestNodeServiceMeshGatewayWithAddrs(t, + "10.0.2.2", 8443, + structs.ServiceAddress{}, + structs.ServiceAddress{Address: "123.us-west-2.elb.notaws.com", Port: 443}), + }, + }, + want: structs.CheckServiceNodes{ + { + Node: &structs.Node{ + Node: "mesh-gateway", + Datacenter: "dc1", + }, + Service: structs.TestNodeServiceMeshGatewayWithAddrs(t, + "gateway.mydomain", 8443, + structs.ServiceAddress{}, + structs.ServiceAddress{Address: "123.us-west-1.elb.notaws.com", Port: 443}), + }, + }, + }, + { + name: "different locality and one WAN hostname endpoint", + localKey: GatewayKey{Datacenter: "dc2", Partition: structs.PartitionOrDefault("")}, + nodes: structs.CheckServiceNodes{ + { + Node: &structs.Node{ + Node: "mesh-gateway", + Datacenter: "dc1", + }, + Service: structs.TestNodeServiceMeshGatewayWithAddrs(t, + "gateway.mydomain", 8443, + structs.ServiceAddress{}, + structs.ServiceAddress{Address: "8.8.8.8", Port: 443}), + }, + { + Node: &structs.Node{ + Node: "mesh-gateway", + Datacenter: "dc1", + }, + Service: structs.TestNodeServiceMeshGatewayWithAddrs(t, + "10.0.2.2", 8443, + structs.ServiceAddress{}, + structs.ServiceAddress{Address: "123.us-west-2.elb.notaws.com", Port: 443}), + }, + }, + want: structs.CheckServiceNodes{ + { + Node: &structs.Node{ + Node: "mesh-gateway", + Datacenter: "dc1", + }, + Service: structs.TestNodeServiceMeshGatewayWithAddrs(t, + "10.0.2.2", 8443, + structs.ServiceAddress{}, + structs.ServiceAddress{Address: "123.us-west-2.elb.notaws.com", Port: 443}), + }, + }, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + run(t, c) + }) + } +} diff --git a/agent/proxycfg/terminating_gateway.go b/agent/proxycfg/terminating_gateway.go index 4018c7801f..c185ed013b 100644 --- a/agent/proxycfg/terminating_gateway.go +++ b/agent/proxycfg/terminating_gateway.go @@ -285,7 +285,10 @@ func (s *handlerTerminatingGateway) handleUpdate(ctx context.Context, u cache.Up if len(resp.Nodes) > 0 { snap.TerminatingGateway.ServiceGroups[sn] = resp.Nodes snap.TerminatingGateway.HostnameServices[sn] = hostnameEndpoints( - s.logger, snap.Datacenter, resp.Nodes) + s.logger, + GatewayKey{Partition: snap.ProxyID.PartitionOrDefault(), Datacenter: snap.Datacenter}, + resp.Nodes, + ) } // Store leaf cert for watched service diff --git a/agent/proxycfg/upstreams.go b/agent/proxycfg/upstreams.go index 1f5060b158..74015c118d 100644 --- a/agent/proxycfg/upstreams.go +++ b/agent/proxycfg/upstreams.go @@ -107,6 +107,8 @@ func (s *handlerUpstreams) handleUpdateUpstreams(ctx context.Context, u cache.Up Addrs: make(map[string]struct{}), } } + + // TODO(partitions) Update to account for upstream in remote partition once tproxy supports it addr, _ := node.BestAddress(false) upstreamsSnapshot.PassthroughUpstreams[svc.String()].Addrs[addr] = struct{}{} } From bbe46e95221e3cc65e80fba75b5524a84b13ac00 Mon Sep 17 00:00:00 2001 From: freddygv Date: Thu, 28 Oct 2021 18:41:58 -0600 Subject: [PATCH 2/5] Update locality check in xds --- agent/xds/clusters.go | 14 +++++++++++--- agent/xds/endpoints.go | 16 ++++++++-------- agent/xds/endpoints_test.go | 2 +- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index 0bcd66fb93..0910eed769 100644 --- a/agent/xds/clusters.go +++ b/agent/xds/clusters.go @@ -209,14 +209,14 @@ func (s *ResourceGenerator) clustersFromSnapshotMeshGateway(cfgSnap *proxycfg.Co // Generate the remote clusters for _, key := range keys { - if key.Matches(cfgSnap.Datacenter, cfgSnap.ProxyID.PartitionOrEmpty()) { + if key.Matches(cfgSnap.Datacenter, cfgSnap.ProxyID.PartitionOrDefault()) { continue // skip local } opts := gatewayClusterOpts{ name: connect.GatewaySNI(key.Datacenter, key.Partition, cfgSnap.Roots.TrustDomain), hostnameEndpoints: cfgSnap.MeshGateway.HostnameDatacenters[key.String()], - isRemote: key.Datacenter != cfgSnap.Datacenter, + isRemote: true, } cluster := s.makeGatewayCluster(cfgSnap, opts) clusters = append(clusters, cluster) @@ -238,7 +238,7 @@ func (s *ResourceGenerator) clustersFromSnapshotMeshGateway(cfgSnap *proxycfg.Co opts := gatewayClusterOpts{ name: cfgSnap.ServerSNIFn(key.Datacenter, ""), hostnameEndpoints: hostnameEndpoints, - isRemote: key.Datacenter != cfgSnap.Datacenter, + isRemote: !key.Matches(cfgSnap.Datacenter, cfgSnap.ProxyID.PartitionOrDefault()), } cluster := s.makeGatewayCluster(cfgSnap, opts) clusters = append(clusters, cluster) @@ -299,10 +299,17 @@ func (s *ResourceGenerator) makeGatewayServiceClusters( hostnameEndpoints = cfgSnap.TerminatingGateway.HostnameServices[svc] } + localKey := proxycfg.GatewayKey{Partition: cfgSnap.ProxyID.PartitionOrDefault(), Datacenter: cfgSnap.Datacenter} + var isRemote bool + if len(services[svc]) > 0 { + isRemote = !localKey.Matches(services[svc][0].Node.Datacenter, services[svc][0].Node.PartitionOrDefault()) + } + opts := gatewayClusterOpts{ name: clusterName, hostnameEndpoints: hostnameEndpoints, connectTimeout: resolver.ConnectTimeout, + isRemote: isRemote, } cluster := s.makeGatewayCluster(cfgSnap, opts) @@ -323,6 +330,7 @@ func (s *ResourceGenerator) makeGatewayServiceClusters( hostnameEndpoints: subsetHostnameEndpoints, onlyPassing: subset.OnlyPassing, connectTimeout: resolver.ConnectTimeout, + isRemote: isRemote, } cluster := s.makeGatewayCluster(cfgSnap, opts) diff --git a/agent/xds/endpoints.go b/agent/xds/endpoints.go index 073a7ac320..9c9a02b2f4 100644 --- a/agent/xds/endpoints.go +++ b/agent/xds/endpoints.go @@ -79,7 +79,7 @@ func (s *ResourceGenerator) endpointsFromSnapshotConnectProxy(cfgSnap *proxycfg. []loadAssignmentEndpointGroup{ {Endpoints: endpoints}, }, - cfgSnap.Datacenter, + proxycfg.GatewayKey{Datacenter: cfgSnap.Datacenter, Partition: cfgSnap.ProxyID.PartitionOrDefault()}, ) resources = append(resources, la) } @@ -114,7 +114,7 @@ func (s *ResourceGenerator) endpointsFromSnapshotMeshGateway(cfgSnap *proxycfg.C resources := make([]proto.Message, 0, len(keys)+len(cfgSnap.MeshGateway.ServiceGroups)) for _, key := range keys { - if key.Matches(cfgSnap.Datacenter, cfgSnap.ProxyID.PartitionOrEmpty()) { + if key.Matches(cfgSnap.Datacenter, cfgSnap.ProxyID.PartitionOrDefault()) { continue // skip local } // Also skip gateways with a hostname as their address. EDS cannot resolve hostnames, @@ -140,7 +140,7 @@ func (s *ResourceGenerator) endpointsFromSnapshotMeshGateway(cfgSnap *proxycfg.C []loadAssignmentEndpointGroup{ {Endpoints: endpoints}, }, - cfgSnap.Datacenter, + proxycfg.GatewayKey{Datacenter: cfgSnap.Datacenter, Partition: cfgSnap.ProxyID.PartitionOrDefault()}, ) resources = append(resources, la) } @@ -155,7 +155,7 @@ func (s *ResourceGenerator) endpointsFromSnapshotMeshGateway(cfgSnap *proxycfg.C []loadAssignmentEndpointGroup{ {Endpoints: endpoints}, }, - cfgSnap.Datacenter, + proxycfg.GatewayKey{Datacenter: cfgSnap.Datacenter, Partition: cfgSnap.ProxyID.PartitionOrDefault()}, ) resources = append(resources, la) } @@ -256,7 +256,7 @@ func (s *ResourceGenerator) endpointsFromServicesAndResolvers( la := makeLoadAssignment( clusterName, groups, - cfgSnap.Datacenter, + proxycfg.GatewayKey{Datacenter: cfgSnap.Datacenter, Partition: cfgSnap.ProxyID.PartitionOrDefault()}, ) resources = append(resources, la) } @@ -427,7 +427,7 @@ func (s *ResourceGenerator) endpointsFromDiscoveryChain( la := makeLoadAssignment( clusterName, endpointGroups, - gatewayKey.Datacenter, + gatewayKey, ) resources = append(resources, la) } @@ -441,7 +441,7 @@ type loadAssignmentEndpointGroup struct { OverrideHealth envoy_core_v3.HealthStatus } -func makeLoadAssignment(clusterName string, endpointGroups []loadAssignmentEndpointGroup, localDatacenter string) *envoy_endpoint_v3.ClusterLoadAssignment { +func makeLoadAssignment(clusterName string, endpointGroups []loadAssignmentEndpointGroup, localKey proxycfg.GatewayKey) *envoy_endpoint_v3.ClusterLoadAssignment { cla := &envoy_endpoint_v3.ClusterLoadAssignment{ ClusterName: clusterName, Endpoints: make([]*envoy_endpoint_v3.LocalityLbEndpoints, 0, len(endpointGroups)), @@ -461,7 +461,7 @@ func makeLoadAssignment(clusterName string, endpointGroups []loadAssignmentEndpo for _, ep := range endpoints { // TODO (mesh-gateway) - should we respect the translate_wan_addrs configuration here or just always use the wan for cross-dc? - addr, port := ep.BestAddress(localDatacenter != ep.Node.Datacenter) + addr, port := ep.BestAddress(!localKey.Matches(ep.Node.Datacenter, ep.Node.PartitionOrDefault())) healthStatus, weight := calculateEndpointHealthAndWeight(ep, endpointGroup.OnlyPassing) if endpointGroup.OverrideHealth != envoy_core_v3.HealthStatus_UNKNOWN { diff --git a/agent/xds/endpoints_test.go b/agent/xds/endpoints_test.go index c52012c8e3..e9a982be0f 100644 --- a/agent/xds/endpoints_test.go +++ b/agent/xds/endpoints_test.go @@ -209,7 +209,7 @@ func Test_makeLoadAssignment(t *testing.T) { got := makeLoadAssignment( tt.clusterName, tt.endpoints, - "dc1", + proxycfg.GatewayKey{Datacenter: "dc1"}, ) require.Equal(t, tt.want, got) }) From 90ce8974565a68d57faea65ca49cba9f3891c6d7 Mon Sep 17 00:00:00 2001 From: freddygv Date: Thu, 28 Oct 2021 18:47:42 -0600 Subject: [PATCH 3/5] Store GatewayKey in proxycfg snapshot for re-use --- agent/proxycfg/manager_test.go | 2 + agent/proxycfg/mesh_gateway.go | 4 +- agent/proxycfg/snapshot.go | 1 + agent/proxycfg/state.go | 1 + agent/proxycfg/terminating_gateway.go | 2 +- agent/proxycfg/testing.go | 66 +++++++++++++++------------ agent/xds/clusters.go | 3 +- agent/xds/endpoints.go | 10 ++-- 8 files changed, 50 insertions(+), 39 deletions(-) diff --git a/agent/proxycfg/manager_test.go b/agent/proxycfg/manager_test.go index 0198289364..6410dc852f 100644 --- a/agent/proxycfg/manager_test.go +++ b/agent/proxycfg/manager_test.go @@ -239,6 +239,7 @@ func TestManager_BasicLifecycle(t *testing.T) { IntentionsSet: true, }, Datacenter: "dc1", + Locality: GatewayKey{Datacenter: "dc1", Partition: structs.PartitionOrDefault("")}, }, }, { @@ -296,6 +297,7 @@ func TestManager_BasicLifecycle(t *testing.T) { IntentionsSet: true, }, Datacenter: "dc1", + Locality: GatewayKey{Datacenter: "dc1", Partition: structs.PartitionOrDefault("")}, }, }, } diff --git a/agent/proxycfg/mesh_gateway.go b/agent/proxycfg/mesh_gateway.go index dffca3e9a5..3bcce18e7a 100644 --- a/agent/proxycfg/mesh_gateway.go +++ b/agent/proxycfg/mesh_gateway.go @@ -144,7 +144,7 @@ func (s *handlerMeshGateway) handleUpdate(ctx context.Context, u cache.UpdateEve for dc, nodes := range dcIndexedNodes.DatacenterNodes { snap.MeshGateway.HostnameDatacenters[dc] = hostnameEndpoints( s.logger.Named(logging.MeshGateway), - GatewayKey{Partition: snap.ProxyID.PartitionOrDefault(), Datacenter: snap.Datacenter}, + snap.Locality, nodes, ) } @@ -327,7 +327,7 @@ func (s *handlerMeshGateway) handleUpdate(ctx context.Context, u cache.UpdateEve snap.MeshGateway.GatewayGroups[key] = resp.Nodes snap.MeshGateway.HostnameDatacenters[key] = hostnameEndpoints( s.logger.Named(logging.MeshGateway), - GatewayKey{Partition: snap.ProxyID.PartitionOrDefault(), Datacenter: snap.Datacenter}, + snap.Locality, resp.Nodes, ) } diff --git a/agent/proxycfg/snapshot.go b/agent/proxycfg/snapshot.go index 6d69ab523d..e318120e7a 100644 --- a/agent/proxycfg/snapshot.go +++ b/agent/proxycfg/snapshot.go @@ -408,6 +408,7 @@ type ConfigSnapshot struct { Proxy structs.ConnectProxyConfig Datacenter string IntentionDefaultAllow bool + Locality GatewayKey ServerSNIFn ServerSNIFunc Roots *structs.IndexedCARoots diff --git a/agent/proxycfg/state.go b/agent/proxycfg/state.go index 31a380b1c3..cd30003531 100644 --- a/agent/proxycfg/state.go +++ b/agent/proxycfg/state.go @@ -254,6 +254,7 @@ func newConfigSnapshotFromServiceInstance(s serviceInstance, config stateConfig) TaggedAddresses: s.taggedAddresses, Proxy: s.proxyCfg, Datacenter: config.source.Datacenter, + Locality: GatewayKey{Datacenter: config.source.Datacenter, Partition: s.proxyID.PartitionOrDefault()}, ServerSNIFn: config.serverSNIFn, IntentionDefaultAllow: config.intentionDefaultAllow, } diff --git a/agent/proxycfg/terminating_gateway.go b/agent/proxycfg/terminating_gateway.go index c185ed013b..b08985b293 100644 --- a/agent/proxycfg/terminating_gateway.go +++ b/agent/proxycfg/terminating_gateway.go @@ -286,7 +286,7 @@ func (s *handlerTerminatingGateway) handleUpdate(ctx context.Context, u cache.Up snap.TerminatingGateway.ServiceGroups[sn] = resp.Nodes snap.TerminatingGateway.HostnameServices[sn] = hostnameEndpoints( s.logger, - GatewayKey{Partition: snap.ProxyID.PartitionOrDefault(), Datacenter: snap.Datacenter}, + snap.Locality, resp.Nodes, ) } diff --git a/agent/proxycfg/testing.go b/agent/proxycfg/testing.go index e8f40c3901..870d8e7a1d 100644 --- a/agent/proxycfg/testing.go +++ b/agent/proxycfg/testing.go @@ -13,6 +13,7 @@ import ( "github.com/mitchellh/go-testing-interface" "github.com/stretchr/testify/require" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/cache" cachetype "github.com/hashicorp/consul/agent/cache-types" "github.com/hashicorp/consul/agent/connect" @@ -674,11 +675,12 @@ func TestConfigSnapshot(t testing.T) *ConfigSnapshot { upstreams := structs.TestUpstreams(t) return &ConfigSnapshot{ - Kind: structs.ServiceKindConnectProxy, - Service: "web-sidecar-proxy", - ProxyID: structs.NewServiceID("web-sidecar-proxy", nil), - Address: "0.0.0.0", - Port: 9999, + Locality: GatewayKey{Datacenter: "dc1", Partition: acl.DefaultPartitionName}, + Kind: structs.ServiceKindConnectProxy, + Service: "web-sidecar-proxy", + ProxyID: structs.NewServiceID("web-sidecar-proxy", nil), + Address: "0.0.0.0", + Port: 9999, Proxy: structs.ConnectProxyConfig{ DestinationServiceID: "web", DestinationServiceName: "web", @@ -798,11 +800,12 @@ func testConfigSnapshotDiscoveryChain(t testing.T, variation string, additionalE roots, leaf := TestCerts(t) snap := &ConfigSnapshot{ - Kind: structs.ServiceKindConnectProxy, - Service: "web-sidecar-proxy", - ProxyID: structs.NewServiceID("web-sidecar-proxy", nil), - Address: "0.0.0.0", - Port: 9999, + Locality: GatewayKey{Datacenter: "dc1", Partition: acl.DefaultPartitionName}, + Kind: structs.ServiceKindConnectProxy, + Service: "web-sidecar-proxy", + ProxyID: structs.NewServiceID("web-sidecar-proxy", nil), + Address: "0.0.0.0", + Port: 9999, Proxy: structs.ConnectProxyConfig{ DestinationServiceID: "web", DestinationServiceName: "web", @@ -1510,11 +1513,12 @@ func TestConfigSnapshotMeshGatewayNoServices(t testing.T) *ConfigSnapshot { func testConfigSnapshotMeshGateway(t testing.T, populateServices bool, useFederationStates bool) *ConfigSnapshot { roots, _ := TestCerts(t) snap := &ConfigSnapshot{ - Kind: structs.ServiceKindMeshGateway, - Service: "mesh-gateway", - ProxyID: structs.NewServiceID("mesh-gateway", nil), - Address: "1.2.3.4", - Port: 8443, + Locality: GatewayKey{Datacenter: "dc1", Partition: acl.DefaultPartitionName}, + Kind: structs.ServiceKindMeshGateway, + Service: "mesh-gateway", + ProxyID: structs.NewServiceID("mesh-gateway", nil), + Address: "1.2.3.4", + Port: 8443, Proxy: structs.ConnectProxyConfig{ Config: map[string]interface{}{}, }, @@ -1721,6 +1725,7 @@ func testConfigSnapshotIngressGateway( roots, leaf := TestCerts(t) snap := &ConfigSnapshot{ + Locality: GatewayKey{Datacenter: "dc1", Partition: acl.DefaultPartitionName}, Kind: structs.ServiceKindIngressGateway, Service: "ingress-gateway", ProxyID: structs.NewServiceID("ingress-gateway", nil), @@ -1760,11 +1765,12 @@ func testConfigSnapshotIngressGateway( func TestConfigSnapshotExposeConfig(t testing.T) *ConfigSnapshot { return &ConfigSnapshot{ - Kind: structs.ServiceKindConnectProxy, - Service: "web-proxy", - ProxyID: structs.NewServiceID("web-proxy", nil), - Address: "1.2.3.4", - Port: 8080, + Locality: GatewayKey{Datacenter: "dc1", Partition: acl.DefaultPartitionName}, + Kind: structs.ServiceKindConnectProxy, + Service: "web-proxy", + ProxyID: structs.NewServiceID("web-proxy", nil), + Address: "1.2.3.4", + Port: 8080, Proxy: structs.ConnectProxyConfig{ DestinationServiceName: "web", DestinationServiceID: "web", @@ -1801,10 +1807,11 @@ func testConfigSnapshotTerminatingGateway(t testing.T, populateServices bool) *C roots, _ := TestCerts(t) snap := &ConfigSnapshot{ - Kind: structs.ServiceKindTerminatingGateway, - Service: "terminating-gateway", - ProxyID: structs.NewServiceID("terminating-gateway", nil), - Address: "1.2.3.4", + Locality: GatewayKey{Datacenter: "dc1", Partition: acl.DefaultPartitionName}, + Kind: structs.ServiceKindTerminatingGateway, + Service: "terminating-gateway", + ProxyID: structs.NewServiceID("terminating-gateway", nil), + Address: "1.2.3.4", TaggedAddresses: map[string]structs.ServiceAddress{ structs.TaggedAddressWAN: { Address: "198.18.0.1", @@ -2035,11 +2042,12 @@ func testConfigSnapshotTerminatingGateway(t testing.T, populateServices bool) *C func TestConfigSnapshotGRPCExposeHTTP1(t testing.T) *ConfigSnapshot { return &ConfigSnapshot{ - Kind: structs.ServiceKindConnectProxy, - Service: "grpc-proxy", - ProxyID: structs.NewServiceID("grpc-proxy", nil), - Address: "1.2.3.4", - Port: 8080, + Locality: GatewayKey{Datacenter: "dc1", Partition: acl.DefaultPartitionName}, + Kind: structs.ServiceKindConnectProxy, + Service: "grpc-proxy", + ProxyID: structs.NewServiceID("grpc-proxy", nil), + Address: "1.2.3.4", + Port: 8080, Proxy: structs.ConnectProxyConfig{ DestinationServiceName: "grpc", DestinationServiceID: "grpc", diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index 0910eed769..441b587355 100644 --- a/agent/xds/clusters.go +++ b/agent/xds/clusters.go @@ -299,10 +299,9 @@ func (s *ResourceGenerator) makeGatewayServiceClusters( hostnameEndpoints = cfgSnap.TerminatingGateway.HostnameServices[svc] } - localKey := proxycfg.GatewayKey{Partition: cfgSnap.ProxyID.PartitionOrDefault(), Datacenter: cfgSnap.Datacenter} var isRemote bool if len(services[svc]) > 0 { - isRemote = !localKey.Matches(services[svc][0].Node.Datacenter, services[svc][0].Node.PartitionOrDefault()) + isRemote = !cfgSnap.Locality.Matches(services[svc][0].Node.Datacenter, services[svc][0].Node.PartitionOrDefault()) } opts := gatewayClusterOpts{ diff --git a/agent/xds/endpoints.go b/agent/xds/endpoints.go index 9c9a02b2f4..57c014995c 100644 --- a/agent/xds/endpoints.go +++ b/agent/xds/endpoints.go @@ -51,7 +51,7 @@ func (s *ResourceGenerator) endpointsFromSnapshotConnectProxy(cfgSnap *proxycfg. es := s.endpointsFromDiscoveryChain( id, chain, - proxycfg.GatewayKey{Datacenter: cfgSnap.Datacenter, Partition: cfgSnap.ProxyID.PartitionOrDefault()}, + cfgSnap.Locality, cfgSnap.ConnectProxy.UpstreamConfig[id], cfgSnap.ConnectProxy.WatchedUpstreamEndpoints[id], cfgSnap.ConnectProxy.WatchedGatewayEndpoints[id], @@ -79,7 +79,7 @@ func (s *ResourceGenerator) endpointsFromSnapshotConnectProxy(cfgSnap *proxycfg. []loadAssignmentEndpointGroup{ {Endpoints: endpoints}, }, - proxycfg.GatewayKey{Datacenter: cfgSnap.Datacenter, Partition: cfgSnap.ProxyID.PartitionOrDefault()}, + cfgSnap.Locality, ) resources = append(resources, la) } @@ -140,7 +140,7 @@ func (s *ResourceGenerator) endpointsFromSnapshotMeshGateway(cfgSnap *proxycfg.C []loadAssignmentEndpointGroup{ {Endpoints: endpoints}, }, - proxycfg.GatewayKey{Datacenter: cfgSnap.Datacenter, Partition: cfgSnap.ProxyID.PartitionOrDefault()}, + cfgSnap.Locality, ) resources = append(resources, la) } @@ -155,7 +155,7 @@ func (s *ResourceGenerator) endpointsFromSnapshotMeshGateway(cfgSnap *proxycfg.C []loadAssignmentEndpointGroup{ {Endpoints: endpoints}, }, - proxycfg.GatewayKey{Datacenter: cfgSnap.Datacenter, Partition: cfgSnap.ProxyID.PartitionOrDefault()}, + cfgSnap.Locality, ) resources = append(resources, la) } @@ -256,7 +256,7 @@ func (s *ResourceGenerator) endpointsFromServicesAndResolvers( la := makeLoadAssignment( clusterName, groups, - proxycfg.GatewayKey{Datacenter: cfgSnap.Datacenter, Partition: cfgSnap.ProxyID.PartitionOrDefault()}, + cfgSnap.Locality, ) resources = append(resources, la) } From e3666b0bc4db040fe96a1da9fe6f038909210757 Mon Sep 17 00:00:00 2001 From: freddygv Date: Mon, 1 Nov 2021 11:23:55 -0600 Subject: [PATCH 4/5] Update GatewayKeys deduplication Federation states data is only keyed on datacenter, so it cannot be directly compared against keys for gateway groups. --- agent/proxycfg/snapshot.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/agent/proxycfg/snapshot.go b/agent/proxycfg/snapshot.go index e318120e7a..acc737c841 100644 --- a/agent/proxycfg/snapshot.go +++ b/agent/proxycfg/snapshot.go @@ -303,12 +303,13 @@ func (c *configSnapshotMeshGateway) GatewayKeys() []GatewayKey { } keys := make([]GatewayKey, 0, sz) - for key := range c.GatewayGroups { + for key := range c.FedStateGateways { keys = append(keys, gatewayKeyFromString(key)) } - for key := range c.FedStateGateways { - if _, ok := c.GatewayGroups[key]; !ok { - keys = append(keys, gatewayKeyFromString(key)) + for key := range c.GatewayGroups { + gk := gatewayKeyFromString(key) + if _, ok := c.FedStateGateways[gk.Datacenter]; !ok { + keys = append(keys, gk) } } From 60066e5154e747e0f96315170a49cfa24ee775e3 Mon Sep 17 00:00:00 2001 From: freddygv Date: Mon, 1 Nov 2021 14:43:44 -0600 Subject: [PATCH 5/5] Exclude default partition from GatewayKey string This will behave the way we handle SNI and SPIFFE IDs, where the default partition is excluded. Excluding the default ensures that don't attempt to compare default.dc2 to dc2 in OSS. --- agent/proxycfg/snapshot.go | 5 +++-- agent/proxycfg/state_test.go | 6 +++--- agent/proxycfg/testing.go | 12 ++++++------ 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/agent/proxycfg/snapshot.go b/agent/proxycfg/snapshot.go index acc737c841..8e9b4fee34 100644 --- a/agent/proxycfg/snapshot.go +++ b/agent/proxycfg/snapshot.go @@ -8,6 +8,7 @@ import ( "github.com/mitchellh/copystructure" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/structs" ) @@ -61,7 +62,7 @@ type GatewayKey struct { func (k GatewayKey) String() string { resp := k.Datacenter - if k.Partition != "" { + if !structs.IsDefaultPartition(k.Partition) { resp = k.Partition + "." + resp } return resp @@ -79,7 +80,7 @@ func gatewayKeyFromString(s string) GatewayKey { split := strings.SplitN(s, ".", 2) if len(split) == 1 { - return GatewayKey{Datacenter: split[0]} + return GatewayKey{Datacenter: split[0], Partition: acl.DefaultPartitionName} } return GatewayKey{Partition: split[0], Datacenter: split[1]} } diff --git a/agent/proxycfg/state_test.go b/agent/proxycfg/state_test.go index 1378ac0a53..b4d1fc202f 100644 --- a/agent/proxycfg/state_test.go +++ b/agent/proxycfg/state_test.go @@ -649,8 +649,8 @@ func TestState_WatchesAndUpdates(t *testing.T) { "upstream-target:api-failover-remote.default.default.dc2:api-failover-remote?dc=dc2": genVerifyServiceWatch("api-failover-remote", "", "dc2", true), "upstream-target:api-failover-local.default.default.dc2:api-failover-local?dc=dc2": genVerifyServiceWatch("api-failover-local", "", "dc2", true), "upstream-target:api-failover-direct.default.default.dc2:api-failover-direct?dc=dc2": genVerifyServiceWatch("api-failover-direct", "", "dc2", true), - "mesh-gateway:default.dc2:api-failover-remote?dc=dc2": genVerifyGatewayWatch("dc2"), - "mesh-gateway:default.dc1:api-failover-local?dc=dc2": genVerifyGatewayWatch("dc1"), + "mesh-gateway:dc2:api-failover-remote?dc=dc2": genVerifyGatewayWatch("dc2"), + "mesh-gateway:dc1:api-failover-local?dc=dc2": genVerifyGatewayWatch("dc1"), }, verifySnapshot: func(t testing.TB, snap *ConfigSnapshot) { require.True(t, snap.Valid()) @@ -673,7 +673,7 @@ func TestState_WatchesAndUpdates(t *testing.T) { } if meshGatewayProxyConfigValue == structs.MeshGatewayModeLocal { - stage1.requiredWatches["mesh-gateway:default.dc1:api-dc2"] = genVerifyGatewayWatch("dc1") + stage1.requiredWatches["mesh-gateway:dc1:api-dc2"] = genVerifyGatewayWatch("dc1") } return testCase{ diff --git a/agent/proxycfg/testing.go b/agent/proxycfg/testing.go index 870d8e7a1d..33444598b4 100644 --- a/agent/proxycfg/testing.go +++ b/agent/proxycfg/testing.go @@ -1432,7 +1432,7 @@ func setupTestVariationConfigEntriesAndSnapshot( TestUpstreamNodesDC2(t) snap.WatchedGatewayEndpoints = map[string]map[string]structs.CheckServiceNodes{ "db": { - "default.dc2": TestGatewayNodesDC2(t), + "dc2": TestGatewayNodesDC2(t), }, } case "failover-through-double-remote-gateway-triggered": @@ -1445,8 +1445,8 @@ func setupTestVariationConfigEntriesAndSnapshot( snap.WatchedUpstreamEndpoints["db"]["db.default.default.dc3"] = TestUpstreamNodesDC2(t) snap.WatchedGatewayEndpoints = map[string]map[string]structs.CheckServiceNodes{ "db": { - "default.dc2": TestGatewayNodesDC2(t), - "default.dc3": TestGatewayNodesDC3(t), + "dc2": TestGatewayNodesDC2(t), + "dc3": TestGatewayNodesDC3(t), }, } case "failover-through-local-gateway-triggered": @@ -1458,7 +1458,7 @@ func setupTestVariationConfigEntriesAndSnapshot( TestUpstreamNodesDC2(t) snap.WatchedGatewayEndpoints = map[string]map[string]structs.CheckServiceNodes{ "db": { - "default.dc1": TestGatewayNodesDC1(t), + "dc1": TestGatewayNodesDC1(t), }, } case "failover-through-double-local-gateway-triggered": @@ -1471,7 +1471,7 @@ func setupTestVariationConfigEntriesAndSnapshot( snap.WatchedUpstreamEndpoints["db"]["db.default.default.dc3"] = TestUpstreamNodesDC2(t) snap.WatchedGatewayEndpoints = map[string]map[string]structs.CheckServiceNodes{ "db": { - "default.dc1": TestGatewayNodesDC1(t), + "dc1": TestGatewayNodesDC1(t), }, } case "splitter-with-resolver-redirect-multidc": @@ -1547,7 +1547,7 @@ func testConfigSnapshotMeshGateway(t testing.T, populateServices bool, useFedera }, WatchedServicesSet: true, WatchedGateways: map[string]context.CancelFunc{ - "default.dc2": nil, + "dc2": nil, }, ServiceGroups: map[structs.ServiceName]structs.CheckServiceNodes{ structs.NewServiceName("foo", nil): TestGatewayServiceGroupFooDC1(t),