AlinsRan opened a new pull request, #2859:
URL: https://github.com/apache/apisix-ingress-controller/pull/2859
### Type of change:
- [x] Bugfix
### What this PR does / why we need it:
Deleting an `ApisixPluginConfig` that an `ApisixRoute` or an `Ingress` still
references leaves the data plane applying its plugins indefinitely.
Both reconcilers treat the missing reference as a validation failure and
return before `Provider.Update` ever runs:
```go
// internal/controller/apisixroute_controller.go
if err = r.processApisixRoute(tctx, &ar); err != nil {
return ctrl.Result{}, err // validatePluginConfig failed; nothing
retracts
}
if err = r.Provider.Update(ctx, tctx, &ar); err != nil {
```
```go
// internal/controller/ingress_controller.go
if err := r.processPluginConfig(tctx, ingress); err != nil {
return ctrl.Result{}, err // same shape
}
...
if err := r.Provider.Update(ctx, tctx, ingress); err != nil {
```
Since the configuration store is what every sync pushes, an entry that is
never rewritten and never deleted keeps being served. The object reports its
spec as invalid while the deleted plugins still take effect, and deleting the
route or the Ingress is the only way to clear them. Recreating the plugin
config under the same name repairs it, which is what makes this look like a
sync delay rather than a missing code path.
This PR retracts the published configuration when the reference is genuinely
absent. That follows the direction set by #2814: reject a route whose plugin
configuration cannot be resolved rather than serve it with the plugins silently
dropped. For a response-header plugin the difference is cosmetic; for an auth
or rate-limit plugin, serving without it is worse than a 404.
A read failure that is not `NotFound` is transient and must not drop a
working route because the API server hiccuped. `validatePluginConfig` used to
fold every `Get` failure into an `InvalidSpec` `ReasonError`, losing that
distinction, so this adds `types.DependencyMissingError` to mark the
absent-reference case and returns other errors unchanged.
### 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?
- [ ] Is this PR backward compatible?
Behavior change worth calling out: a route or Ingress whose plugin config is
deleted now stops serving instead of continuing on the last good configuration.
That is the point of the fix, but it is a visible change for anyone who was
relying on the old behavior.
Tests: `internal/controller/pluginconfig_retract_test.go` covers both
reconcilers for the absent reference, the transient read failure, and the
healthy path. The three retraction assertions fail on `master` and pass here.
--
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]
