AlinsRan commented on code in PR #2893:
URL:
https://github.com/apache/apisix-ingress-controller/pull/2893#discussion_r4534699413
##########
internal/adc/translator/httproute.go:
##########
@@ -788,7 +803,9 @@ func (t *Translator) TranslateHTTPRoute(tctx
*provider.TranslateContext, httpRou
}
}
- t.fillHTTPRoutePoliciesForHTTPRoute(tctx, routes, rule)
+ if err := t.fillHTTPRoutePoliciesForHTTPRoute(tctx, routes,
rule); err != nil {
+ return nil, err
Review Comment:
Could we scope this failure to the rule targeted by the policy? For example,
an HTTPRoute can have `public` and `private` rules, with an HTTPRoutePolicy
targeting only `private` via `targetRefs.sectionName`. If that policy contains
a malformed `vars` item, this `return nil, err` aborts translation of the
entire HTTPRoute and discards any services already translated for other rules.
A new HTTPRoute therefore cannot publish even the unaffected `public` rule; for
an existing HTTPRoute, `Provider.Update` returns before updating the store, so
changes to `public` are also blocked.
I reproduced this with a two-rule HTTPRoute and a malformed policy targeting
only `private`. Please keep the failure handling within the policy's target
scope and add a regression test verifying that the unaffected rule can still be
created and updated.
--
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]
