Skip to content
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

Server-side field validation should be strict by default #2859

Open
dlipovetsky opened this issue Jun 20, 2024 · 2 comments · May be fixed by #2860
Open

Server-side field validation should be strict by default #2859

dlipovetsky opened this issue Jun 20, 2024 · 2 comments · May be fixed by #2860

Comments

@dlipovetsky
Copy link
Contributor

Creating/updating unstructured objects is a supported use case for controller-runtime.

When a controller works with an unstructured objects, there is no Go type to ensure the object conforms to a schema. Instead, the controller relies on server-side validation.

By default, server-side validation is not strict. For example, if a controller creates or updates an object and adds an unknown field, the request is not rejected; at most, a warning message is logged.

I0612 13:30:46.003406  164090 warning_handler.go:65] "msg"="unknown field \"spec.exampleField\"" "logger"="KubeAPIWarningLogger" 

Warning messages are difficult to test against, and can be missed in production. In turn, controller bugs are missed.

All controllers would benefit from enabling strict validation, but some may be unable to use it, e.g. because of backward compatibility (this is also why strict field validation was not enabled by default in the API server).

Individual client requests can be configured so that the API server returns an error, although it requires understanding the use of the "raw" options:

client.Update(ctx, obj, &client.UpdateOptions{Raw: &metav1.UpdateOptions{ FieldValidation: metav1.FieldValidationStrict}})

But, if all controllers would benefit from enabling strict validation, then it's poor UX to require controller authors to enable it in every client request.

Therefore, I propose that controller-runtime

(a) allow the user to configure field validation at client create time, and
(b) consider enabling strict field validation the default

@dlipovetsky
Copy link
Contributor Author

We recently talked about this behavior in #cluster-api slack.

@dlipovetsky
Copy link
Contributor Author

dlipovetsky commented Jun 20, 2024

To address (a), I propose a specialized "strict" client that follows the pattern established by the specialized "field owner" client introduced in #2771

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

Successfully merging a pull request may close this issue.

1 participant