Skip to content

Commit

Permalink
🐛 Fakeclient: Allow manipulating registered types through unstructured
Browse files Browse the repository at this point in the history
Currently, manipulating an object that is registered through
unstructured will break all subsequent List requests that include said
object. This happens because the tracker unconditionally saves objects but
it's List() assumes that what it finds can be assigned to the .Items slice
of an object produced through scheme.New(ListGVK).
  • Loading branch information
alvaroaleman committed Sep 11, 2021
1 parent 0a9a777 commit dcd5e10
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 1 deletion.
43 changes: 42 additions & 1 deletion pkg/client/fake/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,11 @@ func (t versionedTracker) Add(obj runtime.Object) error {
// be recognized
accessor.SetResourceVersion(trackerAddResourceVersion)
}

obj, err = convertFromUnstructuredIfNecessary(t.scheme, obj)
if err != nil {
return err
}
if err := t.ObjectTracker.Add(obj); err != nil {
return err
}
Expand All @@ -192,13 +197,45 @@ func (t versionedTracker) Create(gvr schema.GroupVersionResource, obj runtime.Ob
return apierrors.NewBadRequest("resourceVersion can not be set for Create requests")
}
accessor.SetResourceVersion("1")
obj, err = convertFromUnstructuredIfNecessary(t.scheme, obj)
if err != nil {
return err
}
if err := t.ObjectTracker.Create(gvr, obj, ns); err != nil {
accessor.SetResourceVersion("")
return err
}

return nil
}

// convertFromUnstructuredIfNecessary will convert *unstructured.Unstructured for a GVK that is recocnized
// by the schema into the whatever the schema produces with New() for said GVK.
// This is required because the tracker unconditionally saves on manipulations, but it's List() implementation
// tries to assign whatever it finds into a ListType it gets from schema.New() - Thus we have to ensure
// we save as the very same type, otherwise subsequent List requests will fail.
func convertFromUnstructuredIfNecessary(s *runtime.Scheme, o runtime.Object) (runtime.Object, error) {
u, isUnstructured := o.(*unstructured.Unstructured)
if !isUnstructured || !s.Recognizes(u.GroupVersionKind()) {
return o, nil
}

typed, err := s.New(u.GroupVersionKind())
if err != nil {
return nil, fmt.Errorf("scheme recognizes %s but failed to produce an object for it: %w", u.GroupVersionKind().String(), err)
}

unstructuredSerialized, err := json.Marshal(u)
if err != nil {
return nil, fmt.Errorf("failed to serialize %T: %w", unstructuredSerialized, err)
}
if err := json.Unmarshal(unstructuredSerialized, typed); err != nil {
return nil, fmt.Errorf("failed to unmarshal the content of %T into %T: %w", u, typed, err)
}

return typed, nil
}

func (t versionedTracker) Update(gvr schema.GroupVersionResource, obj runtime.Object, ns string) error {
accessor, err := meta.Accessor(obj)
if err != nil {
Expand Down Expand Up @@ -255,6 +292,10 @@ func (t versionedTracker) Update(gvr schema.GroupVersionResource, obj runtime.Ob
if !accessor.GetDeletionTimestamp().IsZero() && len(accessor.GetFinalizers()) == 0 {
return t.ObjectTracker.Delete(gvr, accessor.GetNamespace(), accessor.GetName())
}
obj, err = convertFromUnstructuredIfNecessary(t.scheme, obj)
if err != nil {
return err
}
return t.ObjectTracker.Update(gvr, obj, ns)
}

Expand Down Expand Up @@ -318,7 +359,7 @@ func (c *fakeClient) List(ctx context.Context, obj client.ObjectList, opts ...cl
}

if _, isUnstructuredList := obj.(*unstructured.UnstructuredList); isUnstructuredList && !c.scheme.Recognizes(gvk) {
// We need tor register the ListKind with UnstructuredList:
// We need to register the ListKind with UnstructuredList:
// https://github.com/kubernetes/kubernetes/blob/7b2776b89fb1be28d4e9203bdeec079be903c103/staging/src/k8s.io/client-go/dynamic/fake/simple.go#L44-L51
c.schemeWriteLock.Lock()
c.scheme.AddKnownTypeWithName(gvk.GroupVersion().WithKind(gvk.Kind+"List"), &unstructured.UnstructuredList{})
Expand Down
45 changes: 45 additions & 0 deletions pkg/client/fake/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,51 @@ var _ = Describe("Fake client", func() {
Expect(list.Items).To(HaveLen(2))
})

It("should be able to retrieve registered objects that got manipulated as unstructured", func() {
list := func() {
By("Listing all endpoints in a namespace")
list := &unstructured.UnstructuredList{}
list.SetAPIVersion("v1")
list.SetKind("EndpointsList")
err := cl.List(context.Background(), list, client.InNamespace("ns1"))
Expect(err).To(BeNil())
Expect(list.Items).To(HaveLen(1))
}

unstructuredEndpoint := func() *unstructured.Unstructured {
item := &unstructured.Unstructured{}
item.SetAPIVersion("v1")
item.SetKind("Endpoints")
item.SetName("test-endpoint")
item.SetNamespace("ns1")
return item
}

By("Adding the object during client initialization")
cl = NewFakeClient(unstructuredEndpoint())
list()
Expect(cl.Delete(context.Background(), unstructuredEndpoint())).To(BeNil())

By("Creating an object")
item := unstructuredEndpoint()
err := cl.Create(context.Background(), item)
Expect(err).To(BeNil())
list()

By("Updating the object")
item.SetAnnotations(map[string]string{"foo": "bar"})
err = cl.Update(context.Background(), item)
Expect(err).To(BeNil())
list()

By("Patching the object")
old := item.DeepCopy()
item.SetAnnotations(map[string]string{"bar": "baz"})
err = cl.Patch(context.Background(), item, client.MergeFrom(old))
Expect(err).To(BeNil())
list()
})

It("should be able to Create an unregistered type using unstructured", func() {
item := &unstructured.Unstructured{}
item.SetAPIVersion("custom/v1")
Expand Down

0 comments on commit dcd5e10

Please sign in to comment.