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

[WIP] Validate external types with CRD machinery #1690

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cartermckinnon
Copy link
Member

Description of changes:

This adds validation for the external API types using the same mechanism that the API server uses for CRD's. Validation directives like required are only expressed in the CRD spec, so we need to leverage the core validation in the API server to leverage it.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@cartermckinnon cartermckinnon force-pushed the validation-with-crd-spec branch from 01236bd to 9b77dcb Compare February 27, 2024 07:17
@@ -28,6 +28,9 @@ func DecodeNodeConfig(data []byte) (*internalapi.NodeConfig, error) {
if gvk.Group != api.GroupName {
return nil, fmt.Errorf("failed to decode %q, unexpected group: %s", gvk.Kind, gvk.Group)
}
if err := ValidateExternalType(obj, *gvk); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

based on tests, this is breaking how we merge partial configs 😞
will we have to do the merging in json/yaml like kubelet? https://github.com/kubernetes/kubernetes/blob/master/cmd/kubelet/app/server.go#L314-L345

@cartermckinnon
Copy link
Member Author

/ci

Copy link
Contributor

@cartermckinnon this PR is not currently mergeable, you'll need to rebase it first.

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 this pull request may close these issues.

3 participants