Skip to content

Commit 892382c

Browse files
committed
Revert "Field validation"
This reverts commit ad1966a. This doesn't work, the server will always error on additional fields when using SSA, even when fieldValiation=None is configured.
1 parent ad1966a commit 892382c

File tree

4 files changed

+7
-113
lines changed

4 files changed

+7
-113
lines changed

pkg/client/client_test.go

Lines changed: 0 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -949,62 +949,6 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
949949
Expect(cm.Data).To(BeComparableTo(obj.Data))
950950
})
951951
})
952-
953-
Context("with FieldValidation", func() {
954-
It("should handle invalid fields based on FieldValidation setting", func() {
955-
cl, err := client.New(cfg, client.Options{})
956-
Expect(err).NotTo(HaveOccurred())
957-
Expect(cl).NotTo(BeNil())
958-
959-
data := map[string]any{
960-
"some-key": "some-value",
961-
}
962-
obj := &unstructured.Unstructured{Object: map[string]any{
963-
"apiVersion": "v1",
964-
"kind": "ConfigMap",
965-
"metadata": map[string]any{
966-
"name": "test-configmap-fieldvalidation",
967-
"namespace": "default",
968-
},
969-
"data": data,
970-
"invalidField": "invalid-value",
971-
}}
972-
973-
// Apply with disabled field validation should succeed
974-
err = cl.Apply(context.Background(), client.ApplyConfigurationFromUnstructured(obj), &client.ApplyOptions{
975-
FieldManager: "test-manager",
976-
FieldValidation: metav1.FieldValidationIgnore,
977-
})
978-
Expect(err).NotTo(HaveOccurred())
979-
980-
cm, err := clientset.CoreV1().ConfigMaps(obj.GetNamespace()).Get(context.Background(), obj.GetName(), metav1.GetOptions{})
981-
Expect(err).NotTo(HaveOccurred())
982-
983-
actualData := map[string]any{}
984-
for k, v := range cm.Data {
985-
actualData[k] = v
986-
}
987-
Expect(actualData).To(BeComparableTo(data))
988-
989-
// Apply with default (strict) field validation should fail
990-
obj2 := &unstructured.Unstructured{Object: map[string]any{
991-
"apiVersion": "v1",
992-
"kind": "ConfigMap",
993-
"metadata": map[string]any{
994-
"name": "test-configmap-fieldvalidation-strict",
995-
"namespace": "default",
996-
},
997-
"data": data,
998-
"invalidField": "invalid-value",
999-
}}
1000-
1001-
err = cl.Apply(context.Background(), client.ApplyConfigurationFromUnstructured(obj2), &client.ApplyOptions{
1002-
FieldManager: "test-manager",
1003-
})
1004-
Expect(err).To(HaveOccurred())
1005-
Expect(err.Error()).To(ContainSubstring("unknown field"))
1006-
})
1007-
})
1008952
})
1009953

1010954
Describe("SubResourceClient", func() {

pkg/client/fieldvalidation.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package client
1818

1919
import (
2020
"context"
21+
"errors"
2122

2223
"k8s.io/apimachinery/pkg/api/meta"
2324
"k8s.io/apimachinery/pkg/runtime"
@@ -54,7 +55,7 @@ func (c *clientWithFieldValidation) Patch(ctx context.Context, obj Object, patch
5455
}
5556

5657
func (c *clientWithFieldValidation) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...ApplyOption) error {
57-
return c.client.Apply(ctx, obj, append([]ApplyOption{c.validation}, opts...)...)
58+
return errors.New("Apply is not supported with field validation")
5859
}
5960

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

pkg/client/fieldvalidation_test.go

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525
corev1 "k8s.io/api/core/v1"
2626
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2727
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
28-
"k8s.io/apimachinery/pkg/runtime"
2928
"k8s.io/apimachinery/pkg/runtime/schema"
3029
"sigs.k8s.io/controller-runtime/pkg/client"
3130
"sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -106,15 +105,14 @@ func TestWithStrictFieldValidation(t *testing.T) {
106105
_ = wrappedClient.Create(ctx, dummyObj)
107106
_ = wrappedClient.Update(ctx, dummyObj)
108107
_ = wrappedClient.Patch(ctx, dummyObj, nil)
109-
_ = wrappedClient.Apply(ctx, nil)
110108
_ = wrappedClient.Status().Create(ctx, dummyObj, dummyObj)
111109
_ = wrappedClient.Status().Update(ctx, dummyObj)
112110
_ = wrappedClient.Status().Patch(ctx, dummyObj, nil)
113111
_ = wrappedClient.SubResource("some-subresource").Create(ctx, dummyObj, dummyObj)
114112
_ = wrappedClient.SubResource("some-subresource").Update(ctx, dummyObj)
115113
_ = wrappedClient.SubResource("some-subresource").Patch(ctx, dummyObj, nil)
116114

117-
if expectedCalls := 10; calls != expectedCalls {
115+
if expectedCalls := 9; calls != expectedCalls {
118116
t.Fatalf("wrong number of calls to assertions: expected=%d; got=%d", expectedCalls, calls)
119117
}
120118
}
@@ -131,15 +129,14 @@ func TestWithStrictFieldValidationOverridden(t *testing.T) {
131129
_ = wrappedClient.Create(ctx, dummyObj, client.FieldValidation(metav1.FieldValidationWarn))
132130
_ = wrappedClient.Update(ctx, dummyObj, client.FieldValidation(metav1.FieldValidationWarn))
133131
_ = wrappedClient.Patch(ctx, dummyObj, nil, client.FieldValidation(metav1.FieldValidationWarn))
134-
_ = wrappedClient.Apply(ctx, nil, client.FieldValidation(metav1.FieldValidationWarn))
135132
_ = wrappedClient.Status().Create(ctx, dummyObj, dummyObj, client.FieldValidation(metav1.FieldValidationWarn))
136133
_ = wrappedClient.Status().Update(ctx, dummyObj, client.FieldValidation(metav1.FieldValidationWarn))
137134
_ = wrappedClient.Status().Patch(ctx, dummyObj, nil, client.FieldValidation(metav1.FieldValidationWarn))
138135
_ = wrappedClient.SubResource("some-subresource").Create(ctx, dummyObj, dummyObj, client.FieldValidation(metav1.FieldValidationWarn))
139136
_ = wrappedClient.SubResource("some-subresource").Update(ctx, dummyObj, client.FieldValidation(metav1.FieldValidationWarn))
140137
_ = wrappedClient.SubResource("some-subresource").Patch(ctx, dummyObj, nil, client.FieldValidation(metav1.FieldValidationWarn))
141138

142-
if expectedCalls := 10; calls != expectedCalls {
139+
if expectedCalls := 9; calls != expectedCalls {
143140
t.Fatalf("wrong number of calls to assertions: expected=%d; got=%d", expectedCalls, calls)
144141
}
145142
}
@@ -275,26 +272,5 @@ func testFieldValidationClient(t *testing.T, expectedFieldValidation string, cal
275272
}
276273
return nil
277274
},
278-
Apply: func(ctx context.Context, c client.WithWatch, obj runtime.ApplyConfiguration, opts ...client.ApplyOption) error {
279-
callback()
280-
out := &client.ApplyOptions{}
281-
for _, f := range opts {
282-
f.ApplyToApply(out)
283-
}
284-
if got := out.FieldValidation; expectedFieldValidation != got {
285-
t.Fatalf("wrong field validation: expected=%q; got=%q", expectedFieldValidation, got)
286-
}
287-
288-
if got := out.AsPatchOptions().FieldValidation; expectedFieldValidation != got {
289-
t.Fatalf("wrong field validation: expected=%q; got=%q", expectedFieldValidation, got)
290-
}
291-
292-
co := &client.ApplyOptions{}
293-
out.ApplyToApply(co)
294-
if got := co.FieldValidation; expectedFieldValidation != got {
295-
t.Fatalf("wrong field validation: expected=%q; got=%q", expectedFieldValidation, got)
296-
}
297-
return nil
298-
},
299275
}).Build()
300276
}

