Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xds: generate listeners directly from API gateway snapshot #17398

Merged
merged 40 commits into from
May 22, 2023
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
af41a67
API Gateway XDS Primitives, endpoints and clusters (#17002)
sarahalsmiller Apr 26, 2023
bdae6b2
Routes for API Gateway (#17158)
sarahalsmiller May 9, 2023
36c0d63
checkpoint, skeleton, tests not passing
sarahalsmiller May 15, 2023
c7d6f79
checkpoint
sarahalsmiller May 17, 2023
463c5fe
endpoints xds cluster configuration
sarahalsmiller May 17, 2023
94188ea
resources test fix
sarahalsmiller May 17, 2023
ed82d8c
fix reversion in resources_test
sarahalsmiller May 17, 2023
e3f1e85
checkpoint
sarahalsmiller May 17, 2023
8f5ea9e
Update agent/proxycfg/api_gateway.go
sarahalsmiller May 17, 2023
890aaed
unit tests passing
sarahalsmiller May 17, 2023
8728cac
gofmt
sarahalsmiller May 17, 2023
562c789
add deterministic sorting to appease the unit test gods
sarahalsmiller May 18, 2023
077741d
remove panic
sarahalsmiller May 18, 2023
029ae91
Find ready upstream matching listener instead of first in list
nathancoleman May 18, 2023
3204325
Clean up, improve TODO
nathancoleman May 18, 2023
a13a37d
Modify getReadyUpstreams to filter upstreams by listener (#17410)
nathancoleman May 18, 2023
b302597
clean up todos, references to api gateway in listeners_ingress
sarahalsmiller May 18, 2023
6cdd69a
Merge branch 'NET-3673-endpointsFromSnapshotAPIGateway' into NET-3671…
sarahalsmiller May 18, 2023
61e78ee
merge in Nathan's fix
sarahalsmiller May 18, 2023
2f794e6
Update agent/consul/discoverychain/gateway.go
sarahalsmiller May 19, 2023
c22a4a3
cleanup current todos, remove snapshot manipulation from generation code
sarahalsmiller May 19, 2023
554ac6f
Update agent/structs/config_entry_gateways.go
sarahalsmiller May 19, 2023
ba3ef70
Update agent/consul/discoverychain/gateway.go
sarahalsmiller May 19, 2023
d1bf338
Update agent/consul/discoverychain/gateway.go
sarahalsmiller May 19, 2023
257ac0e
Update agent/proxycfg/snapshot.go
sarahalsmiller May 19, 2023
b73a1ce
clarified header comment for FlattenHTTPRoute, changed RebuildHTTPRou…
sarahalsmiller May 19, 2023
0d3454c
Merge branch 'NET-3671-makeAPIGatewayListeners' of github.com:hashico…
sarahalsmiller May 19, 2023
606afa2
Merge branch 'main' into NET-3671-makeAPIGatewayListeners
sarahalsmiller May 19, 2023
b9b4e1c
simplify cert logic
sarahalsmiller May 19, 2023
1b413e1
Delete scratch
sarahalsmiller May 22, 2023
c905897
revert route related changes in listener PR
sarahalsmiller May 22, 2023
ff3f6b3
Update agent/consul/discoverychain/gateway.go
sarahalsmiller May 22, 2023
7180fbc
Update agent/proxycfg/snapshot.go
sarahalsmiller May 22, 2023
1700631
clean up uneeded extra lines in endpoints
sarahalsmiller May 22, 2023
9c30c40
Merge branch 'NET-3671-makeAPIGatewayListeners' of github.com:hashico…
sarahalsmiller May 22, 2023
c47dd2a
Merge branch 'main' into NET-3671-makeAPIGatewayListeners
sarahalsmiller May 22, 2023
c4c8e9d
Merge branch 'main' into NET-3671-makeAPIGatewayListeners
sarahalsmiller May 22, 2023
967fe11
clean up extra function
sarahalsmiller May 22, 2023
3826310
Replace TODO with FUTURE comment for removal
nathancoleman May 22, 2023
92351d5
Remove inapplicable TODO
nathancoleman May 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 24 additions & 10 deletions agent/consul/discoverychain/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,13 @@ 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)
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{}
}
Expand Down Expand Up @@ -90,8 +94,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
sarahalsmiller marked this conversation as resolved.
Show resolved Hide resolved
return currentMatches
}

// Synthesize assembles a synthetic discovery chain from multiple other discovery chains
Expand All @@ -116,6 +122,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{
Expand All @@ -126,7 +133,6 @@ func (l *GatewayChainSynthesizer) Synthesize(chains ...*structs.CompiledDiscover
EvaluateInTrustDomain: l.trustDomain,
Entries: entries,
})

if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -188,17 +194,23 @@ 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)
}

// 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
Expand Down Expand Up @@ -258,12 +270,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)
}

Expand Down
2 changes: 1 addition & 1 deletion agent/consul/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion agent/proxycfg/api_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -445,6 +444,7 @@ func (h *handlerAPIGateway) recompileDiscoveryChains(snap *ConfigSnapshot) error

for i, service := range services {
id := NewUpstreamIDFromServiceName(structs.NewServiceName(service.Name, &service.EnterpriseMeta))

if compiled[i].ServiceName != service.Name {
return fmt.Errorf("Compiled Discovery chain for %s does not match service %s", compiled[i].ServiceName, id)
}
Expand Down
1 change: 1 addition & 0 deletions agent/proxycfg/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,7 @@ DOMAIN_LOOP:
}
synthesizer.AddTCPRoute(*route)
for _, service := range route.GetServices() {

sarahalsmiller marked this conversation as resolved.
Show resolved Hide resolved
id := NewUpstreamIDFromServiceName(structs.NewServiceName(service.Name, &service.EnterpriseMeta))
if chain := c.DiscoveryChain[id]; chain != nil {
chains = append(chains, chain)
Expand Down
5 changes: 5 additions & 0 deletions agent/structs/config_entry_gateways.go
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,11 @@ type APIGatewayTLSConfiguration struct {
CipherSuites []types.TLSCipherSuite
}

// 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
}

// 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
Expand Down
46 changes: 39 additions & 7 deletions agent/xds/clusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -816,6 +810,44 @@ 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)
readyUpstreamsList := getReadyUpstreams(cfgSnap)

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
// 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 {
clusters = append(clusters, cluster)
}

createdClusters[uid] = true
}
}
return clusters, nil
}

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) {
Expand Down
2 changes: 2 additions & 0 deletions agent/xds/endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,7 @@ 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
Expand Down Expand Up @@ -606,6 +607,7 @@ func (s *ResourceGenerator) endpointsFromSnapshotAPIGateway(cfgSnap *proxycfg.Co
if err != nil {
return nil, err
}

resources = append(resources, endpoints...)
createdClusters[uid] = struct{}{}
}
Expand Down
9 changes: 2 additions & 7 deletions agent/xds/listeners.go
Original file line number Diff line number Diff line change
Expand Up @@ -860,16 +860,11 @@ 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)
listeners, err := s.makeAPIGatewayListeners(a.Address, cfgSnap)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the crux of the whole change that we're making: instead of converting to an ingress gateway snapshot and generating xDS resources from that, we generate xDS resources directly from our API gateway snapshot 🎉

if err != nil {
return nil, err
}

resources = append(resources, listeners...)
case structs.ServiceKindIngressGateway:
listeners, err := s.makeIngressGatewayListeners(a.Address, cfgSnap)
Expand Down
Loading