AlinsRan commented on code in PR #2813:
URL:
https://github.com/apache/apisix-ingress-controller/pull/2813#discussion_r3662242235
##########
internal/webhook/v1/ingress_webhook.go:
##########
@@ -82,6 +83,10 @@ func (v *IngressCustomValidator) ValidateCreate(ctx
context.Context, obj runtime
return nil, fmt.Errorf("%s",
sslvalidator.FormatConflicts(conflicts))
}
+ if err := validateAnnotations(ingress); err != nil {
Review Comment:
The shipped Helm chart defaults `webhook.enabled: true`
(`charts/apisix-ingress-controller/values.yaml` in apisix-helm-chart). The
`false` you are looking at is the Go default in `NewWebhookConfig()` /
`config/samples/config.yaml`, which only applies when running the binary
against a hand-written config.
Fail-closed on update was the previous revision of this PR and was reverted
in the `csrf.go` thread: `Provider.Update` returns on the translation error
*before* `UpdateConfig`, so the old unprotected route stays live anyway. It
swaps one broken state for another rather than protecting anything.
The gap that is real is that the translator says nothing at all when the
webhook is off. A log there is enough — commented on `csrf.go`.
--
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]
