diff --git a/.changelog/19120.txt b/.changelog/19120.txt new file mode 100644 index 000000000..e7e5d3274 --- /dev/null +++ b/.changelog/19120.txt @@ -0,0 +1,3 @@ +```release-note:bug +api-gateway: fix matching for different hostnames on the same listener +``` diff --git a/agent/consul/discoverychain/gateway.go b/agent/consul/discoverychain/gateway.go index e43e4b631..d4447db73 100644 --- a/agent/consul/discoverychain/gateway.go +++ b/agent/consul/discoverychain/gateway.go @@ -196,9 +196,14 @@ func (l *GatewayChainSynthesizer) consolidateHTTPRoutes() []structs.HTTPRouteCon return consolidateHTTPRoutes(l.matchesByHostname, l.suffix, l.gateway) } -// 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{}) +// ConsolidateHTTPRoutes takes in one or more HTTPRoutes and consolidates them down to the minimum +// set of HTTPRoutes that can represent the same set of rules. This should result in approx. one +// HTTPRoute per hostname. +func ConsolidateHTTPRoutes(gateway *structs.APIGatewayConfigEntry, listener *structs.APIGatewayListener, routes ...*structs.HTTPRouteConfigEntry) []structs.HTTPRouteConfigEntry { + matches := map[string][]hostnameMatch{} + for _, route := range routes { + matches = initHostMatches(listener.GetHostname(), route, matches) + } return consolidateHTTPRoutes(matches, listener.Name, gateway) } diff --git a/agent/xds/listeners_apigateway.go b/agent/xds/listeners_apigateway.go index 633c04f05..1f15145ce 100644 --- a/agent/xds/listeners_apigateway.go +++ b/agent/xds/listeners_apigateway.go @@ -5,6 +5,7 @@ package xds import ( "fmt" + envoy_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" envoy_listener_v3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" envoy_tls_v3 "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3" @@ -202,7 +203,7 @@ type readyListener struct { listenerKey proxycfg.APIGatewayListenerKey listenerCfg structs.APIGatewayListener boundListenerCfg structs.BoundAPIGatewayListener - routeReference structs.ResourceReference + routeReferences map[structs.ResourceReference]struct{} upstreams []structs.Upstream } @@ -240,10 +241,11 @@ func getReadyListeners(cfgSnap *proxycfg.ConfigSnapshot) map[string]readyListene r = readyListener{ listenerKey: listenerKey, listenerCfg: l, + routeReferences: map[structs.ResourceReference]struct{}{}, boundListenerCfg: boundListener, - routeReference: routeRef, } } + r.routeReferences[routeRef] = struct{}{} r.upstreams = append(r.upstreams, upstream) ready[l.Name] = r } diff --git a/agent/xds/routes.go b/agent/xds/routes.go index a86747a9c..4484f6a6a 100644 --- a/agent/xds/routes.go +++ b/agent/xds/routes.go @@ -14,7 +14,8 @@ import ( envoy_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" envoy_route_v3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" envoy_matcher_v3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" - + "golang.org/x/exp/maps" + "golang.org/x/exp/slices" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/durationpb" @@ -429,40 +430,45 @@ func (s *ResourceGenerator) routesForIngressGateway(cfgSnap *proxycfg.ConfigSnap func (s *ResourceGenerator) routesForAPIGateway(cfgSnap *proxycfg.ConfigSnapshot) ([]proto.Message, error) { var result []proto.Message - readyUpstreamsList := getReadyListeners(cfgSnap) + // Build up the routes in a deterministic way + readyListeners := getReadyListeners(cfgSnap) + listenerNames := maps.Keys(readyListeners) + sort.Strings(listenerNames) - for _, readyUpstreams := range readyUpstreamsList { - listenerCfg := readyUpstreams.listenerCfg - // Do not create any route configuration for TCP listeners - if listenerCfg.Protocol != structs.ListenerProtocolHTTP { + for _, listenerName := range listenerNames { + readyListener, ok := readyListeners[listenerName] + if !ok { continue } - routeRef := readyUpstreams.routeReference - listenerKey := readyUpstreams.listenerKey + // Do not create any route configuration for TCP listeners + if readyListener.listenerCfg.Protocol != structs.ListenerProtocolHTTP { + continue + } - defaultRoute := &envoy_route_v3.RouteConfiguration{ - Name: listenerKey.RouteName(), + listenerRoute := &envoy_route_v3.RouteConfiguration{ + Name: readyListener.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) + // Consolidate all routes for this listener into the minimum possible set based on hostname matching. + allRoutesForListener := []*structs.HTTPRouteConfigEntry{} + for _, routeRef := range maps.Keys(readyListener.routeReferences) { + route, ok := cfgSnap.APIGateway.HTTPRoutes.Get(routeRef) + if !ok { + return nil, fmt.Errorf("missing route for route routeRef %s:%s", routeRef.Name, routeRef.Kind) + } + allRoutesForListener = append(allRoutesForListener, route) } + consolidatedRoutes := discoverychain.ConsolidateHTTPRoutes(cfgSnap.APIGateway.GatewayConfig, &readyListener.listenerCfg, allRoutesForListener...) - // 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. - reformatedRoutes := discoverychain.ReformatHTTPRoute(route, &listenerCfg, cfgSnap.APIGateway.GatewayConfig) - - for _, reformatedRoute := range reformatedRoutes { - reformatedRoute := reformatedRoute - - upstream := buildHTTPRouteUpstream(reformatedRoute, listenerCfg) + // Produce one virtual host per hostname. If no hostname is specified for a set of + // Gateway + HTTPRoutes, then the virtual host will be "*". + for _, consolidatedRoute := range consolidatedRoutes { + upstream := buildHTTPRouteUpstream(consolidatedRoute, readyListener.listenerCfg) uid := proxycfg.NewUpstreamID(&upstream) chain := cfgSnap.APIGateway.DiscoveryChain[uid] if chain == nil { @@ -470,18 +476,19 @@ func (s *ResourceGenerator) routesForAPIGateway(cfgSnap *proxycfg.ConfigSnapshot continue } - domains := generateUpstreamAPIsDomains(listenerKey, upstream, reformatedRoute.Hostnames) - + domains := generateUpstreamAPIsDomains(readyListener.listenerKey, upstream, consolidatedRoute.Hostnames) virtualHost, err := s.makeUpstreamRouteForDiscoveryChain(cfgSnap, uid, chain, domains, false) if err != nil { return nil, err } - defaultRoute.VirtualHosts = append(defaultRoute.VirtualHosts, virtualHost) + listenerRoute.VirtualHosts = append(listenerRoute.VirtualHosts, virtualHost) } - if len(defaultRoute.VirtualHosts) > 0 { - result = append(result, defaultRoute) + if len(listenerRoute.VirtualHosts) > 0 { + // Build up the virtual hosts in a deterministic way + slices.SortStableFunc(listenerRoute.VirtualHosts, func(a, b *envoy_route_v3.VirtualHost) bool { return a.Name < b.Name }) + result = append(result, listenerRoute) } } diff --git a/agent/xds/routes_test.go b/agent/xds/routes_test.go index eaa8d48c0..72082ba1c 100644 --- a/agent/xds/routes_test.go +++ b/agent/xds/routes_test.go @@ -10,7 +10,6 @@ import ( "time" envoy_route_v3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" - "github.com/hashicorp/consul/agent/xds/testcommon" testinf "github.com/mitchellh/go-testing-interface" "github.com/stretchr/testify/require" @@ -18,6 +17,7 @@ import ( "github.com/hashicorp/consul/agent/proxycfg" "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/agent/xds/testcommon" "github.com/hashicorp/consul/envoyextensions/xdscommon" "github.com/hashicorp/consul/sdk/testutil" ) @@ -196,6 +196,65 @@ func TestRoutesFromSnapshot(t *testing.T) { name: "terminating-gateway-lb-config", create: proxycfg.TestConfigSnapshotTerminatingGatewayLBConfig, }, + { + name: "api-gateway-with-multiple-hostnames", + create: func(t testinf.T) *proxycfg.ConfigSnapshot { + return proxycfg.TestConfigSnapshotAPIGateway(t, "default", nil, func(entry *structs.APIGatewayConfigEntry, bound *structs.BoundAPIGatewayConfigEntry) { + entry.Listeners = []structs.APIGatewayListener{ + { + Name: "http", + Protocol: structs.ListenerProtocolHTTP, + Port: 8080, + Hostname: "*.example.com", + }, + } + bound.Listeners = []structs.BoundAPIGatewayListener{ + { + Name: "http", + Routes: []structs.ResourceReference{ + {Kind: structs.HTTPRoute, Name: "backend-route"}, + {Kind: structs.HTTPRoute, Name: "frontend-route"}, + {Kind: structs.HTTPRoute, Name: "generic-route"}, + }}, + } + }, + []structs.BoundRoute{ + &structs.HTTPRouteConfigEntry{ + Kind: structs.HTTPRoute, + Name: "backend-route", + Hostnames: []string{"backend.example.com"}, + Parents: []structs.ResourceReference{{Kind: structs.APIGateway, Name: "api-gateway"}}, + Rules: []structs.HTTPRouteRule{ + {Services: []structs.HTTPService{{Name: "backend"}}}, + }, + }, + &structs.HTTPRouteConfigEntry{ + Kind: structs.HTTPRoute, + Name: "frontend-route", + Hostnames: []string{"frontend.example.com"}, + Parents: []structs.ResourceReference{{Kind: structs.APIGateway, Name: "api-gateway"}}, + Rules: []structs.HTTPRouteRule{ + {Services: []structs.HTTPService{{Name: "frontend"}}}, + }, + }, + &structs.HTTPRouteConfigEntry{ + Kind: structs.HTTPRoute, + Name: "generic-route", + Parents: []structs.ResourceReference{{Kind: structs.APIGateway, Name: "api-gateway"}}, + Rules: []structs.HTTPRouteRule{ + { + Matches: []structs.HTTPMatch{{Path: structs.HTTPPathMatch{Match: structs.HTTPPathMatchPrefix, Value: "/frontend"}}}, + Services: []structs.HTTPService{{Name: "frontend"}}, + }, + { + Matches: []structs.HTTPMatch{{Path: structs.HTTPPathMatch{Match: structs.HTTPPathMatchPrefix, Value: "/backend"}}}, + Services: []structs.HTTPService{{Name: "backend"}}, + }, + }, + }, + }, nil, nil) + }, + }, } tests = append(tests, makeRouteDiscoChainTests(false)...) diff --git a/agent/xds/testdata/routes/api-gateway-with-multiple-hostnames.latest.golden b/agent/xds/testdata/routes/api-gateway-with-multiple-hostnames.latest.golden new file mode 100644 index 000000000..c4959e566 --- /dev/null +++ b/agent/xds/testdata/routes/api-gateway-with-multiple-hostnames.latest.golden @@ -0,0 +1,73 @@ +{ + "versionInfo": "00000001", + "resources": [ + { + "@type": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration", + "name": "8080", + "virtualHosts": [ + { + "name": "api-gateway-http-54620b06", + "domains": [ + "frontend.example.com", + "frontend.example.com:8080" + ], + "routes": [ + { + "match": { + "prefix": "/" + }, + "route": { + "cluster": "frontend.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" + } + } + ] + }, + { + "name": "api-gateway-http-5a84e719", + "domains": [ + "backend.example.com", + "backend.example.com:8080" + ], + "routes": [ + { + "match": { + "prefix": "/" + }, + "route": { + "cluster": "backend.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" + } + } + ] + }, + { + "name": "api-gateway-http-aa289ce2", + "domains": [ + "*.example.com", + "*.example.com:8080" + ], + "routes": [ + { + "match": { + "prefix": "/frontend" + }, + "route": { + "cluster": "frontend.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" + } + }, + { + "match": { + "prefix": "/backend" + }, + "route": { + "cluster": "backend.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" + } + } + ] + } + ], + "validateClusters": true + } + ], + "typeUrl": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration", + "nonce": "00000001" +} \ No newline at end of file