Add UnsafeDisableDeepCopy to GetOptions

Author: jonathan-innis

pkg/cache/internal/cache_reader.go

@@ -54,7 +54,10 @@ type CacheReader struct {
 }
 
 // Get checks the indexer for the object and writes a copy of it if found.
-func (c *CacheReader) Get(_ context.Context, key client.ObjectKey, out client.Object, _ ...client.GetOption) error {
+func (c *CacheReader) Get(_ context.Context, key client.ObjectKey, out client.Object, opts ...client.GetOption) error {
+	getOpts := client.GetOptions{}
+	getOpts.ApplyOptions(opts)
+
 	if c.scopeName == apimeta.RESTScopeNameRoot {
 		key.Namespace = ""
 	}
@@ -81,7 +84,7 @@ func (c *CacheReader) Get(_ context.Context, key client.ObjectKey, out client.Ob
 		return fmt.Errorf("cache contained %T, which is not an Object", obj)
 	}
 
-	if c.disableDeepCopy {
+	if c.disableDeepCopy || (getOpts.UnsafeDisableDeepCopy != nil && *getOpts.UnsafeDisableDeepCopy) {
 		// skip deep copy which might be unsafe
 		// you must DeepCopy any object before mutating it outside
 	} else {
@@ -97,7 +100,7 @@ func (c *CacheReader) Get(_ context.Context, key client.ObjectKey, out client.Ob
 		return fmt.Errorf("cache had type %s, but %s was asked for", objVal.Type(), outVal.Type())
 	}
 	reflect.Indirect(outVal).Set(reflect.Indirect(objVal))
-	if !c.disableDeepCopy {
+	if !c.disableDeepCopy && (getOpts.UnsafeDisableDeepCopy == nil || !*getOpts.UnsafeDisableDeepCopy) {
 		out.GetObjectKind().SetGroupVersionKind(c.groupVersionKind)
 	}
 

pkg/client/client_test.go

@@ -1976,6 +1976,30 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
 
 			})
 
+			It("should not deep copy results when UnsafeDisableDeepCopy is set", func() {
+				By("first creating the Deployment")
+				dep, err := clientset.AppsV1().Deployments(ns).Create(ctx, dep, metav1.CreateOptions{})
+				Expect(err).NotTo(HaveOccurred())
+
+				cl, err := client.New(cfg, client.Options{})
+				Expect(err).NotTo(HaveOccurred())
+				Expect(cl).NotTo(BeNil())
+
+				By("fetching the created Deployment")
+				var actual appsv1.Deployment
+				key := client.ObjectKey{Namespace: ns, Name: dep.Name}
+				err = cl.Get(context.TODO(), key, &actual, client.UnsafeDisableDeepCopy)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(actual).NotTo(BeNil())
+
+				var newActual appsv1.Deployment
+				err = cl.Get(context.TODO(), key, &newActual, client.UnsafeDisableDeepCopy)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(newActual).NotTo(BeNil())
+
+				Expect(newActual).To(BeIdenticalTo(actual))
+			})
+
 			// 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{}} {
@@ -2647,6 +2671,33 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
 				Expect(deps.Items[1].Name).To(Equal(dep4.Name))
 			})
 
+			It("should not deep copy results when UnsafeDisableDeepCopy is set", func() {
+				By("creating an initial object")
+				_, err := clientset.AppsV1().Deployments(ns).Create(ctx, dep, metav1.CreateOptions{})
+				Expect(err).NotTo(HaveOccurred())
+
+				cl, err := client.New(cfg, client.Options{})
+				Expect(err).NotTo(HaveOccurred())
+
+				By("listing all objects of that type in the cluster")
+				deps := &appsv1.DeploymentList{}
+				Expect(cl.List(context.Background(), deps), client.UnsafeDisableDeepCopy).NotTo(HaveOccurred())
+
+				Expect(deps.Items).NotTo(BeEmpty())
+
+				By("listing all objects of that type again in the cluster")
+				newDeps := &appsv1.DeploymentList{}
+				Expect(cl.List(context.Background(), newDeps), client.UnsafeDisableDeepCopy).NotTo(HaveOccurred())
+
+				Expect(newDeps.Items).NotTo(BeEmpty())
+
+				Expect(newDeps).To(HaveLen(len(deps.Items)))
+				// Expect the elements to be the same since we didn't DeepCopy them
+				for i := range deps.Items {
+					Expect(deps.Items[i]).To(BeIdenticalTo(newDeps.Items[i]))
+				}
+			})
+
 			PIt("should fail if the object doesn't have meta", func() {
 
 			})

pkg/client/options.go

@@ -431,6 +431,12 @@ type GetOptions struct {
 	// Raw represents raw GetOptions, as passed to the API server.  Note
 	// that these may not be respected by all implementations of interface.
 	Raw *metav1.GetOptions
+
+	// UnsafeDisableDeepCopy indicates not to deep copy objects during get object.
+	// Be very careful with this, when enabled you must DeepCopy any object before mutating it,
+	// otherwise you will mutate the object in the cache.
+	// +optional
+	UnsafeDisableDeepCopy *bool
 }
 
 var _ GetOption = &GetOptions{}
@@ -440,6 +446,9 @@ func (o *GetOptions) ApplyToGet(lo *GetOptions) {
 	if o.Raw != nil {
 		lo.Raw = o.Raw
 	}
+	if o.UnsafeDisableDeepCopy != nil {
+		lo.UnsafeDisableDeepCopy = o.UnsafeDisableDeepCopy
+	}
 }
 
 // AsGetOptions returns these options as a flattened metav1.GetOptions.
@@ -692,6 +701,17 @@ func (l Limit) ApplyToList(opts *ListOptions) {
 // otherwise you will mutate the object in the cache.
 type UnsafeDisableDeepCopyOption bool
 
+// ApplyToGet applies this configuration to the given an Get options.
+func (d UnsafeDisableDeepCopyOption) ApplyToGet(opts *GetOptions) {
+	definitelyTrue := true
+	definitelyFalse := false
+	if d {
+		opts.UnsafeDisableDeepCopy = &definitelyTrue
+	} else {
+		opts.UnsafeDisableDeepCopy = &definitelyFalse
+	}
+}
+
 // ApplyToList applies this configuration to the given an List options.
 func (d UnsafeDisableDeepCopyOption) ApplyToList(opts *ListOptions) {
 	definitelyTrue := true

pkg/client/options_test.go

@@ -66,6 +66,19 @@ var _ = Describe("ListOptions", func() {
 		o.ApplyToList(newListOpts)
 		Expect(newListOpts).To(Equal(o))
 	})
+	It("Should set UnsafeDisableDeepCopy", func() {
+		definitelyTrue := true
+		o := &client.ListOptions{UnsafeDisableDeepCopy: &definitelyTrue}
+		newListOpts := &client.ListOptions{}
+		o.ApplyToList(newListOpts)
+		Expect(newListOpts).To(Equal(o))
+	})
+	It("Should set UnsafeDisableDeepCopy through option", func() {
+		listOpts := &client.ListOptions{}
+		client.UnsafeDisableDeepCopy.ApplyToList(listOpts)
+		Expect(listOpts.UnsafeDisableDeepCopy).ToNot(BeNil())
+		Expect(*listOpts.UnsafeDisableDeepCopy).To(BeTrue())
+	})
 	It("Should not set anything", func() {
 		o := &client.ListOptions{}
 		newListOpts := &client.ListOptions{}
@@ -81,6 +94,19 @@ var _ = Describe("GetOptions", func() {
 		o.ApplyToGet(newGetOpts)
 		Expect(newGetOpts).To(Equal(o))
 	})
+	It("Should set UnsafeDisableDeepCopy", func() {
+		definitelyTrue := true
+		o := &client.GetOptions{UnsafeDisableDeepCopy: &definitelyTrue}
+		newGetOpts := &client.GetOptions{}
+		o.ApplyToGet(newGetOpts)
+		Expect(newGetOpts).To(Equal(o))
+	})
+	It("Should set UnsafeDisableDeepCopy through option", func() {
+		getOpts := &client.GetOptions{}
+		client.UnsafeDisableDeepCopy.ApplyToGet(getOpts)
+		Expect(getOpts.UnsafeDisableDeepCopy).ToNot(BeNil())
+		Expect(*getOpts.UnsafeDisableDeepCopy).To(BeTrue())
+	})
 })
 
 var _ = Describe("CreateOptions", func() {