Skip to content

Commit ed2cf6e

Browse files
apelisseapelisse
committed
FieldManager: Reset if we receive nil or a list with one empty item
1 parent 0281b85 commit ed2cf6e

File tree

4 files changed

+189
-19
lines changed

4 files changed

+189
-19
lines changed

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

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package fieldmanager
1818

1919
import (
2020
"fmt"
21+
"reflect"
2122
"time"
2223

2324
"k8s.io/apimachinery/pkg/api/meta"
@@ -111,21 +112,25 @@ func newDefaultFieldManager(f Manager, objectCreater runtime.ObjectCreater, kind
111112
func (f *FieldManager) Update(liveObj, newObj runtime.Object, manager string) (object runtime.Object, err error) {
112113
// If the object doesn't have metadata, we should just return without trying to
113114
// set the managedFields at all, so creates/updates/patches will work normally.
114-
if _, err = meta.Accessor(newObj); err != nil {
115+
newAccessor, err := meta.Accessor(newObj)
116+
if err != nil {
115117
return newObj, nil
116118
}
117119

118120
// First try to decode the managed fields provided in the update,
119121
// This is necessary to allow directly updating managed fields.
120122
var managed Managed
121-
if managed, err = internal.DecodeObjectManagedFields(newObj); err != nil || len(managed.Fields()) == 0 {
123+
if isResetManagedFields(newAccessor.GetManagedFields()) {
124+
managed = internal.NewEmptyManaged()
125+
} else if managed, err = internal.DecodeObjectManagedFields(newAccessor.GetManagedFields()); err != nil || len(managed.Fields()) == 0 {
126+
liveAccessor, err := meta.Accessor(liveObj)
127+
if err != nil {
128+
return newObj, nil
129+
}
122130
// If the managed field is empty or we failed to decode it,
123131
// let's try the live object. This is to prevent clients who
124132
// don't understand managedFields from deleting it accidentally.
125-
managed, err = internal.DecodeObjectManagedFields(liveObj)
126-
if err != nil {
127-
// If we also can't decode the liveObject, then
128-
// just restart managedFields from scratch.
133+
if managed, err = internal.DecodeObjectManagedFields(liveAccessor.GetManagedFields()); err != nil {
129134
managed = internal.NewEmptyManaged()
130135
}
131136
}
@@ -163,17 +168,33 @@ func (f *FieldManager) UpdateNoErrors(liveObj, newObj runtime.Object, manager st
163168
return obj
164169
}
165170

171+
// Returns true if the managedFields indicate that the user is trying to
172+
// reset the managedFields, i.e. if the list is non-nil but empty, or if
173+
// the list has one empty item.
174+
func isResetManagedFields(managedFields []metav1.ManagedFieldsEntry) bool {
175+
if len(managedFields) == 0 {
176+
return managedFields != nil
177+
}
178+
179+
if len(managedFields) == 1 {
180+
return reflect.DeepEqual(managedFields[0], metav1.ManagedFieldsEntry{})
181+
}
182+
183+
return false
184+
}
185+
166186
// Apply is used when server-side apply is called, as it merges the
167187
// object and updates the managed fields.
168188
func (f *FieldManager) Apply(liveObj, appliedObj runtime.Object, manager string, force bool) (object runtime.Object, err error) {
169189
// If the object doesn't have metadata, apply isn't allowed.
170-
if _, err = meta.Accessor(liveObj); err != nil {
190+
accessor, err := meta.Accessor(liveObj)
191+
if err != nil {
171192
return nil, fmt.Errorf("couldn't get accessor: %v", err)
172193
}
173194

174195
// Decode the managed fields in the live object, since it isn't allowed in the patch.
175-
var managed Managed
176-
if managed, err = internal.DecodeObjectManagedFields(liveObj); err != nil {
196+
managed, err := internal.DecodeObjectManagedFields(accessor.GetManagedFields())
197+
if err != nil {
177198
return nil, fmt.Errorf("failed to decode managed fields: %v", err)
178199
}
179200

‎staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,3 +783,86 @@ func TestNoOpChanges(t *testing.T) {
783783
t.Fatalf("No-op apply has changed the object:\n%v\n---\n%v", before, f.liveObj)
784784
}
785785
}
786+
787+
// Tests that one can reset the managedFields by sending either an empty
788+
// list
789+
func TestResetManagedFieldsEmptyList(t *testing.T) {
790+
f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"))
791+
792+
obj := &unstructured.Unstructured{Object: map[string]interface{}{}}
793+
if err := yaml.Unmarshal([]byte(`{
794+
"apiVersion": "v1",
795+
"kind": "Pod",
796+
"metadata": {
797+
"labels": {
798+
"a": "b"
799+
},
800+
}
801+
}`), &obj.Object); err != nil {
802+
t.Fatalf("error decoding YAML: %v", err)
803+
}
804+
if err := f.Apply(obj, "fieldmanager_test_apply", false); err != nil {
805+
t.Fatalf("failed to apply object: %v", err)
806+
}
807+
808+
if err := yaml.Unmarshal([]byte(`{
809+
"apiVersion": "v1",
810+
"kind": "Pod",
811+
"metadata": {
812+
"managedFields": [],
813+
"labels": {
814+
"a": "b"
815+
},
816+
}
817+
}`), &obj.Object); err != nil {
818+
t.Fatalf("error decoding YAML: %v", err)
819+
}
820+
if err := f.Update(obj, "update_manager"); err != nil {
821+
t.Fatalf("failed to update with empty manager: %v", err)
822+
}
823+
824+
if len(f.ManagedFields()) != 0 {
825+
t.Fatalf("failed to reset managedFields: %v", f.ManagedFields())
826+
}
827+
}
828+
829+
// Tests that one can reset the managedFields by sending either a list with one empty item.
830+
func TestResetManagedFieldsEmptyItem(t *testing.T) {
831+
f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"))
832+
833+
obj := &unstructured.Unstructured{Object: map[string]interface{}{}}
834+
if err := yaml.Unmarshal([]byte(`{
835+
"apiVersion": "v1",
836+
"kind": "Pod",
837+
"metadata": {
838+
"labels": {
839+
"a": "b"
840+
},
841+
}
842+
}`), &obj.Object); err != nil {
843+
t.Fatalf("error decoding YAML: %v", err)
844+
}
845+
if err := f.Apply(obj, "fieldmanager_test_apply", false); err != nil {
846+
t.Fatalf("failed to apply object: %v", err)
847+
}
848+
849+
if err := yaml.Unmarshal([]byte(`{
850+
"apiVersion": "v1",
851+
"kind": "Pod",
852+
"metadata": {
853+
"managedFields": [{}],
854+
"labels": {
855+
"a": "b"
856+
},
857+
}
858+
}`), &obj.Object); err != nil {
859+
t.Fatalf("error decoding YAML: %v", err)
860+
}
861+
if err := f.Update(obj, "update_manager"); err != nil {
862+
t.Fatalf("failed to update with empty manager: %v", err)
863+
}
864+
865+
if len(f.ManagedFields()) != 0 {
866+
t.Fatalf("failed to reset managedFields: %v", f.ManagedFields())
867+
}
868+
}

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

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -78,16 +78,8 @@ func RemoveObjectManagedFields(obj runtime.Object) {
7878
}
7979

8080
// DecodeObjectManagedFields extracts and converts the objects ManagedFields into a fieldpath.ManagedFields.
81-
func DecodeObjectManagedFields(from runtime.Object) (ManagedInterface, error) {
82-
if from == nil {
83-
return &managedStruct{}, nil
84-
}
85-
accessor, err := meta.Accessor(from)
86-
if err != nil {
87-
panic(fmt.Sprintf("couldn't get accessor: %v", err))
88-
}
89-
90-
managed, err := decodeManagedFields(accessor.GetManagedFields())
81+
func DecodeObjectManagedFields(from []metav1.ManagedFieldsEntry) (ManagedInterface, error) {
82+
managed, err := decodeManagedFields(from)
9183
if err != nil {
9284
return nil, fmt.Errorf("failed to convert managed fields from API: %v", err)
9385
}

‎test/integration/apiserver/apply/apply_test.go‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1578,6 +1578,80 @@ func TestErrorsDontFailPatch(t *testing.T) {
15781578

15791579
}
15801580

1581+
// TestClearManagedFieldsWithUpdateEmptyList verifies it's possible to clear the managedFields by sending an empty list.
1582+
func TestClearManagedFieldsWithUpdateEmptyList(t *testing.T) {
1583+
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ServerSideApply, true)()
1584+
1585+
_, client, closeFn := setup(t)
1586+
defer closeFn()
1587+
1588+
_, err := client.CoreV1().RESTClient().Patch(types.ApplyPatchType).
1589+
Namespace("default").
1590+
Resource("configmaps").
1591+
Name("test-cm").
1592+
Param("fieldManager", "apply_test").
1593+
Body([]byte(`{
1594+
"apiVersion": "v1",
1595+
"kind": "ConfigMap",
1596+
"metadata": {
1597+
"name": "test-cm",
1598+
"namespace": "default",
1599+
"labels": {
1600+
"test-label": "test"
1601+
}
1602+
},
1603+
"data": {
1604+
"key": "value"
1605+
}
1606+
}`)).
1607+
Do(context.TODO()).
1608+
Get()
1609+
if err != nil {
1610+
t.Fatalf("Failed to create object using Apply patch: %v", err)
1611+
}
1612+
1613+
_, err = client.CoreV1().RESTClient().Put().
1614+
Namespace("default").
1615+
Resource("configmaps").
1616+
Name("test-cm").
1617+
Body([]byte(`{
1618+
"apiVersion": "v1",
1619+
"kind": "ConfigMap",
1620+
"metadata": {
1621+
"name": "test-cm",
1622+
"namespace": "default",
1623+
"managedFields": [],
1624+
"labels": {
1625+
"test-label": "test"
1626+
}
1627+
},
1628+
"data": {
1629+
"key": "value"
1630+
}
1631+
}`)).Do(context.TODO()).Get()
1632+
if err != nil {
1633+
t.Fatalf("Failed to patch object: %v", err)
1634+
}
1635+
1636+
object, err := client.CoreV1().RESTClient().Get().Namespace("default").Resource("configmaps").Name("test-cm").Do(context.TODO()).Get()
1637+
if err != nil {
1638+
t.Fatalf("Failed to retrieve object: %v", err)
1639+
}
1640+
1641+
accessor, err := meta.Accessor(object)
1642+
if err != nil {
1643+
t.Fatalf("Failed to get meta accessor: %v", err)
1644+
}
1645+
1646+
if managedFields := accessor.GetManagedFields(); len(managedFields) != 0 {
1647+
t.Fatalf("Failed to clear managedFields, got: %v", managedFields)
1648+
}
1649+
1650+
if labels := accessor.GetLabels(); len(labels) < 1 {
1651+
t.Fatalf("Expected other fields to stay untouched, got: %v", object)
1652+
}
1653+
}
1654+
15811655
var podBytes = []byte(`
15821656
apiVersion: v1
15831657
kind: Pod

0 commit comments

Comments
 (0)