From 972b20aee7c764217d8772502856aca5333c59c1 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Wed, 26 Jul 2023 13:45:13 -0400 Subject: [PATCH 1/3] api-gateway: subscribe to bound-api-gateway only after receiving api-gateway This fixes a race condition due to our dependency on having the listener(s) from the api-gateway config entry in order to fully and properly process the resources on the bound-api-gateway config entry. --- agent/proxycfg/api_gateway.go | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/agent/proxycfg/api_gateway.go b/agent/proxycfg/api_gateway.go index 41eb5921259e..35bbc8759618 100644 --- a/agent/proxycfg/api_gateway.go +++ b/agent/proxycfg/api_gateway.go @@ -54,12 +54,6 @@ func (h *handlerAPIGateway) initialize(ctx context.Context) (ConfigSnapshot, err return snap, err } - // Watch the bound-api-gateway's config entry - err = h.subscribeToConfigEntry(ctx, structs.BoundAPIGateway, h.service, h.proxyID.EnterpriseMeta, boundGatewayConfigWatchID) - if err != nil { - return snap, err - } - snap.APIGateway.Listeners = make(map[string]structs.APIGatewayListener) snap.APIGateway.BoundListeners = make(map[string]structs.BoundAPIGatewayListener) snap.APIGateway.HTTPRoutes = watch.NewMap[structs.ResourceReference, *structs.HTTPRouteConfigEntry]() @@ -143,10 +137,12 @@ func (h *handlerAPIGateway) handleRootCAUpdate(u UpdateEvent, snap *ConfigSnapsh return nil } -// handleGatewayConfigUpdate responds to changes in the watched config entry for a gateway. -// In particular, we want to make sure that we're subscribing to any attached resources such -// as routes and certificates. These additional subscriptions will enable us to update the -// config snapshot appropriately for any route or certificate changes. +// handleGatewayConfigUpdate responds to changes in the watched config entries for a gateway. +// Once the base api-gateway config entry has been seen, we store the list of listeners and +// then subscribe to the corresponding bound-api-gateway config entry. We use the bound-api-gateway +// config entry to subscribing to any attached resources, including routes and certificates. +// These additional subscriptions will enable us to update the config snapshot appropriately +// for any route or certificate changes. func (h *handlerAPIGateway) handleGatewayConfigUpdate(ctx context.Context, u UpdateEvent, snap *ConfigSnapshot, correlationID string) error { resp, ok := u.Result.(*structs.ConfigEntryResponse) if !ok { @@ -244,6 +240,12 @@ func (h *handlerAPIGateway) handleGatewayConfigUpdate(ctx context.Context, u Upd } snap.APIGateway.GatewayConfigLoaded = true + + // Watch the corresponding bound-api-gateway's config entry + err := h.subscribeToConfigEntry(ctx, structs.BoundAPIGateway, h.service, h.proxyID.EnterpriseMeta, boundGatewayConfigWatchID) + if err != nil { + return err + } break default: return fmt.Errorf("invalid type for config entry: %T", resp.Entry) From 275681714280e27228a1f9f5ce68c1b1ddb3c33e Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Wed, 26 Jul 2023 13:50:55 -0400 Subject: [PATCH 2/3] Apply suggestions from code review --- agent/proxycfg/api_gateway.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/proxycfg/api_gateway.go b/agent/proxycfg/api_gateway.go index 35bbc8759618..b4954cd3973c 100644 --- a/agent/proxycfg/api_gateway.go +++ b/agent/proxycfg/api_gateway.go @@ -140,7 +140,7 @@ func (h *handlerAPIGateway) handleRootCAUpdate(u UpdateEvent, snap *ConfigSnapsh // handleGatewayConfigUpdate responds to changes in the watched config entries for a gateway. // Once the base api-gateway config entry has been seen, we store the list of listeners and // then subscribe to the corresponding bound-api-gateway config entry. We use the bound-api-gateway -// config entry to subscribing to any attached resources, including routes and certificates. +// config entry to subscribe to any attached resources, including routes and certificates. // These additional subscriptions will enable us to update the config snapshot appropriately // for any route or certificate changes. func (h *handlerAPIGateway) handleGatewayConfigUpdate(ctx context.Context, u UpdateEvent, snap *ConfigSnapshot, correlationID string) error { @@ -241,7 +241,7 @@ func (h *handlerAPIGateway) handleGatewayConfigUpdate(ctx context.Context, u Upd snap.APIGateway.GatewayConfigLoaded = true - // Watch the corresponding bound-api-gateway's config entry + // Watch the corresponding bound-api-gateway config entry err := h.subscribeToConfigEntry(ctx, structs.BoundAPIGateway, h.service, h.proxyID.EnterpriseMeta, boundGatewayConfigWatchID) if err != nil { return err From 72c8f1e4db8c8204a71d0c85b6f9900c231c495a Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Wed, 26 Jul 2023 15:11:48 -0400 Subject: [PATCH 3/3] Add changelog entry --- .changelog/18291.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/18291.txt diff --git a/.changelog/18291.txt b/.changelog/18291.txt new file mode 100644 index 000000000000..bb0ec6f89295 --- /dev/null +++ b/.changelog/18291.txt @@ -0,0 +1,3 @@ +```release-note:bug +api-gateway: fix race condition in proxy config generation when Consul is notified of the bound-api-gateway config entry before it is notified of the api-gateway config entry. +```