Skip to content

Commit 3f10709

Browse files
committed
Fix fieldType being dropped by older go-clients
1 parent ed2cf6e commit 3f10709

File tree

4 files changed

+61
-7
lines changed

4 files changed

+61
-7
lines changed

staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ func ValidateManagedFields(fieldsList []metav1.ManagedFieldsEntry, fldPath *fiel
178178
default:
179179
allErrs = append(allErrs, field.Invalid(fldPath.Child("operation"), fields.Operation, "must be `Apply` or `Update`"))
180180
}
181-
if fields.FieldsType != "FieldsV1" {
181+
if len(fields.FieldsType) > 0 && fields.FieldsType != "FieldsV1" {
182182
allErrs = append(allErrs, field.Invalid(fldPath.Child("fieldsType"), fields.FieldsType, "must be `FieldsV1`"))
183183
}
184184
}

staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -242,12 +242,8 @@ func TestValidateFieldManagerInvalid(t *testing.T) {
242242
}
243243
}
244244

245-
func TestValidateMangedFieldsInvalid(t *testing.T) {
245+
func TestValidateManagedFieldsInvalid(t *testing.T) {
246246
tests := []metav1.ManagedFieldsEntry{
247-
{
248-
Operation: metav1.ManagedFieldsOperationUpdate,
249-
// FieldsType is missing
250-
},
251247
{
252248
Operation: metav1.ManagedFieldsOperationUpdate,
253249
FieldsType: "RandomVersion",
@@ -274,6 +270,10 @@ func TestValidateMangedFieldsInvalid(t *testing.T) {
274270

275271
func TestValidateMangedFieldsValid(t *testing.T) {
276272
tests := []metav1.ManagedFieldsEntry{
273+
{
274+
Operation: metav1.ManagedFieldsOperationUpdate,
275+
// FieldsType is missing
276+
},
277277
{
278278
Operation: metav1.ManagedFieldsOperationUpdate,
279279
FieldsType: "FieldsV1",

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,16 @@ func EncodeObjectManagedFields(obj runtime.Object, managed ManagedInterface) err
107107
func decodeManagedFields(encodedManagedFields []metav1.ManagedFieldsEntry) (managed managedStruct, err error) {
108108
managed.fields = make(fieldpath.ManagedFields, len(encodedManagedFields))
109109
managed.times = make(map[string]*metav1.Time, len(encodedManagedFields))
110-
for _, encodedVersionedSet := range encodedManagedFields {
110+
111+
for i, encodedVersionedSet := range encodedManagedFields {
112+
switch encodedVersionedSet.FieldsType {
113+
case "FieldsV1":
114+
// Valid case.
115+
case "":
116+
return managedStruct{}, fmt.Errorf("missing fieldsType in managed fields entry %d", i)
117+
default:
118+
return managedStruct{}, fmt.Errorf("invalid fieldsType %q in managed fields entry %d", encodedVersionedSet.FieldsType, i)
119+
}
111120
manager, err := BuildManagerIdentifier(&encodedVersionedSet)
112121
if err != nil {
113122
return managedStruct{}, fmt.Errorf("error decoding manager from %v: %v", encodedVersionedSet, err)

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,51 @@ import (
2727
"sigs.k8s.io/yaml"
2828
)
2929

30+
// TestHasFieldsType makes sure that we fail if we don't have a
31+
// FieldsType set properly.
32+
func TestHasFieldsType(t *testing.T) {
33+
var unmarshaled []metav1.ManagedFieldsEntry
34+
if err := yaml.Unmarshal([]byte(`- apiVersion: v1
35+
fieldsType: FieldsV1
36+
fieldsV1:
37+
f:field: {}
38+
manager: foo
39+
operation: Apply
40+
`), &unmarshaled); err != nil {
41+
t.Fatalf("did not expect yaml unmarshalling error but got: %v", err)
42+
}
43+
if _, err := decodeManagedFields(unmarshaled); err != nil {
44+
t.Fatalf("did not expect decoding error but got: %v", err)
45+
}
46+
47+
// Invalid fieldsType V2.
48+
if err := yaml.Unmarshal([]byte(`- apiVersion: v1
49+
fieldsType: FieldsV2
50+
fieldsV1:
51+
f:field: {}
52+
manager: foo
53+
operation: Apply
54+
`), &unmarshaled); err != nil {
55+
t.Fatalf("did not expect yaml unmarshalling error but got: %v", err)
56+
}
57+
if _, err := decodeManagedFields(unmarshaled); err == nil {
58+
t.Fatal("Expect decoding error but got none")
59+
}
60+
61+
// Missing fieldsType.
62+
if err := yaml.Unmarshal([]byte(`- apiVersion: v1
63+
fieldsV1:
64+
f:field: {}
65+
manager: foo
66+
operation: Apply
67+
`), &unmarshaled); err != nil {
68+
t.Fatalf("did not expect yaml unmarshalling error but got: %v", err)
69+
}
70+
if _, err := decodeManagedFields(unmarshaled); err == nil {
71+
t.Fatal("Expect decoding error but got none")
72+
}
73+
}
74+
3075
// TestRoundTripManagedFields will roundtrip ManagedFields from the wire format
3176
// (api format) to the format used by sigs.k8s.io/structured-merge-diff and back
3277
func TestRoundTripManagedFields(t *testing.T) {

0 commit comments

Comments
 (0)