From 3f10709e4ce7e14fa9efc019ade3e99bb0a84b8a Mon Sep 17 00:00:00 2001 From: Antoine Pelisse Date: Thu, 30 Apr 2020 13:38:23 -0700 Subject: [PATCH] Fix fieldType being dropped by older go-clients --- .../pkg/apis/meta/v1/validation/validation.go | 2 +- .../meta/v1/validation/validation_test.go | 10 ++--- .../fieldmanager/internal/managedfields.go | 11 ++++- .../internal/managedfields_test.go | 45 +++++++++++++++++++ 4 files changed, 61 insertions(+), 7 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go index 2743793dde2..fcd491f4c07 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go @@ -178,7 +178,7 @@ func ValidateManagedFields(fieldsList []metav1.ManagedFieldsEntry, fldPath *fiel default: allErrs = append(allErrs, field.Invalid(fldPath.Child("operation"), fields.Operation, "must be `Apply` or `Update`")) } - if fields.FieldsType != "FieldsV1" { + if len(fields.FieldsType) > 0 && fields.FieldsType != "FieldsV1" { allErrs = append(allErrs, field.Invalid(fldPath.Child("fieldsType"), fields.FieldsType, "must be `FieldsV1`")) } } diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go index 30d6289f8ee..aa71a600ae2 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go @@ -242,12 +242,8 @@ func TestValidateFieldManagerInvalid(t *testing.T) { } } -func TestValidateMangedFieldsInvalid(t *testing.T) { +func TestValidateManagedFieldsInvalid(t *testing.T) { tests := []metav1.ManagedFieldsEntry{ - { - Operation: metav1.ManagedFieldsOperationUpdate, - // FieldsType is missing - }, { Operation: metav1.ManagedFieldsOperationUpdate, FieldsType: "RandomVersion", @@ -274,6 +270,10 @@ func TestValidateMangedFieldsInvalid(t *testing.T) { func TestValidateMangedFieldsValid(t *testing.T) { tests := []metav1.ManagedFieldsEntry{ + { + Operation: metav1.ManagedFieldsOperationUpdate, + // FieldsType is missing + }, { Operation: metav1.ManagedFieldsOperationUpdate, FieldsType: "FieldsV1", diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields.go index 9e4fa96f887..c5434b101e0 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields.go @@ -107,7 +107,16 @@ func EncodeObjectManagedFields(obj runtime.Object, managed ManagedInterface) err func decodeManagedFields(encodedManagedFields []metav1.ManagedFieldsEntry) (managed managedStruct, err error) { managed.fields = make(fieldpath.ManagedFields, len(encodedManagedFields)) managed.times = make(map[string]*metav1.Time, len(encodedManagedFields)) - for _, encodedVersionedSet := range encodedManagedFields { + + for i, encodedVersionedSet := range encodedManagedFields { + switch encodedVersionedSet.FieldsType { + case "FieldsV1": + // Valid case. + case "": + return managedStruct{}, 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) + } manager, err := BuildManagerIdentifier(&encodedVersionedSet) if err != nil { return managedStruct{}, fmt.Errorf("error decoding manager from %v: %v", encodedVersionedSet, err) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields_test.go index 4b92ea6ec33..68a6fa5a2b7 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields_test.go @@ -27,6 +27,51 @@ import ( "sigs.k8s.io/yaml" ) +// TestHasFieldsType makes sure that we fail if we don't have a +// FieldsType set properly. +func TestHasFieldsType(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 fieldsType V2. + if err := yaml.Unmarshal([]byte(`- apiVersion: v1 + fieldsType: FieldsV2 + 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") + } + + // Missing fieldsType. + if err := yaml.Unmarshal([]byte(`- apiVersion: v1 + 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") + } +} + // TestRoundTripManagedFields will roundtrip ManagedFields from the wire format // (api format) to the format used by sigs.k8s.io/structured-merge-diff and back func TestRoundTripManagedFields(t *testing.T) {