shreemaan-abhishek opened a new issue, #2842:
URL: https://github.com/apache/apisix-ingress-controller/issues/2842
### Problem
#2814 makes the apiv2 plugin-config paths fail closed: a `config` that does
not unmarshal into an object now fails translation instead of publishing the
plugin with an empty config. Several equivalent sites still log the error and
`continue`, so the plugin silently vanishes and the resource reconciles green.
Still on the log-and-skip path:
| Site | What is dropped |
|---|---|
| `internal/adc/translator/consumer.go` | v1alpha1 `Consumer` credential
config, and `Consumer` plugins |
| `internal/adc/translator/gateway.go` | `GatewayProxy` `plugins` and
`pluginMetadata` (gateway-wide blast radius) |
| `internal/adc/translator/policies.go` | `L4RoutePolicy` plugins
(`mergeL4PolicyPlugins`) |
The failure mode differs from the one #2814 fixes — these `continue`, so the
plugin disappears rather than becoming `{}` — but the user-visible outcome is
the same class: the plugin does not run and nothing reports a problem. Left as
is, the tree carries two deliberate and opposite conventions for the same
situation.
### Expected behavior
Settle on one convention per path. The right treatment is not uniform,
because it depends on whether the failure is observable:
- **v1alpha1 `Consumer`** — `internal/webhook/v1/adc_validation.go` already
has a `case *v1alpha1.Consumer`, so the admission webhook runs the real
translator and a returned error is rejected at apply time. Fail closed here,
matching apiv2 `ApisixConsumer`.
- **`GatewayProxy`** — needs a check of whether `gatewayproxy_webhook.go`
validates through the translator. If not, failing closed would be silent, and
the failure needs a status condition first.
- **`L4RoutePolicy`** — no webhook. Same caveat: decide how the failure
surfaces before changing the behavior.
Whichever way each one lands, the PR should say why, so the convention is
documented rather than implicit.
### Notes
`mergeL4PolicyPlugins` also carries an explicit `cfg == nil` normalization
for a literal `config: null`. 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 entirely.
Verified with envtest against the generated CRDs, on both `ApisixRoute` and
`ApisixPluginConfig`. Worth removing while the file is being touched.
### Context
Raised in review on #2814.
--
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]
