[post-1.20] Fake Dynamic Client requires scheme registration with listType, breaks duck typing.

[n3wscott]

we in Knative attempted to update to k8s client-go 1.20 and found that the fake dynamic client no longer works with duck typing because of this addition: 20c034c#diff-8ba8eeecc835702a1f7fbb6d06b268cb17453f82c5ef5c8294f25fcdf0f53dacR316-R318 @deads2k I will keep looking at the impl, but I fail to see how the dynamic client is useful if scheme registration is required, it means you can not use the dynamic client for unknown types?

it does not look like ListType is even required to list in the dynamic client:

client-go/dynamic/simple.go

Lines 253 to 275 in f6ce18a

	func (c *dynamicResourceClient) List(ctx context.Context, opts metav1.ListOptions) (*unstructured.UnstructuredList, error) {
	result := c.client.client.Get().AbsPath(c.makeURLSegments("")...).SpecificallyVersionedParams(&opts, dynamicParameterCodec, versionV1).Do(ctx)
	if err := result.Error(); err != nil {
	return nil, err
	}
	retBytes, err := result.Raw()
	if err != nil {
	return nil, err
	}
	uncastObj, err := runtime.Decode(unstructured.UnstructuredJSONScheme, retBytes)
	if err != nil {
	return nil, err
	}
	if list, ok := uncastObj.(*unstructured.UnstructuredList); ok {
	return list, nil
	}
	list, err := uncastObj.(*unstructured.Unstructured).ToList()
	if err != nil {
	return nil, err
	}
	return list, nil
	}

unknown list types should be *unstructured.UnstructuredList and I think the change to fake dynamic client is an error/bug.

It does not make sense to require scheme registration with the dynamic client, the whole point is it is dynamic right?

Knative issue: knative/eventing#5185

[deads2k]

A few points

1. this is the fake client
2. This isn't requiring a scheme registration.
3. Since the listkind is provided by the server in the real client, it provides data that is missing in the fake client. Missing this data causes problems of consistency when testing with fake client
4. A fake client used in a unit test is a constrained view of reality that must be predicted in order to write an effective test. Thus asking for this mapping when creating a fake client is reasonable.

I'm open to allowing this to be specified via function if you like, but the data needs to be returned by the fake client to have parity with the real client, so I don't see an effective way to simply stop requiring this mapping.

[n3wscott]

Do you have a way to do this at runtime for the fake dynamic client? We are getting panics with empty listTypes?

[deads2k]

I'm open to allowing this to be specified via function if you like, but the data needs to be returned by the fake client to have parity with the real client, so I don't see an effective way to simply stop requiring this mapping.

I'm open to a PR to make the dynamic client take a mapping function instead of a static mapping. But we have to return the objectmeta on the list values returned from the fake dynamic client.

[n3wscott]

The list type comes from calls like this when registering schema?

scheme.AddKnownTypes(SchemeGroupVersion,
		&Broker{},
		&BrokerList{},
		&Trigger{},
		&TriggerList{},
	)

Perhaps I just need help understanding where the fake dynamic client is assuming the listType value comes from.

[n3wscott]

oh, I see. There is a new method I should be using, the old method will not work with List.

func NewSimpleDynamicClientWithCustomListKinds(scheme *runtime.Scheme, gvrToListKind map[schema.GroupVersionResource]string, objects ...runtime.Object) *FakeDynamicClient {

[n3wscott]

OK, followup, there is code in the original NewSumpleDynamicClient that will look at the scheme registered does explode out the ListKinds. So if you need to work with duck types with the Fake Dynamic Client, you have to register the List Kind:

scheme.AddKnownTypeWithName(schema.GroupVersionKind{Group: "example.com", Version: "v1", Kind: "FooBarList"}, &unstructured.UnstructuredList{})

/close