AlinsRan opened a new pull request, #2858:
URL: https://github.com/apache/apisix-ingress-controller/pull/2858
### Type of change:
- [x] Bugfix
### What this PR does / why we need it:
The five Gateway API route reconcilers only ever publish. They call
`Provider.Update` when the route is accepted and do nothing otherwise, so there
is no path that retracts a route that is still present but no longer
programmable.
That matters because the configuration store, not the data plane, decides
what every sync pushes (`Client.Sync` walks the store and pushes it in full).
An entry that stays in the store keeps being served until the route object
itself is deleted or the controller restarts.
Two exits are affected in each of `httproute`, `grpcroute`, `tcproute`,
`tlsroute` and `udproute`:
```go
// internal/controller/httproute_controller.go
if len(gateways) == 0 {
return ctrl.Result{}, nil // parentRef no longer resolves to a
Gateway of ours
}
...
if isRouteAccepted(gateways) && err == nil {
r.Provider.Update(ctx, tctx, routeToUpdate)
} // no else, so nothing retracts
return ctrl.Result{}, nil
```
Several ordinary edits reach the second exit and leave a live route behind:
- narrowing a listener's `allowedRoutes` so the route's namespace is no
longer admitted, which reports `Accepted=False` / `NotAllowedByListeners`
- removing the listener a `sectionName` points at (`NoMatchingParent`)
- editing `hostnames` until they no longer intersect the listener
(`NoMatchingListenerHostname`, raised by `filterHostnames`)
In each case the route status flips to `Accepted=False` while the data plane
keeps forwarding the configuration an earlier reconcile published. Deleting the
HTTPRoute is currently the only way to clear it, which is exactly the wrong
remedy when the point of the edit was to revoke a tenant's access without
touching their objects.
This PR adds the retraction on both exits in all five reconcilers.
`Provider.Delete` resolves to `ConfigManager.Get(key)` and is a no-op when
nothing was published for that key, so a route that was never accepted costs
one store lookup and no sync.
Scope note: this is the tactical fix for the exits that are reachable by
editing a Gateway or a route. It does not address a reconcile that is never
enqueued at all (a dropped watch or a leader change), which needs a periodic
reconcile of the store against the cache and is worth a separate change.
### Pre-submission checklist:
- [x] Did you explain what problem does this PR solve? Or what new features
have been added?
- [x] Have you added corresponding test cases?
- [ ] Have you modified the corresponding document?
- [x] Is this PR backward compatible?
Tests: `internal/controller/httproute_controller_retract_test.go` covers
both exits, the accepted path, and provider-error propagation. All four fail on
`master` and pass here. E2E coverage that asserts the data plane stops
forwarding would be the natural follow-up; I could not run the e2e suite in
this environment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
