diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index f424d0aab..c1ca90445 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -700,15 +700,15 @@ }, { "ImportPath": "k8s.io/api", - "Rev": "d04530e36602" + "Rev": "8e35c5077a13" }, { "ImportPath": "k8s.io/apimachinery", - "Rev": "283a3268598b" + "Rev": "a5a2acc259d2" }, { "ImportPath": "k8s.io/client-go", - "Rev": "75a1e7fffe07" + "Rev": "b4932b529fd2" }, { "ImportPath": "k8s.io/component-base", diff --git a/go.mod b/go.mod index 64d88bdd9..f01a51938 100644 --- a/go.mod +++ b/go.mod @@ -41,9 +41,9 @@ require ( gopkg.in/natefinch/lumberjack.v2 v2.0.0 gopkg.in/square/go-jose.v2 v2.2.2 gopkg.in/yaml.v2 v2.2.8 - k8s.io/api v0.0.0-20210306132701-d04530e36602 - k8s.io/apimachinery v0.0.0-20210306132128-283a3268598b - k8s.io/client-go v0.0.0-20210306133322-75a1e7fffe07 + k8s.io/api v0.0.0-20210307052319-8e35c5077a13 + k8s.io/apimachinery v0.0.0-20210307052016-a5a2acc259d2 + k8s.io/client-go v0.0.0-20210307052716-b4932b529fd2 k8s.io/component-base v0.0.0-20210306134456-247e7b3ba560 k8s.io/klog/v2 v2.5.0 k8s.io/kube-openapi v0.0.0-20210305001622-591a79e4bda7 @@ -54,8 +54,8 @@ require ( ) replace ( - k8s.io/api => k8s.io/api v0.0.0-20210306132701-d04530e36602 - k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20210306132128-283a3268598b - k8s.io/client-go => k8s.io/client-go v0.0.0-20210306133322-75a1e7fffe07 + k8s.io/api => k8s.io/api v0.0.0-20210307052319-8e35c5077a13 + k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20210307052016-a5a2acc259d2 + k8s.io/client-go => k8s.io/client-go v0.0.0-20210307052716-b4932b529fd2 k8s.io/component-base => k8s.io/component-base v0.0.0-20210306134456-247e7b3ba560 ) diff --git a/go.sum b/go.sum index 163eeca62..f7d4bab5d 100644 --- a/go.sum +++ b/go.sum @@ -583,9 +583,9 @@ honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWh honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg= honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= -k8s.io/api v0.0.0-20210306132701-d04530e36602/go.mod h1:oXIlxI7P0Fm+wAl92K966U84hJ8jYH+vvhPMIFggXJQ= -k8s.io/apimachinery v0.0.0-20210306132128-283a3268598b/go.mod h1:2LERmYT9PI3b4uQt87vnb2UVkblBDzZhucIf8PxvJ2o= -k8s.io/client-go v0.0.0-20210306133322-75a1e7fffe07/go.mod h1:hcawgCmh6c53kdHiIH1dk9ZLukLcM6NPVE8rs3YLqdU= +k8s.io/api v0.0.0-20210307052319-8e35c5077a13/go.mod h1:oXIlxI7P0Fm+wAl92K966U84hJ8jYH+vvhPMIFggXJQ= +k8s.io/apimachinery v0.0.0-20210307052016-a5a2acc259d2/go.mod h1:2LERmYT9PI3b4uQt87vnb2UVkblBDzZhucIf8PxvJ2o= +k8s.io/client-go v0.0.0-20210307052716-b4932b529fd2/go.mod h1:qv0Qrnu6UEKMfXsRgVBD0X/s4dNXOmHHWD79odx0C38= k8s.io/component-base v0.0.0-20210306134456-247e7b3ba560/go.mod h1:I1nomQYfl44UYAQFJJL/eA28WOBZNwpTUseGxPE/jpE= k8s.io/gengo v0.0.0-20200413195148-3a45101e95ac/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE= diff --git a/pkg/endpoints/handlers/create.go b/pkg/endpoints/handlers/create.go index e914eaeea..b2e167f26 100644 --- a/pkg/endpoints/handlers/create.go +++ b/pkg/endpoints/handlers/create.go @@ -35,6 +35,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/audit" + "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager" "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/features" @@ -163,6 +164,7 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int return nil, fmt.Errorf("failed to create new object (Create for %v): %v", scope.Kind, err) } obj = scope.FieldManager.UpdateNoErrors(liveObj, obj, managerOrUserAgent(options.FieldManager, req.UserAgent())) + admit = fieldmanager.NewManagedFieldsValidatingAdmissionController(admit) } if mutatingAdmission, ok := admit.(admission.MutationInterface); ok && mutatingAdmission.Handles(admission.Create) { if err := mutatingAdmission.Admit(ctx, admissionAttributes, scope); err != nil { diff --git a/pkg/endpoints/handlers/fieldmanager/admission.go b/pkg/endpoints/handlers/fieldmanager/admission.go new file mode 100644 index 000000000..20bbab065 --- /dev/null +++ b/pkg/endpoints/handlers/fieldmanager/admission.go @@ -0,0 +1,86 @@ +/* +Copyright 2021 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 fieldmanager + +import ( + "context" + "fmt" + + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/warning" +) + +// InvalidManagedFieldsAfterMutatingAdmissionWarningFormat is the warning that a client receives +// when a create/update/patch request results in invalid managedFields after going through the admission chain. +const InvalidManagedFieldsAfterMutatingAdmissionWarningFormat = ".metadata.managedFields was in an invalid state after admission; this could be caused by an outdated mutating admission controller; please fix your requests: %v" + +// NewManagedFieldsValidatingAdmissionController validates the managedFields after calling +// the provided admission and resets them to their original state if they got changed to an invalid value +func NewManagedFieldsValidatingAdmissionController(wrap admission.Interface) admission.Interface { + if wrap == nil { + return nil + } + return &managedFieldsValidatingAdmissionController{wrap: wrap} +} + +type managedFieldsValidatingAdmissionController struct { + wrap admission.Interface +} + +var _ admission.Interface = &managedFieldsValidatingAdmissionController{} +var _ admission.MutationInterface = &managedFieldsValidatingAdmissionController{} +var _ admission.ValidationInterface = &managedFieldsValidatingAdmissionController{} + +// Handles calls the wrapped admission.Interface if applicable +func (admit *managedFieldsValidatingAdmissionController) Handles(operation admission.Operation) bool { + return admit.wrap.Handles(operation) +} + +// Admit calls the wrapped admission.Interface if applicable and resets the managedFields to their state before admission if they +// got modified in an invalid way +func (admit *managedFieldsValidatingAdmissionController) Admit(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) (err error) { + mutationInterface, isMutationInterface := admit.wrap.(admission.MutationInterface) + if !isMutationInterface { + return nil + } + objectMeta, err := meta.Accessor(a.GetObject()) + if err != nil { + return err + } + managedFieldsBeforeAdmission := objectMeta.GetManagedFields() + if err := mutationInterface.Admit(ctx, a, o); err != nil { + return err + } + managedFieldsAfterAdmission := objectMeta.GetManagedFields() + if _, err := DecodeManagedFields(managedFieldsAfterAdmission); err != nil { + objectMeta.SetManagedFields(managedFieldsBeforeAdmission) + warning.AddWarning(ctx, "", + fmt.Sprintf(InvalidManagedFieldsAfterMutatingAdmissionWarningFormat, + err.Error()), + ) + } + return nil +} + +// Validate calls the wrapped admission.Interface if aplicable +func (admit *managedFieldsValidatingAdmissionController) Validate(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) (err error) { + if validationInterface, isValidationInterface := admit.wrap.(admission.ValidationInterface); isValidationInterface { + return validationInterface.Validate(ctx, a, o) + } + return nil +} diff --git a/pkg/endpoints/handlers/fieldmanager/admission_test.go b/pkg/endpoints/handlers/fieldmanager/admission_test.go new file mode 100644 index 000000000..09df1f476 --- /dev/null +++ b/pkg/endpoints/handlers/fieldmanager/admission_test.go @@ -0,0 +1,122 @@ +/* +Copyright 2021 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 fieldmanager_test + +import ( + "context" + "reflect" + "testing" + + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager" + "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal" + "sigs.k8s.io/structured-merge-diff/v4/fieldpath" +) + +func TestAdmission(t *testing.T) { + wrap := &mockAdmissionController{} + ac := fieldmanager.NewManagedFieldsValidatingAdmissionController(wrap) + now := metav1.Now() + + validFieldsV1, err := internal.SetToFields(*fieldpath.NewSet(fieldpath.MakePathOrDie("metadata", "labels", "test-label"))) + if err != nil { + t.Fatal(err) + } + validManagedFieldsEntry := metav1.ManagedFieldsEntry{ + APIVersion: "v1", + Operation: metav1.ManagedFieldsOperationApply, + Time: &now, + Manager: "test", + FieldsType: "FieldsV1", + FieldsV1: &validFieldsV1, + } + + managedFieldsMutators := map[string]func(in metav1.ManagedFieldsEntry) (out metav1.ManagedFieldsEntry, shouldReset bool){ + "invalid APIVersion": func(managedFields metav1.ManagedFieldsEntry) (metav1.ManagedFieldsEntry, bool) { + managedFields.APIVersion = "" + return managedFields, true + }, + "invalid Operation": func(managedFields metav1.ManagedFieldsEntry) (metav1.ManagedFieldsEntry, bool) { + managedFields.Operation = "invalid operation" + return managedFields, true + }, + "invalid fieldsType": func(managedFields metav1.ManagedFieldsEntry) (metav1.ManagedFieldsEntry, bool) { + managedFields.FieldsType = "invalid fieldsType" + return managedFields, true + }, + "invalid fieldsV1": func(managedFields metav1.ManagedFieldsEntry) (metav1.ManagedFieldsEntry, bool) { + managedFields.FieldsV1 = &metav1.FieldsV1{Raw: []byte("{invalid}")} + return managedFields, true + }, + "invalid manager": func(managedFields metav1.ManagedFieldsEntry) (metav1.ManagedFieldsEntry, bool) { + managedFields.Manager = "" + return managedFields, false + }, + } + + for name, mutate := range managedFieldsMutators { + t.Run(name, func(t *testing.T) { + mutated, shouldReset := mutate(validManagedFieldsEntry) + validEntries := []metav1.ManagedFieldsEntry{validManagedFieldsEntry} + mutatedEntries := []metav1.ManagedFieldsEntry{mutated} + + obj := &v1.ConfigMap{} + obj.SetManagedFields(validEntries) + + wrap.admit = replaceManagedFields(mutatedEntries) + + attrs := admission.NewAttributesRecord(obj, obj, schema.GroupVersionKind{}, "default", "", schema.GroupVersionResource{}, "", admission.Update, nil, false, nil) + if err := ac.(admission.MutationInterface).Admit(context.TODO(), attrs, nil); err != nil { + t.Fatal(err) + } + + if shouldReset && !reflect.DeepEqual(obj.GetManagedFields(), validEntries) { + t.Fatalf("expected: \n%v\ngot:\n%v", validEntries, obj.GetManagedFields()) + } + if !shouldReset && reflect.DeepEqual(obj.GetManagedFields(), validEntries) { + t.Fatalf("expected: \n%v\ngot:\n%v", mutatedEntries, obj.GetManagedFields()) + } + }) + } +} + +func replaceManagedFields(with []metav1.ManagedFieldsEntry) func(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) error { + return func(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) error { + objectMeta, err := meta.Accessor(a.GetObject()) + if err != nil { + return err + } + objectMeta.SetManagedFields(with) + return nil + } +} + +type mockAdmissionController struct { + admit func(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) error +} + +func (c *mockAdmissionController) Handles(operation admission.Operation) bool { + return true +} + +func (c *mockAdmissionController) Admit(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) error { + return c.admit(ctx, a, o) +} diff --git a/pkg/endpoints/handlers/fieldmanager/fieldmanager.go b/pkg/endpoints/handlers/fieldmanager/fieldmanager.go index ec91d772f..71a4eff8e 100644 --- a/pkg/endpoints/handlers/fieldmanager/fieldmanager.go +++ b/pkg/endpoints/handlers/fieldmanager/fieldmanager.go @@ -110,23 +110,22 @@ func newDefaultFieldManager(f Manager, typeConverter TypeConverter, objectConver return NewFieldManager(f, ignoreManagedFieldsFromRequestObject) } -func decodeLiveManagedFields(liveObj runtime.Object) (Managed, error) { +// DecodeManagedFields converts ManagedFields from the wire format (api format) +// to the format used by sigs.k8s.io/structured-merge-diff +func DecodeManagedFields(encodedManagedFields []metav1.ManagedFieldsEntry) (Managed, error) { + return internal.DecodeManagedFields(encodedManagedFields) +} + +func decodeLiveOrNew(liveObj, newObj runtime.Object, ignoreManagedFieldsFromRequestObject bool) (Managed, error) { liveAccessor, err := meta.Accessor(liveObj) if err != nil { return nil, err } - managed, err := internal.DecodeObjectManagedFields(liveAccessor.GetManagedFields()) - if err != nil { - return internal.NewEmptyManaged(), nil - } - return managed, nil -} -func decodeManagedFields(liveObj, newObj runtime.Object, ignoreManagedFieldsFromRequestObject bool) (Managed, error) { // We take the managedFields of the live object in case the request tries to // manually set managedFields via a subresource. if ignoreManagedFieldsFromRequestObject { - return decodeLiveManagedFields(liveObj) + return emptyManagedFieldsOnErr(DecodeManagedFields(liveAccessor.GetManagedFields())) } // If the object doesn't have metadata, we should just return without trying to @@ -140,14 +139,20 @@ func decodeManagedFields(liveObj, newObj runtime.Object, ignoreManagedFieldsFrom return internal.NewEmptyManaged(), nil } - managed, err := internal.DecodeObjectManagedFields(newAccessor.GetManagedFields()) // If the managed field is empty or we failed to decode it, // let's try the live object. This is to prevent clients who // don't understand managedFields from deleting it accidentally. + managed, err := DecodeManagedFields(newAccessor.GetManagedFields()) if err != nil || len(managed.Fields()) == 0 { - return decodeLiveManagedFields(liveObj) + return emptyManagedFieldsOnErr(DecodeManagedFields(liveAccessor.GetManagedFields())) } + return managed, nil +} +func emptyManagedFieldsOnErr(managed Managed, err error) (Managed, error) { + if err != nil { + return internal.NewEmptyManaged(), nil + } return managed, nil } @@ -157,7 +162,7 @@ func decodeManagedFields(liveObj, newObj runtime.Object, ignoreManagedFieldsFrom func (f *FieldManager) Update(liveObj, newObj runtime.Object, manager string) (object runtime.Object, err error) { // First try to decode the managed fields provided in the update, // This is necessary to allow directly updating managed fields. - managed, err := decodeManagedFields(liveObj, newObj, f.ignoreManagedFieldsFromRequestObject) + managed, err := decodeLiveOrNew(liveObj, newObj, f.ignoreManagedFieldsFromRequestObject) if err != nil { return newObj, nil } @@ -219,7 +224,7 @@ func (f *FieldManager) Apply(liveObj, appliedObj runtime.Object, manager string, } // Decode the managed fields in the live object, since it isn't allowed in the patch. - managed, err := internal.DecodeObjectManagedFields(accessor.GetManagedFields()) + managed, err := DecodeManagedFields(accessor.GetManagedFields()) if err != nil { return nil, fmt.Errorf("failed to decode managed fields: %v", err) } diff --git a/pkg/endpoints/handlers/fieldmanager/internal/managedfields.go b/pkg/endpoints/handlers/fieldmanager/internal/managedfields.go index 9a625e2ac..40237cdc6 100644 --- a/pkg/endpoints/handlers/fieldmanager/internal/managedfields.go +++ b/pkg/endpoints/handlers/fieldmanager/internal/managedfields.go @@ -77,15 +77,6 @@ func RemoveObjectManagedFields(obj runtime.Object) { accessor.SetManagedFields(nil) } -// DecodeObjectManagedFields extracts and converts the objects ManagedFields into a fieldpath.ManagedFields. -func DecodeObjectManagedFields(from []metav1.ManagedFieldsEntry) (ManagedInterface, error) { - managed, err := decodeManagedFields(from) - if err != nil { - return nil, fmt.Errorf("failed to convert managed fields from API: %v", err) - } - return &managed, nil -} - // EncodeObjectManagedFields converts and stores the fieldpathManagedFields into the objects ManagedFields func EncodeObjectManagedFields(obj runtime.Object, managed ManagedInterface) error { accessor, err := meta.Accessor(obj) @@ -102,32 +93,41 @@ func EncodeObjectManagedFields(obj runtime.Object, managed ManagedInterface) err return nil } -// decodeManagedFields converts ManagedFields from the wire format (api format) +// DecodeManagedFields converts ManagedFields from the wire format (api format) // to the format used by sigs.k8s.io/structured-merge-diff -func decodeManagedFields(encodedManagedFields []metav1.ManagedFieldsEntry) (managed managedStruct, err error) { +func DecodeManagedFields(encodedManagedFields []metav1.ManagedFieldsEntry) (ManagedInterface, error) { + managed := managedStruct{} managed.fields = make(fieldpath.ManagedFields, len(encodedManagedFields)) managed.times = make(map[string]*metav1.Time, len(encodedManagedFields)) for i, encodedVersionedSet := range encodedManagedFields { + switch encodedVersionedSet.Operation { + case metav1.ManagedFieldsOperationApply, metav1.ManagedFieldsOperationUpdate: + default: + return nil, fmt.Errorf("operation must be `Apply` or `Update`") + } + if len(encodedVersionedSet.APIVersion) < 1 { + return nil, fmt.Errorf("apiVersion must not be empty") + } switch encodedVersionedSet.FieldsType { case "FieldsV1": // Valid case. case "": - return managedStruct{}, fmt.Errorf("missing fieldsType in managed fields entry %d", i) + return nil, fmt.Errorf("missing fieldsType in managed fields entry %d", i) default: - return managedStruct{}, fmt.Errorf("invalid fieldsType %q in managed fields entry %d", encodedVersionedSet.FieldsType, i) + return nil, fmt.Errorf("invalid fieldsType %q in managed fields entry %d", encodedVersionedSet.FieldsType, i) } manager, err := BuildManagerIdentifier(&encodedVersionedSet) if err != nil { - return managedStruct{}, fmt.Errorf("error decoding manager from %v: %v", encodedVersionedSet, err) + return nil, fmt.Errorf("error decoding manager from %v: %v", encodedVersionedSet, err) } managed.fields[manager], err = decodeVersionedSet(&encodedVersionedSet) if err != nil { - return managedStruct{}, fmt.Errorf("error decoding versioned set from %v: %v", encodedVersionedSet, err) + return nil, fmt.Errorf("error decoding versioned set from %v: %v", encodedVersionedSet, err) } managed.times[manager] = encodedVersionedSet.Time } - return managed, nil + return &managed, nil } // BuildManagerIdentifier creates a manager identifier string from a ManagedFieldsEntry diff --git a/pkg/endpoints/handlers/fieldmanager/internal/managedfields_test.go b/pkg/endpoints/handlers/fieldmanager/internal/managedfields_test.go index 68a6fa5a2..f6df7228c 100644 --- a/pkg/endpoints/handlers/fieldmanager/internal/managedfields_test.go +++ b/pkg/endpoints/handlers/fieldmanager/internal/managedfields_test.go @@ -40,7 +40,7 @@ func TestHasFieldsType(t *testing.T) { `), &unmarshaled); err != nil { t.Fatalf("did not expect yaml unmarshalling error but got: %v", err) } - if _, err := decodeManagedFields(unmarshaled); err != nil { + if _, err := DecodeManagedFields(unmarshaled); err != nil { t.Fatalf("did not expect decoding error but got: %v", err) } @@ -54,7 +54,7 @@ func TestHasFieldsType(t *testing.T) { `), &unmarshaled); err != nil { t.Fatalf("did not expect yaml unmarshalling error but got: %v", err) } - if _, err := decodeManagedFields(unmarshaled); err == nil { + if _, err := DecodeManagedFields(unmarshaled); err == nil { t.Fatal("Expect decoding error but got none") } @@ -67,7 +67,85 @@ func TestHasFieldsType(t *testing.T) { `), &unmarshaled); err != nil { t.Fatalf("did not expect yaml unmarshalling error but got: %v", err) } - if _, err := decodeManagedFields(unmarshaled); err == nil { + if _, err := DecodeManagedFields(unmarshaled); err == nil { + t.Fatal("Expect decoding error but got none") + } +} + +// TestHasAPIVersion makes sure that we fail if we don't have an +// APIVersion set. +func TestHasAPIVersion(t *testing.T) { + var unmarshaled []metav1.ManagedFieldsEntry + if err := yaml.Unmarshal([]byte(`- apiVersion: v1 + fieldsType: FieldsV1 + fieldsV1: + f:field: {} + manager: foo + operation: Apply +`), &unmarshaled); err != nil { + t.Fatalf("did not expect yaml unmarshalling error but got: %v", err) + } + if _, err := DecodeManagedFields(unmarshaled); err != nil { + t.Fatalf("did not expect decoding error but got: %v", err) + } + + // Missing apiVersion. + unmarshaled = nil + if err := yaml.Unmarshal([]byte(`- fieldsType: FieldsV1 + fieldsV1: + f:field: {} + manager: foo + operation: Apply +`), &unmarshaled); err != nil { + t.Fatalf("did not expect yaml unmarshalling error but got: %v", err) + } + if _, err := DecodeManagedFields(unmarshaled); err == nil { + t.Fatal("Expect decoding error but got none") + } +} + +// TestHasOperation makes sure that we fail if we don't have an +// Operation set properly. +func TestHasOperation(t *testing.T) { + var unmarshaled []metav1.ManagedFieldsEntry + if err := yaml.Unmarshal([]byte(`- apiVersion: v1 + fieldsType: FieldsV1 + fieldsV1: + f:field: {} + manager: foo + operation: Apply +`), &unmarshaled); err != nil { + t.Fatalf("did not expect yaml unmarshalling error but got: %v", err) + } + if _, err := DecodeManagedFields(unmarshaled); err != nil { + t.Fatalf("did not expect decoding error but got: %v", err) + } + + // Invalid operation. + if err := yaml.Unmarshal([]byte(`- apiVersion: v1 + fieldsType: FieldsV1 + fieldsV1: + f:field: {} + manager: foo + operation: Invalid +`), &unmarshaled); err != nil { + t.Fatalf("did not expect yaml unmarshalling error but got: %v", err) + } + if _, err := DecodeManagedFields(unmarshaled); err == nil { + t.Fatal("Expect decoding error but got none") + } + + // Missing operation. + unmarshaled = nil + if err := yaml.Unmarshal([]byte(`- apiVersion: v1 + fieldsType: FieldsV1 + fieldsV1: + f:field: {} + manager: foo +`), &unmarshaled); err != nil { + t.Fatalf("did not expect yaml unmarshalling error but got: %v", err) + } + if _, err := DecodeManagedFields(unmarshaled); err == nil { t.Fatal("Expect decoding error but got none") } } @@ -189,11 +267,11 @@ func TestRoundTripManagedFields(t *testing.T) { if err := yaml.Unmarshal([]byte(test), &unmarshaled); err != nil { t.Fatalf("did not expect yaml unmarshalling error but got: %v", err) } - decoded, err := decodeManagedFields(unmarshaled) + decoded, err := DecodeManagedFields(unmarshaled) if err != nil { t.Fatalf("did not expect decoding error but got: %v", err) } - encoded, err := encodeManagedFields(&decoded) + encoded, err := encodeManagedFields(decoded) if err != nil { t.Fatalf("did not expect encoding error but got: %v", err) } diff --git a/pkg/endpoints/handlers/patch.go b/pkg/endpoints/handlers/patch.go index 4c0d10230..903307fc2 100644 --- a/pkg/endpoints/handlers/patch.go +++ b/pkg/endpoints/handlers/patch.go @@ -171,6 +171,9 @@ func PatchResource(r rest.Patcher, scope *RequestScope, admit admission.Interfac userInfo, ) + if scope.FieldManager != nil { + admit = fieldmanager.NewManagedFieldsValidatingAdmissionController(admit) + } mutatingAdmission, _ := admit.(admission.MutationInterface) createAuthorizerAttributes := authorizer.AttributesRecord{ User: userInfo, diff --git a/pkg/endpoints/handlers/update.go b/pkg/endpoints/handlers/update.go index fd215bb38..57daefd9c 100644 --- a/pkg/endpoints/handlers/update.go +++ b/pkg/endpoints/handlers/update.go @@ -33,6 +33,7 @@ import ( "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/audit" "k8s.io/apiserver/pkg/authorization/authorizer" + "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager" "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/features" @@ -130,6 +131,7 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa // allows skipping managedFields update if the resulting object is too big shouldUpdateManagedFields := true if scope.FieldManager != nil { + admit = fieldmanager.NewManagedFieldsValidatingAdmissionController(admit) transformers = append(transformers, func(_ context.Context, newObj, liveObj runtime.Object) (runtime.Object, error) { if shouldUpdateManagedFields { return scope.FieldManager.UpdateNoErrors(liveObj, newObj, managerOrUserAgent(options.FieldManager, req.UserAgent())), nil