Skip to content

Commit 7b6b975

Browse files
committed
add test case to show it will not override other status fields when updating
Signed-off-by: Troy Connor <troy0820@users.noreply.github.com>
1 parent b6a7a0e commit 7b6b975

File tree

2 files changed

+38
-24
lines changed

2 files changed

+38
-24
lines changed

pkg/client/fake/client.go

+8-24
Original file line numberDiff line numberDiff line change
@@ -400,9 +400,10 @@ func (t versionedTracker) update(gvr schema.GroupVersionResource, obj runtime.Ob
400400

401401
if t.withStatusSubresource.Has(gvk) {
402402
if isStatus { // copy everything but status and metadata.ResourceVersion from original object
403-
if err := copyNonStatusFrom(oldObject, obj); err != nil {
403+
if err := copyStatusFrom(obj, oldObject); err != nil {
404404
return fmt.Errorf("failed to copy non-status field for object with status subresouce: %w", err)
405405
}
406+
obj = oldObject.DeepCopyObject().(client.Object)
406407
} else { // copy status from original object
407408
if err := copyStatusFrom(oldObject, obj); err != nil {
408409
return fmt.Errorf("failed to copy the status for object with status subresource: %w", err)
@@ -949,6 +950,7 @@ func dryPatch(action testing.PatchActionImpl, tracker testing.ObjectTracker) (ru
949950
return obj, nil
950951
}
951952

953+
//lint:ignore U1000 function is causing errors with status updates
952954
func copyNonStatusFrom(old, new runtime.Object) error {
953955
newClientObject, ok := new.(client.Object)
954956
if !ok {
@@ -1108,30 +1110,12 @@ func (sw *fakeSubResourceClient) Create(ctx context.Context, obj client.Object,
11081110
func (sw *fakeSubResourceClient) Update(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error {
11091111
updateOptions := client.SubResourceUpdateOptions{}
11101112
updateOptions.ApplyOptions(opts)
1111-
gvr, err := getGVRFromObject(obj, sw.client.scheme)
1112-
if err != nil {
1113-
return err
1114-
}
1115-
o, err := sw.client.tracker.Get(gvr, obj.GetNamespace(), obj.GetName())
1116-
if err != nil {
1117-
return err
1118-
}
1119-
gvk, err := apiutil.GVKForObject(obj, sw.client.scheme)
1120-
if err != nil {
1121-
return err
1122-
}
1123-
body := o
1124-
if sw.client.tracker.withStatusSubresource.Has(gvk) {
1125-
err := copyStatusFrom(obj, body)
1126-
if err != nil {
1127-
return err
1128-
}
1129-
}
1130-
bodyObj := body.(client.Object)
1131-
if bodyObj.GetResourceVersion() != obj.GetResourceVersion() {
1132-
return apierrors.NewConflict(gvr.GroupResource(), obj.GetName(), fmt.Errorf("resource version conflict"))
1113+
1114+
body := obj
1115+
if updateOptions.SubResourceBody != nil {
1116+
body = updateOptions.SubResourceBody
11331117
}
1134-
return sw.client.update(bodyObj, true, &updateOptions.UpdateOptions)
1118+
return sw.client.update(body, true, &updateOptions.UpdateOptions)
11351119
}
11361120

11371121
func (sw *fakeSubResourceClient) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.SubResourcePatchOption) error {

pkg/client/fake/client_test.go

+30
Original file line numberDiff line numberDiff line change
@@ -1479,6 +1479,36 @@ var _ = Describe("Fake client", func() {
14791479
objOriginal.Status.NodeInfo.MachineID = "machine-id-from-status-update"
14801480
Expect(cmp.Diff(objOriginal, actual)).To(BeEmpty())
14811481
})
1482+
It("should not overwrite status fields of typed objects that have a status subresource on status update", func() {
1483+
obj := &corev1.Node{
1484+
ObjectMeta: metav1.ObjectMeta{
1485+
Name: "node",
1486+
},
1487+
Spec: corev1.NodeSpec{
1488+
PodCIDR: "old-cidr",
1489+
},
1490+
Status: corev1.NodeStatus{
1491+
NodeInfo: corev1.NodeSystemInfo{
1492+
MachineID: "machine-id",
1493+
},
1494+
},
1495+
}
1496+
cl := NewClientBuilder().WithStatusSubresource(obj).WithObjects(obj).Build()
1497+
objOriginal := obj.DeepCopy()
1498+
1499+
obj.Status.Phase = corev1.NodeRunning
1500+
Expect(cl.Status().Update(context.Background(), obj)).NotTo(HaveOccurred())
1501+
1502+
actual := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: obj.Name}}
1503+
Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(actual), actual)).NotTo(HaveOccurred())
1504+
1505+
objOriginal.APIVersion = actual.APIVersion
1506+
objOriginal.Kind = actual.Kind
1507+
objOriginal.ResourceVersion = actual.ResourceVersion
1508+
Expect(cmp.Diff(objOriginal, actual)).ToNot(BeEmpty())
1509+
Expect(objOriginal.Status.NodeInfo.MachineID).To(Equal(actual.Status.NodeInfo.MachineID))
1510+
Expect(objOriginal.Status.Phase).ToNot(Equal(actual.Status.Phase))
1511+
})
14821512

14831513
It("should not change the status of typed objects that have a status subresource on patch", func() {
14841514
obj := &corev1.Node{

0 commit comments

Comments
 (0)