Fake client does not preserve TypeMeta for PartialObjectMetadata after request

[dimityrmirchev]

Hi folks,

I have code that uses PartialObjectMetadata to potentially send two patch requests for one specific object. The second request fails when using the fake client in test suite because after the first Patch the object loses its TypeMeta even though it was explicitly set. The same requests succeed when ran through the regular client. I managed to reproduce in a minimal test and it seems that the same is true at least for Get requests as well. Here is a reproducer:

package tt_test

import (
	"context"
	"testing"

	corev1 "k8s.io/api/core/v1"
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
	"k8s.io/client-go/kubernetes/scheme"
	"sigs.k8s.io/controller-runtime/pkg/client"
	"sigs.k8s.io/controller-runtime/pkg/client/fake"
)

func TestGetPreserveTypeMeta(t *testing.T) {
	var (
		fakeClient client.Client = fake.NewClientBuilder().WithScheme(scheme.Scheme).Build()
		ctx                      = context.TODO()
	)

	partialObjMeta := &metav1.PartialObjectMetadata{
		ObjectMeta: metav1.ObjectMeta{
			Name:      "foo",
			Namespace: "bar",
		},
	}
	partialObjMeta.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Secret"))

	err := fakeClient.Create(ctx, &corev1.Secret{
		ObjectMeta: metav1.ObjectMeta{
			Name:      "foo",
			Namespace: "bar",
			Labels: map[string]string{
				"one": "two",
			},
		},
	})
	if err != nil {
		t.Errorf("create was not successful: %v", err)
	}

	err = fakeClient.Get(ctx, client.ObjectKeyFromObject(partialObjMeta), partialObjMeta)
	if err != nil {
		t.Errorf("get was not successful: %v", err)
	}

	if partialObjMeta.Kind != "Secret" {
		t.Errorf("got kind: %s, want Secret", partialObjMeta.Kind)
	}
}

I am not sure if this is intended behaviour but I would expect that partialObjMeta.Kind == "Secret" at the end of the test.

[sbueringer]

Hm, seems reasonable to me that PartialObjectMeta keeps kind set. The regular client (cached & apireader) does that, I assume?

(Sort of unrelated, but also wondering how fake client behaves with Unstructured)

[sbueringer]

(cc @alvaroaleman)

[dimityrmirchev]

The regular client (cached & apireader) does that, I assume?

Yes (at least this is what my brief testing shows). The following test will pass if there is a foo Secret already created in the cluster.

package tt_test

import (
	"context"
	"os"
	"testing"

	corev1 "k8s.io/api/core/v1"
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
	"k8s.io/client-go/tools/clientcmd"
	"sigs.k8s.io/controller-runtime/pkg/client"
)

func TestGetPreserveTypeMetaNotFakeClient(t *testing.T) {
	kubeconfigBytes, err := os.ReadFile(os.Getenv("KUBECONFIG"))
	if err != nil {
		t.Fatalf("cannot read KUBECONFIG file: %v", err)
	}

	cfg, err := clientcmd.RESTConfigFromKubeConfig(kubeconfigBytes)
	if err != nil {
		t.Fatalf("cannot create rest config from kubeconfig: %v", err)
	}

	c, err := client.New(cfg, client.Options{})
	if err != nil {
		t.Fatalf("cannot create client config rest config: %v", err)
	}

	partialObjMeta := &metav1.PartialObjectMetadata{
		ObjectMeta: metav1.ObjectMeta{
			Name:      "foo",
			Namespace: "default",
		},
	}
	partialObjMeta.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Secret"))

	ctx := context.TODO()
	err = c.Get(ctx, client.ObjectKeyFromObject(partialObjMeta), partialObjMeta)
	if err != nil {
		t.Errorf("get was not successful: %v", err)
	}

	if partialObjMeta.Kind != "Secret" {
		t.Errorf("got kind: %s, want Secret", partialObjMeta.Kind)
	}
}

[bigkevmcd]

The problematic part of the code is here

controller-runtime/pkg/client/fake/client.go

Lines 524 to 529 in b901db1

	j, err := json.Marshal(o)
	if err != nil {
	return err
	}
	zero(obj)
	return json.Unmarshal(j, obj)

It creates a new value of whatever type is passed in (in the test case a PartialObjectMeta) and then copies (via JSON) the resource to the new value of that type, this means that the version that was created in the Create call overwrites the PartialObjectMeta value, and so loses the TypeMeta.

It could probably be fixed fairly minimally by changing

controller-runtime/pkg/client/fake/client.go

Line 511 in b901db1

if _, isUnstructured := obj.(runtime.Unstructured); isUnstructured {

to test for obj being a PartialObjectMetadata.

If it's either a PartialObjectMetadata or an Unstructured then populate the GVK on the value from the tracker.

[alvaroaleman]

Yeah, it needs to be preserved for unstructured and objectMeta types. Please feel free to submit a fix.

/kind bug