From 685f27bb500fe40ede53379da1675cfa71387a94 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Mon, 26 May 2025 17:35:51 -0400 Subject: [PATCH] :warning: Fakeclient: Clear typemeta for structured The fakeclient currently differs from the liveclient in that if a structured object is created that has typemeta set, it will retain that. In contrast to that, the liveclient always strips it. This change makes the fakeclient strip it just like the live client. --- pkg/client/fake/client.go | 130 +++++++++++++++++----------- pkg/client/fake/client_test.go | 152 +++++++++++++++++++-------------- 2 files changed, 166 insertions(+), 116 deletions(-) diff --git a/pkg/client/fake/client.go b/pkg/client/fake/client.go index 16e2cba512..a4ef75cc83 100644 --- a/pkg/client/fake/client.go +++ b/pkg/client/fake/client.go @@ -332,8 +332,9 @@ func (t versionedTracker) Create(gvr schema.GroupVersionResource, obj runtime.Ob return fmt.Errorf("failed to get accessor for object: %w", err) } if accessor.GetName() == "" { + gvk, _ := apiutil.GVKForObject(obj, t.scheme) return apierrors.NewInvalid( - obj.GetObjectKind().GroupVersionKind().GroupKind(), + gvk.GroupKind(), accessor.GetName(), field.ErrorList{field.Required(field.NewPath("metadata.name"), "name is required")}) } @@ -433,8 +434,9 @@ func (t versionedTracker) updateObject(gvr schema.GroupVersionResource, obj runt } if accessor.GetName() == "" { + gvk, _ := apiutil.GVKForObject(obj, t.scheme) return nil, apierrors.NewInvalid( - obj.GetObjectKind().GroupVersionKind().GroupKind(), + gvk.GroupKind(), accessor.GetName(), field.ErrorList{field.Required(field.NewPath("metadata.name"), "name is required")}) } @@ -527,33 +529,35 @@ func (c *fakeClient) Get(ctx context.Context, key client.ObjectKey, obj client.O if err != nil { return err } + gvk, err := apiutil.GVKForObject(obj, c.scheme) + if err != nil { + return err + } o, err := c.tracker.Get(gvr, key.Namespace, key.Name) if err != nil { return err } - _, isUnstructured := obj.(runtime.Unstructured) - _, isPartialObject := obj.(*metav1.PartialObjectMetadata) - - if isUnstructured || isPartialObject { - gvk, err := apiutil.GVKForObject(obj, c.scheme) - if err != nil { - return err - } - ta, err := meta.TypeAccessor(o) - if err != nil { - return err - } - ta.SetKind(gvk.Kind) - ta.SetAPIVersion(gvk.GroupVersion().String()) + ta, err := meta.TypeAccessor(o) + if err != nil { + return err } + // If the final object is unstructuctured, the json + // representation must contain GVK or the apimachinery + // json serializer will error out. + ta.SetAPIVersion(gvk.GroupVersion().String()) + ta.SetKind(gvk.Kind) + j, err := json.Marshal(o) if err != nil { return err } zero(obj) - return json.Unmarshal(j, obj) + if err := json.Unmarshal(j, obj); err != nil { + return err + } + return ensureTypeMeta(obj, gvk) } func (c *fakeClient) Watch(ctx context.Context, list client.ObjectList, opts ...client.ListOption) (watch.Interface, error) { @@ -579,8 +583,7 @@ func (c *fakeClient) List(ctx context.Context, obj client.ObjectList, opts ...cl return err } - originalKind := gvk.Kind - + originalGVK := gvk gvk.Kind = strings.TrimSuffix(gvk.Kind, "List") if _, isUnstructuredList := obj.(runtime.Unstructured); isUnstructuredList && !c.scheme.Recognizes(gvk) { @@ -602,39 +605,30 @@ func (c *fakeClient) List(ctx context.Context, obj client.ObjectList, opts ...cl return err } - if _, isUnstructured := obj.(runtime.Unstructured); isUnstructured { - ta, err := meta.TypeAccessor(o) - if err != nil { - return err - } - ta.SetKind(originalKind) - ta.SetAPIVersion(gvk.GroupVersion().String()) - } - j, err := json.Marshal(o) if err != nil { return err } zero(obj) + if err := ensureTypeMeta(obj, originalGVK); err != nil { + return err + } objCopy := obj.DeepCopyObject().(client.ObjectList) if err := json.Unmarshal(j, objCopy); err != nil { return err } - if _, isUnstructured := obj.(runtime.Unstructured); isUnstructured { - ta, err := meta.TypeAccessor(obj) - if err != nil { - return err - } - ta.SetKind(originalKind) - ta.SetAPIVersion(gvk.GroupVersion().String()) - } - objs, err := meta.ExtractList(objCopy) if err != nil { return err } + for _, o := range objs { + if err := ensureTypeMeta(o, gvk); err != nil { + return err + } + } + if listOpts.LabelSelector == nil && listOpts.FieldSelector == nil { return meta.SetList(obj, objs) } @@ -775,7 +769,15 @@ func (c *fakeClient) Create(ctx context.Context, obj client.Object, opts ...clie c.trackerWriteLock.Lock() defer c.trackerWriteLock.Unlock() - return c.tracker.Create(gvr, obj, accessor.GetNamespace()) + if err := c.tracker.Create(gvr, obj, accessor.GetNamespace()); err != nil { + return err + } + + gvk, err := apiutil.GVKForObject(obj, c.scheme) + if err != nil { + return err + } + return ensureTypeMeta(obj, gvk) } func (c *fakeClient) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { @@ -892,6 +894,10 @@ func (c *fakeClient) update(obj client.Object, isStatus bool, opts ...client.Upd if err != nil { return err } + gvk, err := apiutil.GVKForObject(obj, c.scheme) + if err != nil { + return err + } accessor, err := meta.Accessor(obj) if err != nil { return err @@ -899,7 +905,11 @@ func (c *fakeClient) update(obj client.Object, isStatus bool, opts ...client.Upd c.trackerWriteLock.Lock() defer c.trackerWriteLock.Unlock() - return c.tracker.update(gvr, obj, accessor.GetNamespace(), isStatus, false, *updateOptions.AsUpdateOptions()) + if err := c.tracker.update(gvr, obj, accessor.GetNamespace(), isStatus, false, *updateOptions.AsUpdateOptions()); err != nil { + return err + } + + return ensureTypeMeta(obj, gvk) } func (c *fakeClient) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { @@ -922,16 +932,15 @@ func (c *fakeClient) patch(obj client.Object, patch client.Patch, opts ...client if err != nil { return err } - accessor, err := meta.Accessor(obj) + gvk, err := apiutil.GVKForObject(obj, c.scheme) if err != nil { return err } - data, err := patch.Data(obj) + accessor, err := meta.Accessor(obj) if err != nil { return err } - - gvk, err := apiutil.GVKForObject(obj, c.scheme) + data, err := patch.Data(obj) if err != nil { return err } @@ -978,13 +987,8 @@ func (c *fakeClient) patch(obj client.Object, patch client.Patch, opts ...client panic("tracker could not handle patch method") } - if _, isUnstructured := obj.(runtime.Unstructured); isUnstructured { - ta, err := meta.TypeAccessor(o) - if err != nil { - return err - } - ta.SetKind(gvk.Kind) - ta.SetAPIVersion(gvk.GroupVersion().String()) + if err := ensureTypeMeta(obj, gvk); err != nil { + return err } j, err := json.Marshal(o) @@ -992,7 +996,11 @@ func (c *fakeClient) patch(obj client.Object, patch client.Patch, opts ...client return err } zero(obj) - return json.Unmarshal(j, obj) + if err := json.Unmarshal(j, obj); err != nil { + return err + } + + return ensureTypeMeta(obj, gvk) } // Applying a patch results in a deletionTimestamp that is truncated to the nearest second. @@ -1600,3 +1608,23 @@ func AddIndex(c client.Client, obj runtime.Object, field string, extractValue cl return nil } + +func ensureTypeMeta(obj runtime.Object, gvk schema.GroupVersionKind) error { + ta, err := meta.TypeAccessor(obj) + if err != nil { + return err + } + _, isUnstructured := obj.(runtime.Unstructured) + _, isPartialObject := obj.(*metav1.PartialObjectMetadata) + _, isPartialObjectList := obj.(*metav1.PartialObjectMetadataList) + if !isUnstructured && !isPartialObject && !isPartialObjectList { + ta.SetKind("") + ta.SetAPIVersion("") + return nil + } + + ta.SetKind(gvk.Kind) + ta.SetAPIVersion(gvk.GroupVersion().String()) + + return nil +} diff --git a/pkg/client/fake/client_test.go b/pkg/client/fake/client_test.go index e46795ec3b..ab728a0a7a 100644 --- a/pkg/client/fake/client_test.go +++ b/pkg/client/fake/client_test.go @@ -66,10 +66,6 @@ var _ = Describe("Fake client", func() { BeforeEach(func() { replicas := int32(1) dep = &appsv1.Deployment{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "apps/v1", - Kind: "Deployment", - }, ObjectMeta: metav1.ObjectMeta{ Name: "test-deployment", Namespace: "ns1", @@ -83,10 +79,6 @@ var _ = Describe("Fake client", func() { }, } dep2 = &appsv1.Deployment{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "apps/v1", - Kind: "Deployment", - }, ObjectMeta: metav1.ObjectMeta{ Name: "test-deployment-2", Namespace: "ns1", @@ -100,10 +92,6 @@ var _ = Describe("Fake client", func() { }, } cm = &corev1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "ConfigMap", - }, ObjectMeta: metav1.ObjectMeta{ Name: "test-cm", Namespace: "ns2", @@ -248,7 +236,7 @@ var _ = Describe("Fake client", func() { Expect(err).ToNot(HaveOccurred()) }) - It("should be able to List an unregistered type using unstructured", func() { + It("should be able to List an unregistered type using unstructured with ListKind", func() { list := &unstructured.UnstructuredList{} list.SetAPIVersion("custom/v3") list.SetKind("ImageList") @@ -258,14 +246,14 @@ var _ = Describe("Fake client", func() { Expect(err).ToNot(HaveOccurred()) }) - It("should be able to List an unregistered type using unstructured", func() { + It("should be able to List an unregistered type using unstructured with Kind", func() { list := &unstructured.UnstructuredList{} list.SetAPIVersion("custom/v4") list.SetKind("Image") err := cl.List(context.Background(), list) + Expect(err).ToNot(HaveOccurred()) Expect(list.GroupVersionKind().GroupVersion().String()).To(Equal("custom/v4")) Expect(list.GetKind()).To(Equal("Image")) - Expect(err).ToNot(HaveOccurred()) }) It("should be able to Update an unregistered type using unstructured", func() { @@ -403,10 +391,6 @@ var _ = Describe("Fake client", func() { It("should reject apply patches, they are not supported in the fake client", func() { By("Creating a new configmap") cm := &corev1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "ConfigMap", - }, ObjectMeta: metav1.ObjectMeta{ Name: "new-test-cm", Namespace: "ns2", @@ -423,10 +407,6 @@ var _ = Describe("Fake client", func() { It("should be able to Create", func() { By("Creating a new configmap") newcm := &corev1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "ConfigMap", - }, ObjectMeta: metav1.ObjectMeta{ Name: "new-test-cm", Namespace: "ns2", @@ -474,10 +454,6 @@ var _ = Describe("Fake client", func() { It("should error on Create with empty Name", func() { By("Creating a new configmap") newcm := &corev1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "ConfigMap", - }, ObjectMeta: metav1.ObjectMeta{ Namespace: "ns2", }, @@ -489,10 +465,6 @@ var _ = Describe("Fake client", func() { It("should error on Update with empty Name", func() { By("Creating a new configmap") newcm := &corev1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "ConfigMap", - }, ObjectMeta: metav1.ObjectMeta{ Namespace: "ns2", }, @@ -504,10 +476,6 @@ var _ = Describe("Fake client", func() { It("should be able to Create with GenerateName", func() { By("Creating a new configmap") newcm := &corev1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "ConfigMap", - }, ObjectMeta: metav1.ObjectMeta{ GenerateName: "new-test-cm", Namespace: "ns2", @@ -533,10 +501,6 @@ var _ = Describe("Fake client", func() { It("should be able to Update", func() { By("Updating a new configmap") newcm := &corev1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "ConfigMap", - }, ObjectMeta: metav1.ObjectMeta{ Name: "test-cm", Namespace: "ns2", @@ -564,10 +528,6 @@ var _ = Describe("Fake client", func() { It("should allow updates with non-set ResourceVersion for a resource that allows unconditional updates", func() { By("Updating a new configmap") newcm := &corev1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "ConfigMap", - }, ObjectMeta: metav1.ObjectMeta{ Name: "test-cm", Namespace: "ns2", @@ -621,10 +581,6 @@ var _ = Describe("Fake client", func() { It("should reject updates with non-set ResourceVersion for a resource that doesn't allow unconditional updates", func() { By("Creating a new binding") binding := &corev1.Binding{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "Binding", - }, ObjectMeta: metav1.ObjectMeta{ Name: "test-binding", Namespace: "ns2", @@ -640,10 +596,6 @@ var _ = Describe("Fake client", func() { By("Updating the binding with a new resource lacking resource version") newBinding := &corev1.Binding{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "Binding", - }, ObjectMeta: metav1.ObjectMeta{ Name: binding.Name, Namespace: binding.Namespace, @@ -659,10 +611,6 @@ var _ = Describe("Fake client", func() { It("should allow create on update for a resource that allows create on update", func() { By("Creating a new lease with update") lease := &coordinationv1.Lease{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "coordination.k8s.io/v1", - Kind: "Lease", - }, ObjectMeta: metav1.ObjectMeta{ Name: "test-lease", Namespace: "ns2", @@ -685,10 +633,6 @@ var _ = Describe("Fake client", func() { It("should reject create on update for a resource that does not allow create on update", func() { By("Attemping to create a new configmap with update") newcm := &corev1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "ConfigMap", - }, ObjectMeta: metav1.ObjectMeta{ Name: "different-test-cm", Namespace: "ns2", @@ -1539,10 +1483,6 @@ var _ = Describe("Fake client", func() { } dep3 := &appsv1.Deployment{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "apps/v1", - Kind: "Deployment", - }, ObjectMeta: metav1.ObjectMeta{ Name: "test-deployment-3", Namespace: "ns1", @@ -1929,8 +1869,6 @@ var _ = Describe("Fake client", func() { actual := &corev1.Pod{} Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(obj), actual)).To(Succeed()) - obj.APIVersion = u.GetAPIVersion() - obj.Kind = u.GetKind() obj.ResourceVersion = actual.ResourceVersion // only the spec mutation should persist obj.Spec.RestartPolicy = corev1.RestartPolicyNever @@ -2445,6 +2383,90 @@ var _ = Describe("Fake client", func() { Expect(cl.SubResource(subResourceScale).Update(context.Background(), obj, client.WithSubResourceBody(scale)).Error()).To(Equal(expectedErr)) }) + It("clears typemeta from structured objects on create", func() { + obj := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "ConfigMap", + }, + } + cl := NewClientBuilder().Build() + Expect(cl.Create(context.Background(), obj)).To(Succeed()) + Expect(obj.TypeMeta).To(Equal(metav1.TypeMeta{})) + }) + + It("clears typemeta from structured objects on update", func() { + obj := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "ConfigMap", + }, + } + cl := NewClientBuilder().WithObjects(obj).Build() + Expect(cl.Update(context.Background(), obj)).To(Succeed()) + Expect(obj.TypeMeta).To(Equal(metav1.TypeMeta{})) + }) + + It("clears typemeta from structured objects on patch", func() { + obj := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + } + cl := NewClientBuilder().WithObjects(obj).Build() + original := obj.DeepCopy() + obj.TypeMeta = metav1.TypeMeta{ + APIVersion: "v1", + Kind: "ConfigMap", + } + Expect(cl.Patch(context.Background(), obj, client.MergeFrom(original))).To(Succeed()) + Expect(obj.TypeMeta).To(Equal(metav1.TypeMeta{})) + }) + + It("clears typemeta from structured objects on get", func() { + obj := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "ConfigMap", + }, + } + cl := NewClientBuilder().WithObjects(obj).Build() + target := &corev1.ConfigMap{} + Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(obj), target)).To(Succeed()) + Expect(target.TypeMeta).To(Equal(metav1.TypeMeta{})) + }) + + It("clears typemeta from structured objects on list", func() { + obj := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "ConfigMap", + }, + } + cl := NewClientBuilder().WithObjects(obj).Build() + target := &corev1.ConfigMapList{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "ConfigMap", + }, + } + Expect(cl.List(context.Background(), target)).To(Succeed()) + Expect(target.TypeMeta).To(Equal(metav1.TypeMeta{})) + Expect(target.Items[0].TypeMeta).To(Equal(metav1.TypeMeta{})) + }) + It("is threadsafe", func() { cl := NewClientBuilder().Build()