From ce47cb355502370c7bceae2a78688987375ffa89 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Wed, 17 May 2023 12:28:34 -0500 Subject: [PATCH 01/14] xds generation for routes api gateway --- agent/consul/discoverychain/gateway.go | 56 +++++++++++++--- agent/consul/server_test.go | 2 +- agent/xds/routes.go | 93 ++++++++++++++++++++++++-- 3 files changed, 133 insertions(+), 18 deletions(-) diff --git a/agent/consul/discoverychain/gateway.go b/agent/consul/discoverychain/gateway.go index c1c6a2b0841..f81715372c4 100644 --- a/agent/consul/discoverychain/gateway.go +++ b/agent/consul/discoverychain/gateway.go @@ -59,9 +59,14 @@ func (l *GatewayChainSynthesizer) SetHostname(hostname string) { // single hostname can be specified in multiple routes. Routing for a given // hostname must behave based on the aggregate of all rules that apply to it. func (l *GatewayChainSynthesizer) AddHTTPRoute(route structs.HTTPRouteConfigEntry) { - hostnames := route.FilteredHostnames(l.hostname) + //TODO maps are pointers in golang, might not need to set it like this, test later + l.matchesByHostname = getHostMatches(l.hostname, &route, l.matchesByHostname) +} + +func getHostMatches(hostname string, route *structs.HTTPRouteConfigEntry, currentMatches map[string][]hostnameMatch) map[string][]hostnameMatch { + hostnames := route.FilteredHostnames(hostname) for _, host := range hostnames { - matches, ok := l.matchesByHostname[host] + matches, ok := currentMatches[host] if !ok { matches = []hostnameMatch{} } @@ -90,8 +95,10 @@ func (l *GatewayChainSynthesizer) AddHTTPRoute(route structs.HTTPRouteConfigEntr } } - l.matchesByHostname[host] = matches + currentMatches[host] = matches } + //TODO def don't think this is needed just testing for now, remove if not needed + return currentMatches } // Synthesize assembles a synthetic discovery chain from multiple other discovery chains @@ -116,6 +123,7 @@ func (l *GatewayChainSynthesizer) Synthesize(chains ...*structs.CompiledDiscover compiledChains := make([]*structs.CompiledDiscoveryChain, 0, len(set)) for i, service := range services { + entries := set[i] compiled, err := Compile(CompileRequest{ @@ -126,7 +134,6 @@ func (l *GatewayChainSynthesizer) Synthesize(chains ...*structs.CompiledDiscover EvaluateInTrustDomain: l.trustDomain, Entries: entries, }) - if err != nil { return nil, nil, err } @@ -188,17 +195,44 @@ func (l *GatewayChainSynthesizer) Synthesize(chains ...*structs.CompiledDiscover // consolidateHTTPRoutes combines all rules into the shortest possible list of routes // with one route per hostname containing all rules for that hostname. func (l *GatewayChainSynthesizer) consolidateHTTPRoutes() []structs.HTTPRouteConfigEntry { + return consolidateHTTPRoutes(l.matchesByHostname, l.suffix, l.gateway) +} + +// FlattenHTTPRoute takes in a route and its parent config entries and returns a list of flattened routes +func FlattenHTTPRoute(route *structs.HTTPRouteConfigEntry, listener *structs.APIGatewayListener, gateway *structs.APIGatewayConfigEntry) []structs.HTTPRouteConfigEntry { + //build map[string][]hostnameMatch for route + matches := map[string][]hostnameMatch{} + matches = getHostMatches(listener.GetHostname(), route, matches) + return consolidateHTTPRoutes(matches, listener.Name, gateway) +} + +func RebuildHTTPRouteUpstream(route structs.HTTPRouteConfigEntry, listener structs.APIGatewayListener) structs.Upstream { + return structs.Upstream{ + DestinationName: route.GetName(), + DestinationNamespace: route.NamespaceOrDefault(), + DestinationPartition: route.PartitionOrDefault(), + IngressHosts: route.Hostnames, + LocalBindPort: listener.Port, + Config: map[string]interface{}{ + "protocol": string(listener.Protocol), + }, + } +} + +// ConsolidateHTTPRoutes combines all rules into the shortest possible list of routes +// with one route per hostname containing all rules for that hostname. +func consolidateHTTPRoutes(matchesByHostname map[string][]hostnameMatch, suffix string, gateway *structs.APIGatewayConfigEntry) []structs.HTTPRouteConfigEntry { var routes []structs.HTTPRouteConfigEntry - for hostname, rules := range l.matchesByHostname { + for hostname, rules := range matchesByHostname { // Create route for this hostname route := structs.HTTPRouteConfigEntry{ Kind: structs.HTTPRoute, - Name: fmt.Sprintf("%s-%s-%s", l.gateway.Name, l.suffix, hostsKey(hostname)), + Name: fmt.Sprintf("%s-%s-%s", gateway.Name, suffix, hostsKey(hostname)), Hostnames: []string{hostname}, Rules: make([]structs.HTTPRouteRule, 0, len(rules)), - Meta: l.gateway.Meta, - EnterpriseMeta: l.gateway.EnterpriseMeta, + Meta: gateway.Meta, + EnterpriseMeta: gateway.EnterpriseMeta, } // Sort rules for this hostname in order of precedence @@ -258,12 +292,14 @@ func (l *GatewayChainSynthesizer) synthesizeEntries() ([]structs.IngressService, entries := []*configentry.DiscoveryChainSet{} for _, route := range l.consolidateHTTPRoutes() { - entrySet := configentry.NewDiscoveryChainSet() ingress, router, splitters, defaults := synthesizeHTTPRouteDiscoveryChain(route) + + services = append(services, ingress) + + entrySet := configentry.NewDiscoveryChainSet() entrySet.AddRouters(router) entrySet.AddSplitters(splitters...) entrySet.AddServices(defaults...) - services = append(services, ingress) entries = append(entries, entrySet) } diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index c246428b2f2..bd39e9676ea 100644 --- a/agent/consul/server_test.go +++ b/agent/consul/server_test.go @@ -1906,7 +1906,7 @@ func TestServer_ReloadConfig(t *testing.T) { defaults := DefaultConfig() got := s.raft.ReloadableConfig() require.Equal(t, uint64(4321), got.SnapshotThreshold, - "should have be reloaded to new value") + "should have been reloaded to new value") require.Equal(t, defaults.RaftConfig.SnapshotInterval, got.SnapshotInterval, "should have remained the default interval") require.Equal(t, defaults.RaftConfig.TrailingLogs, got.TrailingLogs, diff --git a/agent/xds/routes.go b/agent/xds/routes.go index 30907002e80..9f4ef03712d 100644 --- a/agent/xds/routes.go +++ b/agent/xds/routes.go @@ -6,6 +6,7 @@ package xds import ( "errors" "fmt" + "github.com/hashicorp/consul/agent/consul/discoverychain" "net" "sort" "strings" @@ -36,13 +37,7 @@ func (s *ResourceGenerator) routesFromSnapshot(cfgSnap *proxycfg.ConfigSnapshot) case structs.ServiceKindIngressGateway: return s.routesForIngressGateway(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.routesForIngressGateway(cfgSnap) + return s.routesForAPIGateway(cfgSnap) case structs.ServiceKindTerminatingGateway: return s.routesForTerminatingGateway(cfgSnap) case structs.ServiceKindMeshGateway: @@ -430,6 +425,75 @@ func (s *ResourceGenerator) routesForIngressGateway(cfgSnap *proxycfg.ConfigSnap return result, nil } +// routesForAPIGateway returns the xDS API representation of the +// "routes" in the snapshot. +func (s *ResourceGenerator) routesForAPIGateway(cfgSnap *proxycfg.ConfigSnapshot) ([]proto.Message, error) { + var result []proto.Message + + readyUpstreamsList := getReadyUpstreams(cfgSnap) + + for _, readyUpstreams := range readyUpstreamsList { + listenerCfg := readyUpstreams.listenerCfg + // Do not create any route configuration for TCP listeners + if listenerCfg.Protocol == "tcp" { + continue + } + + routeRef := readyUpstreams.routeReference + listenerKey := readyUpstreams.listenerKey + + // Depending on their TLS config, upstreams are either attached to the + // default route or have their own routes. We'll add any upstreams that + // don't have custom filter chains and routes to this. + defaultRoute := &envoy_route_v3.RouteConfiguration{ + Name: listenerKey.RouteName(), + // ValidateClusters defaults to true when defined statically and false + // when done via RDS. Re-set the reasonable value of true to prevent + // null-routing traffic. + ValidateClusters: makeBoolValue(true), + } + + route, ok := cfgSnap.APIGateway.HTTPRoutes.Get(routeRef) + if !ok { + return nil, fmt.Errorf("missing route for route reference %s:%s", routeRef.Name, routeRef.Kind) + } + + flattenedRoutes := discoverychain.FlattenHTTPRoute(route, &listenerCfg, cfgSnap.APIGateway.GatewayConfig) + + for _, flattenedRoute := range flattenedRoutes { + flattenedRoute := flattenedRoute + + upstream := discoverychain.RebuildHTTPRouteUpstream(flattenedRoute, listenerCfg) + uid := proxycfg.NewUpstreamID(&upstream) + chain := cfgSnap.APIGateway.DiscoveryChain[uid] + if chain == nil { + s.Logger.Debug("Discovery chain not found for flattened route", "discovery chain ID", uid) + continue + } + + domains := generateUpstreamAPIsDomains(listenerKey, upstream, flattenedRoute.Hostnames) + + virtualHost, err := s.makeUpstreamRouteForDiscoveryChain(cfgSnap, uid, chain, domains, false) + if err != nil { + return nil, err + } + + injectHeaderManipToVirtualHostAPIGateway(&flattenedRoute, virtualHost) + + // TODO Handle TLS config and add new route if appropriate + // We need something analogous to routeNameForUpstream used below + // But currently ToIngress is not handeling this usecase + defaultRoute.VirtualHosts = append(defaultRoute.VirtualHosts, virtualHost) + } + + if len(defaultRoute.VirtualHosts) > 0 { + result = append(result, defaultRoute) + } + } + + return result, nil +} + func makeHeadersValueOptions(vals map[string]string, add bool) []*envoy_core_v3.HeaderValueOption { opts := make([]*envoy_core_v3.HeaderValueOption, 0, len(vals)) for k, v := range vals { @@ -516,6 +580,11 @@ func generateUpstreamIngressDomains(listenerKey proxycfg.IngressListenerKey, u s return domains } +func generateUpstreamAPIsDomains(listenerKey proxycfg.APIGatewayListenerKey, u structs.Upstream, hosts []string) []string { + u.IngressHosts = hosts + return generateUpstreamIngressDomains(listenerKey, u) +} + func (s *ResourceGenerator) makeUpstreamRouteForDiscoveryChain( cfgSnap *proxycfg.ConfigSnapshot, uid proxycfg.UpstreamID, @@ -1019,6 +1088,16 @@ func injectHeaderManipToRoute(dest *structs.ServiceRouteDestination, r *envoy_ro return nil } +func injectHeaderManipToVirtualHostAPIGateway(dest *structs.HTTPRouteConfigEntry, vh *envoy_route_v3.VirtualHost) { + for _, rule := range dest.Rules { + for _, header := range rule.Filters.Headers { + vh.RequestHeadersToAdd = append(vh.RequestHeadersToAdd, makeHeadersValueOptions(header.Add, true)...) + vh.RequestHeadersToAdd = append(vh.RequestHeadersToAdd, makeHeadersValueOptions(header.Set, false)...) + vh.RequestHeadersToRemove = append(vh.RequestHeadersToRemove, header.Remove...) + } + } +} + func injectHeaderManipToVirtualHost(dest *structs.IngressService, vh *envoy_route_v3.VirtualHost) error { if !dest.RequestHeaders.IsZero() { vh.RequestHeadersToAdd = append( From 7c1c1ffeae1a11b4ebbee7cd060febcef64741bd Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Wed, 17 May 2023 12:32:42 -0500 Subject: [PATCH 02/14] Update gateway.go --- agent/consul/discoverychain/gateway.go | 1 - 1 file changed, 1 deletion(-) diff --git a/agent/consul/discoverychain/gateway.go b/agent/consul/discoverychain/gateway.go index f81715372c4..328431e8e8f 100644 --- a/agent/consul/discoverychain/gateway.go +++ b/agent/consul/discoverychain/gateway.go @@ -59,7 +59,6 @@ func (l *GatewayChainSynthesizer) SetHostname(hostname string) { // single hostname can be specified in multiple routes. Routing for a given // hostname must behave based on the aggregate of all rules that apply to it. func (l *GatewayChainSynthesizer) AddHTTPRoute(route structs.HTTPRouteConfigEntry) { - //TODO maps are pointers in golang, might not need to set it like this, test later l.matchesByHostname = getHostMatches(l.hostname, &route, l.matchesByHostname) } From 27799539113814aaf669f7b739ff11e61c9e92a0 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Fri, 19 May 2023 14:01:04 -0500 Subject: [PATCH 03/14] move buildHttpRoute into xds package --- agent/xds/routes.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/agent/xds/routes.go b/agent/xds/routes.go index 9f4ef03712d..e484da822d5 100644 --- a/agent/xds/routes.go +++ b/agent/xds/routes.go @@ -463,7 +463,7 @@ func (s *ResourceGenerator) routesForAPIGateway(cfgSnap *proxycfg.ConfigSnapshot for _, flattenedRoute := range flattenedRoutes { flattenedRoute := flattenedRoute - upstream := discoverychain.RebuildHTTPRouteUpstream(flattenedRoute, listenerCfg) + upstream := buildHTTPRouteUpstream(flattenedRoute, listenerCfg) uid := proxycfg.NewUpstreamID(&upstream) chain := cfgSnap.APIGateway.DiscoveryChain[uid] if chain == nil { @@ -494,6 +494,19 @@ func (s *ResourceGenerator) routesForAPIGateway(cfgSnap *proxycfg.ConfigSnapshot return result, nil } +func buildHTTPRouteUpstream(route structs.HTTPRouteConfigEntry, listener structs.APIGatewayListener) structs.Upstream { + return structs.Upstream{ + DestinationName: route.GetName(), + DestinationNamespace: route.NamespaceOrDefault(), + DestinationPartition: route.PartitionOrDefault(), + IngressHosts: route.Hostnames, + LocalBindPort: listener.Port, + Config: map[string]interface{}{ + "protocol": string(listener.Protocol), + }, + } +} + func makeHeadersValueOptions(vals map[string]string, add bool) []*envoy_core_v3.HeaderValueOption { opts := make([]*envoy_core_v3.HeaderValueOption, 0, len(vals)) for k, v := range vals { From 9ca8c7e4c2e822bb1f75150c8d99357021ef2bf6 Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Fri, 19 May 2023 14:49:30 -0500 Subject: [PATCH 04/14] Update agent/consul/discoverychain/gateway.go --- agent/consul/discoverychain/gateway.go | 1 - 1 file changed, 1 deletion(-) diff --git a/agent/consul/discoverychain/gateway.go b/agent/consul/discoverychain/gateway.go index 328431e8e8f..22d9cd50b32 100644 --- a/agent/consul/discoverychain/gateway.go +++ b/agent/consul/discoverychain/gateway.go @@ -96,7 +96,6 @@ func getHostMatches(hostname string, route *structs.HTTPRouteConfigEntry, curren currentMatches[host] = matches } - //TODO def don't think this is needed just testing for now, remove if not needed return currentMatches } From 2dd8700f724f0cb10f02b8c7e21e5199254251cd Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Mon, 22 May 2023 09:54:49 -0500 Subject: [PATCH 05/14] remove unneeded function --- agent/consul/discoverychain/gateway.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/agent/consul/discoverychain/gateway.go b/agent/consul/discoverychain/gateway.go index 22d9cd50b32..3914f0e11ea 100644 --- a/agent/consul/discoverychain/gateway.go +++ b/agent/consul/discoverychain/gateway.go @@ -204,19 +204,6 @@ func FlattenHTTPRoute(route *structs.HTTPRouteConfigEntry, listener *structs.API return consolidateHTTPRoutes(matches, listener.Name, gateway) } -func RebuildHTTPRouteUpstream(route structs.HTTPRouteConfigEntry, listener structs.APIGatewayListener) structs.Upstream { - return structs.Upstream{ - DestinationName: route.GetName(), - DestinationNamespace: route.NamespaceOrDefault(), - DestinationPartition: route.PartitionOrDefault(), - IngressHosts: route.Hostnames, - LocalBindPort: listener.Port, - Config: map[string]interface{}{ - "protocol": string(listener.Protocol), - }, - } -} - // ConsolidateHTTPRoutes combines all rules into the shortest possible list of routes // with one route per hostname containing all rules for that hostname. func consolidateHTTPRoutes(matchesByHostname map[string][]hostnameMatch, suffix string, gateway *structs.APIGatewayConfigEntry) []structs.HTTPRouteConfigEntry { From d782497eb172bd4aca651c0b3acdb5c56de460cc Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Mon, 22 May 2023 09:56:52 -0500 Subject: [PATCH 06/14] convert http route code to only run for http protocol to future proof code path --- agent/xds/routes.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/xds/routes.go b/agent/xds/routes.go index e484da822d5..b69592c68a0 100644 --- a/agent/xds/routes.go +++ b/agent/xds/routes.go @@ -355,7 +355,7 @@ func (s *ResourceGenerator) routesForIngressGateway(cfgSnap *proxycfg.ConfigSnap var result []proto.Message for listenerKey, upstreams := range cfgSnap.IngressGateway.Upstreams { // Do not create any route configuration for TCP listeners - if listenerKey.Protocol == "tcp" { + if listenerKey.Protocol != "http" { continue } From d9bd8111cc6a356aa099bebc66dc05e835f7220e Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Mon, 22 May 2023 11:15:45 -0500 Subject: [PATCH 07/14] Update agent/consul/discoverychain/gateway.go Co-authored-by: Mike Morris --- agent/consul/discoverychain/gateway.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/consul/discoverychain/gateway.go b/agent/consul/discoverychain/gateway.go index 3914f0e11ea..72ee44a2d84 100644 --- a/agent/consul/discoverychain/gateway.go +++ b/agent/consul/discoverychain/gateway.go @@ -206,7 +206,7 @@ func FlattenHTTPRoute(route *structs.HTTPRouteConfigEntry, listener *structs.API // ConsolidateHTTPRoutes combines all rules into the shortest possible list of routes // with one route per hostname containing all rules for that hostname. -func consolidateHTTPRoutes(matchesByHostname map[string][]hostnameMatch, suffix string, gateway *structs.APIGatewayConfigEntry) []structs.HTTPRouteConfigEntry { +func consolidateHTTPRoutes(matchesByHostname map[string][]hostnameMatch, listenerName string, gateway *structs.APIGatewayConfigEntry) []structs.HTTPRouteConfigEntry { var routes []structs.HTTPRouteConfigEntry for hostname, rules := range matchesByHostname { From ecbe243ad2c15594b42a50635f0c5ce06d82712b Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Mon, 22 May 2023 11:31:02 -0500 Subject: [PATCH 08/14] fix tests, clean up http check logic --- agent/consul/discoverychain/gateway.go | 2 +- agent/xds/routes.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/agent/consul/discoverychain/gateway.go b/agent/consul/discoverychain/gateway.go index 72ee44a2d84..a10f68f76ef 100644 --- a/agent/consul/discoverychain/gateway.go +++ b/agent/consul/discoverychain/gateway.go @@ -213,7 +213,7 @@ func consolidateHTTPRoutes(matchesByHostname map[string][]hostnameMatch, listene // Create route for this hostname route := structs.HTTPRouteConfigEntry{ Kind: structs.HTTPRoute, - Name: fmt.Sprintf("%s-%s-%s", gateway.Name, suffix, hostsKey(hostname)), + Name: fmt.Sprintf("%s-%s-%s", gateway.Name, listenerName, hostsKey(hostname)), Hostnames: []string{hostname}, Rules: make([]structs.HTTPRouteRule, 0, len(rules)), Meta: gateway.Meta, diff --git a/agent/xds/routes.go b/agent/xds/routes.go index b69592c68a0..c5c09d1bdcb 100644 --- a/agent/xds/routes.go +++ b/agent/xds/routes.go @@ -355,7 +355,7 @@ func (s *ResourceGenerator) routesForIngressGateway(cfgSnap *proxycfg.ConfigSnap var result []proto.Message for listenerKey, upstreams := range cfgSnap.IngressGateway.Upstreams { // Do not create any route configuration for TCP listeners - if listenerKey.Protocol != "http" { + if listenerKey.Protocol == "tcp" { continue } @@ -435,7 +435,7 @@ func (s *ResourceGenerator) routesForAPIGateway(cfgSnap *proxycfg.ConfigSnapshot for _, readyUpstreams := range readyUpstreamsList { listenerCfg := readyUpstreams.listenerCfg // Do not create any route configuration for TCP listeners - if listenerCfg.Protocol == "tcp" { + if listenerCfg.Protocol != "http" && listenerCfg.Protocol != "HTTP" { continue } From e13d041d933ff0e729d3a4e2a7e2937d137e456c Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Mon, 22 May 2023 15:55:57 -0500 Subject: [PATCH 09/14] clean up todo --- agent/xds/routes.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/agent/xds/routes.go b/agent/xds/routes.go index c5c09d1bdcb..f7a506ba0fa 100644 --- a/agent/xds/routes.go +++ b/agent/xds/routes.go @@ -480,9 +480,6 @@ func (s *ResourceGenerator) routesForAPIGateway(cfgSnap *proxycfg.ConfigSnapshot injectHeaderManipToVirtualHostAPIGateway(&flattenedRoute, virtualHost) - // TODO Handle TLS config and add new route if appropriate - // We need something analogous to routeNameForUpstream used below - // But currently ToIngress is not handeling this usecase defaultRoute.VirtualHosts = append(defaultRoute.VirtualHosts, virtualHost) } From 2f07a6d5a39081eeb8b7ec159383621f3fc9b770 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Mon, 22 May 2023 17:29:00 -0400 Subject: [PATCH 10/14] Fix casing in docstring --- agent/consul/discoverychain/gateway.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/consul/discoverychain/gateway.go b/agent/consul/discoverychain/gateway.go index a10f68f76ef..bcc70454ef4 100644 --- a/agent/consul/discoverychain/gateway.go +++ b/agent/consul/discoverychain/gateway.go @@ -204,7 +204,7 @@ func FlattenHTTPRoute(route *structs.HTTPRouteConfigEntry, listener *structs.API return consolidateHTTPRoutes(matches, listener.Name, gateway) } -// ConsolidateHTTPRoutes combines all rules into the shortest possible list of routes +// consolidateHTTPRoutes combines all rules into the shortest possible list of routes // with one route per hostname containing all rules for that hostname. func consolidateHTTPRoutes(matchesByHostname map[string][]hostnameMatch, listenerName string, gateway *structs.APIGatewayConfigEntry) []structs.HTTPRouteConfigEntry { var routes []structs.HTTPRouteConfigEntry From 072aa4574720037e91662e6a7129bd6c82b4f321 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Mon, 22 May 2023 17:56:44 -0400 Subject: [PATCH 11/14] Fix import block, adjust docstrings --- agent/xds/routes.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/agent/xds/routes.go b/agent/xds/routes.go index f7a506ba0fa..8011d3a9d91 100644 --- a/agent/xds/routes.go +++ b/agent/xds/routes.go @@ -6,7 +6,6 @@ package xds import ( "errors" "fmt" - "github.com/hashicorp/consul/agent/consul/discoverychain" "net" "sort" "strings" @@ -20,6 +19,7 @@ import ( "google.golang.org/protobuf/types/known/durationpb" "github.com/hashicorp/consul/agent/connect" + "github.com/hashicorp/consul/agent/consul/discoverychain" "github.com/hashicorp/consul/agent/proxycfg" "github.com/hashicorp/consul/agent/structs" ) @@ -435,16 +435,13 @@ func (s *ResourceGenerator) routesForAPIGateway(cfgSnap *proxycfg.ConfigSnapshot for _, readyUpstreams := range readyUpstreamsList { listenerCfg := readyUpstreams.listenerCfg // Do not create any route configuration for TCP listeners - if listenerCfg.Protocol != "http" && listenerCfg.Protocol != "HTTP" { + if listenerCfg.Protocol != "http" { continue } routeRef := readyUpstreams.routeReference listenerKey := readyUpstreams.listenerKey - // Depending on their TLS config, upstreams are either attached to the - // default route or have their own routes. We'll add any upstreams that - // don't have custom filter chains and routes to this. defaultRoute := &envoy_route_v3.RouteConfiguration{ Name: listenerKey.RouteName(), // ValidateClusters defaults to true when defined statically and false @@ -458,6 +455,9 @@ func (s *ResourceGenerator) routesForAPIGateway(cfgSnap *proxycfg.ConfigSnapshot return nil, fmt.Errorf("missing route for route reference %s:%s", routeRef.Name, routeRef.Kind) } + // Flatten the routes here since discovery chains were indexed earlier using the + // specific naming convention in discoverychain.consolidateHTTPRoutes. If we don't + // convert our route to use the same naming convention, we won't find any chains below. flattenedRoutes := discoverychain.FlattenHTTPRoute(route, &listenerCfg, cfgSnap.APIGateway.GatewayConfig) for _, flattenedRoute := range flattenedRoutes { From 3534acc815848ec40c7ce346d4fbf0d3e3072628 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Wed, 24 May 2023 12:46:18 -0500 Subject: [PATCH 12/14] update name and comment --- agent/consul/discoverychain/gateway.go | 12 +++++------- agent/xds/routes.go | 14 +++++++------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/agent/consul/discoverychain/gateway.go b/agent/consul/discoverychain/gateway.go index bcc70454ef4..8c13bb59a88 100644 --- a/agent/consul/discoverychain/gateway.go +++ b/agent/consul/discoverychain/gateway.go @@ -59,10 +59,10 @@ func (l *GatewayChainSynthesizer) SetHostname(hostname string) { // single hostname can be specified in multiple routes. Routing for a given // hostname must behave based on the aggregate of all rules that apply to it. func (l *GatewayChainSynthesizer) AddHTTPRoute(route structs.HTTPRouteConfigEntry) { - l.matchesByHostname = getHostMatches(l.hostname, &route, l.matchesByHostname) + l.matchesByHostname = initHostMatches(l.hostname, &route, l.matchesByHostname) } -func getHostMatches(hostname string, route *structs.HTTPRouteConfigEntry, currentMatches map[string][]hostnameMatch) map[string][]hostnameMatch { +func initHostMatches(hostname string, route *structs.HTTPRouteConfigEntry, currentMatches map[string][]hostnameMatch) map[string][]hostnameMatch { hostnames := route.FilteredHostnames(hostname) for _, host := range hostnames { matches, ok := currentMatches[host] @@ -196,11 +196,9 @@ func (l *GatewayChainSynthesizer) consolidateHTTPRoutes() []structs.HTTPRouteCon return consolidateHTTPRoutes(l.matchesByHostname, l.suffix, l.gateway) } -// FlattenHTTPRoute takes in a route and its parent config entries and returns a list of flattened routes -func FlattenHTTPRoute(route *structs.HTTPRouteConfigEntry, listener *structs.APIGatewayListener, gateway *structs.APIGatewayConfigEntry) []structs.HTTPRouteConfigEntry { - //build map[string][]hostnameMatch for route - matches := map[string][]hostnameMatch{} - matches = getHostMatches(listener.GetHostname(), route, matches) +// ReformatHTTPRoute takes in an HTTPRoute and reformats it to match the discovery chains generated by the gateway chain synthesizer +func ReformatHTTPRoute(route *structs.HTTPRouteConfigEntry, listener *structs.APIGatewayListener, gateway *structs.APIGatewayConfigEntry) []structs.HTTPRouteConfigEntry { + matches := initHostMatches(listener.GetHostname(), route, map[string][]hostnameMatch{}) return consolidateHTTPRoutes(matches, listener.Name, gateway) } diff --git a/agent/xds/routes.go b/agent/xds/routes.go index 8011d3a9d91..65b59750c26 100644 --- a/agent/xds/routes.go +++ b/agent/xds/routes.go @@ -455,15 +455,15 @@ func (s *ResourceGenerator) routesForAPIGateway(cfgSnap *proxycfg.ConfigSnapshot return nil, fmt.Errorf("missing route for route reference %s:%s", routeRef.Name, routeRef.Kind) } - // Flatten the routes here since discovery chains were indexed earlier using the + // Reformat the route here since discovery chains were indexed earlier using the // specific naming convention in discoverychain.consolidateHTTPRoutes. If we don't // convert our route to use the same naming convention, we won't find any chains below. - flattenedRoutes := discoverychain.FlattenHTTPRoute(route, &listenerCfg, cfgSnap.APIGateway.GatewayConfig) + reformatedRoutes := discoverychain.ReformatHTTPRoute(route, &listenerCfg, cfgSnap.APIGateway.GatewayConfig) - for _, flattenedRoute := range flattenedRoutes { - flattenedRoute := flattenedRoute + for _, reformatedRoute := range reformatedRoutes { + reformatedRoute := reformatedRoute - upstream := buildHTTPRouteUpstream(flattenedRoute, listenerCfg) + upstream := buildHTTPRouteUpstream(reformatedRoute, listenerCfg) uid := proxycfg.NewUpstreamID(&upstream) chain := cfgSnap.APIGateway.DiscoveryChain[uid] if chain == nil { @@ -471,14 +471,14 @@ func (s *ResourceGenerator) routesForAPIGateway(cfgSnap *proxycfg.ConfigSnapshot continue } - domains := generateUpstreamAPIsDomains(listenerKey, upstream, flattenedRoute.Hostnames) + domains := generateUpstreamAPIsDomains(listenerKey, upstream, reformatedRoute.Hostnames) virtualHost, err := s.makeUpstreamRouteForDiscoveryChain(cfgSnap, uid, chain, domains, false) if err != nil { return nil, err } - injectHeaderManipToVirtualHostAPIGateway(&flattenedRoute, virtualHost) + injectHeaderManipToVirtualHostAPIGateway(&reformatedRoute, virtualHost) defaultRoute.VirtualHosts = append(defaultRoute.VirtualHosts, virtualHost) } From 221816caeda2e4bdc90e8fd057b09c3fccff9c08 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Wed, 24 May 2023 13:51:35 -0500 Subject: [PATCH 13/14] use constant value --- agent/xds/routes.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/xds/routes.go b/agent/xds/routes.go index 65b59750c26..e01def1f90a 100644 --- a/agent/xds/routes.go +++ b/agent/xds/routes.go @@ -355,7 +355,7 @@ func (s *ResourceGenerator) routesForIngressGateway(cfgSnap *proxycfg.ConfigSnap var result []proto.Message for listenerKey, upstreams := range cfgSnap.IngressGateway.Upstreams { // Do not create any route configuration for TCP listeners - if listenerKey.Protocol == "tcp" { + if listenerKey.Protocol != structs.ListenerProtocolHTTP { continue } @@ -435,7 +435,7 @@ func (s *ResourceGenerator) routesForAPIGateway(cfgSnap *proxycfg.ConfigSnapshot for _, readyUpstreams := range readyUpstreamsList { listenerCfg := readyUpstreams.listenerCfg // Do not create any route configuration for TCP listeners - if listenerCfg.Protocol != "http" { + if listenerCfg.Protocol != structs.ListenerProtocolHTTP { continue } From 71b808be4687cb0dbacd1556bc340f7c8b2bf85e Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Wed, 24 May 2023 13:59:25 -0500 Subject: [PATCH 14/14] use constant --- agent/xds/listeners_apigateway.go | 2 +- agent/xds/routes.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/xds/listeners_apigateway.go b/agent/xds/listeners_apigateway.go index ed6cc6a9e0c..d731f121ddd 100644 --- a/agent/xds/listeners_apigateway.go +++ b/agent/xds/listeners_apigateway.go @@ -43,7 +43,7 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro return nil, err } - if listenerKey.Protocol == "tcp" { + if listenerCfg.Protocol == structs.ListenerProtocolTCP { // Find the upstream matching this listener // We rely on the invariant of upstreams slice always having at least 1 diff --git a/agent/xds/routes.go b/agent/xds/routes.go index e01def1f90a..0eab72b259f 100644 --- a/agent/xds/routes.go +++ b/agent/xds/routes.go @@ -355,7 +355,7 @@ func (s *ResourceGenerator) routesForIngressGateway(cfgSnap *proxycfg.ConfigSnap var result []proto.Message for listenerKey, upstreams := range cfgSnap.IngressGateway.Upstreams { // Do not create any route configuration for TCP listeners - if listenerKey.Protocol != structs.ListenerProtocolHTTP { + if listenerKey.Protocol == "tcp" { continue }