client.Get() does not zero out slice fields on CRD objects

[nathanperkins]

I've encountered some unusual behavior when using Get() on CRD objects with the controller-runtime client. When the client.Object you pass into Get() has a slice field populated with values and the object on the API server has an empty or nil slice for that field, the client does not properly zero/empty that field.

This is in contrast with the behavior when getting core objects, which does zero/empty fields as expected.

Observations

• Empty slice fields on the API server do not empty/zero populated fields on the client.Object.
• I confirmed this issue occurs for slice fields in ObjectMeta, Spec, and Status.
• Populated slice fields on the API server properly update the client.Object as expected.
• Other field types like int and string update as expected regardless of presence.
• Empty slice fields update properly for core objects, confirmed with v1.ConfigMap.
• I tested and confirmed on v0.7.0 and v0.9.6.

The example test below demonstrates the inconsistency.

var _ = Describe("client.Get", func() {
	ctx := context.Background()

	var (
		crdObject  *v1alpha1.CRDType
		coreObject *corev1.ConfigMap
	)

	BeforeEach(func() {
		crdObject = &v1alpha1.Type{
			ObjectMeta: metav1.ObjectMeta{
				Name:      "crd-object",
				Namespace: "default",
				// Finalizers empty.
			},
		}
		coreObject = &corev1.ConfigMap{
			ObjectMeta: metav1.ObjectMeta{
				Name:      "core-object",
				Namespace: "default",
				// Finalizers empty.
			},
		}
	})

	JustBeforeEach(func() {
		Expect(k8sClient.Create(ctx, crdObject)).Should(Succeed())
		Expect(k8sClient.Create(ctx, coreObject)).Should(Succeed())
	})

	AfterEach(func() {
		Expect(k8sClient.Delete(ctx, crdObject)).Should(Succeed())
		Expect(k8sClient.Delete(ctx, coreObject)).Should(Succeed())
	})

	It("Should overwrite slices that should be empty when getting a CRD object", func() {
		crdObject.Finalizers = []string{"1.1.1.1"}

		Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(crdObject), crdObject)).
			Should(Succeed())

		// Fails: crdObject.Finalizers is []string{"1.1.1.1"}
		Expect(crdObject.Finalizers).To(BeEmpty())
	})

	It("Should overwrite slices that should be empty when getting a core object", func() {
		coreObject.Finalizers = []string{"1.1.1.1"}

		Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(coreObject), coreObject)).
			Should(Succeed())

		// Passes.
		Expect(coreObject.Finalizers).To(BeEmpty())
	})
})

[alvaroaleman]

Yeah, I can confirm this. This happens because the json deserializer (caseSensitiveJSONIterator) doesn't clear the object. caseSensitiveJSONIterator is documented to behave as the stdlib deserializer except that its case sensitive and the stdlib deserializer does the same. Hence its unlikely upstream will change anything (although I personally find the stdlib behavior pretty questionable).

I presume it works with the ConfigMap as expected because we use proto there.

Seems we have to workaround this by resetting the value in at least Get() and List()