Merge pull request #3283 from alvaroaleman/fix-unmarshal

🐛 Fakeclient: Update passed object in Apply

Author: k8s-ci-robot

pkg/client/fake/client.go

@@ -1085,7 +1085,30 @@ func (c *fakeClient) Apply(ctx context.Context, obj runtime.ApplyConfiguration,
 patchOpts := &client.PatchOptions{}
 patchOpts.Raw = applyOpts.AsPatchOptions()
 
-return c.patch(u, applyPatch, patchOpts)
+if err := c.patch(u, applyPatch, patchOpts); err != nil {
+return err
+}
+
+acJSON, err := json.Marshal(u)
+if err != nil {
+return fmt.Errorf("failed to marshal patched object: %w", err)
+}
+
+// We have to zero the object in case it contained a status and there is a
+// status subresource. If its the private `unstructuredApplyConfiguration`
+// we can not zero all of it, as that will cause the embedded Unstructured
+// to be nil which then causes a NPD in the json.Unmarshal below.
+switch reflect.TypeOf(obj).String() {
+case "*client.unstructuredApplyConfiguration":
+zero(reflect.ValueOf(obj).Elem().FieldByName("Unstructured").Interface())
+default:
+zero(obj)
+}
+if err := json.Unmarshal(acJSON, obj); err != nil {
+return fmt.Errorf("failed to unmarshal patched object: %w", err)
+}
+
+return nil
 }
 
 type fakeApplyPatch struct{}

pkg/client/fake/client_test.go

@@ -2542,6 +2542,74 @@ var _ = Describe("Fake client", func() {
 wg.Wait()
 })
 
+DescribeTable("mutating operations return the updated object",
+func(ctx SpecContext, mutate func(ctx SpecContext) (*corev1.ConfigMap, error)) {
+mutated, err := mutate(ctx)
+Expect(err).NotTo(HaveOccurred())
+
+var retrieved corev1.ConfigMap
+Expect(cl.Get(ctx, client.ObjectKeyFromObject(mutated), &retrieved)).To(Succeed())
+
+Expect(&retrieved).To(BeComparableTo(mutated))
+},
+
+Entry("create", func(ctx SpecContext) (*corev1.ConfigMap, error) {
+cl = NewClientBuilder().Build()
+cm.ResourceVersion = ""
+return cm, cl.Create(ctx, cm)
+}),
+Entry("update", func(ctx SpecContext) (*corev1.ConfigMap, error) {
+cl = NewClientBuilder().WithObjects(cm).Build()
+cm.Labels = map[string]string{"updated-label": "update-test"}
+cm.Data["new-key"] = "new-value"
+return cm, cl.Update(ctx, cm)
+}),
+Entry("patch", func(ctx SpecContext) (*corev1.ConfigMap, error) {
+cl = NewClientBuilder().WithObjects(cm).Build()
+original := cm.DeepCopy()
+
+cm.Labels = map[string]string{"updated-label": "update-test"}
+cm.Data["new-key"] = "new-value"
+return cm, cl.Patch(ctx, cm, client.MergeFrom(original))
+}),
+Entry("Create through Apply", func(ctx SpecContext) (*corev1.ConfigMap, error) {
+ac := corev1applyconfigurations.ConfigMap(cm.Name, cm.Namespace).WithData(cm.Data)
+
+cl = NewClientBuilder().Build()
+Expect(cl.Apply(ctx, ac, client.FieldOwner("foo"))).To(Succeed())
+
+serialized, err := json.Marshal(ac)
+Expect(err).NotTo(HaveOccurred())
+
+var cm corev1.ConfigMap
+Expect(json.Unmarshal(serialized, &cm)).To(Succeed())
+
+// ApplyConfigurations always have TypeMeta set as they do not support using the scheme
+// to retrieve gvk.
+cm.TypeMeta = metav1.TypeMeta{}
+return &cm, nil
+}),
+Entry("Update through Apply", func(ctx SpecContext) (*corev1.ConfigMap, error) {
+ac := corev1applyconfigurations.ConfigMap(cm.Name, cm.Namespace).
+WithLabels(map[string]string{"updated-label": "update-test"}).
+WithData(map[string]string{"new-key": "new-value"})
+
+cl = NewClientBuilder().WithObjects(cm).Build()
+Expect(cl.Apply(ctx, ac, client.FieldOwner("foo"))).To(Succeed())
+
+serialized, err := json.Marshal(ac)
+Expect(err).NotTo(HaveOccurred())
+
+var cm corev1.ConfigMap
+Expect(json.Unmarshal(serialized, &cm)).To(Succeed())
+
+// ApplyConfigurations always have TypeMeta set as they do not support using the scheme
+// to retrieve gvk.
+cm.TypeMeta = metav1.TypeMeta{}
+return &cm, nil
+}),
+)
+
 It("supports server-side apply of a client-go resource", func(ctx SpecContext) {
 cl := NewClientBuilder().Build()
 obj := &unstructured.Unstructured{}
@@ -2810,7 +2878,7 @@ var _ = Describe("Fake client", func() {
 })
 
 It("allows to set deletionTimestamp on an object during SSA create", func(ctx SpecContext) {
-now := metav1.Now()
+now := metav1.Time{Time: time.Now().Round(time.Second)}
 obj := corev1applyconfigurations.
 ConfigMap("foo", "default").
 WithDeletionTimestamp(now).
@@ -2823,8 +2891,7 @@ var _ = Describe("Fake client", func() {
 })
 
 It("will silently ignore a deletionTimestamp update through SSA", func(ctx SpecContext) {
-Skip("the apply logic in the managedFieldObjectTracker seems to override this")
-now := metav1.Now()
+now := metav1.Time{Time: time.Now().Round(time.Second)}
 obj := corev1applyconfigurations.
 ConfigMap("foo", "default").
 WithDeletionTimestamp(now).