Plugin config unmarshal failures still log-and-skip in consumer.go, gateway.go and policies.go

[shreemaan-abhishek]

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.

[AlinsRan]

The three sites and all three webhook observations check out against master. Two of the conclusions that follow need changing, and both make this issue smaller.

L4RoutePolicy needs no design decision — GEP-713 already answered it

The caveat "no webhook, decide how the failure surfaces first" doesn't apply. L4RoutePolicy is a Policy Attachment resource: TargetRefs is gatewayv1.LocalPolicyTargetReferenceWithSectionName, PolicyStatus is gatewayv1.PolicyStatus, and policies.go:328 already writes status.ancestors[].conditions using PolicyConditionAccepted / PolicyReasonAccepted / PolicyReasonConflicted. The failure surface exists and is wired. Make mergeL4PolicyPlugins return an error and that path turns it into Accepted=False.

Declaring type: object on the CRD erases three of the four sites

None of these config fields declare a type — they carry only x-kubernetes-preserve-unknown-fields: true, which is why config: [1,2] reaches the translator at all. Adding the type rejects it at apply time and makes consumer.go, gateway.go:254 and policies.go:266 unreachable in a single change, across every path that reaches them rather than just these three. It's breaking for configs that are already invalid today, so it needs a release note.

pluginMetadata is the one site left, and the only one with no GEP to follow

GatewayProxy.PluginMetadata is map[string]apiextensionsv1.JSON (gatewayproxy_types.go:43), and in the generated CRD its additionalProperties carries no type either (apisix.apache.org_gatewayproxies.yaml:45-50) — a marker on Plugin.Config doesn't reach a map value. So fillPluginMetadataFromGatewayProxy still needs the question this issue asks, and GatewayProxy is neither a Route nor a Policy, so there's nothing upstream to model it on. That one is worth discussing; the rest resolves itself.

Also worth keeping in view: failing closed on GatewayProxy has gateway-wide blast radius, and gatewayproxy_webhook.go does not validate through the translator (only collectWarnings and validateGatewayProxyConflict), so a fail-closed change there would swap a silent no-op for a silent rejection until the status condition exists.

Keep the cfg == nil branch

json.Unmarshal([]byte("null"), &cfg) does set a non-nil map back to nil, so the guard is correct Go. What makes it unreachable is the API server pruning the explicit null — deleting it in the same change that rewrites those schemas trades three lines for a dependency on schema behaviour. If it goes, the PR should state that dependency.

The shared defect under this and #2841 is that the translator can't return errors

The fill* / merge* / load* family mostly has no error return, so a parse failure has nowhere to go but a log line. Exactly five of them return an error today — the five #2814 changed. Doing the rest as its own no-behaviour-change PR would unblock both issues and stop the tree from growing a third convention.

Note the fix does not generalise across the two, though, and the line is drawn by gateway-api itself by resource category rather than by which group defined the CRD: Route (HTTPRoute/GRPCRoute) → data-plane error scoped to the rule, per httproute_types.go:847-849; Policy (L4RoutePolicy) → GEP-713 ancestor conditions; neither (Consumer, GatewayProxy) → schema rejection or an ordinary status condition.