luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

[API Gateway] Fix infinite loop in controller and binding non-accepted routes and gateways (#16377)

This commit is contained in:
Andrew Stucki 2023-02-22 14:55:40 -05:00 committed by GitHub
parent 1832998d7d
commit 5e939ae527
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 286 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
agent/consul/gateways/controller_gateways.go
View File

@ -409,7 +409,6 @@ func (r *apiGatewayReconciler) reconcileRoute(_ context.Context, req controller.
}
var triggerOnce sync.Once
validTargets := true
for _, service := range route.GetServiceNames() {
_, chainSet, err := store.ReadDiscoveryChainConfigEntries(ws, service.Name, pointerTo(service.EnterpriseMeta))
if err != nil {
@ -422,11 +421,6 @@ func (r *apiGatewayReconciler) reconcileRoute(_ context.Context, req controller.
r.controller.AddTrigger(req, ws.WatchCtx)
})
if chainSet.IsEmpty() {
updater.SetCondition(conditions.routeInvalidDiscoveryChain(errServiceDoesNotExist))
continue
}
// make sure that we can actually compile a discovery chain based on this route
// the main check is to make sure that all of the protocols align
chain, err := discoverychain.Compile(discoverychain.CompileRequest{
@ -438,28 +432,16 @@ func (r *apiGatewayReconciler) reconcileRoute(_ context.Context, req controller.
Entries: chainSet,
})
if err != nil {
// we only really need to return the first error for an invalid
// discovery chain, but we still want to set watches on everything in the
// store
if validTargets {
updater.SetCondition(conditions.routeInvalidDiscoveryChain(err))
validTargets = false
}
updater.SetCondition(conditions.routeInvalidDiscoveryChain(err))
continue
}
if chain.Protocol != string(route.GetProtocol()) {
if validTargets {
updater.SetCondition(conditions.routeInvalidDiscoveryChain(errInvalidProtocol))
validTargets = false
}
updater.SetCondition(conditions.routeInvalidDiscoveryChain(errInvalidProtocol))
continue
}
// this makes sure we don't override an already set status
if validTargets {
updater.SetCondition(conditions.routeAccepted())
}
updater.SetCondition(conditions.routeAccepted())
}
// if we have no upstream targets, then set the route as invalid
@ -467,17 +449,6 @@ func (r *apiGatewayReconciler) reconcileRoute(_ context.Context, req controller.
// we'll do it here too just in case
if len(route.GetServiceNames()) == 0 {
updater.SetCondition(conditions.routeNoUpstreams())
validTargets = false
}
if !validTargets {
// we return early, but need to make sure we're removed from all referencing
// gateways and our status is updated properly
updated := []*structs.BoundAPIGatewayConfigEntry{}
for _, modifiedGateway := range removeRoute(requestToResourceRef(req), meta...) {
updated = append(updated, modifiedGateway.BoundGateway)
}
return finalize(updated)
}
// the route is valid, attempt to bind it to all gateways
@ -575,6 +546,8 @@ type gatewayMeta struct {
// the map values are pointers so that we can update them directly
// and have the changes propagate back to the container gateways.
boundListeners map[string]*structs.BoundAPIGatewayListener
generator *gatewayConditionGenerator
}
// getAllGatewayMeta returns a pre-constructed list of all valid gateway and state
@ -701,11 +674,22 @@ func (g *gatewayMeta) bindRoute(listener *structs.APIGatewayListener, bound *str
return false, nil
}
// check to make sure we're not binding to an invalid gateway
if !g.Gateway.Status.MatchesConditionStatus(g.generator.gatewayAccepted()) {
return false, fmt.Errorf("failed to bind route to gateway %s: gateway has not been accepted", g.Gateway.Name)
}
// check to make sure we're not binding to an invalid route
status := route.GetStatus()
if !status.MatchesConditionStatus(g.generator.routeAccepted()) {
return false, fmt.Errorf("failed to bind route to gateway %s: route has not been accepted", g.Gateway.Name)
}
if route, ok := route.(*structs.HTTPRouteConfigEntry); ok {
// check our hostnames
hostnames := route.FilteredHostnames(listener.GetHostname())
if len(hostnames) == 0 {
return false, fmt.Errorf("failed to bind route to gateway %s: listener %s is does not have any hostnames that match the route", route.GetName(), g.Gateway.Name)
return false, fmt.Errorf("failed to bind route to gateway %s: listener %s is does not have any hostnames that match the route", g.Gateway.Name, listener.Name)
}
}
@ -809,6 +793,8 @@ func (g *gatewayMeta) setConflicts(updater *structs.StatusUpdater) {
// initialize sets up the listener maps that we use for quickly indexing the listeners in our binding logic
func (g *gatewayMeta) initialize() *gatewayMeta {
g.generator = newGatewayConditionGenerator()
// set up the maps for fast access
g.boundListeners = make(map[string]*structs.BoundAPIGatewayListener, len(g.BoundGateway.Listeners))
for i, listener := range g.BoundGateway.Listeners {

259
agent/consul/gateways/controller_gateways_test.go
View File

@ -49,6 +49,11 @@ func TestBoundAPIGatewayBindRoute(t *testing.T) {
Protocol: structs.ListenerProtocolTCP,
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().gatewayAccepted(),
},
},
},
},
route: &structs.TCPRouteConfigEntry{
@ -61,6 +66,11 @@ func TestBoundAPIGatewayBindRoute(t *testing.T) {
SectionName: "Listener",
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
expectedBoundGateway: structs.BoundAPIGatewayConfigEntry{
Kind: structs.BoundAPIGateway,
@ -116,6 +126,11 @@ func TestBoundAPIGatewayBindRoute(t *testing.T) {
Protocol: structs.ListenerProtocolTCP,
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().gatewayAccepted(),
},
},
},
},
route: &structs.TCPRouteConfigEntry{
@ -127,6 +142,11 @@ func TestBoundAPIGatewayBindRoute(t *testing.T) {
Name: "Gateway",
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
expectedBoundGateway: structs.BoundAPIGatewayConfigEntry{
Kind: structs.BoundAPIGateway,
@ -417,6 +437,11 @@ func TestBindRoutesToGateways(t *testing.T) {
Protocol: structs.ListenerProtocolTCP,
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().gatewayAccepted(),
},
},
},
},
},
@ -431,6 +456,11 @@ func TestBindRoutesToGateways(t *testing.T) {
SectionName: "Listener",
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
},
expectedBoundAPIGateways: []*structs.BoundAPIGatewayConfigEntry{
@ -521,6 +551,11 @@ func TestBindRoutesToGateways(t *testing.T) {
Protocol: structs.ListenerProtocolTCP,
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().gatewayAccepted(),
},
},
},
},
{
@ -541,6 +576,11 @@ func TestBindRoutesToGateways(t *testing.T) {
Protocol: structs.ListenerProtocolTCP,
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().gatewayAccepted(),
},
},
},
},
},
@ -560,6 +600,11 @@ func TestBindRoutesToGateways(t *testing.T) {
SectionName: "Listener",
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
},
expectedBoundAPIGateways: []*structs.BoundAPIGatewayConfigEntry{
@ -624,6 +669,11 @@ func TestBindRoutesToGateways(t *testing.T) {
Protocol: structs.ListenerProtocolTCP,
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().gatewayAccepted(),
},
},
},
},
},
@ -638,6 +688,11 @@ func TestBindRoutesToGateways(t *testing.T) {
SectionName: "Listener 2",
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
},
expectedBoundAPIGateways: []*structs.BoundAPIGatewayConfigEntry{
@ -691,6 +746,11 @@ func TestBindRoutesToGateways(t *testing.T) {
Protocol: structs.ListenerProtocolTCP,
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().gatewayAccepted(),
},
},
},
},
},
@ -705,6 +765,11 @@ func TestBindRoutesToGateways(t *testing.T) {
SectionName: "",
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
},
expectedBoundAPIGateways: []*structs.BoundAPIGatewayConfigEntry{
@ -770,6 +835,11 @@ func TestBindRoutesToGateways(t *testing.T) {
Protocol: structs.ListenerProtocolTCP,
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().gatewayAccepted(),
},
},
},
},
},
@ -784,6 +854,11 @@ func TestBindRoutesToGateways(t *testing.T) {
SectionName: "",
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
},
expectedBoundAPIGateways: []*structs.BoundAPIGatewayConfigEntry{
@ -843,6 +918,11 @@ func TestBindRoutesToGateways(t *testing.T) {
Protocol: structs.ListenerProtocolTCP,
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().gatewayAccepted(),
},
},
},
},
{
@ -871,6 +951,11 @@ func TestBindRoutesToGateways(t *testing.T) {
Protocol: structs.ListenerProtocolTCP,
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().gatewayAccepted(),
},
},
},
},
},
@ -890,6 +975,11 @@ func TestBindRoutesToGateways(t *testing.T) {
SectionName: "Listener 2",
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
},
expectedBoundAPIGateways: []*structs.BoundAPIGatewayConfigEntry{
@ -968,6 +1058,11 @@ func TestBindRoutesToGateways(t *testing.T) {
Protocol: structs.ListenerProtocolTCP,
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().gatewayAccepted(),
},
},
},
},
},
@ -982,6 +1077,11 @@ func TestBindRoutesToGateways(t *testing.T) {
SectionName: "Listener 2",
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
},
expectedBoundAPIGateways: []*structs.BoundAPIGatewayConfigEntry{
@ -1027,6 +1127,11 @@ func TestBindRoutesToGateways(t *testing.T) {
Protocol: structs.ListenerProtocolTCP,
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().gatewayAccepted(),
},
},
},
},
{
@ -1047,6 +1152,11 @@ func TestBindRoutesToGateways(t *testing.T) {
Protocol: structs.ListenerProtocolTCP,
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().gatewayAccepted(),
},
},
},
},
},
@ -1061,6 +1171,11 @@ func TestBindRoutesToGateways(t *testing.T) {
SectionName: "Listener 1",
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
&structs.TCPRouteConfigEntry{
Name: "TCP Route 2",
@ -1072,6 +1187,11 @@ func TestBindRoutesToGateways(t *testing.T) {
SectionName: "Listener 2",
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
},
expectedBoundAPIGateways: []*structs.BoundAPIGatewayConfigEntry{
@ -1128,6 +1248,11 @@ func TestBindRoutesToGateways(t *testing.T) {
Protocol: structs.ListenerProtocolHTTP,
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().gatewayAccepted(),
},
},
},
},
},
@ -1142,6 +1267,11 @@ func TestBindRoutesToGateways(t *testing.T) {
SectionName: "Listener",
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
},
expectedBoundAPIGateways: []*structs.BoundAPIGatewayConfigEntry{},
@ -1181,6 +1311,11 @@ func TestBindRoutesToGateways(t *testing.T) {
Protocol: structs.ListenerProtocolTCP,
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().gatewayAccepted(),
},
},
},
},
},
@ -1195,6 +1330,11 @@ func TestBindRoutesToGateways(t *testing.T) {
SectionName: "",
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
},
expectedBoundAPIGateways: []*structs.BoundAPIGatewayConfigEntry{
@ -1289,6 +1429,11 @@ func TestBindRoutesToGateways(t *testing.T) {
Protocol: structs.ListenerProtocolTCP,
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().gatewayAccepted(),
},
},
},
},
},
@ -1302,6 +1447,11 @@ func TestBindRoutesToGateways(t *testing.T) {
Kind: structs.APIGateway,
},
},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
},
expectedBoundAPIGateways: []*structs.BoundAPIGatewayConfigEntry{},
@ -1430,7 +1580,7 @@ func TestAPIGatewayController(t *testing.T) {
},
},
},
"tcp-route-no-gateways-invalid-targets": {
"tcp-route-not-accepted-bind": {
requests: []controller.Request{{
Kind: structs.TCPRoute,
Name: "tcp-route",
@ -1444,6 +1594,27 @@ func TestAPIGatewayController(t *testing.T) {
Services: []structs.TCPService{{
Name: "tcp-upstream",
}},
Parents: []structs.ResourceReference{{
Name: "api-gateway",
EnterpriseMeta: *defaultMeta,
}},
},
&structs.APIGatewayConfigEntry{
Kind: structs.APIGateway,
Name: "api-gateway",
EnterpriseMeta: *defaultMeta,
Listeners: []structs.APIGatewayListener{{
Name: "listener",
Port: 80,
}},
},
&structs.BoundAPIGatewayConfigEntry{
Kind: structs.BoundAPIGateway,
Name: "api-gateway",
EnterpriseMeta: *defaultMeta,
Listeners: []structs.BoundAPIGatewayListener{{
Name: "listener",
}},
},
},
finalEntries: []structs.ConfigEntry{
@ -1453,10 +1624,41 @@ func TestAPIGatewayController(t *testing.T) {
EnterpriseMeta: *defaultMeta,
Status: structs.Status{
Conditions: []structs.Condition{
conditions.routeInvalidDiscoveryChain(errServiceDoesNotExist),
conditions.routeAccepted(),
conditions.routeUnbound(structs.ResourceReference{
Name: "api-gateway",
EnterpriseMeta: *defaultMeta,
}, errors.New("failed to bind route to gateway api-gateway: gateway has not been accepted")),
},
},
},
&structs.APIGatewayConfigEntry{
Kind: structs.APIGateway,
Name: "api-gateway",
EnterpriseMeta: *defaultMeta,
Listeners: []structs.APIGatewayListener{{
Name: "listener",
Port: 80,
}},
Status: structs.Status{
Conditions: []structs.Condition{
conditions.gatewayListenerNoConflicts(structs.ResourceReference{
Kind: structs.APIGateway,
Name: "api-gateway",
SectionName: "listener",
EnterpriseMeta: *defaultMeta,
}),
},
},
},
&structs.BoundAPIGatewayConfigEntry{
Kind: structs.BoundAPIGateway,
Name: "api-gateway",
EnterpriseMeta: *defaultMeta,
Listeners: []structs.BoundAPIGatewayListener{{
Name: "listener",
}},
},
},
},
"tcp-route-no-gateways-invalid-targets-bad-protocol": {
@ -1748,6 +1950,11 @@ func TestAPIGatewayController(t *testing.T) {
Name: "gateway",
EnterpriseMeta: *defaultMeta,
}},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
&structs.ServiceConfigEntry{
Kind: structs.ServiceDefaults,
@ -1763,6 +1970,11 @@ func TestAPIGatewayController(t *testing.T) {
Protocol: structs.ListenerProtocolTCP,
Port: 22,
}},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().gatewayAccepted(),
},
},
},
&structs.BoundAPIGatewayConfigEntry{
Kind: structs.BoundAPIGateway,
@ -1840,6 +2052,11 @@ func TestAPIGatewayController(t *testing.T) {
Name: "gateway",
EnterpriseMeta: *defaultMeta,
}},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
&structs.ServiceConfigEntry{
Kind: structs.ServiceDefaults,
@ -1931,6 +2148,11 @@ func TestAPIGatewayController(t *testing.T) {
Name: "gateway",
EnterpriseMeta: *defaultMeta,
}},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
&structs.TCPRouteConfigEntry{
Kind: structs.TCPRoute,
@ -1944,6 +2166,11 @@ func TestAPIGatewayController(t *testing.T) {
Name: "gateway",
EnterpriseMeta: *defaultMeta,
}},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
&structs.ServiceConfigEntry{
Kind: structs.ServiceDefaults,
@ -2061,6 +2288,11 @@ func TestAPIGatewayController(t *testing.T) {
Name: "gateway",
EnterpriseMeta: *defaultMeta,
}},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
&structs.HTTPRouteConfigEntry{
Kind: structs.HTTPRoute,
@ -2076,6 +2308,11 @@ func TestAPIGatewayController(t *testing.T) {
Name: "gateway",
EnterpriseMeta: *defaultMeta,
}},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
&structs.ServiceConfigEntry{
Kind: structs.ServiceDefaults,
@ -2174,6 +2411,14 @@ func TestAPIGatewayController(t *testing.T) {
Kind: structs.TCPRoute,
Name: "tcp-route",
Meta: acl.DefaultEnterpriseMeta(),
}, {
Kind: structs.TCPRoute,
Name: "tcp-route",
Meta: acl.DefaultEnterpriseMeta(),
}, {
Kind: structs.HTTPRoute,
Name: "http-route",
Meta: acl.DefaultEnterpriseMeta(),
}, {
Kind: structs.HTTPRoute,
Name: "http-route",
@ -2327,6 +2572,11 @@ func TestAPIGatewayController(t *testing.T) {
Name: "gateway",
EnterpriseMeta: *defaultMeta,
}},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
&structs.TCPRouteConfigEntry{
Kind: structs.TCPRoute,
@ -2340,6 +2590,11 @@ func TestAPIGatewayController(t *testing.T) {
Name: "gateway",
EnterpriseMeta: *defaultMeta,
}},
Status: structs.Status{
Conditions: []structs.Condition{
newGatewayConditionGenerator().routeAccepted(),
},
},
},
&structs.ServiceConfigEntry{
Kind: structs.ServiceDefaults,

10
agent/structs/config_entry_status.go
View File

@ -50,6 +50,16 @@ type Status struct {
Conditions []Condition
}
func (s *Status) MatchesConditionStatus(condition Condition) bool {
for _, c := range s.Conditions {
if c.IsCondition(&condition) &&
c.Status == condition.Status {
return true
}
}
return false
}
func (s Status) SameConditions(other Status) bool {
if len(s.Conditions) != len(other.Conditions) {
return false