From e36814913e653c03771e57349cd5fb257ef96eda Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Sat, 9 Sep 2023 16:25:11 -0400 Subject: [PATCH] :bug: Fix status subresource getting updated on Update when it is empty Before this, Update and Patch requests will override to-level status fields for objects that have a status resource if the status was initially empty. This happens because the code to set the status to what we already have in the unupdated object converts the object into a `map[string]any` using json serialization/deserialization, copies the original `status` field, then converts back. The json deserializer unfortunatelly does not properly clear the target, allowing data to remain that is absent in the json serialization. --- pkg/client/fake/client.go | 1 + pkg/client/fake/client_test.go | 22 +++++++--------------- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/pkg/client/fake/client.go b/pkg/client/fake/client.go index eb219cff53..91886d278f 100644 --- a/pkg/client/fake/client.go +++ b/pkg/client/fake/client.go @@ -995,6 +995,7 @@ func fromMapStringAny(u map[string]any, target runtime.Object) error { return fmt.Errorf("failed to serialize: %w", err) } + zero(target) if err := json.Unmarshal(serialized, &target); err != nil { return fmt.Errorf("failed to deserialize: %w", err) } diff --git a/pkg/client/fake/client_test.go b/pkg/client/fake/client_test.go index 533037743d..81a2d4ac43 100644 --- a/pkg/client/fake/client_test.go +++ b/pkg/client/fake/client_test.go @@ -1409,22 +1409,19 @@ var _ = Describe("Fake client", func() { }) It("should not change the status of typed objects that have a status subresource on update", func() { - obj := &corev1.Node{ + obj := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ - Name: "node", - }, - Status: corev1.NodeStatus{ - NodeInfo: corev1.NodeSystemInfo{MachineID: "machine-id"}, + Name: "pod", }, } cl := NewClientBuilder().WithStatusSubresource(obj).WithObjects(obj).Build() - obj.Status.NodeInfo.MachineID = "updated-machine-id" + obj.Status.Phase = "Running" Expect(cl.Update(context.Background(), obj)).To(Succeed()) Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(obj), obj)).To(Succeed()) - Expect(obj.Status).To(BeEquivalentTo(corev1.NodeStatus{NodeInfo: corev1.NodeSystemInfo{MachineID: "machine-id"}})) + Expect(obj.Status).To(BeEquivalentTo(corev1.PodStatus{})) }) It("should return a conflict error when an incorrect RV is used on status update", func() { @@ -1511,25 +1508,20 @@ var _ = Describe("Fake client", func() { }) It("should not change the status of typed objects that have a status subresource on patch", func() { - obj := &corev1.Node{ + obj := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "node", }, - Status: corev1.NodeStatus{ - NodeInfo: corev1.NodeSystemInfo{ - MachineID: "machine-id", - }, - }, } Expect(cl.Create(context.Background(), obj)).To(Succeed()) original := obj.DeepCopy() - obj.Status.NodeInfo.MachineID = "machine-id-from-patch" + obj.Status.Phase = "Running" Expect(cl.Patch(context.Background(), obj, client.MergeFrom(original))).To(Succeed()) Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(obj), obj)).To(Succeed()) - Expect(obj.Status).To(BeEquivalentTo(corev1.NodeStatus{NodeInfo: corev1.NodeSystemInfo{MachineID: "machine-id"}})) + Expect(obj.Status).To(BeEquivalentTo(corev1.PodStatus{})) }) It("should not change non-status field of typed objects that have a status subresource on status patch", func() {