diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index b01746b3cc..f74e8e1077 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -39,7 +39,10 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/internal/log" ) -var log = logf.RuntimeLog.WithName("object-cache") +var ( + log = logf.RuntimeLog.WithName("object-cache") + defaultResyncTime = 10 * time.Hour +) // Cache knows how to load Kubernetes objects, fetch informers to request // to receive events for Kubernetes objects (at a low-level), @@ -100,14 +103,6 @@ type Informer interface { HasSynced() bool } -// ObjectSelector is an alias name of internal.Selector. -type ObjectSelector internal.Selector - -// SelectorsByObject associate a client.Object's GVK to a field/label selector. -// There is also `DefaultSelector` to set a global default (which will be overridden by -// a more specific setting here, if any). -type SelectorsByObject map[client.Object]ObjectSelector - // Options are the optional arguments for creating a new InformersMap object. type Options struct { // HTTPClient is the http client to use for the REST client @@ -125,39 +120,41 @@ type Options struct { // So that all informers will not send list requests simultaneously. ResyncEvery *time.Duration - // View restricts the cache's ListWatch to the desired fields per GVK - // Default watches all fields and all namespaces. - View ViewOptions -} - -// ViewOptions are the optional arguments for creating a cache view. -// A cache view restricts the Get(), List(), and internal ListWatch operations -// to the desired parameter. -type ViewOptions struct { // Namespaces restricts the cache's ListWatch to the desired namespaces // Default watches all namespaces Namespaces []string - // DefaultSelector will be used as selectors for all object types + // DefaultLabelSelector will be used as a label selectors for all object types // unless they have a more specific selector set in ByObject. - DefaultSelector ObjectSelector + DefaultLabelSelector labels.Selector + + // DefaultFieldSelector will be used as a field selectors for all object types + // unless they have a more specific selector set in ByObject. + DefaultFieldSelector fields.Selector // DefaultTransform will be used as transform for all object types // unless they have a more specific transform set in ByObject. DefaultTransform toolscache.TransformFunc // ByObject restricts the cache's ListWatch to the desired fields per GVK at the specified object. - ByObject ViewByObject + ByObject map[client.Object]ByObject + + // UnsafeDisableDeepCopy indicates not to deep copy objects during get or + // list objects for EVERY 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. + // + // This is a global setting for all objects, and can be overridden by the ByObject setting. + UnsafeDisableDeepCopy *bool } -// ViewByObject offers more fine-grained control over the cache's ListWatch by object. -type ViewByObject struct { - // Selectors restricts the cache's ListWatch to the desired - // fields per GVK at the specified object, the map's value must implement - // Selectors [1] using for example a Set [2] - // [1] https://pkg.go.dev/k8s.io/apimachinery/pkg/fields#Selectors - // [2] https://pkg.go.dev/k8s.io/apimachinery/pkg/fields#Set - Selectors SelectorsByObject +// ByObject offers more fine-grained control over the cache's ListWatch by object. +type ByObject struct { + // Label represents a label selector for the object. + Label labels.Selector + + // Field represents a field selector for the object. + Field fields.Selector // Transform is a map from objects to transformer functions which // get applied when objects of the transformation are about to be committed @@ -165,48 +162,39 @@ type ViewByObject struct { // // This function is called both for new objects to enter the cache, // and for updated objects. - Transform TransformByObject + Transform toolscache.TransformFunc // UnsafeDisableDeepCopy indicates not to deep copy objects during get or // list objects per GVK at the specified 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. - UnsafeDisableDeepCopy DisableDeepCopyByObject + UnsafeDisableDeepCopy *bool } -var defaultResyncTime = 10 * time.Hour - // New initializes and returns a new Cache. func New(config *rest.Config, opts Options) (Cache, error) { opts, err := defaultOpts(config, opts) if err != nil { return nil, err } - selectorsByGVK, err := convertToByGVK(opts.View.ByObject.Selectors, opts.View.DefaultSelector, opts.Scheme) - if err != nil { - return nil, err + if len(opts.Namespaces) == 0 { + opts.Namespaces = []string{metav1.NamespaceAll} } - disableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(opts.View.ByObject.UnsafeDisableDeepCopy, opts.Scheme) - if err != nil { - return nil, err + if len(opts.Namespaces) > 1 { + return newMultiNamespaceCache(config, opts) } - transformers, err := convertToByGVK(opts.View.ByObject.Transform, opts.View.DefaultTransform, opts.Scheme) + byGVK, err := convertToInformerOptsByGVK(opts.ByObject, opts.Scheme) if err != nil { return nil, err } - transformByGVK := internal.TransformFuncByGVKFromMap(transformers) - - internalSelectorsByGVK := internal.SelectorsByGVK{} - for gvk, selector := range selectorsByGVK { - internalSelectorsByGVK[gvk] = internal.Selector(selector) - } - - if len(opts.View.Namespaces) == 0 { - opts.View.Namespaces = []string{metav1.NamespaceAll} - } - - if len(opts.View.Namespaces) > 1 { - return newMultiNamespaceCache(config, opts) + // Set the default selector and transform. + byGVK[schema.GroupVersionKind{}] = internal.InformersOptsByGVK{ + Selector: internal.Selector{ + Label: opts.DefaultLabelSelector, + Field: opts.DefaultFieldSelector, + }, + Transform: opts.DefaultTransform, + UnsafeDisableDeepCopy: opts.UnsafeDisableDeepCopy, } return &informerCache{ @@ -216,22 +204,18 @@ func New(config *rest.Config, opts Options) (Cache, error) { Scheme: opts.Scheme, Mapper: opts.Mapper, ResyncPeriod: *opts.ResyncEvery, - Namespace: opts.View.Namespaces[0], - ByGVK: internal.InformersOptsByGVK{ - Selectors: internalSelectorsByGVK, - DisableDeepCopy: disableDeepCopyByGVK, - Transformers: transformByGVK, - }, + Namespace: opts.Namespaces[0], + ByGVK: byGVK, }), }, nil } // BuilderWithOptions returns a Cache constructor that will build a cache // honoring the options argument, this is useful to specify options like -// SelectorsByObject -// WARNING: If SelectorsByObject is specified, filtered out resources are not +// ByObjects, DefaultSelector, DefaultTransform, etc. +// WARNING: If ByObject selectors are specified, filtered out resources are not // returned. -// WARNING: If UnsafeDisableDeepCopy is enabled, you must DeepCopy any object +// WARNING: If ByObject UnsafeDisableDeepCopy is enabled, you must DeepCopy any object // returned from cache get/list before mutating it. func BuilderWithOptions(options Options) NewCacheFunc { return func(config *rest.Config, inherited Options) (Cache, error) { @@ -260,16 +244,22 @@ func (options Options) inheritFrom(inherited Options) (*Options, error) { combined.Scheme = combineScheme(inherited.Scheme, options.Scheme) combined.Mapper = selectMapper(inherited.Mapper, options.Mapper) combined.ResyncEvery = selectResync(inherited.ResyncEvery, options.ResyncEvery) - combined.View.Namespaces = selectNamespaces(inherited.View.Namespaces, options.View.Namespaces) - combined.View.ByObject.Selectors, combined.View.DefaultSelector, err = combineSelectors(inherited, options, combined.Scheme) - if err != nil { - return nil, err + combined.Namespaces = selectNamespaces(inherited.Namespaces, options.Namespaces) + combined.DefaultLabelSelector = combineSelector( + internal.Selector{Label: inherited.DefaultLabelSelector}, + internal.Selector{Label: options.DefaultLabelSelector}, + ).Label + combined.DefaultFieldSelector = combineSelector( + internal.Selector{Field: inherited.DefaultFieldSelector}, + internal.Selector{Field: options.DefaultFieldSelector}, + ).Field + combined.DefaultTransform = combineTransform(inherited.DefaultTransform, options.DefaultTransform) + combined.ByObject, err = combineByObject(inherited, options, combined.Scheme) + if options.UnsafeDisableDeepCopy != nil { + combined.UnsafeDisableDeepCopy = options.UnsafeDisableDeepCopy + } else { + combined.UnsafeDisableDeepCopy = inherited.UnsafeDisableDeepCopy } - combined.View.ByObject.UnsafeDisableDeepCopy, err = combineUnsafeDeepCopy(inherited, options, combined.Scheme) - if err != nil { - return nil, err - } - combined.View.ByObject.Transform, combined.View.DefaultTransform, err = combineTransforms(inherited, options, combined.Scheme) if err != nil { return nil, err } @@ -313,37 +303,37 @@ func selectNamespaces(def, override []string) []string { return def } -func combineSelectors(inherited, options Options, scheme *runtime.Scheme) (SelectorsByObject, ObjectSelector, error) { - // Selectors are combined via logical AND. - // - Combined label selector is a union of the selectors requirements from both sets of options. - // - Combined field selector uses fields.AndSelectors with the combined list of non-nil field selectors - // defined in both sets of options. - // - // There is a bunch of complexity here because we need to convert to SelectorsByGVK - // to be able to match keys between options and inherited and then convert back to SelectorsByObject - optionsSelectorsByGVK, err := convertToByGVK(options.View.ByObject.Selectors, options.View.DefaultSelector, scheme) +func combineByObject(inherited, options Options, scheme *runtime.Scheme) (map[client.Object]ByObject, error) { + optionsByGVK, err := convertToInformerOptsByGVK(options.ByObject, scheme) if err != nil { - return nil, ObjectSelector{}, err + return nil, err } - inheritedSelectorsByGVK, err := convertToByGVK(inherited.View.ByObject.Selectors, inherited.View.DefaultSelector, inherited.Scheme) + inheritedByGVK, err := convertToInformerOptsByGVK(inherited.ByObject, scheme) if err != nil { - return nil, ObjectSelector{}, err + return nil, err } - - for gvk, inheritedSelector := range inheritedSelectorsByGVK { - optionsSelectorsByGVK[gvk] = combineSelector(inheritedSelector, optionsSelectorsByGVK[gvk]) + for gvk, inheritedByGVK := range inheritedByGVK { + unsafeDisableDeepCopy := options.UnsafeDisableDeepCopy + if current, ok := optionsByGVK[gvk]; ok { + unsafeDisableDeepCopy = current.UnsafeDisableDeepCopy + } + optionsByGVK[gvk] = internal.InformersOptsByGVK{ + Selector: combineSelector(inheritedByGVK.Selector, optionsByGVK[gvk].Selector), + Transform: combineTransform(inheritedByGVK.Transform, optionsByGVK[gvk].Transform), + UnsafeDisableDeepCopy: unsafeDisableDeepCopy, + } } - return convertToByObject(optionsSelectorsByGVK, scheme) + return convertToByObject(optionsByGVK, scheme) } -func combineSelector(selectors ...ObjectSelector) ObjectSelector { +func combineSelector(selectors ...internal.Selector) internal.Selector { ls := make([]labels.Selector, 0, len(selectors)) fs := make([]fields.Selector, 0, len(selectors)) for _, s := range selectors { ls = append(ls, s.Label) fs = append(fs, s.Field) } - return ObjectSelector{ + return internal.Selector{ Label: combineLabelSelectors(ls...), Field: combineFieldSelectors(fs...), } @@ -381,51 +371,6 @@ func combineFieldSelectors(fs ...fields.Selector) fields.Selector { return fields.AndSelectors(nonNil...) } -func combineUnsafeDeepCopy(inherited, options Options, scheme *runtime.Scheme) (DisableDeepCopyByObject, error) { - // UnsafeDisableDeepCopyByObject is combined via precedence. Only if a value for a particular GVK is unset - // in options will a value from inherited be used. - optionsDisableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(options.View.ByObject.UnsafeDisableDeepCopy, options.Scheme) - if err != nil { - return nil, err - } - inheritedDisableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(inherited.View.ByObject.UnsafeDisableDeepCopy, inherited.Scheme) - if err != nil { - return nil, err - } - - for gvk, inheritedDeepCopy := range inheritedDisableDeepCopyByGVK { - if _, ok := optionsDisableDeepCopyByGVK[gvk]; !ok { - if optionsDisableDeepCopyByGVK == nil { - optionsDisableDeepCopyByGVK = map[schema.GroupVersionKind]bool{} - } - optionsDisableDeepCopyByGVK[gvk] = inheritedDeepCopy - } - } - return convertToDisableDeepCopyByObject(optionsDisableDeepCopyByGVK, scheme) -} - -func combineTransforms(inherited, options Options, scheme *runtime.Scheme) (TransformByObject, toolscache.TransformFunc, error) { - // Transform functions are combined via chaining. If both inherited and options define a transform - // function, the transform function from inherited will be called first, and the transform function from - // options will be called second. - optionsTransformByGVK, err := convertToByGVK(options.View.ByObject.Transform, options.View.DefaultTransform, options.Scheme) - if err != nil { - return nil, nil, err - } - inheritedTransformByGVK, err := convertToByGVK(inherited.View.ByObject.Transform, inherited.View.DefaultTransform, inherited.Scheme) - if err != nil { - return nil, nil, err - } - - for gvk, inheritedTransform := range inheritedTransformByGVK { - if optionsTransformByGVK == nil { - optionsTransformByGVK = map[schema.GroupVersionKind]toolscache.TransformFunc{} - } - optionsTransformByGVK[gvk] = combineTransform(inheritedTransform, optionsTransformByGVK[gvk]) - } - return convertToByObject(optionsTransformByGVK, scheme) -} - func combineTransform(inherited, current toolscache.TransformFunc) toolscache.TransformFunc { if inherited == nil { return current @@ -477,77 +422,32 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) { return opts, nil } -func convertToByGVK[T any](byObject map[client.Object]T, def T, scheme *runtime.Scheme) (map[schema.GroupVersionKind]T, error) { - byGVK := map[schema.GroupVersionKind]T{} - for object, value := range byObject { +func convertToInformerOptsByGVK(in map[client.Object]ByObject, scheme *runtime.Scheme) (map[schema.GroupVersionKind]internal.InformersOptsByGVK, error) { + out := map[schema.GroupVersionKind]internal.InformersOptsByGVK{} + for object, byObject := range in { gvk, err := apiutil.GVKForObject(object, scheme) if err != nil { return nil, err } - byGVK[gvk] = value - } - byGVK[schema.GroupVersionKind{}] = def - return byGVK, nil -} - -func convertToByObject[T any](byGVK map[schema.GroupVersionKind]T, scheme *runtime.Scheme) (map[client.Object]T, T, error) { - var byObject map[client.Object]T - def := byGVK[schema.GroupVersionKind{}] - for gvk, value := range byGVK { - if gvk == (schema.GroupVersionKind{}) { - continue + if _, ok := out[gvk]; ok { + return nil, fmt.Errorf("duplicate cache options for GVK %v, cache.Options.ByObject has multiple types with the same GroupVersionKind", gvk) } - obj, err := scheme.New(gvk) - if err != nil { - return nil, def, err - } - cObj, ok := obj.(client.Object) - if !ok { - return nil, def, fmt.Errorf("object %T for GVK %q does not implement client.Object", obj, gvk) - } - if byObject == nil { - byObject = map[client.Object]T{} - } - byObject[cObj] = value - } - return byObject, def, nil -} - -// DisableDeepCopyByObject associate a client.Object's GVK to disable DeepCopy during get or list from cache. -type DisableDeepCopyByObject map[client.Object]bool - -var _ client.Object = &ObjectAll{} - -// ObjectAll is the argument to represent all objects' types. -type ObjectAll struct { - client.Object -} - -func convertToDisableDeepCopyByGVK(disableDeepCopyByObject DisableDeepCopyByObject, scheme *runtime.Scheme) (internal.DisableDeepCopyByGVK, error) { - disableDeepCopyByGVK := internal.DisableDeepCopyByGVK{} - for obj, disable := range disableDeepCopyByObject { - switch obj.(type) { - case ObjectAll, *ObjectAll: - disableDeepCopyByGVK[internal.GroupVersionKindAll] = disable - default: - gvk, err := apiutil.GVKForObject(obj, scheme) - if err != nil { - return nil, err - } - disableDeepCopyByGVK[gvk] = disable + out[gvk] = internal.InformersOptsByGVK{ + Selector: internal.Selector{ + Field: byObject.Field, + Label: byObject.Label, + }, + Transform: byObject.Transform, + UnsafeDisableDeepCopy: byObject.UnsafeDisableDeepCopy, } } - return disableDeepCopyByGVK, nil + return out, nil } -func convertToDisableDeepCopyByObject(byGVK internal.DisableDeepCopyByGVK, scheme *runtime.Scheme) (DisableDeepCopyByObject, error) { - var byObject DisableDeepCopyByObject - for gvk, value := range byGVK { - if byObject == nil { - byObject = DisableDeepCopyByObject{} - } +func convertToByObject(in map[schema.GroupVersionKind]internal.InformersOptsByGVK, scheme *runtime.Scheme) (map[client.Object]ByObject, error) { + out := map[client.Object]ByObject{} + for gvk, opts := range in { if gvk == (schema.GroupVersionKind{}) { - byObject[ObjectAll{}] = value continue } obj, err := scheme.New(gvk) @@ -558,12 +458,12 @@ func convertToDisableDeepCopyByObject(byGVK internal.DisableDeepCopyByGVK, schem if !ok { return nil, fmt.Errorf("object %T for GVK %q does not implement client.Object", obj, gvk) } - - byObject[cObj] = value + out[cObj] = ByObject{ + Field: opts.Selector.Field, + Label: opts.Selector.Label, + Transform: opts.Transform, + UnsafeDisableDeepCopy: opts.UnsafeDisableDeepCopy, + } } - return byObject, nil + return out, nil } - -// TransformByObject associate a client.Object's GVK to a transformer function -// to be applied when storing the object into the cache. -type TransformByObject map[client.Object]toolscache.TransformFunc diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index 4e882f0f8f..36213c6ffd 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -38,6 +38,7 @@ import ( kscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" kcache "k8s.io/client-go/tools/cache" + "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" @@ -119,13 +120,10 @@ var _ = Describe("Informer Cache", func() { var _ = Describe("Multi-Namespace Informer Cache", func() { CacheTest(cache.MultiNamespacedCacheBuilder([]string{testNamespaceOne, testNamespaceTwo, "default"}), cache.Options{}) }) -var _ = Describe("Informer Cache without DeepCopy", func() { + +var _ = Describe("Informer Cache without global DeepCopy", func() { CacheTest(cache.New, cache.Options{ - View: cache.ViewOptions{ - ByObject: cache.ViewByObject{ - UnsafeDisableDeepCopy: cache.DisableDeepCopyByObject{cache.ObjectAll{}: true}, - }, - }, + UnsafeDisableDeepCopy: pointer.Bool(true), }) }) @@ -190,48 +188,46 @@ var _ = Describe("Cache with transformers", func() { By("creating the informer cache") informerCache, err = cache.New(cfg, cache.Options{ - View: cache.ViewOptions{ - DefaultTransform: func(i interface{}) (interface{}, error) { - obj := i.(runtime.Object) - Expect(obj).NotTo(BeNil()) + DefaultTransform: func(i interface{}) (interface{}, error) { + obj := i.(runtime.Object) + Expect(obj).NotTo(BeNil()) - accessor, err := meta.Accessor(obj) - Expect(err).To(BeNil()) - annotations := accessor.GetAnnotations() - - if _, exists := annotations["transformed"]; exists { - // Avoid performing transformation multiple times. - return i, nil - } + accessor, err := meta.Accessor(obj) + Expect(err).To(BeNil()) + annotations := accessor.GetAnnotations() - if annotations == nil { - annotations = make(map[string]string) - } - annotations["transformed"] = "default" - accessor.SetAnnotations(annotations) + if _, exists := annotations["transformed"]; exists { + // Avoid performing transformation multiple times. return i, nil - }, - ByObject: cache.ViewByObject{ - Transform: cache.TransformByObject{ - &corev1.Pod{}: func(i interface{}) (interface{}, error) { - obj := i.(runtime.Object) - Expect(obj).NotTo(BeNil()) - accessor, err := meta.Accessor(obj) - Expect(err).To(BeNil()) - - annotations := accessor.GetAnnotations() - if _, exists := annotations["transformed"]; exists { - // Avoid performing transformation multiple times. - return i, nil - } - - if annotations == nil { - annotations = make(map[string]string) - } - annotations["transformed"] = "explicit" - accessor.SetAnnotations(annotations) + } + + if annotations == nil { + annotations = make(map[string]string) + } + annotations["transformed"] = "default" + accessor.SetAnnotations(annotations) + return i, nil + }, + ByObject: map[client.Object]cache.ByObject{ + &corev1.Pod{}: { + Transform: func(i interface{}) (interface{}, error) { + obj := i.(runtime.Object) + Expect(obj).NotTo(BeNil()) + accessor, err := meta.Accessor(obj) + Expect(err).To(BeNil()) + + annotations := accessor.GetAnnotations() + if _, exists := annotations["transformed"]; exists { + // Avoid performing transformation multiple times. return i, nil - }, + } + + if annotations == nil { + annotations = make(map[string]string) + } + annotations["transformed"] = "explicit" + accessor.SetAnnotations(annotations) + return i, nil }, }, }, @@ -370,13 +366,9 @@ var _ = Describe("Cache with selectors", func() { } opts := cache.Options{ - View: cache.ViewOptions{ - DefaultSelector: cache.ObjectSelector{Field: fields.OneTermEqualSelector("metadata.namespace", testNamespaceTwo)}, - ByObject: cache.ViewByObject{ - Selectors: cache.SelectorsByObject{ - &corev1.ServiceAccount{}: {Field: fields.OneTermEqualSelector("metadata.namespace", testNamespaceOne)}, - }, - }, + DefaultFieldSelector: fields.OneTermEqualSelector("metadata.namespace", testNamespaceTwo), + ByObject: map[client.Object]cache.ByObject{ + &corev1.ServiceAccount{}: {Field: fields.OneTermEqualSelector("metadata.namespace", testNamespaceOne)}, }, } @@ -798,7 +790,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca It("should be able to restrict cache to a namespace", func() { By("creating a namespaced cache") - namespacedCache, err := cache.New(cfg, cache.Options{View: cache.ViewOptions{Namespaces: []string{testNamespaceOne}}}) + namespacedCache, err := cache.New(cfg, cache.Options{Namespaces: []string{testNamespaceOne}}) Expect(err).NotTo(HaveOccurred()) By("running the cache and waiting for it to sync") @@ -1079,7 +1071,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca It("should be able to restrict cache to a namespace", func() { By("creating a namespaced cache") - namespacedCache, err := cache.New(cfg, cache.Options{View: cache.ViewOptions{Namespaces: []string{testNamespaceOne}}}) + namespacedCache, err := cache.New(cfg, cache.Options{Namespaces: []string{testNamespaceOne}}) Expect(err).NotTo(HaveOccurred()) By("running the cache and waiting for it to sync") @@ -1233,14 +1225,10 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca By("creating the cache") builder := cache.BuilderWithOptions( cache.Options{ - View: cache.ViewOptions{ - ByObject: cache.ViewByObject{ - Selectors: cache.SelectorsByObject{ - &corev1.Pod{}: { - Label: labels.Set(tc.labelSelectors).AsSelector(), - Field: fields.Set(tc.fieldSelectors).AsSelector(), - }, - }, + ByObject: map[client.Object]cache.ByObject{ + &corev1.Pod{}: { + Label: labels.Set(tc.labelSelectors).AsSelector(), + Field: fields.Set(tc.fieldSelectors).AsSelector(), }, }, }, @@ -1822,12 +1810,11 @@ func isKubeService(svc metav1.Object) bool { } func isPodDisableDeepCopy(opts cache.Options) bool { - if d, ok := opts.View.ByObject.UnsafeDisableDeepCopy[&corev1.Pod{}]; ok { - return d - } else if d, ok = opts.View.ByObject.UnsafeDisableDeepCopy[cache.ObjectAll{}]; ok { - return d - } else if d, ok = opts.View.ByObject.UnsafeDisableDeepCopy[&cache.ObjectAll{}]; ok { - return d + if opts.ByObject[&corev1.Pod{}].UnsafeDisableDeepCopy != nil { + return *opts.ByObject[&corev1.Pod{}].UnsafeDisableDeepCopy + } + if opts.UnsafeDisableDeepCopy != nil { + return *opts.UnsafeDisableDeepCopy } return false } diff --git a/pkg/cache/cache_unit_test.go b/pkg/cache/cache_unit_test.go index 8f74e2407f..1006c72812 100644 --- a/pkg/cache/cache_unit_test.go +++ b/pkg/cache/cache_unit_test.go @@ -14,7 +14,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/cache" "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" @@ -110,184 +109,231 @@ var _ = Describe("cache.inheritFrom", func() { }) }) Context("Namespace", func() { - It("has zero length when View.Namespaces specified and inherited are unset", func() { - Expect(checkError(specified.inheritFrom(inherited)).View.Namespaces).To(HaveLen(0)) + It("has zero length when Namespaces specified and inherited are unset", func() { + Expect(checkError(specified.inheritFrom(inherited)).Namespaces).To(HaveLen(0)) }) It("is unchanged when only specified is set", func() { - specified.View.Namespaces = []string{"specified"} - Expect(checkError(specified.inheritFrom(inherited)).View.Namespaces).To(Equal(specified.View.Namespaces)) + specified.Namespaces = []string{"specified"} + Expect(checkError(specified.inheritFrom(inherited)).Namespaces).To(Equal(specified.Namespaces)) }) It("is inherited when only inherited is set", func() { - inherited.View.Namespaces = []string{"inherited"} - Expect(checkError(specified.inheritFrom(inherited)).View.Namespaces).To(Equal(inherited.View.Namespaces)) + inherited.Namespaces = []string{"inherited"} + Expect(checkError(specified.inheritFrom(inherited)).Namespaces).To(Equal(inherited.Namespaces)) }) It("in unchanged when both inherited and specified are set", func() { - specified.View.Namespaces = []string{"specified"} - inherited.View.Namespaces = []string{"inherited"} - Expect(checkError(specified.inheritFrom(inherited)).View.Namespaces).To(Equal(specified.View.Namespaces)) + specified.Namespaces = []string{"specified"} + inherited.Namespaces = []string{"inherited"} + Expect(checkError(specified.inheritFrom(inherited)).Namespaces).To(Equal(specified.Namespaces)) }) }) Context("SelectorsByObject", func() { It("is unchanged when specified and inherited are unset", func() { - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.Selectors).To(BeNil()) + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(0)) }) It("is unchanged when only specified is set", func() { specified.Scheme = coreScheme - specified.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.Pod{}: {}} - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.Selectors).To(HaveLen(1)) + specified.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: {}, + } + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(1)) }) It("is inherited when only inherited is set", func() { inherited.Scheme = coreScheme - inherited.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.ConfigMap{}: {}} - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.Selectors).To(HaveLen(1)) + inherited.ByObject = map[client.Object]ByObject{ + &corev1.ConfigMap{}: {}, + } + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(1)) }) It("is combined when both inherited and specified are set", func() { specified.Scheme = coreScheme inherited.Scheme = coreScheme - specified.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.Pod{}: {}} - inherited.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.ConfigMap{}: {}} - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.Selectors).To(HaveLen(2)) + specified.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: {}, + } + inherited.ByObject = map[client.Object]ByObject{ + &corev1.ConfigMap{}: {}, + } + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(2)) }) It("combines selectors if specified and inherited specify selectors for the same object", func() { specified.Scheme = coreScheme inherited.Scheme = coreScheme - specified.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.Pod{}: { - Label: labels.Set{"specified": "true"}.AsSelector(), - Field: fields.Set{"metadata.name": "specified"}.AsSelector(), - }} - inherited.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.Pod{}: { - Label: labels.Set{"inherited": "true"}.AsSelector(), - Field: fields.Set{"metadata.namespace": "inherited"}.AsSelector(), - }} - combined := checkError(specified.inheritFrom(inherited)).View.ByObject.Selectors + specified.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + Label: labels.Set{"specified": "true"}.AsSelector(), + Field: fields.Set{"metadata.name": "specified"}.AsSelector(), + }, + } + inherited.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + Label: labels.Set{"inherited": "true"}.AsSelector(), + Field: fields.Set{"metadata.namespace": "inherited"}.AsSelector(), + }, + } + combined := checkError(specified.inheritFrom(inherited)).ByObject Expect(combined).To(HaveLen(1)) var ( obj client.Object - selector ObjectSelector + byObject ByObject ) - for obj, selector = range combined { + for obj, byObject = range combined { } Expect(obj).To(BeAssignableToTypeOf(&corev1.Pod{})) - Expect(selector.Label.Matches(labels.Set{"specified": "true"})).To(BeFalse()) - Expect(selector.Label.Matches(labels.Set{"inherited": "true"})).To(BeFalse()) - Expect(selector.Label.Matches(labels.Set{"specified": "true", "inherited": "true"})).To(BeTrue()) + Expect(byObject.Label.Matches(labels.Set{"specified": "true"})).To(BeFalse()) + Expect(byObject.Label.Matches(labels.Set{"inherited": "true"})).To(BeFalse()) + Expect(byObject.Label.Matches(labels.Set{"specified": "true", "inherited": "true"})).To(BeTrue()) - Expect(selector.Field.Matches(fields.Set{"metadata.name": "specified", "metadata.namespace": "other"})).To(BeFalse()) - Expect(selector.Field.Matches(fields.Set{"metadata.name": "other", "metadata.namespace": "inherited"})).To(BeFalse()) - Expect(selector.Field.Matches(fields.Set{"metadata.name": "specified", "metadata.namespace": "inherited"})).To(BeTrue()) + Expect(byObject.Field.Matches(fields.Set{"metadata.name": "specified", "metadata.namespace": "other"})).To(BeFalse()) + Expect(byObject.Field.Matches(fields.Set{"metadata.name": "other", "metadata.namespace": "inherited"})).To(BeFalse()) + Expect(byObject.Field.Matches(fields.Set{"metadata.name": "specified", "metadata.namespace": "inherited"})).To(BeTrue()) }) It("uses inherited scheme for inherited selectors", func() { inherited.Scheme = coreScheme - inherited.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.ConfigMap{}: {}} - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.Selectors).To(HaveLen(1)) - }) - It("does not use specified scheme for inherited selectors", func() { - inherited.Scheme = runtime.NewScheme() - specified.Scheme = coreScheme - inherited.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.ConfigMap{}: {}} - _, err := specified.inheritFrom(inherited) - Expect(err).To(WithTransform(runtime.IsNotRegisteredError, BeTrue())) + inherited.ByObject = map[client.Object]ByObject{ + &corev1.ConfigMap{}: {}, + } + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(1)) }) It("uses inherited scheme for specified selectors", func() { inherited.Scheme = coreScheme - specified.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.ConfigMap{}: {}} - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.Selectors).To(HaveLen(1)) + specified.ByObject = map[client.Object]ByObject{ + &corev1.ConfigMap{}: {}, + } + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(1)) }) It("uses specified scheme for specified selectors", func() { specified.Scheme = coreScheme - specified.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.ConfigMap{}: {}} - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.Selectors).To(HaveLen(1)) + specified.ByObject = map[client.Object]ByObject{ + &corev1.ConfigMap{}: {}, + } + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(1)) }) }) Context("DefaultSelector", func() { It("is unchanged when specified and inherited are unset", func() { - Expect(specified.View.DefaultSelector).To(Equal(ObjectSelector{})) - Expect(inherited.View.DefaultSelector).To(Equal(ObjectSelector{})) - Expect(checkError(specified.inheritFrom(inherited)).View.DefaultSelector).To(Equal(ObjectSelector{})) + Expect(specified.DefaultLabelSelector).To(BeNil()) + Expect(inherited.DefaultLabelSelector).To(BeNil()) + Expect(specified.DefaultFieldSelector).To(BeNil()) + Expect(inherited.DefaultFieldSelector).To(BeNil()) + Expect(checkError(specified.inheritFrom(inherited)).DefaultLabelSelector).To(BeNil()) + Expect(checkError(specified.inheritFrom(inherited)).DefaultFieldSelector).To(BeNil()) }) It("is unchanged when only specified is set", func() { - specified.View.DefaultSelector = ObjectSelector{Label: labels.Set{"specified": "true"}.AsSelector()} - Expect(checkError(specified.inheritFrom(inherited)).View.DefaultSelector).To(Equal(specified.View.DefaultSelector)) + specified.DefaultLabelSelector = labels.Set{"specified": "true"}.AsSelector() + specified.DefaultFieldSelector = fields.Set{"specified": "true"}.AsSelector() + Expect(checkError(specified.inheritFrom(inherited)).DefaultLabelSelector).To(Equal(specified.DefaultLabelSelector)) + Expect(checkError(specified.inheritFrom(inherited)).DefaultFieldSelector).To(Equal(specified.DefaultFieldSelector)) }) It("is inherited when only inherited is set", func() { - inherited.View.DefaultSelector = ObjectSelector{Label: labels.Set{"inherited": "true"}.AsSelector()} - Expect(checkError(specified.inheritFrom(inherited)).View.DefaultSelector).To(Equal(inherited.View.DefaultSelector)) + inherited.DefaultLabelSelector = labels.Set{"inherited": "true"}.AsSelector() + inherited.DefaultFieldSelector = fields.Set{"inherited": "true"}.AsSelector() + Expect(checkError(specified.inheritFrom(inherited)).DefaultLabelSelector).To(Equal(inherited.DefaultLabelSelector)) + Expect(checkError(specified.inheritFrom(inherited)).DefaultFieldSelector).To(Equal(inherited.DefaultFieldSelector)) }) It("is combined when both inherited and specified are set", func() { - specified.View.DefaultSelector = ObjectSelector{ - Label: labels.Set{"specified": "true"}.AsSelector(), - Field: fields.Set{"metadata.name": "specified"}.AsSelector(), + specified.DefaultLabelSelector = labels.Set{"specified": "true"}.AsSelector() + specified.DefaultFieldSelector = fields.Set{"metadata.name": "specified"}.AsSelector() + + inherited.DefaultLabelSelector = labels.Set{"inherited": "true"}.AsSelector() + inherited.DefaultFieldSelector = fields.Set{"metadata.namespace": "inherited"}.AsSelector() + { + combined := checkError(specified.inheritFrom(inherited)).DefaultLabelSelector + Expect(combined).NotTo(BeNil()) + Expect(combined.Matches(labels.Set{"specified": "true"})).To(BeFalse()) + Expect(combined.Matches(labels.Set{"inherited": "true"})).To(BeFalse()) + Expect(combined.Matches(labels.Set{"specified": "true", "inherited": "true"})).To(BeTrue()) } - inherited.View.DefaultSelector = ObjectSelector{ - Label: labels.Set{"inherited": "true"}.AsSelector(), - Field: fields.Set{"metadata.namespace": "inherited"}.AsSelector(), + + { + combined := checkError(specified.inheritFrom(inherited)).DefaultFieldSelector + Expect(combined.Matches(fields.Set{"metadata.name": "specified", "metadata.namespace": "other"})).To(BeFalse()) + Expect(combined.Matches(fields.Set{"metadata.name": "other", "metadata.namespace": "inherited"})).To(BeFalse()) + Expect(combined.Matches(fields.Set{"metadata.name": "specified", "metadata.namespace": "inherited"})).To(BeTrue()) } - combined := checkError(specified.inheritFrom(inherited)).View.DefaultSelector - Expect(combined).NotTo(BeNil()) - Expect(combined.Label.Matches(labels.Set{"specified": "true"})).To(BeFalse()) - Expect(combined.Label.Matches(labels.Set{"inherited": "true"})).To(BeFalse()) - Expect(combined.Label.Matches(labels.Set{"specified": "true", "inherited": "true"})).To(BeTrue()) - Expect(combined.Field.Matches(fields.Set{"metadata.name": "specified", "metadata.namespace": "other"})).To(BeFalse()) - Expect(combined.Field.Matches(fields.Set{"metadata.name": "other", "metadata.namespace": "inherited"})).To(BeFalse()) - Expect(combined.Field.Matches(fields.Set{"metadata.name": "specified", "metadata.namespace": "inherited"})).To(BeTrue()) }) }) Context("UnsafeDisableDeepCopyByObject", func() { It("is unchanged when specified and inherited are unset", func() { - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.UnsafeDisableDeepCopy).To(BeNil()) + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(0)) }) It("is unchanged when only specified is set", func() { specified.Scheme = coreScheme - specified.View.ByObject.UnsafeDisableDeepCopy = map[client.Object]bool{ObjectAll{}: true} - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.UnsafeDisableDeepCopy).To(HaveLen(1)) + specified.UnsafeDisableDeepCopy = pointer.Bool(true) + Expect(*(checkError(specified.inheritFrom(inherited)).UnsafeDisableDeepCopy)).To(BeTrue()) }) It("is inherited when only inherited is set", func() { inherited.Scheme = coreScheme - inherited.View.ByObject.UnsafeDisableDeepCopy = map[client.Object]bool{ObjectAll{}: true} - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.UnsafeDisableDeepCopy).To(HaveLen(1)) + inherited.UnsafeDisableDeepCopy = pointer.Bool(true) + Expect(*(checkError(specified.inheritFrom(inherited)).UnsafeDisableDeepCopy)).To(BeTrue()) }) It("is combined when both inherited and specified are set for different keys", func() { specified.Scheme = coreScheme inherited.Scheme = coreScheme - specified.View.ByObject.UnsafeDisableDeepCopy = map[client.Object]bool{&corev1.Pod{}: true} - inherited.View.ByObject.UnsafeDisableDeepCopy = map[client.Object]bool{&corev1.ConfigMap{}: true} - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.UnsafeDisableDeepCopy).To(HaveLen(2)) + specified.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + UnsafeDisableDeepCopy: pointer.Bool(true), + }, + } + inherited.ByObject = map[client.Object]ByObject{ + &corev1.ConfigMap{}: { + UnsafeDisableDeepCopy: pointer.Bool(true), + }, + } + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(2)) }) It("is true when inherited=false and specified=true for the same key", func() { specified.Scheme = coreScheme inherited.Scheme = coreScheme - specified.View.ByObject.UnsafeDisableDeepCopy = map[client.Object]bool{&corev1.Pod{}: true} - inherited.View.ByObject.UnsafeDisableDeepCopy = map[client.Object]bool{&corev1.Pod{}: false} - combined := checkError(specified.inheritFrom(inherited)).View.ByObject.UnsafeDisableDeepCopy + specified.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + UnsafeDisableDeepCopy: pointer.Bool(true), + }, + } + inherited.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + UnsafeDisableDeepCopy: pointer.Bool(false), + }, + } + combined := checkError(specified.inheritFrom(inherited)).ByObject Expect(combined).To(HaveLen(1)) var ( - obj client.Object - disableDeepCopy bool + obj client.Object + byObject ByObject ) - for obj, disableDeepCopy = range combined { + for obj, byObject = range combined { } Expect(obj).To(BeAssignableToTypeOf(&corev1.Pod{})) - Expect(disableDeepCopy).To(BeTrue()) + Expect(byObject.UnsafeDisableDeepCopy).ToNot(BeNil()) + Expect(*byObject.UnsafeDisableDeepCopy).To(BeTrue()) }) It("is false when inherited=true and specified=false for the same key", func() { specified.Scheme = coreScheme inherited.Scheme = coreScheme - specified.View.ByObject.UnsafeDisableDeepCopy = map[client.Object]bool{&corev1.Pod{}: false} - inherited.View.ByObject.UnsafeDisableDeepCopy = map[client.Object]bool{&corev1.Pod{}: true} - combined := checkError(specified.inheritFrom(inherited)).View.ByObject.UnsafeDisableDeepCopy + specified.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + UnsafeDisableDeepCopy: pointer.Bool(false), + }, + } + inherited.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + UnsafeDisableDeepCopy: pointer.Bool(true), + }, + } + combined := checkError(specified.inheritFrom(inherited)).ByObject Expect(combined).To(HaveLen(1)) var ( - obj client.Object - disableDeepCopy bool + obj client.Object + byObject ByObject ) - for obj, disableDeepCopy = range combined { + for obj, byObject = range combined { } Expect(obj).To(BeAssignableToTypeOf(&corev1.Pod{})) - Expect(disableDeepCopy).To(BeFalse()) + Expect(byObject.UnsafeDisableDeepCopy).ToNot(BeNil()) + Expect(*byObject.UnsafeDisableDeepCopy).To(BeFalse()) }) }) Context("TransformByObject", func() { @@ -302,20 +348,24 @@ var _ = Describe("cache.inheritFrom", func() { tx = transformed{} }) It("is unchanged when specified and inherited are unset", func() { - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.Transform).To(BeNil()) + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(0)) }) It("is unchanged when only specified is set", func() { specified.Scheme = coreScheme - specified.View.ByObject.Transform = map[client.Object]cache.TransformFunc{&corev1.Pod{}: func(i interface{}) (interface{}, error) { - ti := i.(transformed) - ti.podSpecified = true - return ti, nil - }} - combined := checkError(specified.inheritFrom(inherited)).View.ByObject.Transform + specified.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + Transform: func(i interface{}) (interface{}, error) { + ti := i.(transformed) + ti.podSpecified = true + return ti, nil + }, + }, + } + combined := checkError(specified.inheritFrom(inherited)).ByObject Expect(combined).To(HaveLen(1)) - for obj, fn := range combined { + for obj, byObject := range combined { Expect(obj).To(BeAssignableToTypeOf(&corev1.Pod{})) - out, _ := fn(tx) + out, _ := byObject.Transform(tx) Expect(out).To(And( BeAssignableToTypeOf(tx), WithTransform(func(i transformed) bool { return i.podSpecified }, BeTrue()), @@ -325,16 +375,20 @@ var _ = Describe("cache.inheritFrom", func() { }) It("is inherited when only inherited is set", func() { inherited.Scheme = coreScheme - inherited.View.ByObject.Transform = map[client.Object]cache.TransformFunc{&corev1.Pod{}: func(i interface{}) (interface{}, error) { - ti := i.(transformed) - ti.podInherited = true - return ti, nil - }} - combined := checkError(specified.inheritFrom(inherited)).View.ByObject.Transform + inherited.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + Transform: func(i interface{}) (interface{}, error) { + ti := i.(transformed) + ti.podInherited = true + return ti, nil + }, + }, + } + combined := checkError(specified.inheritFrom(inherited)).ByObject Expect(combined).To(HaveLen(1)) - for obj, fn := range combined { + for obj, byObject := range combined { Expect(obj).To(BeAssignableToTypeOf(&corev1.Pod{})) - out, _ := fn(tx) + out, _ := byObject.Transform(tx) Expect(out).To(And( BeAssignableToTypeOf(tx), WithTransform(func(i transformed) bool { return i.podSpecified }, BeFalse()), @@ -345,20 +399,28 @@ var _ = Describe("cache.inheritFrom", func() { It("is combined when both inherited and specified are set for different keys", func() { specified.Scheme = coreScheme inherited.Scheme = coreScheme - specified.View.ByObject.Transform = map[client.Object]cache.TransformFunc{&corev1.Pod{}: func(i interface{}) (interface{}, error) { - ti := i.(transformed) - ti.podSpecified = true - return ti, nil - }} - inherited.View.ByObject.Transform = map[client.Object]cache.TransformFunc{&corev1.ConfigMap{}: func(i interface{}) (interface{}, error) { - ti := i.(transformed) - ti.configmapInherited = true - return ti, nil - }} - combined := checkError(specified.inheritFrom(inherited)).View.ByObject.Transform + specified.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + Transform: func(i interface{}) (interface{}, error) { + ti := i.(transformed) + ti.podSpecified = true + return ti, nil + }, + }, + } + inherited.ByObject = map[client.Object]ByObject{ + &corev1.ConfigMap{}: { + Transform: func(i interface{}) (interface{}, error) { + ti := i.(transformed) + ti.configmapInherited = true + return ti, nil + }, + }, + } + combined := checkError(specified.inheritFrom(inherited)).ByObject Expect(combined).To(HaveLen(2)) - for obj, fn := range combined { - out, _ := fn(tx) + for obj, byObject := range combined { + out, _ := byObject.Transform(tx) if reflect.TypeOf(obj) == reflect.TypeOf(&corev1.Pod{}) { Expect(out).To(And( BeAssignableToTypeOf(tx), @@ -382,21 +444,29 @@ var _ = Describe("cache.inheritFrom", func() { It("is combined into a single transform function when both inherited and specified are set for the same key", func() { specified.Scheme = coreScheme inherited.Scheme = coreScheme - specified.View.ByObject.Transform = map[client.Object]cache.TransformFunc{&corev1.Pod{}: func(i interface{}) (interface{}, error) { - ti := i.(transformed) - ti.podSpecified = true - return ti, nil - }} - inherited.View.ByObject.Transform = map[client.Object]cache.TransformFunc{&corev1.Pod{}: func(i interface{}) (interface{}, error) { - ti := i.(transformed) - ti.podInherited = true - return ti, nil - }} - combined := checkError(specified.inheritFrom(inherited)).View.ByObject.Transform + specified.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + Transform: func(i interface{}) (interface{}, error) { + ti := i.(transformed) + ti.podSpecified = true + return ti, nil + }, + }, + } + inherited.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + Transform: func(i interface{}) (interface{}, error) { + ti := i.(transformed) + ti.podInherited = true + return ti, nil + }, + }, + } + combined := checkError(specified.inheritFrom(inherited)).ByObject Expect(combined).To(HaveLen(1)) - for obj, fn := range combined { + for obj, byObject := range combined { Expect(obj).To(BeAssignableToTypeOf(&corev1.Pod{})) - out, _ := fn(tx) + out, _ := byObject.Transform(tx) Expect(out).To(And( BeAssignableToTypeOf(tx), WithTransform(func(i transformed) bool { return i.podSpecified }, BeTrue()), @@ -417,15 +487,15 @@ var _ = Describe("cache.inheritFrom", func() { tx = transformed{} }) It("is unchanged when specified and inherited are unset", func() { - Expect(checkError(specified.inheritFrom(inherited)).View.DefaultTransform).To(BeNil()) + Expect(checkError(specified.inheritFrom(inherited)).DefaultTransform).To(BeNil()) }) It("is unchanged when only specified is set", func() { - specified.View.DefaultTransform = func(i interface{}) (interface{}, error) { + specified.DefaultTransform = func(i interface{}) (interface{}, error) { ti := i.(transformed) ti.specified = true return ti, nil } - combined := checkError(specified.inheritFrom(inherited)).View.DefaultTransform + combined := checkError(specified.inheritFrom(inherited)).DefaultTransform out, _ := combined(tx) Expect(out).To(And( BeAssignableToTypeOf(tx), @@ -434,12 +504,12 @@ var _ = Describe("cache.inheritFrom", func() { )) }) It("is inherited when only inherited is set", func() { - inherited.View.DefaultTransform = func(i interface{}) (interface{}, error) { + inherited.DefaultTransform = func(i interface{}) (interface{}, error) { ti := i.(transformed) ti.inherited = true return ti, nil } - combined := checkError(specified.inheritFrom(inherited)).View.DefaultTransform + combined := checkError(specified.inheritFrom(inherited)).DefaultTransform out, _ := combined(tx) Expect(out).To(And( BeAssignableToTypeOf(tx), @@ -448,17 +518,17 @@ var _ = Describe("cache.inheritFrom", func() { )) }) It("is combined when the transform function is defined in both inherited and specified", func() { - specified.View.DefaultTransform = func(i interface{}) (interface{}, error) { + specified.DefaultTransform = func(i interface{}) (interface{}, error) { ti := i.(transformed) ti.specified = true return ti, nil } - inherited.View.DefaultTransform = func(i interface{}) (interface{}, error) { + inherited.DefaultTransform = func(i interface{}) (interface{}, error) { ti := i.(transformed) ti.inherited = true return ti, nil } - combined := checkError(specified.inheritFrom(inherited)).View.DefaultTransform + combined := checkError(specified.inheritFrom(inherited)).DefaultTransform Expect(combined).NotTo(BeNil()) out, _ := combined(tx) Expect(out).To(And( diff --git a/pkg/cache/internal/disabledeepcopy.go b/pkg/cache/internal/disabledeepcopy.go deleted file mode 100644 index 54bd7eec93..0000000000 --- a/pkg/cache/internal/disabledeepcopy.go +++ /dev/null @@ -1,35 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package internal - -import "k8s.io/apimachinery/pkg/runtime/schema" - -// GroupVersionKindAll is the argument to represent all GroupVersionKind types. -var GroupVersionKindAll = schema.GroupVersionKind{} - -// DisableDeepCopyByGVK associate a GroupVersionKind to disable DeepCopy during get or list from cache. -type DisableDeepCopyByGVK map[schema.GroupVersionKind]bool - -// IsDisabled returns whether a GroupVersionKind is disabled DeepCopy. -func (disableByGVK DisableDeepCopyByGVK) IsDisabled(gvk schema.GroupVersionKind) bool { - if d, ok := disableByGVK[gvk]; ok { - return d - } else if d, ok = disableByGVK[GroupVersionKindAll]; ok { - return d - } - return false -} diff --git a/pkg/cache/internal/informers.go b/pkg/cache/internal/informers.go index c69c4985ef..bc3e5f23da 100644 --- a/pkg/cache/internal/informers.go +++ b/pkg/cache/internal/informers.go @@ -50,15 +50,15 @@ type InformersOpts struct { Mapper meta.RESTMapper ResyncPeriod time.Duration Namespace string - ByGVK InformersOptsByGVK + ByGVK map[schema.GroupVersionKind]InformersOptsByGVK } // InformersOptsByGVK configured additional by group version kind (or object) // in an InformerMap. type InformersOptsByGVK struct { - Selectors SelectorsByGVK - Transformers TransformFuncByGVK - DisableDeepCopy DisableDeepCopyByGVK + Selector Selector + Transform cache.TransformFunc + UnsafeDisableDeepCopy *bool } // NewInformers creates a new InformersMap that can create informers under the hood. @@ -73,14 +73,12 @@ func NewInformers(config *rest.Config, options *InformersOpts) *Informers { Unstructured: make(map[schema.GroupVersionKind]*Cache), Metadata: make(map[schema.GroupVersionKind]*Cache), }, - codecs: serializer.NewCodecFactory(options.Scheme), - paramCodec: runtime.NewParameterCodec(options.Scheme), - resync: options.ResyncPeriod, - startWait: make(chan struct{}), - namespace: options.Namespace, - selectorByGVK: options.ByGVK.Selectors.forGVK, - disableDeepCopyByGVK: options.ByGVK.DisableDeepCopy, - transformersByGVK: options.ByGVK.Transformers, + codecs: serializer.NewCodecFactory(options.Scheme), + paramCodec: runtime.NewParameterCodec(options.Scheme), + resync: options.ResyncPeriod, + startWait: make(chan struct{}), + namespace: options.Namespace, + byGVK: options.ByGVK, } } @@ -151,15 +149,46 @@ type Informers struct { // default or empty string means all namespaces namespace string - // selectorByGVK are the label or field selectorByGVK that will be added to the - // ListWatch ListOptions. - selectorByGVK func(gvk schema.GroupVersionKind) Selector + byGVK map[schema.GroupVersionKind]InformersOptsByGVK +} - // disableDeepCopyByGVK indicates not to deep copy objects during get or list objects. - disableDeepCopyByGVK DisableDeepCopyByGVK +func (ip *Informers) getSelector(gvk schema.GroupVersionKind) Selector { + if ip.byGVK == nil { + return Selector{} + } + if res, ok := ip.byGVK[gvk]; ok { + return res.Selector + } + if res, ok := ip.byGVK[schema.GroupVersionKind{}]; ok { + return res.Selector + } + return Selector{} +} - // transform funcs are applied to objects before they are committed to the cache - transformersByGVK TransformFuncByGVK +func (ip *Informers) getTransform(gvk schema.GroupVersionKind) cache.TransformFunc { + if ip.byGVK == nil { + return nil + } + if res, ok := ip.byGVK[gvk]; ok { + return res.Transform + } + if res, ok := ip.byGVK[schema.GroupVersionKind{}]; ok { + return res.Transform + } + return nil +} + +func (ip *Informers) getDisableDeepCopy(gvk schema.GroupVersionKind) bool { + if ip.byGVK == nil { + return false + } + if res, ok := ip.byGVK[gvk]; ok && res.UnsafeDisableDeepCopy != nil { + return *res.UnsafeDisableDeepCopy + } + if res, ok := ip.byGVK[schema.GroupVersionKind{}]; ok && res.UnsafeDisableDeepCopy != nil { + return *res.UnsafeDisableDeepCopy + } + return false } // Start calls Run on each of the informers and sets started to true. Blocks on the context. @@ -305,11 +334,11 @@ func (ip *Informers) addInformerToMap(gvk schema.GroupVersionKind, obj runtime.O } sharedIndexInformer := cache.NewSharedIndexInformer(&cache.ListWatch{ ListFunc: func(opts metav1.ListOptions) (runtime.Object, error) { - ip.selectorByGVK(gvk).ApplyToList(&opts) + ip.getSelector(gvk).ApplyToList(&opts) return listWatcher.ListFunc(opts) }, WatchFunc: func(opts metav1.ListOptions) (watch.Interface, error) { - ip.selectorByGVK(gvk).ApplyToList(&opts) + ip.getSelector(gvk).ApplyToList(&opts) opts.Watch = true // Watch needs to be set to true separately return listWatcher.WatchFunc(opts) }, @@ -318,7 +347,7 @@ func (ip *Informers) addInformerToMap(gvk schema.GroupVersionKind, obj runtime.O }) // Check to see if there is a transformer for this gvk - if err := sharedIndexInformer.SetTransform(ip.transformersByGVK.Get(gvk)); err != nil { + if err := sharedIndexInformer.SetTransform(ip.getTransform(gvk)); err != nil { return nil, false, err } @@ -334,7 +363,7 @@ func (ip *Informers) addInformerToMap(gvk schema.GroupVersionKind, obj runtime.O indexer: sharedIndexInformer.GetIndexer(), groupVersionKind: gvk, scopeName: mapping.Scope.Name(), - disableDeepCopy: ip.disableDeepCopyByGVK.IsDisabled(gvk), + disableDeepCopy: ip.getDisableDeepCopy(gvk), }, } ip.informersByType(obj)[gvk] = i @@ -358,7 +387,7 @@ func (ip *Informers) makeListWatcher(gvk schema.GroupVersionKind, obj runtime.Ob // Figure out if the GVK we're dealing with is global, or namespace scoped. var namespace string if mapping.Scope.Name() == meta.RESTScopeNameNamespace { - namespace = restrictNamespaceBySelector(ip.namespace, ip.selectorByGVK(gvk)) + namespace = restrictNamespaceBySelector(ip.namespace, ip.getSelector(gvk)) } switch obj.(type) { diff --git a/pkg/cache/internal/selector.go b/pkg/cache/internal/selector.go index 4eff32fb35..c674379b99 100644 --- a/pkg/cache/internal/selector.go +++ b/pkg/cache/internal/selector.go @@ -20,23 +20,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/runtime/schema" ) -// SelectorsByGVK associate a GroupVersionKind to a field/label selector. -type SelectorsByGVK map[schema.GroupVersionKind]Selector - -func (s SelectorsByGVK) forGVK(gvk schema.GroupVersionKind) Selector { - if specific, found := s[gvk]; found { - return specific - } - if defaultSelector, found := s[schema.GroupVersionKind{}]; found { - return defaultSelector - } - - return Selector{} -} - // Selector specify the label/field selector to fill in ListOptions. type Selector struct { Label labels.Selector diff --git a/pkg/cache/multi_namespace_cache.go b/pkg/cache/multi_namespace_cache.go index 1bd2a8c96b..ee33fb0dd5 100644 --- a/pkg/cache/multi_namespace_cache.go +++ b/pkg/cache/multi_namespace_cache.go @@ -47,13 +47,13 @@ const globalCache = "_cluster-scope" // Deprecated: Use cache.Options.View.Namespaces instead. func MultiNamespacedCacheBuilder(namespaces []string) NewCacheFunc { return func(config *rest.Config, opts Options) (Cache, error) { - opts.View.Namespaces = namespaces + opts.Namespaces = namespaces return newMultiNamespaceCache(config, opts) } } func newMultiNamespaceCache(config *rest.Config, opts Options) (Cache, error) { - if len(opts.View.Namespaces) < 2 { + if len(opts.Namespaces) < 2 { return nil, fmt.Errorf("must specify more than one namespace to use multi-namespace cache") } opts, err := defaultOpts(config, opts) @@ -63,8 +63,8 @@ func newMultiNamespaceCache(config *rest.Config, opts Options) (Cache, error) { // Create every namespace cache. caches := map[string]Cache{} - for _, ns := range opts.View.Namespaces { - opts.View.Namespaces = []string{ns} + for _, ns := range opts.Namespaces { + opts.Namespaces = []string{ns} c, err := New(config, opts) if err != nil { return nil, err @@ -73,7 +73,7 @@ func newMultiNamespaceCache(config *rest.Config, opts Options) (Cache, error) { } // Create a cache for cluster scoped resources. - opts.View.Namespaces = []string{} + opts.Namespaces = []string{} gCache, err := New(config, opts) if err != nil { return nil, fmt.Errorf("error creating global cache: %w", err) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 2bf61b7141..332c7a5e5d 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -176,9 +176,7 @@ func New(config *rest.Config, opts ...Option) (Cluster, error) { Scheme: options.Scheme, Mapper: mapper, ResyncEvery: options.SyncPeriod, - View: cache.ViewOptions{ - Namespaces: []string{options.Namespace}, - }, + Namespaces: []string{options.Namespace}, }) if err != nil { return nil, err diff --git a/pkg/manager/example_test.go b/pkg/manager/example_test.go index 12c421bb83..360008e72b 100644 --- a/pkg/manager/example_test.go +++ b/pkg/manager/example_test.go @@ -60,7 +60,7 @@ func ExampleNew_limitToNamespaces() { mgr, err := manager.New(cfg, manager.Options{ NewCache: func(config *rest.Config, opts cache.Options) (cache.Cache, error) { - opts.View.Namespaces = []string{"namespace1", "namespace2"} + opts.Namespaces = []string{"namespace1", "namespace2"} return cache.New(config, opts) }}, )