AlinsRan commented on code in PR #2813:
URL:
https://github.com/apache/apisix-ingress-controller/pull/2813#discussion_r3654014927
##########
internal/adc/translator/annotations/plugins/plugins.go:
##########
@@ -60,8 +58,9 @@ func (p *plugins) Parse(e annotations.Extractor) (any, error)
{
for _, handler := range handlers {
out, err := handler.Handle(e)
if err != nil {
- log.Error(err, "Failed to handle annotation",
"handler", handler.PluginName())
- continue
+ // Fail closed: abort translation so the route is never
programmed
+ // while a requested plugin is missing.
+ return nil, fmt.Errorf("%s: %w", handler.PluginName(),
err)
Review Comment:
This goes far beyond csrf. With every parser error now joined, an Ingress
also fails translation on `upstream-scheme`, `upstream-retry`, the three
timeouts (upstream.go:51-82), and a non-compiling rewrite regex (rewrite.go:54).
An Ingress carrying a typo'd annotation that has been serving traffic fine
stops reconciling on upgrade, and after the next controller restart rebuilds
the ADC baseline the route disappears from the data plane.
Keep the fail-closed semantics, but this needs an entry in
`docs/en/latest/upgrade-guide.md`.
--
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]
