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()