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

✨ Adds a client that enables strict field validation by default. #2860

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 107 additions & 0 deletions pkg/client/fieldvalidation.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/*
Copyright 2024 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package client

import (
"context"

"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
)

// WithStrictFieldValidation wraps a Client and configures strict field
// validation, by default, for all write requests from this client. Users
// can override the field validation for individual requests.
func WithStrictFieldValidation(c Client) Client {
return &clientWithFieldValidation{
validation: metav1.FieldValidationStrict,
c: c,
Reader: c,
}
}

type clientWithFieldValidation struct {
validation string
c Client
Reader
}

func (f *clientWithFieldValidation) Create(ctx context.Context, obj Object, opts ...CreateOption) error {
return f.c.Create(ctx, obj, append([]CreateOption{FieldValidation(f.validation)}, opts...)...)
}

func (f *clientWithFieldValidation) Update(ctx context.Context, obj Object, opts ...UpdateOption) error {
return f.c.Update(ctx, obj, append([]UpdateOption{FieldValidation(f.validation)}, opts...)...)
}

func (f *clientWithFieldValidation) Patch(ctx context.Context, obj Object, patch Patch, opts ...PatchOption) error {
return f.c.Patch(ctx, obj, patch, append([]PatchOption{FieldValidation(f.validation)}, opts...)...)
}

func (f *clientWithFieldValidation) Delete(ctx context.Context, obj Object, opts ...DeleteOption) error {
return f.c.Delete(ctx, obj, opts...)
}

func (f *clientWithFieldValidation) DeleteAllOf(ctx context.Context, obj Object, opts ...DeleteAllOfOption) error {
return f.c.DeleteAllOf(ctx, obj, opts...)
}

func (f *clientWithFieldValidation) Scheme() *runtime.Scheme { return f.c.Scheme() }
func (f *clientWithFieldValidation) RESTMapper() meta.RESTMapper { return f.c.RESTMapper() }
func (f *clientWithFieldValidation) GroupVersionKindFor(obj runtime.Object) (schema.GroupVersionKind, error) {
return f.c.GroupVersionKindFor(obj)
}

func (f *clientWithFieldValidation) IsObjectNamespaced(obj runtime.Object) (bool, error) {
return f.c.IsObjectNamespaced(obj)
}

func (f *clientWithFieldValidation) Status() StatusWriter {
return &subresourceClientWithFieldValidation{
validation: f.validation,
subresourceWriter: f.c.Status(),
}
}

func (f *clientWithFieldValidation) SubResource(subresource string) SubResourceClient {
c := f.c.SubResource(subresource)
return &subresourceClientWithFieldValidation{
validation: f.validation,
subresourceWriter: c,
SubResourceReader: c,
}
}

type subresourceClientWithFieldValidation struct {
validation string
subresourceWriter SubResourceWriter
SubResourceReader
}

func (f *subresourceClientWithFieldValidation) Create(ctx context.Context, obj Object, subresource Object, opts ...SubResourceCreateOption) error {
return f.subresourceWriter.Create(ctx, obj, subresource, append([]SubResourceCreateOption{FieldValidation(f.validation)}, opts...)...)
}

func (f *subresourceClientWithFieldValidation) Update(ctx context.Context, obj Object, opts ...SubResourceUpdateOption) error {
return f.subresourceWriter.Update(ctx, obj, append([]SubResourceUpdateOption{FieldValidation(f.validation)}, opts...)...)
}

func (f *subresourceClientWithFieldValidation) Patch(ctx context.Context, obj Object, patch Patch, opts ...SubResourcePatchOption) error {
return f.subresourceWriter.Patch(ctx, obj, patch, append([]SubResourcePatchOption{FieldValidation(f.validation)}, opts...)...)
}
149 changes: 149 additions & 0 deletions pkg/client/fieldvalidation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
/*
Copyright 2024 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package client_test

import (
"context"
"testing"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
)

func TestWithStrictFieldValidation(t *testing.T) {
calls := 0
fakeClient := testFieldValidationClient(t, metav1.FieldValidationStrict, func() { calls++ })
wrappedClient := client.WithStrictFieldValidation(fakeClient)

ctx := context.Background()
dummyObj := &corev1.Namespace{}

_ = wrappedClient.Create(ctx, dummyObj)
_ = wrappedClient.Update(ctx, dummyObj)
_ = wrappedClient.Patch(ctx, dummyObj, nil)
_ = wrappedClient.Status().Create(ctx, dummyObj, dummyObj)
_ = wrappedClient.Status().Update(ctx, dummyObj)
_ = wrappedClient.Status().Patch(ctx, dummyObj, nil)
_ = wrappedClient.SubResource("some-subresource").Create(ctx, dummyObj, dummyObj)
_ = wrappedClient.SubResource("some-subresource").Update(ctx, dummyObj)
_ = wrappedClient.SubResource("some-subresource").Patch(ctx, dummyObj, nil)

if expectedCalls := 9; calls != expectedCalls {
t.Fatalf("wrong number of calls to assertions: expected=%d; got=%d", expectedCalls, calls)
}
}

func TestWithStrictFieldValidationOverridden(t *testing.T) {
calls := 0

fakeClient := testFieldValidationClient(t, metav1.FieldValidationWarn, func() { calls++ })
wrappedClient := client.WithStrictFieldValidation(fakeClient)

ctx := context.Background()
dummyObj := &corev1.Namespace{}

_ = wrappedClient.Create(ctx, dummyObj, client.FieldValidation(metav1.FieldValidationWarn))
_ = wrappedClient.Update(ctx, dummyObj, client.FieldValidation(metav1.FieldValidationWarn))
_ = wrappedClient.Patch(ctx, dummyObj, nil, client.FieldValidation(metav1.FieldValidationWarn))
_ = wrappedClient.Status().Create(ctx, dummyObj, dummyObj, client.FieldValidation(metav1.FieldValidationWarn))
_ = wrappedClient.Status().Update(ctx, dummyObj, client.FieldValidation(metav1.FieldValidationWarn))
_ = wrappedClient.Status().Patch(ctx, dummyObj, nil, client.FieldValidation(metav1.FieldValidationWarn))
_ = wrappedClient.SubResource("some-subresource").Create(ctx, dummyObj, dummyObj, client.FieldValidation(metav1.FieldValidationWarn))
_ = wrappedClient.SubResource("some-subresource").Update(ctx, dummyObj, client.FieldValidation(metav1.FieldValidationWarn))
_ = wrappedClient.SubResource("some-subresource").Patch(ctx, dummyObj, nil, client.FieldValidation(metav1.FieldValidationWarn))

if expectedCalls := 9; calls != expectedCalls {
t.Fatalf("wrong number of calls to assertions: expected=%d; got=%d", expectedCalls, calls)
}
}

// testFieldValidationClient is a helper function that checks if calls have the expected field validation,
// and calls the callback function on each intercepted call.
func testFieldValidationClient(t *testing.T, expectedFieldValidation string, callback func()) client.Client {
Copy link
Contributor Author

@dlipovetsky dlipovetsky Jun 20, 2024

Choose a reason for hiding this comment

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

I've shamelessly copied and adapted the test code from fieldowner_test.go.

This is almost identical to the testClient helper function in that file. I would be happy to refactor the two helper functions into one that can be used by both tests. I just think it's wise to copy first, and refactor second.

// TODO: we could use the dummyClient in interceptor pkg if we move it to an internal pkg
return fake.NewClientBuilder().WithInterceptorFuncs(interceptor.Funcs{
Create: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.CreateOption) error {
callback()
out := &client.CreateOptions{}
for _, f := range opts {
f.ApplyToCreate(out)
}
if got := out.FieldValidation; expectedFieldValidation != got {
t.Fatalf("wrong field validation: expected=%q; got=%q", expectedFieldValidation, got)
}
return nil
},
Update: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.UpdateOption) error {
callback()
out := &client.UpdateOptions{}
for _, f := range opts {
f.ApplyToUpdate(out)
}
if got := out.FieldValidation; expectedFieldValidation != got {
t.Fatalf("wrong field validation: expected=%q; got=%q", expectedFieldValidation, got)
}
return nil
},
Patch: func(ctx context.Context, c client.WithWatch, obj client.Object, patch client.Patch, opts ...client.PatchOption) error {
callback()
out := &client.PatchOptions{}
for _, f := range opts {
f.ApplyToPatch(out)
}
if got := out.FieldValidation; expectedFieldValidation != got {
t.Fatalf("wrong field validation: expected=%q; got=%q", expectedFieldValidation, got)
}
return nil
},
SubResourceCreate: func(ctx context.Context, c client.Client, subResourceName string, obj client.Object, subResource client.Object, opts ...client.SubResourceCreateOption) error {
callback()
out := &client.SubResourceCreateOptions{}
for _, f := range opts {
f.ApplyToSubResourceCreate(out)
}
if got := out.FieldValidation; expectedFieldValidation != got {
t.Fatalf("wrong field validation: expected=%q; got=%q", expectedFieldValidation, got)
}
return nil
},
SubResourceUpdate: func(ctx context.Context, c client.Client, subResourceName string, obj client.Object, opts ...client.SubResourceUpdateOption) error {
callback()
out := &client.SubResourceUpdateOptions{}
for _, f := range opts {
f.ApplyToSubResourceUpdate(out)
}
if got := out.FieldValidation; expectedFieldValidation != got {
t.Fatalf("wrong field validation: expected=%q; got=%q", expectedFieldValidation, got)
}
return nil
},
SubResourcePatch: func(ctx context.Context, c client.Client, subResourceName string, obj client.Object, patch client.Patch, opts ...client.SubResourcePatchOption) error {
callback()
out := &client.SubResourcePatchOptions{}
for _, f := range opts {
f.ApplyToSubResourcePatch(out)
}
if got := out.FieldValidation; expectedFieldValidation != got {
t.Fatalf("wrong field validation: expected=%q; got=%q", expectedFieldValidation, got)
}
return nil
},
}).Build()
}
90 changes: 90 additions & 0 deletions pkg/client/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,39 @@ func (f FieldOwner) ApplyToSubResourceUpdate(opts *SubResourceUpdateOptions) {
opts.FieldManager = string(f)
}

// FieldValidation configures field validation for the given requests.
type FieldValidation string

// ApplyToPatch applies this configuration to the given patch options.
func (f FieldValidation) ApplyToPatch(opts *PatchOptions) {
opts.FieldValidation = string(f)
}

// ApplyToCreate applies this configuration to the given create options.
func (f FieldValidation) ApplyToCreate(opts *CreateOptions) {
opts.FieldValidation = string(f)
}

// ApplyToUpdate applies this configuration to the given update options.
func (f FieldValidation) ApplyToUpdate(opts *UpdateOptions) {
opts.FieldValidation = string(f)
}

// ApplyToSubResourcePatch applies this configuration to the given patch options.
func (f FieldValidation) ApplyToSubResourcePatch(opts *SubResourcePatchOptions) {
opts.FieldValidation = string(f)
}

// ApplyToSubResourceCreate applies this configuration to the given create options.
func (f FieldValidation) ApplyToSubResourceCreate(opts *SubResourceCreateOptions) {
opts.FieldValidation = string(f)
}

// ApplyToSubResourceUpdate applies this configuration to the given update options.
func (f FieldValidation) ApplyToSubResourceUpdate(opts *SubResourceUpdateOptions) {
opts.FieldValidation = string(f)
}

// }}}

// {{{ Create Options
Expand All @@ -187,6 +220,25 @@ type CreateOptions struct {
// this request. It must be set with server-side apply.
FieldManager string

// fieldValidation instructs the server on how to handle
// objects in the request (POST/PUT/PATCH) containing unknown
// or duplicate fields. Valid values are:
// - Ignore: This will ignore any unknown fields that are silently
// dropped from the object, and will ignore all but the last duplicate
// field that the decoder encounters. This is the default behavior
// prior to v1.23.
// - Warn: This will send a warning via the standard warning response
// header for each unknown field that is dropped from the object, and
// for each duplicate field that is encountered. The request will
// still succeed if there are no other errors, and will only persist
// the last of any duplicate fields. This is the default in v1.23+
// - Strict: This will fail the request with a BadRequest error if
// any unknown fields would be dropped from the object, or if any
// duplicate fields are present. The error returned from the server
// will contain all unknown and duplicate fields encountered.
// +optional
FieldValidation string

// Raw represents raw CreateOptions, as passed to the API server.
Raw *metav1.CreateOptions
}
Expand Down Expand Up @@ -679,6 +731,25 @@ type UpdateOptions struct {
// this request. It must be set with server-side apply.
FieldManager string

// fieldValidation instructs the server on how to handle
// objects in the request (POST/PUT/PATCH) containing unknown
// or duplicate fields. Valid values are:
// - Ignore: This will ignore any unknown fields that are silently
// dropped from the object, and will ignore all but the last duplicate
// field that the decoder encounters. This is the default behavior
// prior to v1.23.
// - Warn: This will send a warning via the standard warning response
// header for each unknown field that is dropped from the object, and
// for each duplicate field that is encountered. The request will
// still succeed if there are no other errors, and will only persist
// the last of any duplicate fields. This is the default in v1.23+
// - Strict: This will fail the request with a BadRequest error if
// any unknown fields would be dropped from the object, or if any
// duplicate fields are present. The error returned from the server
// will contain all unknown and duplicate fields encountered.
// +optional
FieldValidation string

// Raw represents raw UpdateOptions, as passed to the API server.
Raw *metav1.UpdateOptions
}
Expand Down Expand Up @@ -745,6 +816,25 @@ type PatchOptions struct {
// this request. It must be set with server-side apply.
FieldManager string

// fieldValidation instructs the server on how to handle
// objects in the request (POST/PUT/PATCH) containing unknown
// or duplicate fields. Valid values are:
// - Ignore: This will ignore any unknown fields that are silently
// dropped from the object, and will ignore all but the last duplicate
// field that the decoder encounters. This is the default behavior
// prior to v1.23.
// - Warn: This will send a warning via the standard warning response
// header for each unknown field that is dropped from the object, and
// for each duplicate field that is encountered. The request will
// still succeed if there are no other errors, and will only persist
// the last of any duplicate fields. This is the default in v1.23+
// - Strict: This will fail the request with a BadRequest error if
// any unknown fields would be dropped from the object, or if any
// duplicate fields are present. The error returned from the server
// will contain all unknown and duplicate fields encountered.
// +optional
FieldValidation string

// Raw represents raw PatchOptions, as passed to the API server.
Raw *metav1.PatchOptions
}
Expand Down