pkg/client/options.go

Lines changed: 3 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -218,11 +218,6 @@ func (f FieldValidation) ApplyToSubResourceUpdate(opts *SubResourceUpdateOptions
218218
opts.FieldValidation = string(f)
219219
}
220220

221-
// ApplyToApply applies this configuration to the given apply options.
222-
func (f FieldValidation) ApplyToApply(opts *ApplyOptions) {
223-
opts.FieldValidation = string(f)
224-
}
225-
226221
// }}}
227222

228223
// {{{ Create Options
@@ -998,24 +993,6 @@ type ApplyOptions struct {
998993
// as defined by https://golang.org/pkg/unicode/#IsPrint. This
999994
// field is required.
1000995
FieldManager string `json:"fieldManager" protobuf:"bytes,3,name=fieldManager"`
1001-
1002-
// fieldValidation instructs the server on how to handle
1003-
// objects in the apply request containing unknown
1004-
// or duplicate fields. Valid values are:
1005-
// - Ignore: This will ignore any unknown fields that are silently
1006-
// dropped from the object, and will ignore all but the last duplicate
1007-
// field that the decoder encounters.
1008-
// - Warn: This will send a warning via the standard warning response
1009-
// header for each unknown field that is dropped from the object, and
1010-
// for each duplicate field that is encountered. The request will
1011-
// still succeed if there are no other errors, and will only persist
1012-
// the last of any duplicate fields.
1013-
// - Strict: This will fail the request with a BadRequest error if
1014-
// any unknown fields would be dropped from the object, or if any
1015-
// duplicate fields are present. The error returned from the server
1016-
// will contain all unknown and duplicate fields encountered.
1017-
// This is the default for apply requests.
1018-
FieldValidation string `json:"fieldValidation,omitempty" protobuf:"bytes,4,opt,name=fieldValidation"`
1019996
}
1020997

1021998
// ApplyOptions applies the given opts onto the ApplyOptions
@@ -1038,17 +1015,13 @@ func (o *ApplyOptions) ApplyToApply(opts *ApplyOptions) {
10381015
if o.FieldManager != "" {
10391016
opts.FieldManager = o.FieldManager
10401017
}
1041-
if o.FieldValidation != "" {
1042-
opts.FieldValidation = o.FieldValidation
1043-
}
10441018
}
10451019

10461020
// AsPatchOptions constructs patch options from the given ApplyOptions
10471021
func (o *ApplyOptions) AsPatchOptions() *metav1.PatchOptions {
10481022
return &metav1.PatchOptions{
1049-
DryRun: o.DryRun,
1050-
Force: o.Force,
1051-
FieldManager: o.FieldManager,
1052-
FieldValidation: o.FieldValidation,
1023+
DryRun: o.DryRun,
1024+
Force: o.Force,
1025+
FieldManager: o.FieldManager,
10531026
}
10541027
}

0 commit comments

Comments
 (0)