Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ support disable deepcopy on list funcion #2076

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 24 additions & 0 deletions pkg/cache/cache_test.go
Expand Up @@ -22,6 +22,7 @@ import (
"reflect"
"sort"
"strconv"
"strings"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -1735,6 +1736,29 @@ 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
}
}

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
}
}
})
})
})
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/cache/internal/cache_reader.go
Expand Up @@ -161,7 +161,7 @@ func (c *CacheReader) List(_ context.Context, out client.ObjectList, opts ...cli
}

var outObj runtime.Object
if c.disableDeepCopy {
if c.disableDeepCopy || (listOpts.UnsafeDisableDeepCopy != nil && *listOpts.UnsafeDisableDeepCopy) {
// skip deep copy which might be unsafe
// you must DeepCopy any object before mutating it outside
outObj = obj
Expand Down
28 changes: 28 additions & 0 deletions pkg/client/options.go
Expand Up @@ -417,6 +417,12 @@ type ListOptions struct {
// it has expired. This field is not supported if watch is true in the Raw ListOptions.
Continue string

// UnsafeDisableDeepCopy indicates not to deep copy objects during list objects.
// 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

// Raw represents raw ListOptions, as passed to the API server. Note
// that these may not be respected by all implementations of interface,
// and the LabelSelector, FieldSelector, Limit and Continue fields are ignored.
Expand Down Expand Up @@ -445,6 +451,9 @@ func (o *ListOptions) ApplyToList(lo *ListOptions) {
if o.Continue != "" {
lo.Continue = o.Continue
}
if o.UnsafeDisableDeepCopy != nil {
lo.UnsafeDisableDeepCopy = o.UnsafeDisableDeepCopy
}
}

// AsListOptions returns these options as a flattened metav1.ListOptions.
Expand Down Expand Up @@ -587,6 +596,25 @@ func (l Limit) ApplyToList(opts *ListOptions) {
opts.Limit = int64(l)
}

// UnsafeDisableDeepCopyOption indicates not to deep copy objects during list objects.
// Be very careful with this, when enabled you must DeepCopy any object before mutating it,
// otherwise you will mutate the object in the cache.
type UnsafeDisableDeepCopyOption bool

// 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
}
}

// UnsafeDisableDeepCopy indicates not to deep copy objects during list objects.
const UnsafeDisableDeepCopy = UnsafeDisableDeepCopyOption(true)

// Continue sets a continuation token to retrieve chunks of results when using limit.
// Continue does not implement DeleteAllOfOption interface because the server
// does not support setting it for deletecollection operations.
Expand Down