From af41a67559e9730213f24bc74bf6cf4b8be4a52e Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Wed, 26 Apr 2023 10:17:16 -0500 Subject: [PATCH 01/34] API Gateway XDS Primitives, endpoints and clusters (#17002) * XDS primitive generation for endpoints and clusters Co-authored-by: Nathan Coleman * server_test * deleted extra file * add missing parents to test --------- Co-authored-by: Nathan Coleman --- agent/consul/server_test.go | 2 +- agent/proxycfg/api_gateway.go | 38 +++++++++++++++++++- agent/proxycfg/snapshot.go | 1 + agent/xds/clusters.go | 54 ++++++++++++++++++++++++---- agent/xds/endpoints.go | 68 +++++++++++++++++++++++++++++++---- agent/xds/resources_test.go | 41 +++++++++++++-------- 6 files changed, 174 insertions(+), 30 deletions(-) diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index c246428b2f29..bd39e9676ea3 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/proxycfg/api_gateway.go b/agent/proxycfg/api_gateway.go index 18abed7b25dc..4557e5322ec3 100644 --- a/agent/proxycfg/api_gateway.go +++ b/agent/proxycfg/api_gateway.go @@ -128,7 +128,7 @@ func (h *handlerAPIGateway) handleUpdate(ctx context.Context, u UpdateEvent, sna return (*handlerUpstreams)(h).handleUpdateUpstreams(ctx, u, snap) } - return nil + return h.recompileDiscoveryChains(snap) } // handleRootCAUpdate responds to changes in the watched root CA for a gateway @@ -420,6 +420,42 @@ 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] + } + } + + snap.APIGateway.DiscoveryChain = synthesizedChains + 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/proxycfg/snapshot.go b/agent/proxycfg/snapshot.go index 6c1e116cf688..266ab90e9f46 100644 --- a/agent/proxycfg/snapshot.go +++ b/agent/proxycfg/snapshot.go @@ -913,6 +913,7 @@ DOMAIN_LOOP: return services, upstreams, compiled, err } +// TODO use this in listener code func (c *configSnapshotAPIGateway) toIngressTLS(key IngressListenerKey, listener structs.APIGatewayListener, bound structs.BoundAPIGatewayListener) (*structs.GatewayTLSConfig, error) { if len(listener.TLS.Certificates) == 0 { return nil, nil diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index d29d00a25646..84884bf4a78a 100644 --- a/agent/xds/clusters.go +++ b/agent/xds/clusters.go @@ -65,13 +65,7 @@ func (s *ResourceGenerator) clustersFromSnapshot(cfgSnap *proxycfg.ConfigSnapsho } return res, nil 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 - } - res, err := s.clustersFromSnapshotIngressGateway(cfgSnap) + res, err := s.clustersFromSnapshotAPIGateway(cfgSnap) if err != nil { return nil, err } @@ -816,6 +810,52 @@ func (s *ResourceGenerator) clustersFromSnapshotIngressGateway(cfgSnap *proxycfg return clusters, nil } +func (s *ResourceGenerator) clustersFromSnapshotAPIGateway(cfgSnap *proxycfg.ConfigSnapshot) ([]proto.Message, error) { + var clusters []proto.Message + createdClusters := make(map[proxycfg.UpstreamID]bool) + readyUpstreams := getReadyUpstreams(cfgSnap) + + for listenerKey, upstreams := range readyUpstreams { + for _, upstream := range upstreams { + uid := proxycfg.NewUpstreamID(&upstream) + + // If we've already created a cluster for this upstream, skip it. Multiple listeners may + // reference the same upstream, so we don't need to create duplicate clusters in that case. + if createdClusters[uid] { + continue + } + + // Grab the discovery chain compiled in handlerAPIGateway.recompileDiscoveryChains + chain, ok := cfgSnap.APIGateway.DiscoveryChain[uid] + if !ok { + // this should not happen + return nil, fmt.Errorf("no discovery chain for upstream %q", uid) + } + + // Generate the list of upstream clusters for the discovery chain + upstreamClusters, err := s.makeUpstreamClustersForDiscoveryChain(uid, &upstream, chain, cfgSnap, false) + if err != nil { + return nil, err + } + + for _, cluster := range upstreamClusters { + // TODO Something analogous to s.configIngressUpstreamCluster(c, cfgSnap, listenerKey, &u) + // but not sure what that func does yet + s.configAPIUpstreamCluster(cluster, cfgSnap, listenerKey, &upstream) + clusters = append(clusters, cluster) + } + createdClusters[uid] = true + + } + } + return clusters, nil +} + +func (s *ResourceGenerator) configAPIUpstreamCluster(c *envoy_cluster_v3.Cluster, cfgSnap *proxycfg.ConfigSnapshot, listenerKey proxycfg.APIGatewayListenerKey, u *structs.Upstream) { + //TODO I don't think this is currently needed with what api gateway supports, but will be needed in the future + +} + func (s *ResourceGenerator) configIngressUpstreamCluster(c *envoy_cluster_v3.Cluster, cfgSnap *proxycfg.ConfigSnapshot, listenerKey proxycfg.IngressListenerKey, u *structs.Upstream) { var threshold *envoy_cluster_v3.CircuitBreakers_Thresholds setThresholdLimit := func(limitType string, limit int) { diff --git a/agent/xds/endpoints.go b/agent/xds/endpoints.go index 8c5b487ea97f..92291d08106d 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,65 @@ func (s *ResourceGenerator) endpointsFromSnapshotIngressGateway(cfgSnap *proxycf return resources, nil } +func getReadyUpstreams(cfgSnap *proxycfg.ConfigSnapshot) map[proxycfg.APIGatewayListenerKey][]structs.Upstream { + + readyUpstreams := map[proxycfg.APIGatewayListenerKey][]structs.Upstream{} + 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 listenerKey, upstreams := range upstreamMap { + for _, u := range upstreams { + readyUpstreams[listenerKey] = append(readyUpstreams[listenerKey], u) + } + } + } + } + return readyUpstreams +} + +func (s *ResourceGenerator) endpointsFromSnapshotAPIGateway(cfgSnap *proxycfg.ConfigSnapshot) ([]proto.Message, error) { + var resources []proto.Message + createdClusters := make(map[proxycfg.UpstreamID]bool) + + readyUpstreams := getReadyUpstreams(cfgSnap) + + for _, upstreams := range readyUpstreams { + for _, u := range 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 +681,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/agent/xds/resources_test.go b/agent/xds/resources_test.go index 5028cc67e47a..3a068d850bd1 100644 --- a/agent/xds/resources_test.go +++ b/agent/xds/resources_test.go @@ -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 bdae6b239d64c71b217df2c6721b55e2e077bbf3 Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Tue, 9 May 2023 14:51:21 -0500 Subject: [PATCH 02/34] Routes for API Gateway (#17158) * XDS primitive generation for endpoints and clusters Co-authored-by: Nathan Coleman * server_test * deleted extra file * add missing parents to test * checkpoint * delete extra file * httproute flattening code * linting issue * so close on this, calling for tonight * unit test passing * add in header manip to virtual host * upstream rebuild commented out * Use consistent upstream name whether or not we're rebuilding * Start working through route naming logic * Fix typos in test descriptions * Simplify route naming logic * Simplify RebuildHTTPRouteUpstream * Merge additional compiled discovery chains instead of overwriting * Use correct chain for flattened route, clean up + add TODOs * Remove empty conditional branch * Restore previous variable declaration Limit the scope of this PR * Clean up, improve TODO * add logging, clean up todos * clean up function --------- Co-authored-by: Nathan Coleman --- agent/consul/discoverychain/gateway.go | 56 +++++++++-- agent/proxycfg/api_gateway.go | 13 ++- agent/proxycfg/snapshot.go | 1 + agent/xds/clusters.go | 16 +--- agent/xds/endpoints.go | 38 ++++++-- agent/xds/routes.go | 93 +++++++++++++++++-- .../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 +- 11 files changed, 185 insertions(+), 50 deletions(-) diff --git a/agent/consul/discoverychain/gateway.go b/agent/consul/discoverychain/gateway.go index c1c6a2b0841d..f81715372c4c 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/proxycfg/api_gateway.go b/agent/proxycfg/api_gateway.go index 4557e5322ec3..19daf3d74006 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,7 +124,9 @@ 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 h.recompileDiscoveryChains(snap) @@ -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{}{ @@ -452,7 +453,11 @@ func (h *handlerAPIGateway) recompileDiscoveryChains(snap *ConfigSnapshot) error } } - snap.APIGateway.DiscoveryChain = synthesizedChains + // Merge in additional discovery chains + for id, chain := range synthesizedChains { + snap.APIGateway.DiscoveryChain[id] = chain + } + return nil } diff --git a/agent/proxycfg/snapshot.go b/agent/proxycfg/snapshot.go index 266ab90e9f46..b2195ddefd1a 100644 --- a/agent/proxycfg/snapshot.go +++ b/agent/proxycfg/snapshot.go @@ -876,6 +876,7 @@ DOMAIN_LOOP: } synthesizer.AddTCPRoute(*route) for _, service := range route.GetServices() { + id := NewUpstreamIDFromServiceName(structs.NewServiceName(service.Name, &service.EnterpriseMeta)) if chain := c.DiscoveryChain[id]; chain != nil { chains = append(chains, chain) diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index 84884bf4a78a..259013400b9a 100644 --- a/agent/xds/clusters.go +++ b/agent/xds/clusters.go @@ -813,10 +813,10 @@ func (s *ResourceGenerator) clustersFromSnapshotIngressGateway(cfgSnap *proxycfg func (s *ResourceGenerator) clustersFromSnapshotAPIGateway(cfgSnap *proxycfg.ConfigSnapshot) ([]proto.Message, error) { var clusters []proto.Message createdClusters := make(map[proxycfg.UpstreamID]bool) - readyUpstreams := getReadyUpstreams(cfgSnap) + readyUpstreamsList := getReadyUpstreams(cfgSnap) - for listenerKey, upstreams := range readyUpstreams { - for _, upstream := range upstreams { + for _, readyUpstreams := range readyUpstreamsList { + for _, upstream := range readyUpstreams.upstreams { uid := proxycfg.NewUpstreamID(&upstream) // If we've already created a cluster for this upstream, skip it. Multiple listeners may @@ -839,23 +839,15 @@ func (s *ResourceGenerator) clustersFromSnapshotAPIGateway(cfgSnap *proxycfg.Con } for _, cluster := range upstreamClusters { - // TODO Something analogous to s.configIngressUpstreamCluster(c, cfgSnap, listenerKey, &u) - // but not sure what that func does yet - s.configAPIUpstreamCluster(cluster, cfgSnap, listenerKey, &upstream) clusters = append(clusters, cluster) } - createdClusters[uid] = true + createdClusters[uid] = true } } return clusters, nil } -func (s *ResourceGenerator) configAPIUpstreamCluster(c *envoy_cluster_v3.Cluster, cfgSnap *proxycfg.ConfigSnapshot, listenerKey proxycfg.APIGatewayListenerKey, u *structs.Upstream) { - //TODO I don't think this is currently needed with what api gateway supports, but will be needed in the future - -} - func (s *ResourceGenerator) configIngressUpstreamCluster(c *envoy_cluster_v3.Cluster, cfgSnap *proxycfg.ConfigSnapshot, listenerKey proxycfg.IngressListenerKey, u *structs.Upstream) { var threshold *envoy_cluster_v3.CircuitBreakers_Thresholds setThresholdLimit := func(limitType string, limit int) { diff --git a/agent/xds/endpoints.go b/agent/xds/endpoints.go index 92291d08106d..6f10c382b149 100644 --- a/agent/xds/endpoints.go +++ b/agent/xds/endpoints.go @@ -521,9 +521,18 @@ func (s *ResourceGenerator) endpointsFromSnapshotIngressGateway(cfgSnap *proxycf return resources, nil } -func getReadyUpstreams(cfgSnap *proxycfg.ConfigSnapshot) map[proxycfg.APIGatewayListenerKey][]structs.Upstream { +// 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 { - readyUpstreams := map[proxycfg.APIGatewayListenerKey][]structs.Upstream{} + 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) { @@ -534,24 +543,37 @@ func getReadyUpstreams(cfgSnap *proxycfg.ConfigSnapshot) map[proxycfg.APIGateway for _, routeRef := range boundListener.Routes { //get upstreams upstreamMap := cfgSnap.APIGateway.Upstreams[routeRef] - for listenerKey, upstreams := range upstreamMap { + for _, upstreams := range upstreamMap { for _, u := range upstreams { - readyUpstreams[listenerKey] = append(readyUpstreams[listenerKey], u) + 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 readyUpstreams + return ready } func (s *ResourceGenerator) endpointsFromSnapshotAPIGateway(cfgSnap *proxycfg.ConfigSnapshot) ([]proto.Message, error) { var resources []proto.Message createdClusters := make(map[proxycfg.UpstreamID]bool) - readyUpstreams := getReadyUpstreams(cfgSnap) + readyUpstreamsList := getReadyUpstreams(cfgSnap) - for _, upstreams := range readyUpstreams { - for _, u := range upstreams { + 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 diff --git a/agent/xds/routes.go b/agent/xds/routes.go index 30907002e80f..9f4ef03712d0 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( 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 ba109ea6f9dd..7aaee6da796f 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 72686b3c4f25..e62e979bf8b1 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 aeb0b7fd6cff..4d99c49e6951 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 e96f473be4f4..536c99d7c749 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 5e28909bfa5f..5bdddadd9d26 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 36c0d63c9b0c1affef95b0243c335936efa9db4a Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Mon, 15 May 2023 15:50:06 -0500 Subject: [PATCH 03/34] checkpoint, skeleton, tests not passing --- agent/structs/config_entry_gateways.go | 5 + agent/xds/listeners.go | 19 +- agent/xds/listeners_apigateway.go | 420 +++++++++++++++++++++++++ 3 files changed, 437 insertions(+), 7 deletions(-) create mode 100644 agent/xds/listeners_apigateway.go diff --git a/agent/structs/config_entry_gateways.go b/agent/structs/config_entry_gateways.go index cd66a0ae3991..05af66b21bf4 100644 --- a/agent/structs/config_entry_gateways.go +++ b/agent/structs/config_entry_gateways.go @@ -912,6 +912,11 @@ type APIGatewayTLSConfiguration struct { CipherSuites []types.TLSCipherSuite } +// IsEmpty returns if struct is empty because field isn't currently nullable +func (a *APIGatewayTLSConfiguration) IsEmpty() bool { + return len(a.Certificates) == 0 && len(a.MaxVersion) == 0 && len(a.MinVersion) == 0 && len(a.CipherSuites) == 0 +} + // BoundAPIGatewayConfigEntry manages the configuration for a bound API // gateway with the given name. This type is never written from the client. // It is only written by the controller in order to represent an API gateway diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index fe8d8e116977..1ba5a8a861ff 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -851,13 +851,18 @@ func (s *ResourceGenerator) listenersFromSnapshotGateway(cfgSnap *proxycfg.Confi return nil, err } 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 - } - listeners, err := s.makeIngressGatewayListeners(a.Address, cfgSnap) + //// 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 + //} + //listeners, err := s.makeIngressGatewayListeners(a.Address, cfgSnap) + //if err != nil { + // return nil, err + //} + + listeners, err := s.makeAPIGatewayListeners(a.Address, cfgSnap) if err != nil { return nil, err } diff --git a/agent/xds/listeners_apigateway.go b/agent/xds/listeners_apigateway.go new file mode 100644 index 000000000000..e9f7be20c08f --- /dev/null +++ b/agent/xds/listeners_apigateway.go @@ -0,0 +1,420 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +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" + + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/wrapperspb" + + "github.com/hashicorp/consul/agent/proxycfg" + "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/types" +) + +func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *proxycfg.ConfigSnapshot) ([]proto.Message, error) { + var resources []proto.Message + + readyUpstreamsList := getReadyUpstreams(cfgSnap) + + for _, readyUpstreams := range readyUpstreamsList { + listenerCfg := readyUpstreams.listenerCfg + listenerKey := readyUpstreams.listenerKey + + var isAPIGatewayWithTLS bool + var certs []structs.InlineCertificateConfigEntry + if cfgSnap.APIGateway.ListenerCertificates != nil { + certs = cfgSnap.APIGateway.ListenerCertificates[listenerKey] + } + if certs != nil { + isAPIGatewayWithTLS = true + } + + tlsContext, err := makeDownstreamTLSContextFromSnapshotAPIListenerConfig(cfgSnap, listenerCfg) + if err != nil { + return nil, err + } + + if listenerKey.Protocol == "tcp" { + //TODO not sure if we can rely on this the same way ingress can + u := readyUpstreams.upstreams[0] + uid := proxycfg.NewUpstreamID(&u) + + chain := cfgSnap.IngressGateway.DiscoveryChain[uid] + if chain == nil { + // Wait until a chain is present in the snapshot. + continue + } + cfg := s.getAndModifyUpstreamConfigForListener(uid, &u, chain) + useRDS := cfg.Protocol != "tcp" && !chain.Default + + var clusterName string + if !useRDS { + // When not using RDS we must generate a cluster name to attach to the filter chain. + // With RDS, cluster names get attached to the dynamic routes instead. + target, err := simpleChainTarget(chain) + if err != nil { + return nil, err + } + clusterName = CustomizeClusterName(target.Name, chain) + } + + filterName := fmt.Sprintf("%s.%s.%s.%s", chain.ServiceName, chain.Namespace, chain.Partition, chain.Datacenter) + + opts := makeListenerOpts{ + name: uid.EnvoyID(), + accessLogs: cfgSnap.Proxy.AccessLogs, + addr: address, + port: u.LocalBindPort, + direction: envoy_core_v3.TrafficDirection_OUTBOUND, + logger: s.Logger, + } + l := makeListener(opts) + + filterChain, err := s.makeUpstreamFilterChain(filterChainOpts{ + accessLogs: &cfgSnap.Proxy.AccessLogs, + routeName: uid.EnvoyID(), + useRDS: useRDS, + clusterName: clusterName, + filterName: filterName, + protocol: cfg.Protocol, + tlsContext: tlsContext, + }) + if err != nil { + return nil, err + } + l.FilterChains = []*envoy_listener_v3.FilterChain{ + filterChain, + } + + if isAPIGatewayWithTLS { + // construct SNI filter chains + l.FilterChains, err = makeInlineOverrideFilterChains(cfgSnap, cfgSnap.IngressGateway.TLSConfig, listenerKey, listenerFilterOpts{ + useRDS: useRDS, + protocol: listenerKey.Protocol, + routeName: listenerKey.RouteName(), + cluster: clusterName, + statPrefix: "ingress_upstream_", + accessLogs: &cfgSnap.Proxy.AccessLogs, + logger: s.Logger, + }, certs) + if err != nil { + return nil, err + } + // add the tls inspector to do SNI introspection + tlsInspector, err := makeTLSInspectorListenerFilter() + if err != nil { + return nil, err + } + l.ListenerFilters = []*envoy_listener_v3.ListenerFilter{tlsInspector} + } + resources = append(resources, l) + + } else { + // If multiple upstreams share this port, make a special listener for the protocol. + listenerOpts := makeListenerOpts{ + name: listenerKey.Protocol, + accessLogs: cfgSnap.Proxy.AccessLogs, + addr: address, + port: listenerKey.Port, + direction: envoy_core_v3.TrafficDirection_OUTBOUND, + logger: s.Logger, + } + listener := makeListener(listenerOpts) + filterOpts := listenerFilterOpts{ + useRDS: true, + protocol: listenerKey.Protocol, + filterName: listenerKey.RouteName(), + routeName: listenerKey.RouteName(), + cluster: "", + statPrefix: "ingress_upstream_", + routePath: "", + httpAuthzFilters: nil, + accessLogs: &cfgSnap.Proxy.AccessLogs, + logger: s.Logger, + } + + //TODO equivalent of makeSDSOverrideFilterChains + // Generate any filter chains needed for services with custom TLS certs + // via SDS. + sniFilterChains := []*envoy_listener_v3.FilterChain{} + + if isAPIGatewayWithTLS { + sniFilterChains, err = makeInlineOverrideFilterChains(cfgSnap, cfgSnap.IngressGateway.TLSConfig, listenerKey, filterOpts, certs) + if err != nil { + return nil, err + } + } + + // If there are any sni filter chains, we need a TLS inspector filter! + if len(sniFilterChains) > 0 { + tlsInspector, err := makeTLSInspectorListenerFilter() + if err != nil { + return nil, err + } + listener.ListenerFilters = []*envoy_listener_v3.ListenerFilter{tlsInspector} + } + + listener.FilterChains = sniFilterChains + + // See if there are other services that didn't have specific SNI-matching + // filter chains. If so add a default filterchain to serve them. + //TODO I'm not sure if we can delete this code block or not + if len(sniFilterChains) < len(readyUpstreams.upstreams) && !isAPIGatewayWithTLS { + defaultFilter, err := makeListenerFilter(filterOpts) + if err != nil { + return nil, err + } + + transportSocket, err := makeDownstreamTLSTransportSocket(tlsContext) + if err != nil { + return nil, err + } + listener.FilterChains = append(listener.FilterChains, + &envoy_listener_v3.FilterChain{ + Filters: []*envoy_listener_v3.Filter{ + defaultFilter, + }, + TransportSocket: transportSocket, + }) + } + resources = append(resources, listener) + + } + + } + + return resources, nil +} + +func makeDownstreamTLSContextFromSnapshotAPIListenerConfig(cfgSnap *proxycfg.ConfigSnapshot, listenerCfg structs.APIGatewayListener) (*envoy_tls_v3.DownstreamTlsContext, error) { + var downstreamContext *envoy_tls_v3.DownstreamTlsContext + + tlsContext, err := makeCommonTLSContextFromSnapshotAPIGatewayListenerConfig(cfgSnap, listenerCfg) + if err != nil { + return nil, err + } + + if tlsContext != nil { + // Configure alpn protocols on TLSContext + tlsContext.AlpnProtocols = getAlpnProtocols(string(listenerCfg.Protocol)) + + downstreamContext = &envoy_tls_v3.DownstreamTlsContext{ + CommonTlsContext: tlsContext, + RequireClientCertificate: &wrapperspb.BoolValue{Value: false}, + } + } + + return downstreamContext, nil +} + +func makeCommonTLSContextFromSnapshotAPIGatewayListenerConfig(cfgSnap *proxycfg.ConfigSnapshot, listenerCfg structs.APIGatewayListener) (*envoy_tls_v3.CommonTlsContext, error) { + var tlsContext *envoy_tls_v3.CommonTlsContext + + //TODO if needed, get TLS configuration from the gateway itself + + //TODO gateway config doesn't seem to support TLS config so we might only need to pull from listener at the moment + tlsCfg, err := resolveAPIListenerTLSConfig(listenerCfg.TLS) + if err != nil { + return nil, err + } + + //TODO do we have an equivalent of Enabled field for API Gateway + connectTLSEnabled := (!listenerCfg.TLS.IsEmpty()) + + if tlsCfg.SDS != nil { + //TODO I think this is unreachable with current setup + //but apparently it isn't so what do I know + // Set up listener TLS from SDS + tlsContext = makeCommonTLSContextFromGatewayTLSConfig(*tlsCfg) + } else if connectTLSEnabled { + tlsContext = makeCommonTLSContext(cfgSnap.Leaf(), cfgSnap.RootPEMs(), makeTLSParametersFromGatewayTLSConfig(*tlsCfg)) + } + + return tlsContext, nil +} + +// API-ified +// TODO if multiple tls configuragtions are available, pass them in here to be consolidated +func resolveAPIListenerTLSConfig(listenerTLSCfg structs.APIGatewayTLSConfiguration) (*structs.GatewayTLSConfig, error) { + var mergedCfg structs.GatewayTLSConfig + + //TODO, handle SDS configuration , some equivalent of resolveListenerSDSConfig + //tracing through out code I think this is always going to be empty on our end, but I'm not sure if it needs + //to be initialized + mergedCfg.SDS = &structs.GatewayTLSSDSConfig{} + + //TODO check gateway config if needed? We might only be supporting listener tls at this time + + if !listenerTLSCfg.IsEmpty() { + if listenerTLSCfg.MinVersion != types.TLSVersionUnspecified { + mergedCfg.TLSMinVersion = listenerTLSCfg.MinVersion + } + if listenerTLSCfg.MaxVersion != types.TLSVersionUnspecified { + mergedCfg.TLSMaxVersion = listenerTLSCfg.MaxVersion + } + if len(listenerTLSCfg.CipherSuites) != 0 { + mergedCfg.CipherSuites = listenerTLSCfg.CipherSuites + } + } + + if err := validateListenerTLSConfig(mergedCfg.TLSMinVersion, mergedCfg.CipherSuites); err != nil { + return nil, err + } + + return &mergedCfg, nil +} + +func routeNameForAPIGatewayUpstream(l structs.IngressListener, s structs.IngressService) string { + key := proxycfg.IngressListenerKeyFromListener(l) + + // If the upstream service doesn't have any TLS overrides then it can just use + // the combined filterchain with all the merged routes. + if !ingressServiceHasSDSOverrides(s) { + return key.RouteName() + } + + // Return a specific route for this service as it needs a custom FilterChain + // to serve its custom cert so we should attach its routes to a separate Route + // too. We need this to be consistent between OSS and Enterprise to avoid xDS + // config golden files in tests conflicting so we can't use ServiceID.String() + // which normalizes to included all identifiers in Enterprise. + sn := s.ToServiceName() + svcIdentifier := sn.Name + if !sn.InDefaultPartition() || !sn.InDefaultNamespace() { + // Non-default partition/namespace, use a full identifier + svcIdentifier = sn.String() + } + return fmt.Sprintf("%s_%s", key.RouteName(), svcIdentifier) +} + +//TODO Delete, this is a reference +//func (c *configSnapshotAPIGateway) toIngressTLS(key IngressListenerKey, listener structs.APIGatewayListener, bound structs.BoundAPIGatewayListener) (*structs.GatewayTLSConfig, error) { +// if len(listener.TLS.Certificates) == 0 { +// return nil, nil +// } +// +// for _, certRef := range bound.Certificates { +// cert, ok := c.Certificates.Get(certRef) +// if !ok { +// continue +// } +// c.ListenerCertificates[key] = append(c.ListenerCertificates[key], *cert) +// } +// +// return &structs.GatewayTLSConfig{ +// Enabled: true, +// TLSMinVersion: listener.TLS.MinVersion, +// TLSMaxVersion: listener.TLS.MaxVersion, +// CipherSuites: listener.TLS.CipherSuites, +// }, nil +//} +// + +// when we have multiple certificates on a single listener, we need +// to duplicate the filter chains with multiple TLS contexts + +// TODO this and makeInlineOverrideFilterChains can be consolidated since all it needed was the protocold +func makeInlineOverrideFilterChainsAPIGateway(cfgSnap *proxycfg.ConfigSnapshot, + tlsCfg structs.GatewayTLSConfig, + protocol string, + filterOpts listenerFilterOpts, + certs []structs.InlineCertificateConfigEntry) ([]*envoy_listener_v3.FilterChain, error) { + + var chains []*envoy_listener_v3.FilterChain + + constructChain := func(name string, hosts []string, tlsContext *envoy_tls_v3.CommonTlsContext) error { + filterOpts.filterName = name + filter, err := makeListenerFilter(filterOpts) + if err != nil { + return err + } + + // Configure alpn protocols on TLSContext + tlsContext.AlpnProtocols = getAlpnProtocols(protocol) + transportSocket, err := makeDownstreamTLSTransportSocket(&envoy_tls_v3.DownstreamTlsContext{ + CommonTlsContext: tlsContext, + RequireClientCertificate: &wrapperspb.BoolValue{Value: false}, + }) + if err != nil { + return err + } + + chains = append(chains, &envoy_listener_v3.FilterChain{ + FilterChainMatch: makeSNIFilterChainMatch(hosts...), + Filters: []*envoy_listener_v3.Filter{ + filter, + }, + TransportSocket: transportSocket, + }) + + return nil + } + + multipleCerts := len(certs) > 1 + + allCertHosts := map[string]struct{}{} + overlappingHosts := map[string]struct{}{} + + if multipleCerts { + // we only need to prune out overlapping hosts if we have more than + // one certificate + for _, cert := range certs { + hosts, err := cert.Hosts() + if err != nil { + return nil, fmt.Errorf("unable to parse hosts from x509 certificate: %v", hosts) + } + for _, host := range hosts { + if _, ok := allCertHosts[host]; ok { + overlappingHosts[host] = struct{}{} + } + allCertHosts[host] = struct{}{} + } + } + } + + for _, cert := range certs { + var hosts []string + + // if we only have one cert, we just use it for all ingress + if multipleCerts { + // otherwise, we need an SNI per cert and to fallback to our ingress + // gateway certificate signed by our Consul CA + certHosts, err := cert.Hosts() + if err != nil { + return nil, fmt.Errorf("unable to parse hosts from x509 certificate: %v", hosts) + } + // filter out any overlapping hosts so we don't have collisions in our filter chains + for _, host := range certHosts { + if _, ok := overlappingHosts[host]; !ok { + hosts = append(hosts, host) + } + } + + if len(hosts) == 0 { + // all of our hosts are overlapping, so we just skip this filter and it'll be + // handled by the default filter chain + continue + } + } + + if err := constructChain(cert.Name, hosts, makeInlineTLSContextFromGatewayTLSConfig(tlsCfg, cert)); err != nil { + return nil, err + } + } + + if multipleCerts { + // if we have more than one cert, add a default handler that uses the leaf cert from connect + if err := constructChain("default", nil, makeCommonTLSContext(cfgSnap.Leaf(), cfgSnap.RootPEMs(), makeTLSParametersFromGatewayTLSConfig(tlsCfg))); err != nil { + return nil, err + } + } + + return chains, nil +} From c7d6f799c6bd291f9832d7e30e701e0428799f50 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Wed, 17 May 2023 11:52:25 -0500 Subject: [PATCH 04/34] checkpoint --- agent/xds/listeners.go | 1 + agent/xds/listeners_apigateway.go | 6 +++++- agent/xds/listeners_ingress.go | 5 +++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index 1ba5a8a861ff..662efa6ddf62 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -866,6 +866,7 @@ func (s *ResourceGenerator) listenersFromSnapshotGateway(cfgSnap *proxycfg.Confi if err != nil { return nil, err } + resources = append(resources, listeners...) case structs.ServiceKindIngressGateway: listeners, err := s.makeIngressGatewayListeners(a.Address, cfgSnap) diff --git a/agent/xds/listeners_apigateway.go b/agent/xds/listeners_apigateway.go index e9f7be20c08f..adbe8a884586 100644 --- a/agent/xds/listeners_apigateway.go +++ b/agent/xds/listeners_apigateway.go @@ -41,6 +41,9 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro return nil, err } + fmt.Println(tlsContext) + panic("hi") + if listenerKey.Protocol == "tcp" { //TODO not sure if we can rely on this the same way ingress can u := readyUpstreams.upstreams[0] @@ -232,6 +235,7 @@ func makeCommonTLSContextFromSnapshotAPIGatewayListenerConfig(cfgSnap *proxycfg. //TODO I think this is unreachable with current setup //but apparently it isn't so what do I know // Set up listener TLS from SDS + panic("hihihi") tlsContext = makeCommonTLSContextFromGatewayTLSConfig(*tlsCfg) } else if connectTLSEnabled { tlsContext = makeCommonTLSContext(cfgSnap.Leaf(), cfgSnap.RootPEMs(), makeTLSParametersFromGatewayTLSConfig(*tlsCfg)) @@ -248,7 +252,7 @@ func resolveAPIListenerTLSConfig(listenerTLSCfg structs.APIGatewayTLSConfigurati //TODO, handle SDS configuration , some equivalent of resolveListenerSDSConfig //tracing through out code I think this is always going to be empty on our end, but I'm not sure if it needs //to be initialized - mergedCfg.SDS = &structs.GatewayTLSSDSConfig{} + //mergedCfg.SDS = &structs.GatewayTLSSDSConfig{} //TODO check gateway config if needed? We might only be supporting listener tls at this time diff --git a/agent/xds/listeners_ingress.go b/agent/xds/listeners_ingress.go index c16a1b74cc28..e4c060fadc53 100644 --- a/agent/xds/listeners_ingress.go +++ b/agent/xds/listeners_ingress.go @@ -43,6 +43,11 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap return nil, err } + fmt.Println(listenerCfg.TLS) + + fmt.Println(tlsContext) + panic("hi2") + if listenerKey.Protocol == "tcp" { // We rely on the invariant of upstreams slice always having at least 1 // member, because this key/value pair is created only when a From 463c5fe8580dfd08867cef101ca3a0bd58ca6808 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Wed, 17 May 2023 12:04:42 -0500 Subject: [PATCH 05/34] 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 18abed7b25dc..19daf3d74006 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 8c5b487ea97f..6f10c382b149 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 ba109ea6f9dd..7aaee6da796f 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 72686b3c4f25..e62e979bf8b1 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 aeb0b7fd6cff..4d99c49e6951 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 e96f473be4f4..536c99d7c749 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 5e28909bfa5f..5bdddadd9d26 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 06/34] 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 9204a990a2e4..3a068d850bd1 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 07/34] 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 3a068d850bd1..9e878e02c91b 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 e3f1e855caff092da8bb668f585cb91374843955 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Wed, 17 May 2023 14:39:13 -0500 Subject: [PATCH 08/34] checkpoint --- agent/xds/listeners.go | 23 +++++++++++---------- agent/xds/listeners_apigateway.go | 34 ++++++++++++++++++++++++++----- agent/xds/listeners_ingress.go | 21 +++++++++++++++---- 3 files changed, 58 insertions(+), 20 deletions(-) diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index 662efa6ddf62..67df3ebacca0 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -851,18 +851,19 @@ func (s *ResourceGenerator) listenersFromSnapshotGateway(cfgSnap *proxycfg.Confi return nil, err } case structs.ServiceKindAPIGateway: + cfgSnapPreToIngress := *cfgSnap //// 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 - //} - //listeners, err := s.makeIngressGatewayListeners(a.Address, cfgSnap) - //if err != nil { - // return nil, err - //} - - listeners, err := s.makeAPIGatewayListeners(a.Address, cfgSnap) + var err error + cfgSnap.IngressGateway, err = cfgSnap.APIGateway.ToIngress(cfgSnap.Datacenter) + if err != nil { + return nil, err + } + listeners, err := s.makeIngressGatewayListeners(a.Address, cfgSnap) + if err != nil { + return nil, err + } + + listeners, err = s.makeAPIGatewayListeners(a.Address, &cfgSnapPreToIngress) if err != nil { return nil, err } diff --git a/agent/xds/listeners_apigateway.go b/agent/xds/listeners_apigateway.go index adbe8a884586..bd21daa82ddc 100644 --- a/agent/xds/listeners_apigateway.go +++ b/agent/xds/listeners_apigateway.go @@ -26,10 +26,25 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro for _, readyUpstreams := range readyUpstreamsList { listenerCfg := readyUpstreams.listenerCfg listenerKey := readyUpstreams.listenerKey + boundListener := readyUpstreams.boundListenerCfg + + //TODO I'm positive this doesn't go here but I'm just trying to get this thing to work + if cfgSnap.APIGateway.ListenerCertificates == nil { + cfgSnap.APIGateway.ListenerCertificates = make(map[proxycfg.IngressListenerKey][]structs.InlineCertificateConfigEntry) + } + + for _, certRef := range boundListener.Certificates { + cert, ok := cfgSnap.APIGateway.Certificates.Get(certRef) + if !ok { + continue + } + cfgSnap.APIGateway.ListenerCertificates[listenerKey] = append(cfgSnap.APIGateway.ListenerCertificates[listenerKey], *cert) + } var isAPIGatewayWithTLS bool var certs []structs.InlineCertificateConfigEntry if cfgSnap.APIGateway.ListenerCertificates != nil { + certs = cfgSnap.APIGateway.ListenerCertificates[listenerKey] } if certs != nil { @@ -41,27 +56,29 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro return nil, err } - fmt.Println(tlsContext) - panic("hi") - if listenerKey.Protocol == "tcp" { //TODO not sure if we can rely on this the same way ingress can u := readyUpstreams.upstreams[0] uid := proxycfg.NewUpstreamID(&u) - chain := cfgSnap.IngressGateway.DiscoveryChain[uid] + chain := cfgSnap.APIGateway.DiscoveryChain[uid] if chain == nil { + fmt.Println("hello") // Wait until a chain is present in the snapshot. continue } + cfg := s.getAndModifyUpstreamConfigForListener(uid, &u, chain) useRDS := cfg.Protocol != "tcp" && !chain.Default var clusterName string if !useRDS { + fmt.Println("we ain't using RDS in API gateway") // When not using RDS we must generate a cluster name to attach to the filter chain. // With RDS, cluster names get attached to the dynamic routes instead. target, err := simpleChainTarget(chain) + fmt.Println("target") + fmt.Println(target) if err != nil { return nil, err } @@ -98,7 +115,8 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro if isAPIGatewayWithTLS { // construct SNI filter chains - l.FilterChains, err = makeInlineOverrideFilterChains(cfgSnap, cfgSnap.IngressGateway.TLSConfig, listenerKey, listenerFilterOpts{ + fmt.Println(cfgSnap.APIGateway.TLSConfig) + l.FilterChains, err = makeInlineOverrideFilterChains(cfgSnap, cfgSnap.APIGateway.TLSConfig, listenerKey, listenerFilterOpts{ useRDS: useRDS, protocol: listenerKey.Protocol, routeName: listenerKey.RouteName(), @@ -108,8 +126,12 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro logger: s.Logger, }, certs) if err != nil { + fmt.Println(err) + panic("hi") + return nil, err } + // add the tls inspector to do SNI introspection tlsInspector, err := makeTLSInspectorListenerFilter() if err != nil { @@ -193,6 +215,8 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro } + fmt.Println(len(resources)) + return resources, nil } diff --git a/agent/xds/listeners_ingress.go b/agent/xds/listeners_ingress.go index e4c060fadc53..b8e20cf8791f 100644 --- a/agent/xds/listeners_ingress.go +++ b/agent/xds/listeners_ingress.go @@ -42,11 +42,8 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap if err != nil { return nil, err } - - fmt.Println(listenerCfg.TLS) - + fmt.Println("ingress tls context") fmt.Println(tlsContext) - panic("hi2") if listenerKey.Protocol == "tcp" { // We rely on the invariant of upstreams slice always having at least 1 @@ -56,12 +53,18 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap uid := proxycfg.NewUpstreamID(&u) + fmt.Println("ingress Gateway") + fmt.Println(u) + fmt.Println(uid) + chain := cfgSnap.IngressGateway.DiscoveryChain[uid] if chain == nil { // Wait until a chain is present in the snapshot. continue } + fmt.Println("ingress Gateway") + cfg := s.getAndModifyUpstreamConfigForListener(uid, &u, chain) // RDS, Envoy's Route Discovery Service, is only used for HTTP services with a customized discovery chain. @@ -70,13 +73,16 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap var clusterName string if !useRDS { + // When not using RDS we must generate a cluster name to attach to the filter chain. // With RDS, cluster names get attached to the dynamic routes instead. target, err := simpleChainTarget(chain) + if err != nil { return nil, err } clusterName = CustomizeClusterName(target.Name, chain) + } filterName := fmt.Sprintf("%s.%s.%s.%s", chain.ServiceName, chain.Namespace, chain.Partition, chain.Datacenter) @@ -100,6 +106,7 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap protocol: cfg.Protocol, tlsContext: tlsContext, }) + if err != nil { return nil, err } @@ -109,6 +116,8 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap if isAPIGatewayWithTLS { // construct SNI filter chains + fmt.Println(cfgSnap.IngressGateway.TLSConfig) + l.FilterChains, err = makeInlineOverrideFilterChains(cfgSnap, cfgSnap.IngressGateway.TLSConfig, listenerKey, listenerFilterOpts{ useRDS: useRDS, protocol: listenerKey.Protocol, @@ -205,6 +214,8 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap } } + fmt.Println(len(resources)) + return resources, nil } @@ -427,11 +438,13 @@ func makeSDSOverrideFilterChains(cfgSnap *proxycfg.ConfigSnapshot, return chains, nil } +// TODO this function is the issue // when we have multiple certificates on a single listener, we need // to duplicate the filter chains with multiple TLS contexts func makeInlineOverrideFilterChains(cfgSnap *proxycfg.ConfigSnapshot, tlsCfg structs.GatewayTLSConfig, listenerKey proxycfg.IngressListenerKey, + filterOpts listenerFilterOpts, certs []structs.InlineCertificateConfigEntry) ([]*envoy_listener_v3.FilterChain, error) { 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 09/34] 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 19daf3d74006..2367da9d5469 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 890aaed5a85a11028eb4c3fdfddaf1df503d65ac Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Wed, 17 May 2023 16:44:55 -0500 Subject: [PATCH 10/34] unit tests passing --- agent/xds/listeners_apigateway.go | 18 ++++-------------- agent/xds/listeners_ingress.go | 18 ------------------ 2 files changed, 4 insertions(+), 32 deletions(-) diff --git a/agent/xds/listeners_apigateway.go b/agent/xds/listeners_apigateway.go index bd21daa82ddc..f4413f921c2f 100644 --- a/agent/xds/listeners_apigateway.go +++ b/agent/xds/listeners_apigateway.go @@ -63,7 +63,6 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro chain := cfgSnap.APIGateway.DiscoveryChain[uid] if chain == nil { - fmt.Println("hello") // Wait until a chain is present in the snapshot. continue } @@ -73,12 +72,9 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro var clusterName string if !useRDS { - fmt.Println("we ain't using RDS in API gateway") // When not using RDS we must generate a cluster name to attach to the filter chain. // With RDS, cluster names get attached to the dynamic routes instead. target, err := simpleChainTarget(chain) - fmt.Println("target") - fmt.Println(target) if err != nil { return nil, err } @@ -115,8 +111,7 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro if isAPIGatewayWithTLS { // construct SNI filter chains - fmt.Println(cfgSnap.APIGateway.TLSConfig) - l.FilterChains, err = makeInlineOverrideFilterChains(cfgSnap, cfgSnap.APIGateway.TLSConfig, listenerKey, listenerFilterOpts{ + l.FilterChains, err = makeInlineOverrideFilterChainsAPIGateway(cfgSnap, cfgSnap.APIGateway.TLSConfig, listenerKey.Protocol, listenerFilterOpts{ useRDS: useRDS, protocol: listenerKey.Protocol, routeName: listenerKey.RouteName(), @@ -126,9 +121,6 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro logger: s.Logger, }, certs) if err != nil { - fmt.Println(err) - panic("hi") - return nil, err } @@ -171,7 +163,7 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro sniFilterChains := []*envoy_listener_v3.FilterChain{} if isAPIGatewayWithTLS { - sniFilterChains, err = makeInlineOverrideFilterChains(cfgSnap, cfgSnap.IngressGateway.TLSConfig, listenerKey, filterOpts, certs) + sniFilterChains, err = makeInlineOverrideFilterChainsAPIGateway(cfgSnap, cfgSnap.IngressGateway.TLSConfig, listenerKey.Protocol, filterOpts, certs) if err != nil { return nil, err } @@ -214,9 +206,7 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro } } - - fmt.Println(len(resources)) - + return resources, nil } @@ -348,7 +338,7 @@ func routeNameForAPIGatewayUpstream(l structs.IngressListener, s structs.Ingress // when we have multiple certificates on a single listener, we need // to duplicate the filter chains with multiple TLS contexts -// TODO this and makeInlineOverrideFilterChains can be consolidated since all it needed was the protocold +// TODO this function and makeInlineOverrideFilterChains can be consolidated since it only uses the protocol from listener key func makeInlineOverrideFilterChainsAPIGateway(cfgSnap *proxycfg.ConfigSnapshot, tlsCfg structs.GatewayTLSConfig, protocol string, diff --git a/agent/xds/listeners_ingress.go b/agent/xds/listeners_ingress.go index b8e20cf8791f..c16a1b74cc28 100644 --- a/agent/xds/listeners_ingress.go +++ b/agent/xds/listeners_ingress.go @@ -42,8 +42,6 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap if err != nil { return nil, err } - fmt.Println("ingress tls context") - fmt.Println(tlsContext) if listenerKey.Protocol == "tcp" { // We rely on the invariant of upstreams slice always having at least 1 @@ -53,18 +51,12 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap uid := proxycfg.NewUpstreamID(&u) - fmt.Println("ingress Gateway") - fmt.Println(u) - fmt.Println(uid) - chain := cfgSnap.IngressGateway.DiscoveryChain[uid] if chain == nil { // Wait until a chain is present in the snapshot. continue } - fmt.Println("ingress Gateway") - cfg := s.getAndModifyUpstreamConfigForListener(uid, &u, chain) // RDS, Envoy's Route Discovery Service, is only used for HTTP services with a customized discovery chain. @@ -73,16 +65,13 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap var clusterName string if !useRDS { - // When not using RDS we must generate a cluster name to attach to the filter chain. // With RDS, cluster names get attached to the dynamic routes instead. target, err := simpleChainTarget(chain) - if err != nil { return nil, err } clusterName = CustomizeClusterName(target.Name, chain) - } filterName := fmt.Sprintf("%s.%s.%s.%s", chain.ServiceName, chain.Namespace, chain.Partition, chain.Datacenter) @@ -106,7 +95,6 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap protocol: cfg.Protocol, tlsContext: tlsContext, }) - if err != nil { return nil, err } @@ -116,8 +104,6 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap if isAPIGatewayWithTLS { // construct SNI filter chains - fmt.Println(cfgSnap.IngressGateway.TLSConfig) - l.FilterChains, err = makeInlineOverrideFilterChains(cfgSnap, cfgSnap.IngressGateway.TLSConfig, listenerKey, listenerFilterOpts{ useRDS: useRDS, protocol: listenerKey.Protocol, @@ -214,8 +200,6 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap } } - fmt.Println(len(resources)) - return resources, nil } @@ -438,13 +422,11 @@ func makeSDSOverrideFilterChains(cfgSnap *proxycfg.ConfigSnapshot, return chains, nil } -// TODO this function is the issue // when we have multiple certificates on a single listener, we need // to duplicate the filter chains with multiple TLS contexts func makeInlineOverrideFilterChains(cfgSnap *proxycfg.ConfigSnapshot, tlsCfg structs.GatewayTLSConfig, listenerKey proxycfg.IngressListenerKey, - filterOpts listenerFilterOpts, certs []structs.InlineCertificateConfigEntry) ([]*envoy_listener_v3.FilterChain, error) { From 8728cac94592116cd0e4a7c647c30380db1e0952 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Wed, 17 May 2023 16:47:56 -0500 Subject: [PATCH 11/34] 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 2367da9d5469..2f94874a7e65 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 562c789cb63e6a0aae2f37b98433cd781b53396c Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Thu, 18 May 2023 10:07:05 -0500 Subject: [PATCH 12/34] add deterministic sorting to appease the unit test gods --- agent/xds/listeners.go | 26 +++++++++++++------------- agent/xds/listeners_apigateway.go | 8 ++++++-- agent/xds/listeners_ingress.go | 1 + 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index 67df3ebacca0..f5c620573c79 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -851,19 +851,19 @@ func (s *ResourceGenerator) listenersFromSnapshotGateway(cfgSnap *proxycfg.Confi return nil, err } case structs.ServiceKindAPIGateway: - cfgSnapPreToIngress := *cfgSnap - //// 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 - } - listeners, err := s.makeIngressGatewayListeners(a.Address, cfgSnap) - if err != nil { - return nil, err - } - - listeners, err = s.makeAPIGatewayListeners(a.Address, &cfgSnapPreToIngress) + //cfgSnapPreToIngress := *cfgSnap + ////// 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 + //} + //listeners, err := s.makeIngressGatewayListeners(a.Address, cfgSnap) + //if err != nil { + // return nil, err + //} + + listeners, err := s.makeAPIGatewayListeners(a.Address, cfgSnap) if err != nil { return nil, err } diff --git a/agent/xds/listeners_apigateway.go b/agent/xds/listeners_apigateway.go index f4413f921c2f..32e2e51b2e84 100644 --- a/agent/xds/listeners_apigateway.go +++ b/agent/xds/listeners_apigateway.go @@ -5,6 +5,7 @@ package xds import ( "fmt" + "sort" 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" @@ -57,7 +58,10 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro } if listenerKey.Protocol == "tcp" { - //TODO not sure if we can rely on this the same way ingress can + //TODO this is required for a deterministic unit test output, but I'm not sure it is needed + sort.Slice(readyUpstreams.upstreams, func(i, j int) bool { + return readyUpstreams.upstreams[i].LocalBindPort < readyUpstreams.upstreams[j].LocalBindPort + }) u := readyUpstreams.upstreams[0] uid := proxycfg.NewUpstreamID(&u) @@ -206,7 +210,7 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro } } - + return resources, nil } diff --git a/agent/xds/listeners_ingress.go b/agent/xds/listeners_ingress.go index c16a1b74cc28..260293b35500 100644 --- a/agent/xds/listeners_ingress.go +++ b/agent/xds/listeners_ingress.go @@ -135,6 +135,7 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap direction: envoy_core_v3.TrafficDirection_OUTBOUND, logger: s.Logger, } + listener := makeListener(listenerOpts) filterOpts := listenerFilterOpts{ From 077741d4c42a861ecf296e11e62fdf7e55dc2f4f Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Thu, 18 May 2023 10:15:03 -0500 Subject: [PATCH 13/34] remove panic --- agent/xds/listeners_apigateway.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/agent/xds/listeners_apigateway.go b/agent/xds/listeners_apigateway.go index 32e2e51b2e84..3e7176935ac7 100644 --- a/agent/xds/listeners_apigateway.go +++ b/agent/xds/listeners_apigateway.go @@ -238,9 +238,7 @@ func makeDownstreamTLSContextFromSnapshotAPIListenerConfig(cfgSnap *proxycfg.Con func makeCommonTLSContextFromSnapshotAPIGatewayListenerConfig(cfgSnap *proxycfg.ConfigSnapshot, listenerCfg structs.APIGatewayListener) (*envoy_tls_v3.CommonTlsContext, error) { var tlsContext *envoy_tls_v3.CommonTlsContext - //TODO if needed, get TLS configuration from the gateway itself - - //TODO gateway config doesn't seem to support TLS config so we might only need to pull from listener at the moment + //API Gateway TLS config is per listener tlsCfg, err := resolveAPIListenerTLSConfig(listenerCfg.TLS) if err != nil { return nil, err @@ -250,10 +248,7 @@ func makeCommonTLSContextFromSnapshotAPIGatewayListenerConfig(cfgSnap *proxycfg. connectTLSEnabled := (!listenerCfg.TLS.IsEmpty()) if tlsCfg.SDS != nil { - //TODO I think this is unreachable with current setup - //but apparently it isn't so what do I know // Set up listener TLS from SDS - panic("hihihi") tlsContext = makeCommonTLSContextFromGatewayTLSConfig(*tlsCfg) } else if connectTLSEnabled { tlsContext = makeCommonTLSContext(cfgSnap.Leaf(), cfgSnap.RootPEMs(), makeTLSParametersFromGatewayTLSConfig(*tlsCfg)) From 029ae91618525b6262299f2fd886da2dd2684776 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 18 May 2023 11:46:44 -0400 Subject: [PATCH 14/34] Find ready upstream matching listener instead of first in list --- agent/xds/listeners_apigateway.go | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/agent/xds/listeners_apigateway.go b/agent/xds/listeners_apigateway.go index 3e7176935ac7..0114df75dedc 100644 --- a/agent/xds/listeners_apigateway.go +++ b/agent/xds/listeners_apigateway.go @@ -5,8 +5,6 @@ package xds import ( "fmt" - "sort" - 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" @@ -58,12 +56,20 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro } if listenerKey.Protocol == "tcp" { - //TODO this is required for a deterministic unit test output, but I'm not sure it is needed - sort.Slice(readyUpstreams.upstreams, func(i, j int) bool { - return readyUpstreams.upstreams[i].LocalBindPort < readyUpstreams.upstreams[j].LocalBindPort - }) - u := readyUpstreams.upstreams[0] - uid := proxycfg.NewUpstreamID(&u) + //TODO not sure if we can rely on this the same way ingress can + //u := readyUpstreams.upstreams[0] + //uid := proxycfg.NewUpstreamID(&u) + + // Find the upstream matching this listener + // TODO This might be simpler if getReadyUpstreams were keyed by listenerKey + var u structs.Upstream + var uid proxycfg.UpstreamID + for _, upstream := range readyUpstreams.upstreams { + if upstream.LocalBindPort == listenerKey.Port { + u = upstream + uid = proxycfg.NewUpstreamID(&u) + } + } chain := cfgSnap.APIGateway.DiscoveryChain[uid] if chain == nil { From 3204325bda8f2df29ec338ceffb7bd72d30baf42 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 18 May 2023 11:48:43 -0400 Subject: [PATCH 15/34] Clean up, improve TODO --- agent/xds/listeners_apigateway.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/agent/xds/listeners_apigateway.go b/agent/xds/listeners_apigateway.go index 0114df75dedc..451a0e54b8e8 100644 --- a/agent/xds/listeners_apigateway.go +++ b/agent/xds/listeners_apigateway.go @@ -56,12 +56,11 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro } if listenerKey.Protocol == "tcp" { - //TODO not sure if we can rely on this the same way ingress can - //u := readyUpstreams.upstreams[0] - //uid := proxycfg.NewUpstreamID(&u) - // Find the upstream matching this listener - // TODO This might be simpler if getReadyUpstreams were keyed by listenerKey + // TODO This might be simpler if getReadyUpstreams were keyed by listenerKey. + // Today, it's keyed by route, which means a route attaching to multiple listeners + // will have multiple ready upstreams, one for each listener. We aren't certain + // if anything depends on the upstreams being grouped by route currently vs. listener. var u structs.Upstream var uid proxycfg.UpstreamID for _, upstream := range readyUpstreams.upstreams { From a13a37dbb37f62cd4cf9e82a80ccec3f2c0ed1de Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 18 May 2023 14:56:14 -0400 Subject: [PATCH 16/34] 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 2f94874a7e65..56ee4821068c 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 6c1e116cf688..91d41b522238 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 6f10c382b149..e77703d51251 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 b302597e261d43d3674a53bb770a2cd6fc98348b Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Thu, 18 May 2023 14:43:10 -0500 Subject: [PATCH 17/34] clean up todos, references to api gateway in listeners_ingress --- agent/xds/listeners.go | 12 ---------- agent/xds/listeners_apigateway.go | 16 +++---------- agent/xds/listeners_ingress.go | 40 +------------------------------ 3 files changed, 4 insertions(+), 64 deletions(-) diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index f5c620573c79..00f033416213 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -851,18 +851,6 @@ func (s *ResourceGenerator) listenersFromSnapshotGateway(cfgSnap *proxycfg.Confi return nil, err } case structs.ServiceKindAPIGateway: - //cfgSnapPreToIngress := *cfgSnap - ////// 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 - //} - //listeners, err := s.makeIngressGatewayListeners(a.Address, cfgSnap) - //if err != nil { - // return nil, err - //} - listeners, err := s.makeAPIGatewayListeners(a.Address, cfgSnap) if err != nil { return nil, err diff --git a/agent/xds/listeners_apigateway.go b/agent/xds/listeners_apigateway.go index 451a0e54b8e8..8116e536027d 100644 --- a/agent/xds/listeners_apigateway.go +++ b/agent/xds/listeners_apigateway.go @@ -57,10 +57,7 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro if listenerKey.Protocol == "tcp" { // Find the upstream matching this listener - // TODO This might be simpler if getReadyUpstreams were keyed by listenerKey. - // Today, it's keyed by route, which means a route attaching to multiple listeners - // will have multiple ready upstreams, one for each listener. We aren't certain - // if anything depends on the upstreams being grouped by route currently vs. listener. + // TODO update this block of code to match Nathan's change when its merged in var u structs.Upstream var uid proxycfg.UpstreamID for _, upstream := range readyUpstreams.upstreams { @@ -166,7 +163,8 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro logger: s.Logger, } - //TODO equivalent of makeSDSOverrideFilterChains + //TODO equivalent of makeSDSOverrideFilterChains, if needed + // Generate any filter chains needed for services with custom TLS certs // via SDS. sniFilterChains := []*envoy_listener_v3.FilterChain{} @@ -191,7 +189,6 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro // See if there are other services that didn't have specific SNI-matching // filter chains. If so add a default filterchain to serve them. - //TODO I'm not sure if we can delete this code block or not if len(sniFilterChains) < len(readyUpstreams.upstreams) && !isAPIGatewayWithTLS { defaultFilter, err := makeListenerFilter(filterOpts) if err != nil { @@ -262,17 +259,10 @@ func makeCommonTLSContextFromSnapshotAPIGatewayListenerConfig(cfgSnap *proxycfg. return tlsContext, nil } -// API-ified -// TODO if multiple tls configuragtions are available, pass them in here to be consolidated func resolveAPIListenerTLSConfig(listenerTLSCfg structs.APIGatewayTLSConfiguration) (*structs.GatewayTLSConfig, error) { var mergedCfg structs.GatewayTLSConfig //TODO, handle SDS configuration , some equivalent of resolveListenerSDSConfig - //tracing through out code I think this is always going to be empty on our end, but I'm not sure if it needs - //to be initialized - //mergedCfg.SDS = &structs.GatewayTLSSDSConfig{} - - //TODO check gateway config if needed? We might only be supporting listener tls at this time if !listenerTLSCfg.IsEmpty() { if listenerTLSCfg.MinVersion != types.TLSVersionUnspecified { diff --git a/agent/xds/listeners_ingress.go b/agent/xds/listeners_ingress.go index 260293b35500..d61b4aeec5df 100644 --- a/agent/xds/listeners_ingress.go +++ b/agent/xds/listeners_ingress.go @@ -29,15 +29,6 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap return nil, fmt.Errorf("no listener config found for listener on proto/port %s/%d", listenerKey.Protocol, listenerKey.Port) } - var isAPIGatewayWithTLS bool - var certs []structs.InlineCertificateConfigEntry - if cfgSnap.APIGateway.ListenerCertificates != nil { - certs = cfgSnap.APIGateway.ListenerCertificates[listenerKey] - } - if certs != nil { - isAPIGatewayWithTLS = true - } - tlsContext, err := makeDownstreamTLSContextFromSnapshotListenerConfig(cfgSnap, listenerCfg) if err != nil { return nil, err @@ -102,28 +93,6 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap filterChain, } - if isAPIGatewayWithTLS { - // construct SNI filter chains - l.FilterChains, err = makeInlineOverrideFilterChains(cfgSnap, cfgSnap.IngressGateway.TLSConfig, listenerKey, listenerFilterOpts{ - useRDS: useRDS, - protocol: listenerKey.Protocol, - routeName: listenerKey.RouteName(), - cluster: clusterName, - statPrefix: "ingress_upstream_", - accessLogs: &cfgSnap.Proxy.AccessLogs, - logger: s.Logger, - }, certs) - if err != nil { - return nil, err - } - // add the tls inspector to do SNI introspection - tlsInspector, err := makeTLSInspectorListenerFilter() - if err != nil { - return nil, err - } - l.ListenerFilters = []*envoy_listener_v3.ListenerFilter{tlsInspector} - } - resources = append(resources, l) } else { // If multiple upstreams share this port, make a special listener for the protocol. @@ -158,13 +127,6 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap return nil, err } - if isAPIGatewayWithTLS { - sniFilterChains, err = makeInlineOverrideFilterChains(cfgSnap, cfgSnap.IngressGateway.TLSConfig, listenerKey, filterOpts, certs) - if err != nil { - return nil, err - } - } - // If there are any sni filter chains, we need a TLS inspector filter! if len(sniFilterChains) > 0 { tlsInspector, err := makeTLSInspectorListenerFilter() @@ -178,7 +140,7 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap // See if there are other services that didn't have specific SNI-matching // filter chains. If so add a default filterchain to serve them. - if len(sniFilterChains) < len(upstreams) && !isAPIGatewayWithTLS { + if len(sniFilterChains) < len(upstreams) { defaultFilter, err := makeListenerFilter(filterOpts) if err != nil { return nil, err From 61e78eee1b05e1569245a78523bc94bb641af870 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Thu, 18 May 2023 15:08:11 -0500 Subject: [PATCH 18/34] merge in Nathan's fix --- agent/xds/listeners_apigateway.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/agent/xds/listeners_apigateway.go b/agent/xds/listeners_apigateway.go index 8116e536027d..0dae244a9b52 100644 --- a/agent/xds/listeners_apigateway.go +++ b/agent/xds/listeners_apigateway.go @@ -43,7 +43,6 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro var isAPIGatewayWithTLS bool var certs []structs.InlineCertificateConfigEntry if cfgSnap.APIGateway.ListenerCertificates != nil { - certs = cfgSnap.APIGateway.ListenerCertificates[listenerKey] } if certs != nil { @@ -57,15 +56,12 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro if listenerKey.Protocol == "tcp" { // Find the upstream matching this listener - // TODO update this block of code to match Nathan's change when its merged in - var u structs.Upstream - var uid proxycfg.UpstreamID - for _, upstream := range readyUpstreams.upstreams { - if upstream.LocalBindPort == listenerKey.Port { - u = upstream - uid = proxycfg.NewUpstreamID(&u) - } - } + + // We rely on the invariant of upstreams slice always having at least 1 + // member, because this key/value pair is created only when a + // GatewayService is returned in the RPC + u := readyUpstreams.upstreams[0] + uid := proxycfg.NewUpstreamID(&u) chain := cfgSnap.APIGateway.DiscoveryChain[uid] if chain == nil { From 2f794e6f0279570026e22b02dd595635e31c2659 Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Fri, 19 May 2023 12:46:55 -0500 Subject: [PATCH 19/34] 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 f81715372c4c..328431e8e8f7 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 c22a4a35134409d404ec981937b2bce8f35f6a0a Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Fri, 19 May 2023 12:56:31 -0500 Subject: [PATCH 20/34] cleanup current todos, remove snapshot manipulation from generation code --- agent/xds/listeners_apigateway.go | 46 ++++++------------------------- agent/xds/scratch | 4 +++ 2 files changed, 12 insertions(+), 38 deletions(-) create mode 100644 agent/xds/scratch diff --git a/agent/xds/listeners_apigateway.go b/agent/xds/listeners_apigateway.go index 0dae244a9b52..01943191d141 100644 --- a/agent/xds/listeners_apigateway.go +++ b/agent/xds/listeners_apigateway.go @@ -27,27 +27,23 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro listenerKey := readyUpstreams.listenerKey boundListener := readyUpstreams.boundListenerCfg - //TODO I'm positive this doesn't go here but I'm just trying to get this thing to work - if cfgSnap.APIGateway.ListenerCertificates == nil { - cfgSnap.APIGateway.ListenerCertificates = make(map[proxycfg.IngressListenerKey][]structs.InlineCertificateConfigEntry) - } + //TODO, it's possible that this can be pulled off of the snapshot APIGateway.ListenerCertificates + // if that value is being appropriately updated so we don't have to build this out every time + certificates := make(map[proxycfg.IngressListenerKey][]structs.InlineCertificateConfigEntry) for _, certRef := range boundListener.Certificates { cert, ok := cfgSnap.APIGateway.Certificates.Get(certRef) if !ok { continue } - cfgSnap.APIGateway.ListenerCertificates[listenerKey] = append(cfgSnap.APIGateway.ListenerCertificates[listenerKey], *cert) + certificates[listenerKey] = append(certificates[listenerKey], *cert) } - var isAPIGatewayWithTLS bool var certs []structs.InlineCertificateConfigEntry - if cfgSnap.APIGateway.ListenerCertificates != nil { - certs = cfgSnap.APIGateway.ListenerCertificates[listenerKey] - } - if certs != nil { - isAPIGatewayWithTLS = true + if certificates != nil { + certs = certificates[listenerKey] } + isAPIGatewayWithTLS := certs != nil tlsContext, err := makeDownstreamTLSContextFromSnapshotAPIListenerConfig(cfgSnap, listenerCfg) if err != nil { @@ -159,7 +155,7 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro logger: s.Logger, } - //TODO equivalent of makeSDSOverrideFilterChains, if needed + //TODO equivalent of makeSDSOverrideFilterChains, when needed // Generate any filter chains needed for services with custom TLS certs // via SDS. @@ -242,7 +238,6 @@ func makeCommonTLSContextFromSnapshotAPIGatewayListenerConfig(cfgSnap *proxycfg. return nil, err } - //TODO do we have an equivalent of Enabled field for API Gateway connectTLSEnabled := (!listenerCfg.TLS.IsEmpty()) if tlsCfg.SDS != nil { @@ -258,8 +253,6 @@ func makeCommonTLSContextFromSnapshotAPIGatewayListenerConfig(cfgSnap *proxycfg. func resolveAPIListenerTLSConfig(listenerTLSCfg structs.APIGatewayTLSConfiguration) (*structs.GatewayTLSConfig, error) { var mergedCfg structs.GatewayTLSConfig - //TODO, handle SDS configuration , some equivalent of resolveListenerSDSConfig - if !listenerTLSCfg.IsEmpty() { if listenerTLSCfg.MinVersion != types.TLSVersionUnspecified { mergedCfg.TLSMinVersion = listenerTLSCfg.MinVersion @@ -302,29 +295,6 @@ func routeNameForAPIGatewayUpstream(l structs.IngressListener, s structs.Ingress return fmt.Sprintf("%s_%s", key.RouteName(), svcIdentifier) } -//TODO Delete, this is a reference -//func (c *configSnapshotAPIGateway) toIngressTLS(key IngressListenerKey, listener structs.APIGatewayListener, bound structs.BoundAPIGatewayListener) (*structs.GatewayTLSConfig, error) { -// if len(listener.TLS.Certificates) == 0 { -// return nil, nil -// } -// -// for _, certRef := range bound.Certificates { -// cert, ok := c.Certificates.Get(certRef) -// if !ok { -// continue -// } -// c.ListenerCertificates[key] = append(c.ListenerCertificates[key], *cert) -// } -// -// return &structs.GatewayTLSConfig{ -// Enabled: true, -// TLSMinVersion: listener.TLS.MinVersion, -// TLSMaxVersion: listener.TLS.MaxVersion, -// CipherSuites: listener.TLS.CipherSuites, -// }, nil -//} -// - // when we have multiple certificates on a single listener, we need // to duplicate the filter chains with multiple TLS contexts diff --git a/agent/xds/scratch b/agent/xds/scratch new file mode 100644 index 000000000000..7d00679f72e0 --- /dev/null +++ b/agent/xds/scratch @@ -0,0 +1,4 @@ +&{true []} +common_tls_context:{tls_params:{} tls_certificates:{certificate_chain:{inline_string:"-----BEGIN CERTIFICATE-----\nMIICjDCCAjKgAwIBAgIIC5llxGV1gB8wCgYIKoZIzj0EAwIwFDESMBAGA1UEAxMJ\nVGVzdCBDQSAyMB4XDTE5MDMyMjEzNTgyNloXDTI5MDMyMjEzNTgyNlowDjEMMAoG\nA1UEAxMDd2ViMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEADPv1RHVNRfa2VKR\nAB16b6rZnEt7tuhaxCFpQXPj7M2omb0B9Favq5E0ivpNtv1QnFhxtPd7d5k4e+T7\nSkW1TaOCAXIwggFuMA4GA1UdDwEB/wQEAwIDuDAdBgNVHSUEFjAUBggrBgEFBQcD\nAgYIKwYBBQUHAwEwDAYDVR0TAQH/BAIwADBoBgNVHQ4EYQRfN2Q6MDc6ODc6M2E6\nNDA6MTk6NDc6YzM6NWE6YzA6YmE6NjI6ZGY6YWY6NGI6ZDQ6MDU6MjU6NzY6M2Q6\nNWE6OGQ6MTY6OGQ6Njc6NWU6MmU6YTA6MzQ6N2Q6ZGM6ZmYwagYDVR0jBGMwYYBf\nZDE6MTE6MTE6YWM6MmE6YmE6OTc6YjI6M2Y6YWM6N2I6YmQ6ZGE6YmU6YjE6OGE6\nZmM6OWE6YmE6YjU6YmM6ODM6ZTc6NWU6NDE6NmY6ZjI6NzM6OTU6NTg6MGM6ZGIw\nWQYDVR0RBFIwUIZOc3BpZmZlOi8vMTExMTExMTEtMjIyMi0zMzMzLTQ0NDQtNTU1\nNTU1NTU1NTU1LmNvbnN1bC9ucy9kZWZhdWx0L2RjL2RjMS9zdmMvd2ViMAoGCCqG\nSM49BAMCA0gAMEUCIGC3TTvvjj76KMrguVyFf4tjOqaSCRie3nmHMRNNRav7AiEA\npY0heYeK9A6iOLrzqxSerkXXQyj5e9bE4VgUnxgPU6g=\n-----END CERTIFICATE-----\n"} private_key:{inline_string:"-----BEGIN EC PRIVATE KEY-----\nMHcCAQEEIMoTkpRggp3fqZzFKh82yS4LjtJI+XY+qX/7DefHFrtdoAoGCCqGSM49\nAwEHoUQDQgAEADPv1RHVNRfa2VKRAB16b6rZnEt7tuhaxCFpQXPj7M2omb0B9Fav\nq5E0ivpNtv1QnFhxtPd7d5k4e+T7SkW1TQ==\n-----END EC PRIVATE KEY-----\n"}} validation_context:{trusted_ca:{inline_string:"-----BEGIN CERTIFICATE-----\nMIICXDCCAgKgAwIBAgIICpZq70Z9LyUwCgYIKoZIzj0EAwIwFDESMBAGA1UEAxMJ\nVGVzdCBDQSAyMB4XDTE5MDMyMjEzNTgyNloXDTI5MDMyMjEzNTgyNlowFDESMBAG\nA1UEAxMJVGVzdCBDQSAyMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEIhywH1gx\nAsMwuF3ukAI5YL2jFxH6Usnma1HFSfVyxbXX1/uoZEYrj8yCAtdU2yoHETyd+Zx2\nThhRLP79pYegCaOCATwwggE4MA4GA1UdDwEB/wQEAwIBhjAPBgNVHRMBAf8EBTAD\nAQH/MGgGA1UdDgRhBF9kMToxMToxMTphYzoyYTpiYTo5NzpiMjozZjphYzo3Yjpi\nZDpkYTpiZTpiMTo4YTpmYzo5YTpiYTpiNTpiYzo4MzplNzo1ZTo0MTo2ZjpmMjo3\nMzo5NTo1ODowYzpkYjBqBgNVHSMEYzBhgF9kMToxMToxMTphYzoyYTpiYTo5Nzpi\nMjozZjphYzo3YjpiZDpkYTpiZTpiMTo4YTpmYzo5YTpiYTpiNTpiYzo4MzplNzo1\nZTo0MTo2ZjpmMjo3Mzo5NTo1ODowYzpkYjA/BgNVHREEODA2hjRzcGlmZmU6Ly8x\nMTExMTExMS0yMjIyLTMzMzMtNDQ0NC01NTU1NTU1NTU1NTUuY29uc3VsMAoGCCqG\nSM49BAMCA0gAMEUCICOY0i246rQHJt8o8Oya0D5PLL1FnmsQmQqIGCi31RwnAiEA\noR5f6Ku+cig2Il8T8LJujOp2/2A72QcHZA57B13y+8o=\n-----END CERTIFICATE-----\n"}}} require_client_certificate:{} + +common_tls_context:{tls_params:{} tls_certificates:{certificate_chain:{inline_string:"-----BEGIN CERTIFICATE-----\nMIICjDCCAjKgAwIBAgIIC5llxGV1gB8wCgYIKoZIzj0EAwIwFDESMBAGA1UEAxMJ\nVGVzdCBDQSAyMB4XDTE5MDMyMjEzNTgyNloXDTI5MDMyMjEzNTgyNlowDjEMMAoG\nA1UEAxMDd2ViMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEADPv1RHVNRfa2VKR\nAB16b6rZnEt7tuhaxCFpQXPj7M2omb0B9Favq5E0ivpNtv1QnFhxtPd7d5k4e+T7\nSkW1TaOCAXIwggFuMA4GA1UdDwEB/wQEAwIDuDAdBgNVHSUEFjAUBggrBgEFBQcD\nAgYIKwYBBQUHAwEwDAYDVR0TAQH/BAIwADBoBgNVHQ4EYQRfN2Q6MDc6ODc6M2E6\nNDA6MTk6NDc6YzM6NWE6YzA6YmE6NjI6ZGY6YWY6NGI6ZDQ6MDU6MjU6NzY6M2Q6\nNWE6OGQ6MTY6OGQ6Njc6NWU6MmU6YTA6MzQ6N2Q6ZGM6ZmYwagYDVR0jBGMwYYBf\nZDE6MTE6MTE6YWM6MmE6YmE6OTc6YjI6M2Y6YWM6N2I6YmQ6ZGE6YmU6YjE6OGE6\nZmM6OWE6YmE6YjU6YmM6ODM6ZTc6NWU6NDE6NmY6ZjI6NzM6OTU6NTg6MGM6ZGIw\nWQYDVR0RBFIwUIZOc3BpZmZlOi8vMTExMTExMTEtMjIyMi0zMzMzLTQ0NDQtNTU1\nNTU1NTU1NTU1LmNvbnN1bC9ucy9kZWZhdWx0L2RjL2RjMS9zdmMvd2ViMAoGCCqG\nSM49BAMCA0gAMEUCIGC3TTvvjj76KMrguVyFf4tjOqaSCRie3nmHMRNNRav7AiEA\npY0heYeK9A6iOLrzqxSerkXXQyj5e9bE4VgUnxgPU6g=\n-----END CERTIFICATE-----\n"} private_key:{inline_string:"-----BEGIN EC PRIVATE KEY-----\nMHcCAQEEIMoTkpRggp3fqZzFKh82yS4LjtJI+XY+qX/7DefHFrtdoAoGCCqGSM49\nAwEHoUQDQgAEADPv1RHVNRfa2VKRAB16b6rZnEt7tuhaxCFpQXPj7M2omb0B9Fav\nq5E0ivpNtv1QnFhxtPd7d5k4e+T7SkW1TQ==\n-----END EC PRIVATE KEY-----\n"}} validation_context:{trusted_ca:{inline_string:"-----BEGIN CERTIFICATE-----\nMIICXDCCAgKgAwIBAgIICpZq70Z9LyUwCgYIKoZIzj0EAwIwFDESMBAGA1UEAxMJ\nVGVzdCBDQSAyMB4XDTE5MDMyMjEzNTgyNloXDTI5MDMyMjEzNTgyNlowFDESMBAG\nA1UEAxMJVGVzdCBDQSAyMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEIhywH1gx\nAsMwuF3ukAI5YL2jFxH6Usnma1HFSfVyxbXX1/uoZEYrj8yCAtdU2yoHETyd+Zx2\nThhRLP79pYegCaOCATwwggE4MA4GA1UdDwEB/wQEAwIBhjAPBgNVHRMBAf8EBTAD\nAQH/MGgGA1UdDgRhBF9kMToxMToxMTphYzoyYTpiYTo5NzpiMjozZjphYzo3Yjpi\nZDpkYTpiZTpiMTo4YTpmYzo5YTpiYTpiNTpiYzo4MzplNzo1ZTo0MTo2ZjpmMjo3\nMzo5NTo1ODowYzpkYjBqBgNVHSMEYzBhgF9kMToxMToxMTphYzoyYTpiYTo5Nzpi\nMjozZjphYzo3YjpiZDpkYTpiZTpiMTo4YTpmYzo5YTpiYTpiNTpiYzo4MzplNzo1\nZTo0MTo2ZjpmMjo3Mzo5NTo1ODowYzpkYjA/BgNVHREEODA2hjRzcGlmZmU6Ly8x\nMTExMTExMS0yMjIyLTMzMzMtNDQ0NC01NTU1NTU1NTU1NTUuY29uc3VsMAoGCCqG\nSM49BAMCA0gAMEUCICOY0i246rQHJt8o8Oya0D5PLL1FnmsQmQqIGCi31RwnAiEA\noR5f6Ku+cig2Il8T8LJujOp2/2A72QcHZA57B13y+8o=\n-----END CERTIFICATE-----\n"}}} require_client_certificate:{} \ No newline at end of file From 554ac6f56db4b6bef99d19cf3dcb0fc9d17bb306 Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Fri, 19 May 2023 12:57:16 -0500 Subject: [PATCH 21/34] Update agent/structs/config_entry_gateways.go Co-authored-by: Thomas Eckert --- agent/structs/config_entry_gateways.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/structs/config_entry_gateways.go b/agent/structs/config_entry_gateways.go index 05af66b21bf4..0be410d19870 100644 --- a/agent/structs/config_entry_gateways.go +++ b/agent/structs/config_entry_gateways.go @@ -912,7 +912,7 @@ type APIGatewayTLSConfiguration struct { CipherSuites []types.TLSCipherSuite } -// IsEmpty returns if struct is empty because field isn't currently nullable +// IsEmpty returns true if all values in the struct are nil or empty. func (a *APIGatewayTLSConfiguration) IsEmpty() bool { return len(a.Certificates) == 0 && len(a.MaxVersion) == 0 && len(a.MinVersion) == 0 && len(a.CipherSuites) == 0 } From ba3ef7040f4240b41b90b3f582f6d74777d6754d Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Fri, 19 May 2023 12:58:28 -0500 Subject: [PATCH 22/34] Update agent/consul/discoverychain/gateway.go Co-authored-by: Nathan Coleman --- 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 328431e8e8f7..bbde217e3cbf 100644 --- a/agent/consul/discoverychain/gateway.go +++ b/agent/consul/discoverychain/gateway.go @@ -199,7 +199,6 @@ func (l *GatewayChainSynthesizer) consolidateHTTPRoutes() []structs.HTTPRouteCon // 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) From d1bf338f69bf7aa815647c393189ee37435c42de Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Fri, 19 May 2023 13:29:59 -0500 Subject: [PATCH 23/34] Update agent/consul/discoverychain/gateway.go Co-authored-by: Nathan Coleman --- 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 bbde217e3cbf..ea5f786f9b41 100644 --- a/agent/consul/discoverychain/gateway.go +++ b/agent/consul/discoverychain/gateway.go @@ -217,7 +217,7 @@ func RebuildHTTPRouteUpstream(route structs.HTTPRouteConfigEntry, listener struc } } -// 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, suffix string, gateway *structs.APIGatewayConfigEntry) []structs.HTTPRouteConfigEntry { var routes []structs.HTTPRouteConfigEntry From 257ac0e67bf709890b6557f1216e88fa17ef5913 Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Fri, 19 May 2023 13:30:39 -0500 Subject: [PATCH 24/34] Update agent/proxycfg/snapshot.go Co-authored-by: Nathan Coleman --- agent/proxycfg/snapshot.go | 1 - 1 file changed, 1 deletion(-) diff --git a/agent/proxycfg/snapshot.go b/agent/proxycfg/snapshot.go index 712e0337a846..0b5ed23f70c4 100644 --- a/agent/proxycfg/snapshot.go +++ b/agent/proxycfg/snapshot.go @@ -914,7 +914,6 @@ DOMAIN_LOOP: return services, upstreams, compiled, err } -// TODO use this in listener code func (c *configSnapshotAPIGateway) toIngressTLS(key IngressListenerKey, listener structs.APIGatewayListener, bound structs.BoundAPIGatewayListener) (*structs.GatewayTLSConfig, error) { if len(listener.TLS.Certificates) == 0 { return nil, nil From b73a1cee6c6a59373ebaee925e8e40a2bb0ea4fe Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Fri, 19 May 2023 13:36:36 -0500 Subject: [PATCH 25/34] clarified header comment for FlattenHTTPRoute, changed RebuildHTTPRouteUpstream to BuildHTTPRouteUpstream --- agent/consul/discoverychain/gateway.go | 5 +++-- agent/xds/routes.go | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/agent/consul/discoverychain/gateway.go b/agent/consul/discoverychain/gateway.go index bbde217e3cbf..60ddf683ba9c 100644 --- a/agent/consul/discoverychain/gateway.go +++ b/agent/consul/discoverychain/gateway.go @@ -197,14 +197,15 @@ 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 +// FlattenHTTPRoute takes in a route and its parent config entries and consolidates the routes by hostname func FlattenHTTPRoute(route *structs.HTTPRouteConfigEntry, listener *structs.APIGatewayListener, gateway *structs.APIGatewayConfigEntry) []structs.HTTPRouteConfigEntry { 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 { +// BuilsHTTPRouteUpstream takes a route and a listener and builds out a corresponding Upstream struct +func BuildHTTPRouteUpstream(route structs.HTTPRouteConfigEntry, listener structs.APIGatewayListener) structs.Upstream { return structs.Upstream{ DestinationName: route.GetName(), DestinationNamespace: route.NamespaceOrDefault(), diff --git a/agent/xds/routes.go b/agent/xds/routes.go index 9f4ef03712d0..630d874d481d 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 := discoverychain.BuildHTTPRouteUpstream(flattenedRoute, listenerCfg) uid := proxycfg.NewUpstreamID(&upstream) chain := cfgSnap.APIGateway.DiscoveryChain[uid] if chain == nil { From b9b4e1c2e30855233a1288e011ebe9a5fc0f9e85 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Fri, 19 May 2023 15:26:15 -0500 Subject: [PATCH 26/34] simplify cert logic --- agent/xds/listeners_apigateway.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/agent/xds/listeners_apigateway.go b/agent/xds/listeners_apigateway.go index 01943191d141..b5d7bd59e635 100644 --- a/agent/xds/listeners_apigateway.go +++ b/agent/xds/listeners_apigateway.go @@ -28,22 +28,18 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro boundListener := readyUpstreams.boundListenerCfg //TODO, it's possible that this can be pulled off of the snapshot APIGateway.ListenerCertificates - // if that value is being appropriately updated so we don't have to build this out every time - certificates := make(map[proxycfg.IngressListenerKey][]structs.InlineCertificateConfigEntry) + // if that value is being appropriately updated so we don't have to build this list out every time + var certs []structs.InlineCertificateConfigEntry for _, certRef := range boundListener.Certificates { cert, ok := cfgSnap.APIGateway.Certificates.Get(certRef) if !ok { continue } - certificates[listenerKey] = append(certificates[listenerKey], *cert) + certs = append(certs, *cert) } - var certs []structs.InlineCertificateConfigEntry - if certificates != nil { - certs = certificates[listenerKey] - } - isAPIGatewayWithTLS := certs != nil + isAPIGatewayWithTLS := len(boundListener.Certificates) > 0 tlsContext, err := makeDownstreamTLSContextFromSnapshotAPIListenerConfig(cfgSnap, listenerCfg) if err != nil { From 1b413e1049cec90d09c1b3303694d8ba59c84ec3 Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Mon, 22 May 2023 10:00:32 -0500 Subject: [PATCH 27/34] Delete scratch --- agent/xds/scratch | 4 ---- 1 file changed, 4 deletions(-) delete mode 100644 agent/xds/scratch diff --git a/agent/xds/scratch b/agent/xds/scratch deleted file mode 100644 index 7d00679f72e0..000000000000 --- a/agent/xds/scratch +++ /dev/null @@ -1,4 +0,0 @@ -&{true []} -common_tls_context:{tls_params:{} tls_certificates:{certificate_chain:{inline_string:"-----BEGIN CERTIFICATE-----\nMIICjDCCAjKgAwIBAgIIC5llxGV1gB8wCgYIKoZIzj0EAwIwFDESMBAGA1UEAxMJ\nVGVzdCBDQSAyMB4XDTE5MDMyMjEzNTgyNloXDTI5MDMyMjEzNTgyNlowDjEMMAoG\nA1UEAxMDd2ViMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEADPv1RHVNRfa2VKR\nAB16b6rZnEt7tuhaxCFpQXPj7M2omb0B9Favq5E0ivpNtv1QnFhxtPd7d5k4e+T7\nSkW1TaOCAXIwggFuMA4GA1UdDwEB/wQEAwIDuDAdBgNVHSUEFjAUBggrBgEFBQcD\nAgYIKwYBBQUHAwEwDAYDVR0TAQH/BAIwADBoBgNVHQ4EYQRfN2Q6MDc6ODc6M2E6\nNDA6MTk6NDc6YzM6NWE6YzA6YmE6NjI6ZGY6YWY6NGI6ZDQ6MDU6MjU6NzY6M2Q6\nNWE6OGQ6MTY6OGQ6Njc6NWU6MmU6YTA6MzQ6N2Q6ZGM6ZmYwagYDVR0jBGMwYYBf\nZDE6MTE6MTE6YWM6MmE6YmE6OTc6YjI6M2Y6YWM6N2I6YmQ6ZGE6YmU6YjE6OGE6\nZmM6OWE6YmE6YjU6YmM6ODM6ZTc6NWU6NDE6NmY6ZjI6NzM6OTU6NTg6MGM6ZGIw\nWQYDVR0RBFIwUIZOc3BpZmZlOi8vMTExMTExMTEtMjIyMi0zMzMzLTQ0NDQtNTU1\nNTU1NTU1NTU1LmNvbnN1bC9ucy9kZWZhdWx0L2RjL2RjMS9zdmMvd2ViMAoGCCqG\nSM49BAMCA0gAMEUCIGC3TTvvjj76KMrguVyFf4tjOqaSCRie3nmHMRNNRav7AiEA\npY0heYeK9A6iOLrzqxSerkXXQyj5e9bE4VgUnxgPU6g=\n-----END CERTIFICATE-----\n"} private_key:{inline_string:"-----BEGIN EC PRIVATE KEY-----\nMHcCAQEEIMoTkpRggp3fqZzFKh82yS4LjtJI+XY+qX/7DefHFrtdoAoGCCqGSM49\nAwEHoUQDQgAEADPv1RHVNRfa2VKRAB16b6rZnEt7tuhaxCFpQXPj7M2omb0B9Fav\nq5E0ivpNtv1QnFhxtPd7d5k4e+T7SkW1TQ==\n-----END EC PRIVATE KEY-----\n"}} validation_context:{trusted_ca:{inline_string:"-----BEGIN CERTIFICATE-----\nMIICXDCCAgKgAwIBAgIICpZq70Z9LyUwCgYIKoZIzj0EAwIwFDESMBAGA1UEAxMJ\nVGVzdCBDQSAyMB4XDTE5MDMyMjEzNTgyNloXDTI5MDMyMjEzNTgyNlowFDESMBAG\nA1UEAxMJVGVzdCBDQSAyMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEIhywH1gx\nAsMwuF3ukAI5YL2jFxH6Usnma1HFSfVyxbXX1/uoZEYrj8yCAtdU2yoHETyd+Zx2\nThhRLP79pYegCaOCATwwggE4MA4GA1UdDwEB/wQEAwIBhjAPBgNVHRMBAf8EBTAD\nAQH/MGgGA1UdDgRhBF9kMToxMToxMTphYzoyYTpiYTo5NzpiMjozZjphYzo3Yjpi\nZDpkYTpiZTpiMTo4YTpmYzo5YTpiYTpiNTpiYzo4MzplNzo1ZTo0MTo2ZjpmMjo3\nMzo5NTo1ODowYzpkYjBqBgNVHSMEYzBhgF9kMToxMToxMTphYzoyYTpiYTo5Nzpi\nMjozZjphYzo3YjpiZDpkYTpiZTpiMTo4YTpmYzo5YTpiYTpiNTpiYzo4MzplNzo1\nZTo0MTo2ZjpmMjo3Mzo5NTo1ODowYzpkYjA/BgNVHREEODA2hjRzcGlmZmU6Ly8x\nMTExMTExMS0yMjIyLTMzMzMtNDQ0NC01NTU1NTU1NTU1NTUuY29uc3VsMAoGCCqG\nSM49BAMCA0gAMEUCICOY0i246rQHJt8o8Oya0D5PLL1FnmsQmQqIGCi31RwnAiEA\noR5f6Ku+cig2Il8T8LJujOp2/2A72QcHZA57B13y+8o=\n-----END CERTIFICATE-----\n"}}} require_client_certificate:{} - -common_tls_context:{tls_params:{} tls_certificates:{certificate_chain:{inline_string:"-----BEGIN CERTIFICATE-----\nMIICjDCCAjKgAwIBAgIIC5llxGV1gB8wCgYIKoZIzj0EAwIwFDESMBAGA1UEAxMJ\nVGVzdCBDQSAyMB4XDTE5MDMyMjEzNTgyNloXDTI5MDMyMjEzNTgyNlowDjEMMAoG\nA1UEAxMDd2ViMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEADPv1RHVNRfa2VKR\nAB16b6rZnEt7tuhaxCFpQXPj7M2omb0B9Favq5E0ivpNtv1QnFhxtPd7d5k4e+T7\nSkW1TaOCAXIwggFuMA4GA1UdDwEB/wQEAwIDuDAdBgNVHSUEFjAUBggrBgEFBQcD\nAgYIKwYBBQUHAwEwDAYDVR0TAQH/BAIwADBoBgNVHQ4EYQRfN2Q6MDc6ODc6M2E6\nNDA6MTk6NDc6YzM6NWE6YzA6YmE6NjI6ZGY6YWY6NGI6ZDQ6MDU6MjU6NzY6M2Q6\nNWE6OGQ6MTY6OGQ6Njc6NWU6MmU6YTA6MzQ6N2Q6ZGM6ZmYwagYDVR0jBGMwYYBf\nZDE6MTE6MTE6YWM6MmE6YmE6OTc6YjI6M2Y6YWM6N2I6YmQ6ZGE6YmU6YjE6OGE6\nZmM6OWE6YmE6YjU6YmM6ODM6ZTc6NWU6NDE6NmY6ZjI6NzM6OTU6NTg6MGM6ZGIw\nWQYDVR0RBFIwUIZOc3BpZmZlOi8vMTExMTExMTEtMjIyMi0zMzMzLTQ0NDQtNTU1\nNTU1NTU1NTU1LmNvbnN1bC9ucy9kZWZhdWx0L2RjL2RjMS9zdmMvd2ViMAoGCCqG\nSM49BAMCA0gAMEUCIGC3TTvvjj76KMrguVyFf4tjOqaSCRie3nmHMRNNRav7AiEA\npY0heYeK9A6iOLrzqxSerkXXQyj5e9bE4VgUnxgPU6g=\n-----END CERTIFICATE-----\n"} private_key:{inline_string:"-----BEGIN EC PRIVATE KEY-----\nMHcCAQEEIMoTkpRggp3fqZzFKh82yS4LjtJI+XY+qX/7DefHFrtdoAoGCCqGSM49\nAwEHoUQDQgAEADPv1RHVNRfa2VKRAB16b6rZnEt7tuhaxCFpQXPj7M2omb0B9Fav\nq5E0ivpNtv1QnFhxtPd7d5k4e+T7SkW1TQ==\n-----END EC PRIVATE KEY-----\n"}} validation_context:{trusted_ca:{inline_string:"-----BEGIN CERTIFICATE-----\nMIICXDCCAgKgAwIBAgIICpZq70Z9LyUwCgYIKoZIzj0EAwIwFDESMBAGA1UEAxMJ\nVGVzdCBDQSAyMB4XDTE5MDMyMjEzNTgyNloXDTI5MDMyMjEzNTgyNlowFDESMBAG\nA1UEAxMJVGVzdCBDQSAyMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEIhywH1gx\nAsMwuF3ukAI5YL2jFxH6Usnma1HFSfVyxbXX1/uoZEYrj8yCAtdU2yoHETyd+Zx2\nThhRLP79pYegCaOCATwwggE4MA4GA1UdDwEB/wQEAwIBhjAPBgNVHRMBAf8EBTAD\nAQH/MGgGA1UdDgRhBF9kMToxMToxMTphYzoyYTpiYTo5NzpiMjozZjphYzo3Yjpi\nZDpkYTpiZTpiMTo4YTpmYzo5YTpiYTpiNTpiYzo4MzplNzo1ZTo0MTo2ZjpmMjo3\nMzo5NTo1ODowYzpkYjBqBgNVHSMEYzBhgF9kMToxMToxMTphYzoyYTpiYTo5Nzpi\nMjozZjphYzo3YjpiZDpkYTpiZTpiMTo4YTpmYzo5YTpiYTpiNTpiYzo4MzplNzo1\nZTo0MTo2ZjpmMjo3Mzo5NTo1ODowYzpkYjA/BgNVHREEODA2hjRzcGlmZmU6Ly8x\nMTExMTExMS0yMjIyLTMzMzMtNDQ0NC01NTU1NTU1NTU1NTUuY29uc3VsMAoGCCqG\nSM49BAMCA0gAMEUCICOY0i246rQHJt8o8Oya0D5PLL1FnmsQmQqIGCi31RwnAiEA\noR5f6Ku+cig2Il8T8LJujOp2/2A72QcHZA57B13y+8o=\n-----END CERTIFICATE-----\n"}}} require_client_certificate:{} \ No newline at end of file From c905897af41dde88c1b66f3251aa26d2407de01b Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Mon, 22 May 2023 11:07:52 -0500 Subject: [PATCH 28/34] revert route related changes in listener PR --- agent/consul/discoverychain/gateway.go | 21 ------ agent/xds/routes.go | 93 ++------------------------ 2 files changed, 7 insertions(+), 107 deletions(-) diff --git a/agent/consul/discoverychain/gateway.go b/agent/consul/discoverychain/gateway.go index 0361afbbe098..200ae34b3ffe 100644 --- a/agent/consul/discoverychain/gateway.go +++ b/agent/consul/discoverychain/gateway.go @@ -197,27 +197,6 @@ 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 consolidates the routes by hostname -func FlattenHTTPRoute(route *structs.HTTPRouteConfigEntry, listener *structs.APIGatewayListener, gateway *structs.APIGatewayConfigEntry) []structs.HTTPRouteConfigEntry { - matches := map[string][]hostnameMatch{} - matches = getHostMatches(listener.GetHostname(), route, matches) - return consolidateHTTPRoutes(matches, listener.Name, gateway) -} - -// BuilsHTTPRouteUpstream takes a route and a listener and builds out a corresponding Upstream struct -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), - }, - } -} - // 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 { diff --git a/agent/xds/routes.go b/agent/xds/routes.go index 630d874d481d..30907002e80f 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" @@ -37,7 +36,13 @@ func (s *ResourceGenerator) routesFromSnapshot(cfgSnap *proxycfg.ConfigSnapshot) case structs.ServiceKindIngressGateway: return s.routesForIngressGateway(cfgSnap) case structs.ServiceKindAPIGateway: - return s.routesForAPIGateway(cfgSnap) + // 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) case structs.ServiceKindTerminatingGateway: return s.routesForTerminatingGateway(cfgSnap) case structs.ServiceKindMeshGateway: @@ -425,75 +430,6 @@ 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.BuildHTTPRouteUpstream(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 { @@ -580,11 +516,6 @@ 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, @@ -1088,16 +1019,6 @@ 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 ff3f6b330e9dfe950a15bdbf84b628f14c6b9f49 Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Mon, 22 May 2023 11:08:32 -0500 Subject: [PATCH 29/34] 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 200ae34b3ffe..054e892e02bc 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 7180fbcbbfa773e9b9a03dd38453e707b537d64d Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Mon, 22 May 2023 11:08:56 -0500 Subject: [PATCH 30/34] Update agent/proxycfg/snapshot.go --- agent/proxycfg/snapshot.go | 1 - 1 file changed, 1 deletion(-) diff --git a/agent/proxycfg/snapshot.go b/agent/proxycfg/snapshot.go index 0b5ed23f70c4..91d41b522238 100644 --- a/agent/proxycfg/snapshot.go +++ b/agent/proxycfg/snapshot.go @@ -876,7 +876,6 @@ DOMAIN_LOOP: } synthesizer.AddTCPRoute(*route) for _, service := range route.GetServices() { - id := NewUpstreamIDFromServiceName(structs.NewServiceName(service.Name, &service.EnterpriseMeta)) if chain := c.DiscoveryChain[id]; chain != nil { chains = append(chains, chain) From 1700631a3074a479f5569d8b6580aeae8fe7dea4 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Mon, 22 May 2023 11:09:46 -0500 Subject: [PATCH 31/34] clean up uneeded extra lines in endpoints --- agent/xds/endpoints.go | 1 - 1 file changed, 1 deletion(-) diff --git a/agent/xds/endpoints.go b/agent/xds/endpoints.go index e04ee27affe8..113038ff4ea3 100644 --- a/agent/xds/endpoints.go +++ b/agent/xds/endpoints.go @@ -544,7 +544,6 @@ 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, ok := cfgSnap.APIGateway.Upstreams[routeRef] if !ok { continue From 967fe111ac4216e7cc221aff2e90025fc945ecfe Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Mon, 22 May 2023 15:21:35 -0500 Subject: [PATCH 32/34] clean up extra function --- agent/xds/listeners_apigateway.go | 8 +-- agent/xds/listeners_ingress.go | 105 ------------------------------ 2 files changed, 3 insertions(+), 110 deletions(-) diff --git a/agent/xds/listeners_apigateway.go b/agent/xds/listeners_apigateway.go index b5d7bd59e635..7ab7a66529bf 100644 --- a/agent/xds/listeners_apigateway.go +++ b/agent/xds/listeners_apigateway.go @@ -105,7 +105,7 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro if isAPIGatewayWithTLS { // construct SNI filter chains - l.FilterChains, err = makeInlineOverrideFilterChainsAPIGateway(cfgSnap, cfgSnap.APIGateway.TLSConfig, listenerKey.Protocol, listenerFilterOpts{ + l.FilterChains, err = makeInlineOverrideFilterChains(cfgSnap, cfgSnap.APIGateway.TLSConfig, listenerKey.Protocol, listenerFilterOpts{ useRDS: useRDS, protocol: listenerKey.Protocol, routeName: listenerKey.RouteName(), @@ -158,7 +158,7 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro sniFilterChains := []*envoy_listener_v3.FilterChain{} if isAPIGatewayWithTLS { - sniFilterChains, err = makeInlineOverrideFilterChainsAPIGateway(cfgSnap, cfgSnap.IngressGateway.TLSConfig, listenerKey.Protocol, filterOpts, certs) + sniFilterChains, err = makeInlineOverrideFilterChains(cfgSnap, cfgSnap.IngressGateway.TLSConfig, listenerKey.Protocol, filterOpts, certs) if err != nil { return nil, err } @@ -293,9 +293,7 @@ func routeNameForAPIGatewayUpstream(l structs.IngressListener, s structs.Ingress // when we have multiple certificates on a single listener, we need // to duplicate the filter chains with multiple TLS contexts - -// TODO this function and makeInlineOverrideFilterChains can be consolidated since it only uses the protocol from listener key -func makeInlineOverrideFilterChainsAPIGateway(cfgSnap *proxycfg.ConfigSnapshot, +func makeInlineOverrideFilterChains(cfgSnap *proxycfg.ConfigSnapshot, tlsCfg structs.GatewayTLSConfig, protocol string, filterOpts listenerFilterOpts, diff --git a/agent/xds/listeners_ingress.go b/agent/xds/listeners_ingress.go index d61b4aeec5df..ea4c311cddbc 100644 --- a/agent/xds/listeners_ingress.go +++ b/agent/xds/listeners_ingress.go @@ -385,111 +385,6 @@ func makeSDSOverrideFilterChains(cfgSnap *proxycfg.ConfigSnapshot, return chains, nil } -// when we have multiple certificates on a single listener, we need -// to duplicate the filter chains with multiple TLS contexts -func makeInlineOverrideFilterChains(cfgSnap *proxycfg.ConfigSnapshot, - tlsCfg structs.GatewayTLSConfig, - listenerKey proxycfg.IngressListenerKey, - filterOpts listenerFilterOpts, - certs []structs.InlineCertificateConfigEntry) ([]*envoy_listener_v3.FilterChain, error) { - - listenerCfg, ok := cfgSnap.IngressGateway.Listeners[listenerKey] - if !ok { - return nil, fmt.Errorf("no listener config found for listener on port %d", listenerKey.Port) - } - - var chains []*envoy_listener_v3.FilterChain - - constructChain := func(name string, hosts []string, tlsContext *envoy_tls_v3.CommonTlsContext) error { - filterOpts.filterName = name - filter, err := makeListenerFilter(filterOpts) - if err != nil { - return err - } - - // Configure alpn protocols on TLSContext - tlsContext.AlpnProtocols = getAlpnProtocols(listenerCfg.Protocol) - transportSocket, err := makeDownstreamTLSTransportSocket(&envoy_tls_v3.DownstreamTlsContext{ - CommonTlsContext: tlsContext, - RequireClientCertificate: &wrapperspb.BoolValue{Value: false}, - }) - if err != nil { - return err - } - - chains = append(chains, &envoy_listener_v3.FilterChain{ - FilterChainMatch: makeSNIFilterChainMatch(hosts...), - Filters: []*envoy_listener_v3.Filter{ - filter, - }, - TransportSocket: transportSocket, - }) - - return nil - } - - multipleCerts := len(certs) > 1 - - allCertHosts := map[string]struct{}{} - overlappingHosts := map[string]struct{}{} - - if multipleCerts { - // we only need to prune out overlapping hosts if we have more than - // one certificate - for _, cert := range certs { - hosts, err := cert.Hosts() - if err != nil { - return nil, fmt.Errorf("unable to parse hosts from x509 certificate: %v", hosts) - } - for _, host := range hosts { - if _, ok := allCertHosts[host]; ok { - overlappingHosts[host] = struct{}{} - } - allCertHosts[host] = struct{}{} - } - } - } - - for _, cert := range certs { - var hosts []string - - // if we only have one cert, we just use it for all ingress - if multipleCerts { - // otherwise, we need an SNI per cert and to fallback to our ingress - // gateway certificate signed by our Consul CA - certHosts, err := cert.Hosts() - if err != nil { - return nil, fmt.Errorf("unable to parse hosts from x509 certificate: %v", hosts) - } - // filter out any overlapping hosts so we don't have collisions in our filter chains - for _, host := range certHosts { - if _, ok := overlappingHosts[host]; !ok { - hosts = append(hosts, host) - } - } - - if len(hosts) == 0 { - // all of our hosts are overlapping, so we just skip this filter and it'll be - // handled by the default filter chain - continue - } - } - - if err := constructChain(cert.Name, hosts, makeInlineTLSContextFromGatewayTLSConfig(tlsCfg, cert)); err != nil { - return nil, err - } - } - - if multipleCerts { - // if we have more than one cert, add a default handler that uses the leaf cert from connect - if err := constructChain("default", nil, makeCommonTLSContext(cfgSnap.Leaf(), cfgSnap.RootPEMs(), makeTLSParametersFromGatewayTLSConfig(tlsCfg))); err != nil { - return nil, err - } - } - - return chains, nil -} - func makeTLSParametersFromGatewayTLSConfig(tlsCfg structs.GatewayTLSConfig) *envoy_tls_v3.TlsParameters { return makeTLSParametersFromTLSConfig(tlsCfg.TLSMinVersion, tlsCfg.TLSMaxVersion, tlsCfg.CipherSuites) } From 3826310c5941528c44998b6cba48ec4ac80b75d1 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Mon, 22 May 2023 16:38:48 -0400 Subject: [PATCH 33/34] Replace TODO with FUTURE comment for removal --- agent/proxycfg/snapshot.go | 3 ++- agent/xds/listeners_apigateway.go | 3 --- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/agent/proxycfg/snapshot.go b/agent/proxycfg/snapshot.go index 49e3195bced9..1baa155d5a0b 100644 --- a/agent/proxycfg/snapshot.go +++ b/agent/proxycfg/snapshot.go @@ -736,6 +736,7 @@ type configSnapshotAPIGateway struct { // entry to save us trying to pass fields through Upstreams Listeners map[string]structs.APIGatewayListener // this acts as an intermediary for inlining certificates + // FUTURE(nathancoleman) Remove when ToIngress is removed ListenerCertificates map[IngressListenerKey][]structs.InlineCertificateConfigEntry BoundListeners map[string]structs.BoundAPIGatewayListener @@ -745,7 +746,7 @@ type configSnapshotAPIGateway struct { // This is temporary, for the sake of re-using existing codepaths when integrating // Consul API Gateway into Consul core. // -// FUTURE: Remove when API gateways have custom snapshot generation +// FUTURE(nathancoleman): Remove when API gateways have custom snapshot generation func (c *configSnapshotAPIGateway) ToIngress(datacenter string) (configSnapshotIngressGateway, error) { // Convert API Gateway Listeners to Ingress Listeners. ingressListeners := make(map[IngressListenerKey]structs.IngressListener, len(c.Listeners)) diff --git a/agent/xds/listeners_apigateway.go b/agent/xds/listeners_apigateway.go index 7ab7a66529bf..e3b55c8f24b1 100644 --- a/agent/xds/listeners_apigateway.go +++ b/agent/xds/listeners_apigateway.go @@ -27,9 +27,6 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro listenerKey := readyUpstreams.listenerKey boundListener := readyUpstreams.boundListenerCfg - //TODO, it's possible that this can be pulled off of the snapshot APIGateway.ListenerCertificates - // if that value is being appropriately updated so we don't have to build this list out every time - var certs []structs.InlineCertificateConfigEntry for _, certRef := range boundListener.Certificates { cert, ok := cfgSnap.APIGateway.Certificates.Get(certRef) From 92351d50917e2fce27abb9b19d679cc8ca7c048b Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Mon, 22 May 2023 16:47:20 -0400 Subject: [PATCH 34/34] Remove inapplicable TODO --- agent/xds/listeners_apigateway.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/agent/xds/listeners_apigateway.go b/agent/xds/listeners_apigateway.go index e3b55c8f24b1..ed6cc6a9e0c4 100644 --- a/agent/xds/listeners_apigateway.go +++ b/agent/xds/listeners_apigateway.go @@ -148,8 +148,6 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro logger: s.Logger, } - //TODO equivalent of makeSDSOverrideFilterChains, when needed - // Generate any filter chains needed for services with custom TLS certs // via SDS. sniFilterChains := []*envoy_listener_v3.FilterChain{} @@ -193,9 +191,7 @@ func (s *ResourceGenerator) makeAPIGatewayListeners(address string, cfgSnap *pro }) } resources = append(resources, listener) - } - } return resources, nil