Skip validation errors in mutating webhooks #2865
Conversation
Request processing by kube-apiserver is roughly:
{ mutating webhooks } => { OpenAPI schema validation } => { validating webhooks }
In this PR, we disable validation errors (at least, JSON unmarshalling
validation errors) during mutating webhook processing. It seems
counter-intuitive not to return an error in mutating webhooks for a
serious violation such as invalid JSON, but doing so allows OpenAPI
schema validation to proceed. Schema validation gives us nice errors,
so e.g.:
instead of:
```
The Fleet "" is invalid
```
we get:
```
The Fleet "fleet-example" is invalid: spec.template.spec.sdkServer.grpcPort: Invalid value: 33332229357: spec.template.spec.sdkServer.grpcPort in body should be less than or equal to 65535
```
This will become all the more important if we eventually move to CEL
based validations.
I'm also changing the error in googleforgames#2863 to an internal error - after this
PR, any hard error from a handler (vs a cause) is unexpected.
Fixes googleforgames#1770
|
Build Succeeded 👏 Build Id: d50534e4-375c-4107-a4fa-2b84c7d367af The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
zmerlynn
requested review from
markmandel and
gongmax
and removed request for
aLekSer and
dzlier-gcp
|
Code LGTM. Is there any downside to not fall fast on an invalid request? Maybe not a big deal but potential increasing latency for those request? |
Yes, there's a minor latency difference, I suspect. In theory there's also a minor load difference on the Kubernetes Control Plane. But, importantly, it only effects the error case, which is uncommon to optimize for. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: gongmax, zmerlynn The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Request processing by kube-apiserver is roughly:
{ mutating webhooks } => { OpenAPI schema validation } => { validating webhooks }
In this PR, we disable validation errors (at least, JSON unmarshalling validation errors) during mutating webhook processing. It seems counter-intuitive not to return an error in mutating webhooks for a serious violation such as invalid JSON, but doing so allows OpenAPI schema validation to proceed. Schema validation gives us nice errors, so e.g.:
instead of:
we get:
This will become all the more important if we eventually move to CEL based validations.
I'm also changing the error in #2863 to an internal error - after this PR, any hard error from a handler (vs a cause) is unexpected.
Fixes #1770