Merge pull request #3227 from jonathan-innis/add-unsafe-disable-get-options

✨ Add UnsafeDisableDeepCopy to GetOptions

Author: k8s-ci-robot

pkg/cache/cache_test.go

@@ -2462,27 +2462,43 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
 				})
 			})
 		})
-		Describe("use UnsafeDisableDeepCopy list options", func() {
-			It("should be able to change object in informer cache", func() {
-				By("listing pods")
-				out := corev1.PodList{}
-				Expect(informerCache.List(context.Background(), &out, client.UnsafeDisableDeepCopy)).To(Succeed())
-				for _, item := range out.Items {
-					if strings.Compare(item.Name, "test-pod-3") == 0 { // test-pod-3 has labels
-						item.Labels["UnsafeDisableDeepCopy"] = "true"
-						break
+		Context("using UnsafeDisableDeepCopy", func() {
+			Describe("with ListOptions", func() {
+				It("should be able to change object in informer cache", func() {
+					By("listing pods")
+					out := corev1.PodList{}
+					Expect(informerCache.List(context.Background(), &out, client.UnsafeDisableDeepCopy)).To(Succeed())
+					for _, item := range out.Items {
+						if strings.Compare(item.Name, "test-pod-3") == 0 { // test-pod-3 has labels
+							item.Labels["UnsafeDisableDeepCopy"] = "true"
+							break
+						}
 					}
-				}
 
-				By("verifying that the returned pods were changed")
-				out2 := corev1.PodList{}
-				Expect(informerCache.List(context.Background(), &out, client.UnsafeDisableDeepCopy)).To(Succeed())
-				for _, item := range out2.Items {
-					if strings.Compare(item.Name, "test-pod-3") == 0 {
-						Expect(item.Labels["UnsafeDisableDeepCopy"]).To(Equal("true"))
-						break
+					By("verifying that the returned pods were changed")
+					out2 := corev1.PodList{}
+					Expect(informerCache.List(context.Background(), &out, client.UnsafeDisableDeepCopy)).To(Succeed())
+					for _, item := range out2.Items {
+						if strings.Compare(item.Name, "test-pod-3") == 0 {
+							Expect(item.Labels["UnsafeDisableDeepCopy"]).To(Equal("true"))
+							break
+						}
 					}
-				}
+				})
+			})
+			Describe("with GetOptions", func() {
+				It("should be able to change object in informer cache", func() {
+					out := corev1.Pod{}
+					podKey := client.ObjectKey{Name: "test-pod-2", Namespace: testNamespaceTwo}
+					Expect(informerCache.Get(context.Background(), podKey, &out, client.UnsafeDisableDeepCopy)).To(Succeed())
+
+					out.Labels["UnsafeDisableDeepCopy"] = "true"
+
+					By("verifying that the returned pod was changed")
+					out2 := corev1.Pod{}
+					Expect(informerCache.Get(context.Background(), podKey, &out2, client.UnsafeDisableDeepCopy)).To(Succeed())
+					Expect(out2.Labels["UnsafeDisableDeepCopy"]).To(Equal("true"))
+				})
 			})
 		})
 	})

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/options.go

@@ -21,6 +21,7 @@ import (
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/selection"
+	"k8s.io/utils/ptr"
 )
 
 // {{{ "Functional" Option Interfaces
@@ -431,6 +432,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 +447,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,15 +702,14 @@ 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) {
+	opts.UnsafeDisableDeepCopy = ptr.To(bool(d))
+}
+
 // ApplyToList applies this configuration to the given an List options.
 func (d UnsafeDisableDeepCopyOption) ApplyToList(opts *ListOptions) {
-	definitelyTrue := true
-	definitelyFalse := false
-	if d {
-		opts.UnsafeDisableDeepCopy = &definitelyTrue
-	} else {
-		opts.UnsafeDisableDeepCopy = &definitelyFalse
-	}
+	opts.UnsafeDisableDeepCopy = ptr.To(bool(d))
 }
 
 // UnsafeDisableDeepCopy indicates not to deep copy objects during list objects.

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() {