diff --git a/pkg/client/apiutil/apimachinery.go b/pkg/client/apiutil/apimachinery.go index 2611a20c64..e21eb22389 100644 --- a/pkg/client/apiutil/apimachinery.go +++ b/pkg/client/apiutil/apimachinery.go @@ -21,6 +21,7 @@ package apiutil import ( "fmt" + "reflect" "sync" "k8s.io/apimachinery/pkg/api/meta" @@ -163,9 +164,35 @@ func createRestConfig(gvk schema.GroupVersionKind, isUnstructured bool, baseConf // Use our own custom serializer. cfg.NegotiatedSerializer = serializerWithDecodedGVK{serializer.WithoutConversionCodecFactory{CodecFactory: codecs}} } else { - cfg.NegotiatedSerializer = serializer.WithoutConversionCodecFactory{CodecFactory: codecs} + cfg.NegotiatedSerializer = serializerWithTargetZeroingDecode{NegotiatedSerializer: serializer.WithoutConversionCodecFactory{CodecFactory: codecs}} } } return cfg } + +type serializerWithTargetZeroingDecode struct { + runtime.NegotiatedSerializer +} + +func (s serializerWithTargetZeroingDecode) DecoderToVersion(serializer runtime.Decoder, r runtime.GroupVersioner) runtime.Decoder { + return targetZeroingDecoder{upstream: s.NegotiatedSerializer.DecoderToVersion(serializer, r)} +} + +type targetZeroingDecoder struct { + upstream runtime.Decoder +} + +func (t targetZeroingDecoder) Decode(data []byte, defaults *schema.GroupVersionKind, into runtime.Object) (runtime.Object, *schema.GroupVersionKind, error) { + zero(into) + return t.upstream.Decode(data, defaults, into) +} + +// zero zeros the value of a pointer. +func zero(x interface{}) { + if x == nil { + return + } + res := reflect.ValueOf(x).Elem() + res.Set(reflect.Zero(res.Type())) +} diff --git a/pkg/client/client_suite_test.go b/pkg/client/client_suite_test.go index 980150c91b..c7ed32e7bc 100644 --- a/pkg/client/client_suite_test.go +++ b/pkg/client/client_suite_test.go @@ -22,7 +22,9 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/examples/crd/pkg" "sigs.k8s.io/controller-runtime/pkg/envtest" "sigs.k8s.io/controller-runtime/pkg/envtest/printer" @@ -43,7 +45,7 @@ var clientset *kubernetes.Clientset var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) - testenv = &envtest.Environment{} + testenv = &envtest.Environment{CRDDirectoryPaths: []string{"./testdata"}} var err error cfg, err = testenv.Start() @@ -51,6 +53,8 @@ var _ = BeforeSuite(func() { clientset, err = kubernetes.NewForConfig(cfg) Expect(err).NotTo(HaveOccurred()) + + Expect(pkg.AddToScheme(scheme.Scheme)).NotTo(HaveOccurred()) }, 60) var _ = AfterSuite(func() { diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 45fbf6903c..fac8c97ab8 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -34,6 +34,7 @@ import ( "k8s.io/apimachinery/pkg/types" kscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/examples/crd/pkg" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -1397,6 +1398,33 @@ var _ = Describe("Client", func() { PIt("should fail if the GVK cannot be mapped to a Resource", func() { }) + + // Test this with an integrated type and a CRD to make sure it covers both proto + // and json deserialization. + for idx, object := range []client.Object{&corev1.ConfigMap{}, &pkg.ChaosPod{}} { + idx, object := idx, object + It(fmt.Sprintf("should not retain any data in the obj variable that is not on the server for %T", object), func() { + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) + + object.SetName(fmt.Sprintf("retain-test-%d", idx)) + object.SetNamespace(ns) + + By("First creating the object") + toCreate := object.DeepCopyObject().(client.Object) + Expect(cl.Create(ctx, toCreate)).NotTo(HaveOccurred()) + + By("Fetching it into a variable that has finalizers set") + toGetInto := object.DeepCopyObject().(client.Object) + toGetInto.SetFinalizers([]string{"some-finalizer"}) + Expect(cl.Get(ctx, client.ObjectKeyFromObject(object), toGetInto)).NotTo(HaveOccurred()) + + By("Ensuring the created and the received object are equal") + Expect(toCreate).Should(Equal(toGetInto)) + }) + } + }) Context("with unstructured objects", func() { @@ -1469,6 +1497,30 @@ var _ = Describe("Client", func() { err = cl.Get(context.TODO(), key, u) Expect(err).To(HaveOccurred()) }) + + It("should not retain any data in the obj variable that is not on the server", func() { + object := &unstructured.Unstructured{} + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) + + object.SetName("retain-unstructured") + object.SetNamespace(ns) + object.SetAPIVersion("chaosapps.metamagical.io/v1") + object.SetKind("ChaosPod") + + By("First creating the object") + toCreate := object.DeepCopyObject().(client.Object) + Expect(cl.Create(ctx, toCreate)).NotTo(HaveOccurred()) + + By("Fetching it into a variable that has finalizers set") + toGetInto := object.DeepCopyObject().(client.Object) + toGetInto.SetFinalizers([]string{"some-finalizer"}) + Expect(cl.Get(ctx, client.ObjectKeyFromObject(object), toGetInto)).NotTo(HaveOccurred()) + + By("Ensuring the created and the received object are equal") + Expect(toCreate).Should(Equal(toGetInto)) + }) }) Context("with metadata objects", func() { It("should fetch an existing object for a go struct", func() { @@ -1547,6 +1599,27 @@ var _ = Describe("Client", func() { PIt("should fail if the GVK cannot be mapped to a Resource", func() { }) + + It("should not retain any data in the obj variable that is not on the server", func() { + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) + + By("First creating the object") + toCreate := &pkg.ChaosPod{ObjectMeta: metav1.ObjectMeta{Name: "retain-metadata", Namespace: ns}} + Expect(cl.Create(ctx, toCreate)).NotTo(HaveOccurred()) + + By("Fetching it into a variable that has finalizers set") + toGetInto := &metav1.PartialObjectMetadata{ + TypeMeta: metav1.TypeMeta{APIVersion: "chaosapps.metamagical.io/v1", Kind: "ChaosPod"}, + ObjectMeta: metav1.ObjectMeta{Namespace: ns, Name: "retain-metadata"}, + } + toGetInto.SetFinalizers([]string{"some-finalizer"}) + Expect(cl.Get(ctx, client.ObjectKeyFromObject(toGetInto), toGetInto)).NotTo(HaveOccurred()) + + By("Ensuring the created and the received objects metadata are equal") + Expect(toCreate.ObjectMeta).Should(Equal(toGetInto.ObjectMeta)) + }) }) }) diff --git a/pkg/client/testdata/examplecrd.yaml b/pkg/client/testdata/examplecrd.yaml new file mode 100644 index 0000000000..5409ee9789 --- /dev/null +++ b/pkg/client/testdata/examplecrd.yaml @@ -0,0 +1,17 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: chaospods.chaosapps.metamagical.io +spec: + group: chaosapps.metamagical.io + names: + kind: ChaosPod + plural: chaospods + scope: Namespaced + versions: + - name: "v1" + storage: true + served: true + schema: + openAPIV3Schema: + type: object