shreemaan-abhishek commented on PR #2814: URL: https://github.com/apache/apisix-ingress-controller/pull/2814#issuecomment-5215756232
Thanks, all three land. Dropped `httproute.go` and `grpcroute.go` from this PR; it is now the apiv2 and Ingress paths only. **Gateway API conformance.** Agreed, and returning an error is the worse of the two wrong answers: the route is never programmed, so a request either falls through to another matching route and is served without the filter (the outcome the spec forbids) or 404s. The `backendErr` → `fault-injection` 500 block is the right shape to reuse, scoped per rule with `ResolvedRefs=False`. **Blast radius and visibility.** Confirmed both halves. `TranslateHTTPRoute` aborts on the first bad filter, so every rule of the route goes, and `Reconcile` persists the status before calling `Provider.Update`, so a translation error only requeues while the object still reports `Accepted=True`. `grpcroute_controller.go` has the same ordering. That one is worth fixing on its own regardless of this PR, since it makes any failure on those paths invisible. For what it is worth, the paths that remain here do surface the failure: `ApisixRoute` and `ApisixConsumer` run the real translator through `adcValidator` in the admission webhook, and `ApisixGlobalRule` and `Ingress` call `Provider.Update` before writing status, so the error reaches a condition. That asymmetry is exactly why the split makes sense. **Two conventions.** Fair, and I would rather not leave it implicit, so the description now states what is excluded and why. The remaining sites need individual decisions rather than one sweep: v1alpha1 `Consumer` has an `adcValidator` case and could fail closed like apiv2 `ApisixConsumer`, but `GatewayProxy` and `L4RoutePolicy` need their failure to be observable first, and `L4RoutePolicy` has no webhook at all. One aside from the earlier `config: null` discussion: `mergeL4PolicyPlugins` normalizes a nil `cfg`, but that branch is unreachable. For a field typed `apiextensionsv1.JSON` with `x-kubernetes-preserve-unknown-fields` and no `nullable: true`, the API server prunes the explicit null, so `Config.Raw` arrives empty and the `len(Raw) > 0` guard skips the unmarshal. Verified with envtest against the generated CRDs on both `ApisixRoute` and `ApisixPluginConfig`. Worth removing whenever that file is touched. -- 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]
