Skip to content

MultiValidatingHandler returns as soon as one handler fails, making it impossible to follow convention of reporting all validation errors #3203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dlipovetsky opened this issue Apr 29, 2025 · 1 comment

Comments

@dlipovetsky
Copy link
Contributor

dlipovetsky commented Apr 29, 2025

The prevailing convention in Kubernetes resource validation is to report all validation errors, not only the first discovered error. The built-in API types follow this convention (example). The OpenAPI and CEL rule validation follows this convention, too. Most webhooks I have seen also follow it.

We provide a utility function for webhook authors that executes multiple validators:

// MultiValidatingHandler combines multiple validating webhook handlers into a single
// validating webhook handler. Handlers are called in sequential order, and the first
// `allowed: false` response may short-circuit the rest.
func MultiValidatingHandler(handlers ...Handler) Handler {
return multiValidating(handlers)
}

It returns as soon as one validator fails. That means that subsequent validators are not called, and any errors they might discover are not reported.

I think we should provide an alternative implementation that calls all validators, even if some fail, and aggregates their errors.

Also, because the existing utility function does not follow the convention, I think we should consider deprecating it.

@dlipovetsky dlipovetsky changed the title MultiValidatingHandler should always call all handlers MultiValidatingHandler returns as soon as one handler fails, making it impossible to follow convention of reporting all validation errors Apr 29, 2025
@dlipovetsky
Copy link
Contributor Author

An implementation that aggregates errors from multiple handlers would have to merge handler responses., and I don't see a unambiguous way to do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant