From 463c5fe8580dfd08867cef101ca3a0bd58ca6808 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Wed, 17 May 2023 12:04:42 -0500 Subject: [PATCH 01/11] endpoints xds cluster configuration --- agent/proxycfg/api_gateway.go | 49 +++++++++- agent/xds/endpoints.go | 90 +++++++++++++++++-- .../verify.bats | 4 +- .../case-api-gateway-http-simple/verify.bats | 4 +- .../verify.bats | 4 +- .../case-api-gateway-tcp-simple/verify.bats | 2 +- .../verify.bats | 4 +- 7 files changed, 137 insertions(+), 20 deletions(-) diff --git a/agent/proxycfg/api_gateway.go b/agent/proxycfg/api_gateway.go index 18abed7b25d..19daf3d7400 100644 --- a/agent/proxycfg/api_gateway.go +++ b/agent/proxycfg/api_gateway.go @@ -6,7 +6,6 @@ package proxycfg import ( "context" "fmt" - "github.com/hashicorp/consul/acl" cachetype "github.com/hashicorp/consul/agent/cache-types" "github.com/hashicorp/consul/agent/proxycfg/internal/watch" @@ -125,10 +124,12 @@ func (h *handlerAPIGateway) handleUpdate(ctx context.Context, u UpdateEvent, sna return err } default: - return (*handlerUpstreams)(h).handleUpdateUpstreams(ctx, u, snap) + if err := (*handlerUpstreams)(h).handleUpdateUpstreams(ctx, u, snap); err != nil { + return err + } } - return nil + return h.recompileDiscoveryChains(snap) } // handleRootCAUpdate responds to changes in the watched root CA for a gateway @@ -308,7 +309,7 @@ func (h *handlerAPIGateway) handleRouteConfigUpdate(ctx context.Context, u Updat DestinationNamespace: service.NamespaceOrDefault(), DestinationPartition: service.PartitionOrDefault(), LocalBindPort: listener.Port, - // TODO IngressHosts: g.Hosts, + //IngressHosts: g.Hosts, // Pass the protocol that was configured on the listener in order // to force that protocol on the Envoy listener. Config: map[string]interface{}{ @@ -420,6 +421,46 @@ func (h *handlerAPIGateway) handleRouteConfigUpdate(ctx context.Context, u Updat return nil } +func (h *handlerAPIGateway) recompileDiscoveryChains(snap *ConfigSnapshot) error { + synthesizedChains := map[UpstreamID]*structs.CompiledDiscoveryChain{} + + for name, listener := range snap.APIGateway.Listeners { + boundListener, ok := snap.APIGateway.BoundListeners[name] + if !ok { + // Skip any listeners that don't have a bound listener. Once the bound listener is created, this will be run again. + continue + } + + if !snap.APIGateway.GatewayConfig.ListenerIsReady(name) { + // skip any listeners that might be in an invalid state + continue + } + + // Create a synthesized discovery chain for each service. + services, upstreams, compiled, err := snap.APIGateway.synthesizeChains(h.source.Datacenter, listener, boundListener) + if err != nil { + return err + } + + if len(upstreams) == 0 { + // skip if we can't construct any upstreams + continue + } + + for i, service := range services { + id := NewUpstreamIDFromServiceName(structs.NewServiceName(service.Name, &service.EnterpriseMeta)) + synthesizedChains[id] = compiled[i] + } + } + + // Merge in additional discovery chains + for id, chain := range synthesizedChains { + snap.APIGateway.DiscoveryChain[id] = chain + } + + return nil +} + // referenceIsForListener returns whether the provided structs.ResourceReference // targets the provided structs.APIGatewayListener. For this to be true, the kind // and name must match the structs.APIGatewayConfigEntry containing the listener, diff --git a/agent/xds/endpoints.go b/agent/xds/endpoints.go index 8c5b487ea97..6f10c382b14 100644 --- a/agent/xds/endpoints.go +++ b/agent/xds/endpoints.go @@ -41,13 +41,7 @@ func (s *ResourceGenerator) endpointsFromSnapshot(cfgSnap *proxycfg.ConfigSnapsh case structs.ServiceKindIngressGateway: return s.endpointsFromSnapshotIngressGateway(cfgSnap) case structs.ServiceKindAPIGateway: - // TODO Find a cleaner solution, can't currently pass unexported property types - var err error - cfgSnap.IngressGateway, err = cfgSnap.APIGateway.ToIngress(cfgSnap.Datacenter) - if err != nil { - return nil, err - } - return s.endpointsFromSnapshotIngressGateway(cfgSnap) + return s.endpointsFromSnapshotAPIGateway(cfgSnap) default: return nil, fmt.Errorf("Invalid service kind: %v", cfgSnap.Kind) } @@ -527,6 +521,87 @@ func (s *ResourceGenerator) endpointsFromSnapshotIngressGateway(cfgSnap *proxycf return resources, nil } +// helper struct to persist upstream parent information when ready upstream list is built out +type readyUpstreams struct { + listenerKey proxycfg.APIGatewayListenerKey + listenerCfg structs.APIGatewayListener + boundListenerCfg structs.BoundAPIGatewayListener + routeReference structs.ResourceReference + upstreams []structs.Upstream +} + +func getReadyUpstreams(cfgSnap *proxycfg.ConfigSnapshot) map[string]readyUpstreams { + + ready := map[string]readyUpstreams{} + for _, l := range cfgSnap.APIGateway.Listeners { + //need to account for the state of the Listener when building the upstreams list + if !cfgSnap.APIGateway.GatewayConfig.ListenerIsReady(l.Name) { + continue + } + boundListener := cfgSnap.APIGateway.BoundListeners[l.Name] + //get route ref + for _, routeRef := range boundListener.Routes { + //get upstreams + upstreamMap := cfgSnap.APIGateway.Upstreams[routeRef] + for _, upstreams := range upstreamMap { + for _, u := range upstreams { + r, ok := ready[l.Name] + if !ok { + r = readyUpstreams{ + listenerKey: proxycfg.APIGatewayListenerKey{ + Protocol: string(l.Protocol), + Port: l.Port, + }, + listenerCfg: l, + boundListenerCfg: boundListener, + routeReference: routeRef, + } + } + r.upstreams = append(r.upstreams, u) + ready[l.Name] = r + } + } + } + } + return ready +} + +func (s *ResourceGenerator) endpointsFromSnapshotAPIGateway(cfgSnap *proxycfg.ConfigSnapshot) ([]proto.Message, error) { + var resources []proto.Message + createdClusters := make(map[proxycfg.UpstreamID]bool) + + readyUpstreamsList := getReadyUpstreams(cfgSnap) + + for _, readyUpstreams := range readyUpstreamsList { + for _, u := range readyUpstreams.upstreams { + uid := proxycfg.NewUpstreamID(&u) + + // If we've already created endpoints for this upstream, skip it. Multiple listeners may + // reference the same upstream, so we don't need to create duplicate endpoints in that case. + if createdClusters[uid] { + continue + } + + es, err := s.endpointsFromDiscoveryChain( + uid, + cfgSnap.APIGateway.DiscoveryChain[uid], + cfgSnap, + proxycfg.GatewayKey{Datacenter: cfgSnap.Datacenter, Partition: u.DestinationPartition}, + u.Config, + cfgSnap.APIGateway.WatchedUpstreamEndpoints[uid], + cfgSnap.APIGateway.WatchedGatewayEndpoints[uid], + false, + ) + if err != nil { + return nil, err + } + resources = append(resources, es...) + createdClusters[uid] = true + } + } + return resources, nil +} + // used in clusters.go func makeEndpoint(host string, port int) *envoy_endpoint_v3.LbEndpoint { return &envoy_endpoint_v3.LbEndpoint{ @@ -628,6 +703,7 @@ func (s *ResourceGenerator) endpointsFromDiscoveryChain( var escapeHatchCluster *envoy_cluster_v3.Cluster if !forMeshGateway { + cfg, err := structs.ParseUpstreamConfigNoDefaults(upstreamConfigMap) if err != nil { // Don't hard fail on a config typo, just warn. The parse func returns diff --git a/test/integration/connect/envoy/case-api-gateway-http-hostnames/verify.bats b/test/integration/connect/envoy/case-api-gateway-http-hostnames/verify.bats index ba109ea6f9d..7aaee6da796 100644 --- a/test/integration/connect/envoy/case-api-gateway-http-hostnames/verify.bats +++ b/test/integration/connect/envoy/case-api-gateway-http-hostnames/verify.bats @@ -6,7 +6,7 @@ load helpers retry_default curl -f -s localhost:20000/stats -o /dev/null } -@test "api gateway should have be accepted and not conflicted" { +@test "api gateway should have been accepted and not conflicted" { assert_config_entry_status Accepted True Accepted primary api-gateway api-gateway assert_config_entry_status Conflicted False NoConflict primary api-gateway api-gateway } @@ -63,4 +63,4 @@ load helpers run retry_long curl -H "Host: foo.bar.baz" -s -f -d hello localhost:9995 [ "$status" -eq 0 ] [[ "$output" == *"hello"* ]] -} \ No newline at end of file +} diff --git a/test/integration/connect/envoy/case-api-gateway-http-simple/verify.bats b/test/integration/connect/envoy/case-api-gateway-http-simple/verify.bats index 72686b3c4f2..e62e979bf8b 100644 --- a/test/integration/connect/envoy/case-api-gateway-http-simple/verify.bats +++ b/test/integration/connect/envoy/case-api-gateway-http-simple/verify.bats @@ -6,7 +6,7 @@ load helpers retry_default curl -f -s localhost:20000/stats -o /dev/null } -@test "api gateway should have be accepted and not conflicted" { +@test "api gateway should have been accepted and not conflicted" { assert_config_entry_status Accepted True Accepted primary api-gateway api-gateway assert_config_entry_status Conflicted False NoConflict primary api-gateway api-gateway } @@ -31,4 +31,4 @@ load helpers run retry_default sh -c "curl -s localhost:9998 | grep RBAC" [ "$status" -eq 0 ] [[ "$output" == "RBAC: access denied" ]] -} \ No newline at end of file +} diff --git a/test/integration/connect/envoy/case-api-gateway-http-tls-overlapping-hosts/verify.bats b/test/integration/connect/envoy/case-api-gateway-http-tls-overlapping-hosts/verify.bats index aeb0b7fd6cf..4d99c49e695 100644 --- a/test/integration/connect/envoy/case-api-gateway-http-tls-overlapping-hosts/verify.bats +++ b/test/integration/connect/envoy/case-api-gateway-http-tls-overlapping-hosts/verify.bats @@ -6,7 +6,7 @@ load helpers retry_default curl -f -s localhost:20000/stats -o /dev/null } -@test "api gateway should have be accepted and not conflicted" { +@test "api gateway should have been accepted and not conflicted" { assert_config_entry_status Accepted True Accepted primary api-gateway api-gateway assert_config_entry_status Conflicted False NoConflict primary api-gateway api-gateway } @@ -45,4 +45,4 @@ load helpers @test "api gateway should fall back to a connect certificate on conflicted SNI on listener 2" { assert_cert_has_cn localhost:9998 pri host.consul.example -} \ No newline at end of file +} diff --git a/test/integration/connect/envoy/case-api-gateway-tcp-simple/verify.bats b/test/integration/connect/envoy/case-api-gateway-tcp-simple/verify.bats index e96f473be4f..536c99d7c74 100644 --- a/test/integration/connect/envoy/case-api-gateway-tcp-simple/verify.bats +++ b/test/integration/connect/envoy/case-api-gateway-tcp-simple/verify.bats @@ -6,7 +6,7 @@ load helpers retry_default curl -f -s localhost:20000/stats -o /dev/null } -@test "api gateway should have be accepted and not conflicted" { +@test "api gateway should have been accepted and not conflicted" { assert_config_entry_status Accepted True Accepted primary api-gateway api-gateway assert_config_entry_status Conflicted False NoConflict primary api-gateway api-gateway } diff --git a/test/integration/connect/envoy/case-api-gateway-tcp-tls-overlapping-hosts/verify.bats b/test/integration/connect/envoy/case-api-gateway-tcp-tls-overlapping-hosts/verify.bats index 5e28909bfa5..5bdddadd9d2 100644 --- a/test/integration/connect/envoy/case-api-gateway-tcp-tls-overlapping-hosts/verify.bats +++ b/test/integration/connect/envoy/case-api-gateway-tcp-tls-overlapping-hosts/verify.bats @@ -6,7 +6,7 @@ load helpers retry_default curl -f -s localhost:20000/stats -o /dev/null } -@test "api gateway should have be accepted and not conflicted" { +@test "api gateway should have been accepted and not conflicted" { assert_config_entry_status Accepted True Accepted primary api-gateway api-gateway assert_config_entry_status Conflicted False NoConflict primary api-gateway api-gateway } @@ -40,4 +40,4 @@ load helpers @test "api gateway should fall back to a connect certificate on conflicted SNI on listener 2" { assert_cert_has_cn localhost:9998 pri host.consul.example -} \ No newline at end of file +} From 94188ea899f7ccb57c5defc68e074a22d8c6d4a4 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Wed, 17 May 2023 12:07:23 -0500 Subject: [PATCH 02/11] resources test fix --- agent/xds/resources_test.go | 45 ++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/agent/xds/resources_test.go b/agent/xds/resources_test.go index 9204a990a2e..3a068d850bd 100644 --- a/agent/xds/resources_test.go +++ b/agent/xds/resources_test.go @@ -172,8 +172,8 @@ func TestAllResourcesFromSnapshot(t *testing.T) { create: proxycfg.TestConfigSnapshotPeeringLocalMeshGateway, }, { - name: "telemetry-collector", - create: proxycfg.TestConfigSnapshotTelemetryCollector, + name: "hcp-metrics", + create: proxycfg.TestConfigSnapshotHCPMetrics, }, } tests = append(tests, getConnectProxyTransparentProxyGoldenTestCases()...) @@ -365,20 +365,27 @@ func getAPIGatewayGoldenTestCases(t *testing.T) []goldenTestCase { }}, }, } - }, []structs.BoundRoute{ - &structs.TCPRouteConfigEntry{ - Kind: structs.TCPRoute, - Name: "route", - Services: []structs.TCPService{{ - Name: "service", - }}, - }, - }, []structs.InlineCertificateConfigEntry{{ - Kind: structs.InlineCertificate, - Name: "certificate", - PrivateKey: gatewayTestPrivateKey, - Certificate: gatewayTestCertificate, - }}, nil) + }, + []structs.BoundRoute{ + &structs.TCPRouteConfigEntry{ + Kind: structs.TCPRoute, + Name: "route", + Services: []structs.TCPService{{ + Name: "service", + }}, + Parents: []structs.ResourceReference{ + { + Kind: structs.APIGateway, + Name: "api-gateway", + }, + }, + }, + }, []structs.InlineCertificateConfigEntry{{ + Kind: structs.InlineCertificate, + Name: "certificate", + PrivateKey: gatewayTestPrivateKey, + Certificate: gatewayTestCertificate, + }}, nil) }, }, { @@ -410,6 +417,12 @@ func getAPIGatewayGoldenTestCases(t *testing.T) []goldenTestCase { Name: "service", }}, }}, + Parents: []structs.ResourceReference{ + { + Kind: structs.APIGateway, + Name: "api-gateway", + }, + }, }, }, nil, []proxycfg.UpdateEvent{{ CorrelationID: "discovery-chain:" + serviceUID.String(), From ed82d8c120b99b68f5f4af341d3917ad152f5774 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Wed, 17 May 2023 12:13:30 -0500 Subject: [PATCH 03/11] fix reversion in resources_test --- agent/xds/resources_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/xds/resources_test.go b/agent/xds/resources_test.go index 3a068d850bd..9e878e02c91 100644 --- a/agent/xds/resources_test.go +++ b/agent/xds/resources_test.go @@ -172,8 +172,8 @@ func TestAllResourcesFromSnapshot(t *testing.T) { create: proxycfg.TestConfigSnapshotPeeringLocalMeshGateway, }, { - name: "hcp-metrics", - create: proxycfg.TestConfigSnapshotHCPMetrics, + name: "telemetry-collector", + create: proxycfg.TestConfigSnapshotTelemetryCollector, }, } tests = append(tests, getConnectProxyTransparentProxyGoldenTestCases()...) From 8f5ea9e69176d036686a1197442e3c009bf1a1fc Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Wed, 17 May 2023 16:19:55 -0500 Subject: [PATCH 04/11] Update agent/proxycfg/api_gateway.go Co-authored-by: John Maguire --- agent/proxycfg/api_gateway.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/agent/proxycfg/api_gateway.go b/agent/proxycfg/api_gateway.go index 19daf3d7400..2367da9d546 100644 --- a/agent/proxycfg/api_gateway.go +++ b/agent/proxycfg/api_gateway.go @@ -426,12 +426,8 @@ func (h *handlerAPIGateway) recompileDiscoveryChains(snap *ConfigSnapshot) error for name, listener := range snap.APIGateway.Listeners { boundListener, ok := snap.APIGateway.BoundListeners[name] - if !ok { + if !(ok && snap.APIGateway.GatewayConfig.ListenerIsReady(name)){ // Skip any listeners that don't have a bound listener. Once the bound listener is created, this will be run again. - continue - } - - if !snap.APIGateway.GatewayConfig.ListenerIsReady(name) { // skip any listeners that might be in an invalid state continue } From 8728cac94592116cd0e4a7c647c30380db1e0952 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Wed, 17 May 2023 16:47:56 -0500 Subject: [PATCH 05/11] gofmt --- agent/proxycfg/api_gateway.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/proxycfg/api_gateway.go b/agent/proxycfg/api_gateway.go index 2367da9d546..2f94874a7e6 100644 --- a/agent/proxycfg/api_gateway.go +++ b/agent/proxycfg/api_gateway.go @@ -426,7 +426,7 @@ func (h *handlerAPIGateway) recompileDiscoveryChains(snap *ConfigSnapshot) error for name, listener := range snap.APIGateway.Listeners { boundListener, ok := snap.APIGateway.BoundListeners[name] - if !(ok && snap.APIGateway.GatewayConfig.ListenerIsReady(name)){ + if !(ok && snap.APIGateway.GatewayConfig.ListenerIsReady(name)) { // Skip any listeners that don't have a bound listener. Once the bound listener is created, this will be run again. // skip any listeners that might be in an invalid state continue From a13a37dbb37f62cd4cf9e82a80ccec3f2c0ed1de Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 18 May 2023 14:56:14 -0400 Subject: [PATCH 06/11] Modify getReadyUpstreams to filter upstreams by listener (#17410) Each listener would previously have all upstreams from any route that bound to the listener. This is problematic when a route bound to one listener also binds to other listeners and so includes upstreams for multiple listeners. The list for a given listener would then wind up including upstreams for other listeners. --- agent/proxycfg/api_gateway.go | 4 ++-- agent/proxycfg/snapshot.go | 4 ++++ agent/xds/endpoints.go | 42 +++++++++++++++++++---------------- 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/agent/proxycfg/api_gateway.go b/agent/proxycfg/api_gateway.go index 2f94874a7e6..56ee4821068 100644 --- a/agent/proxycfg/api_gateway.go +++ b/agent/proxycfg/api_gateway.go @@ -317,7 +317,7 @@ func (h *handlerAPIGateway) handleRouteConfigUpdate(ctx context.Context, u Updat }, } - listenerKey := APIGatewayListenerKey{Protocol: string(listener.Protocol), Port: listener.Port} + listenerKey := APIGatewayListenerKeyFromListener(listener) upstreams[listenerKey] = append(upstreams[listenerKey], upstream) } @@ -371,7 +371,7 @@ func (h *handlerAPIGateway) handleRouteConfigUpdate(ctx context.Context, u Updat }, } - listenerKey := APIGatewayListenerKey{Protocol: string(listener.Protocol), Port: listener.Port} + listenerKey := APIGatewayListenerKeyFromListener(listener) upstreams[listenerKey] = append(upstreams[listenerKey], upstream) } diff --git a/agent/proxycfg/snapshot.go b/agent/proxycfg/snapshot.go index 6c1e116cf68..91d41b52223 100644 --- a/agent/proxycfg/snapshot.go +++ b/agent/proxycfg/snapshot.go @@ -984,6 +984,10 @@ func (c *configSnapshotIngressGateway) isEmpty() bool { type APIGatewayListenerKey = IngressListenerKey +func APIGatewayListenerKeyFromListener(l structs.APIGatewayListener) APIGatewayListenerKey { + return APIGatewayListenerKey{Protocol: string(l.Protocol), Port: l.Port} +} + type IngressListenerKey struct { Protocol string Port int diff --git a/agent/xds/endpoints.go b/agent/xds/endpoints.go index 6f10c382b14..e77703d5125 100644 --- a/agent/xds/endpoints.go +++ b/agent/xds/endpoints.go @@ -530,36 +530,40 @@ type readyUpstreams struct { upstreams []structs.Upstream } +// getReadyUpstreams returns a map containing the list of upstreams for each listener that is ready func getReadyUpstreams(cfgSnap *proxycfg.ConfigSnapshot) map[string]readyUpstreams { ready := map[string]readyUpstreams{} for _, l := range cfgSnap.APIGateway.Listeners { - //need to account for the state of the Listener when building the upstreams list + // Only include upstreams for listeners that are ready if !cfgSnap.APIGateway.GatewayConfig.ListenerIsReady(l.Name) { continue } + + // For each route bound to the listener boundListener := cfgSnap.APIGateway.BoundListeners[l.Name] - //get route ref for _, routeRef := range boundListener.Routes { - //get upstreams - upstreamMap := cfgSnap.APIGateway.Upstreams[routeRef] - for _, upstreams := range upstreamMap { - for _, u := range upstreams { - r, ok := ready[l.Name] - if !ok { - r = readyUpstreams{ - listenerKey: proxycfg.APIGatewayListenerKey{ - Protocol: string(l.Protocol), - Port: l.Port, - }, - listenerCfg: l, - boundListenerCfg: boundListener, - routeReference: routeRef, - } + // Get all upstreams for the route + routeUpstreams := cfgSnap.APIGateway.Upstreams[routeRef] + + // Filter to upstreams that attach to this specific listener since + // a route can bind to + have upstreams for multiple listeners + listenerKey := proxycfg.APIGatewayListenerKeyFromListener(l) + routeUpstreamsForListener := routeUpstreams[listenerKey] + + for _, upstream := range routeUpstreamsForListener { + // Insert or update readyUpstreams for the listener to include this upstream + r, ok := ready[l.Name] + if !ok { + r = readyUpstreams{ + listenerKey: listenerKey, + listenerCfg: l, + boundListenerCfg: boundListener, + routeReference: routeRef, } - r.upstreams = append(r.upstreams, u) - ready[l.Name] = r } + r.upstreams = append(r.upstreams, upstream) + ready[l.Name] = r } } } From 35cb61b9c3a17426d4d36b4f392bdbfa9f8d1008 Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Thu, 18 May 2023 14:47:04 -0500 Subject: [PATCH 07/11] Update agent/proxycfg/api_gateway.go Co-authored-by: Nathan Coleman --- agent/proxycfg/api_gateway.go | 1 - 1 file changed, 1 deletion(-) diff --git a/agent/proxycfg/api_gateway.go b/agent/proxycfg/api_gateway.go index 56ee4821068..973d3c36f4f 100644 --- a/agent/proxycfg/api_gateway.go +++ b/agent/proxycfg/api_gateway.go @@ -309,7 +309,6 @@ func (h *handlerAPIGateway) handleRouteConfigUpdate(ctx context.Context, u Updat DestinationNamespace: service.NamespaceOrDefault(), DestinationPartition: service.PartitionOrDefault(), LocalBindPort: listener.Port, - //IngressHosts: g.Hosts, // Pass the protocol that was configured on the listener in order // to force that protocol on the Envoy listener. Config: map[string]interface{}{ From f1c14b291662e69b42d75a8e47b9bc876ad42e80 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 18 May 2023 15:53:53 -0400 Subject: [PATCH 08/11] Restore import blocking --- agent/proxycfg/api_gateway.go | 1 + 1 file changed, 1 insertion(+) diff --git a/agent/proxycfg/api_gateway.go b/agent/proxycfg/api_gateway.go index 973d3c36f4f..1589448599a 100644 --- a/agent/proxycfg/api_gateway.go +++ b/agent/proxycfg/api_gateway.go @@ -6,6 +6,7 @@ package proxycfg import ( "context" "fmt" + "github.com/hashicorp/consul/acl" cachetype "github.com/hashicorp/consul/agent/cache-types" "github.com/hashicorp/consul/agent/proxycfg/internal/watch" From e3e8bb83e93febcd5ba6b0f49fdf0864806f53c6 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Fri, 19 May 2023 14:11:13 -0400 Subject: [PATCH 09/11] Skip to next route if route has no upstreams --- agent/xds/endpoints.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/agent/xds/endpoints.go b/agent/xds/endpoints.go index e77703d5125..3be0935d99c 100644 --- a/agent/xds/endpoints.go +++ b/agent/xds/endpoints.go @@ -544,7 +544,10 @@ func getReadyUpstreams(cfgSnap *proxycfg.ConfigSnapshot) map[string]readyUpstrea boundListener := cfgSnap.APIGateway.BoundListeners[l.Name] for _, routeRef := range boundListener.Routes { // Get all upstreams for the route - routeUpstreams := cfgSnap.APIGateway.Upstreams[routeRef] + routeUpstreams, ok := cfgSnap.APIGateway.Upstreams[routeRef] + if !ok { + continue + } // Filter to upstreams that attach to this specific listener since // a route can bind to + have upstreams for multiple listeners From 5f6d24852cf085edc7375588cdfd85d69a34b18b Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Fri, 19 May 2023 13:14:18 -0500 Subject: [PATCH 10/11] cleanup --- agent/proxycfg/api_gateway.go | 3 +++ agent/xds/endpoints.go | 10 ++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/agent/proxycfg/api_gateway.go b/agent/proxycfg/api_gateway.go index 1589448599a..a97312a7a8c 100644 --- a/agent/proxycfg/api_gateway.go +++ b/agent/proxycfg/api_gateway.go @@ -445,6 +445,9 @@ func (h *handlerAPIGateway) recompileDiscoveryChains(snap *ConfigSnapshot) error for i, service := range services { id := NewUpstreamIDFromServiceName(structs.NewServiceName(service.Name, &service.EnterpriseMeta)) + if compiled[i].ServiceName != service.Name { + return fmt.Errorf("Compiled Discovery chain for %s does not match service %s", compiled[i].ServiceName, id) + } synthesizedChains[id] = compiled[i] } } diff --git a/agent/xds/endpoints.go b/agent/xds/endpoints.go index e77703d5125..fd8ff56c2c2 100644 --- a/agent/xds/endpoints.go +++ b/agent/xds/endpoints.go @@ -544,12 +544,18 @@ func getReadyUpstreams(cfgSnap *proxycfg.ConfigSnapshot) map[string]readyUpstrea boundListener := cfgSnap.APIGateway.BoundListeners[l.Name] for _, routeRef := range boundListener.Routes { // Get all upstreams for the route - routeUpstreams := cfgSnap.APIGateway.Upstreams[routeRef] + routeUpstreams, ok := cfgSnap.APIGateway.Upstreams[routeRef] + if !ok { + continue + } // Filter to upstreams that attach to this specific listener since // a route can bind to + have upstreams for multiple listeners listenerKey := proxycfg.APIGatewayListenerKeyFromListener(l) - routeUpstreamsForListener := routeUpstreams[listenerKey] + routeUpstreamsForListener, ok := routeUpstreams[listenerKey] + if !ok { + continue + } for _, upstream := range routeUpstreamsForListener { // Insert or update readyUpstreams for the listener to include this upstream From 550c33ab5ddad134efc9dfdf9722d4229acd9583 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Fri, 19 May 2023 13:21:45 -0500 Subject: [PATCH 11/11] change set from bool to empty struct --- agent/xds/endpoints.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/agent/xds/endpoints.go b/agent/xds/endpoints.go index fd8ff56c2c2..bd88aebcc75 100644 --- a/agent/xds/endpoints.go +++ b/agent/xds/endpoints.go @@ -578,7 +578,7 @@ func getReadyUpstreams(cfgSnap *proxycfg.ConfigSnapshot) map[string]readyUpstrea func (s *ResourceGenerator) endpointsFromSnapshotAPIGateway(cfgSnap *proxycfg.ConfigSnapshot) ([]proto.Message, error) { var resources []proto.Message - createdClusters := make(map[proxycfg.UpstreamID]bool) + createdClusters := make(map[proxycfg.UpstreamID]struct{}) readyUpstreamsList := getReadyUpstreams(cfgSnap) @@ -588,11 +588,12 @@ func (s *ResourceGenerator) endpointsFromSnapshotAPIGateway(cfgSnap *proxycfg.Co // If we've already created endpoints for this upstream, skip it. Multiple listeners may // reference the same upstream, so we don't need to create duplicate endpoints in that case. - if createdClusters[uid] { + _, ok := createdClusters[uid] + if ok { continue } - es, err := s.endpointsFromDiscoveryChain( + endpoints, err := s.endpointsFromDiscoveryChain( uid, cfgSnap.APIGateway.DiscoveryChain[uid], cfgSnap, @@ -605,8 +606,8 @@ func (s *ResourceGenerator) endpointsFromSnapshotAPIGateway(cfgSnap *proxycfg.Co if err != nil { return nil, err } - resources = append(resources, es...) - createdClusters[uid] = true + resources = append(resources, endpoints...) + createdClusters[uid] = struct{}{} } } return resources, nil