From b38c4a526b4cf9ffbcaebd0fe9363d9d64182dd2 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Tue, 9 Aug 2022 09:03:51 -0400 Subject: [PATCH] fix: cache.BuilderWithOptions inherit options from caller using cache.BuilderWithOptions does not properly inherit all options passed in from the caller: - scheme was overridden instead of merged - selectors were not inherited at all, even if specified options selectors remained unset - transforms were not inherited at all, even if specified options transforms remained unset - disable deep copy settings were not inherited at all, even if specified options for disabling deep copies remained unset This commit resolves this issues by implementing merge logic for all fields in the cache.Options struct: - Schemes are merged - RESTMapper is chosen by precedence only falling back to inherited options if left unset in specified options - Resync is chosen by precedence only falling back to inherited options if left unset in specified options - Namespace is chosen by precedence only falling back to inherited options if left unset in specified options - Selectors are merged. If both inherited and specified Options defined selectors for a given type, those selectors are merged via logical AND. - DisableDeepCopy is combined via precedence. Only if a value for a particular GVK is unset in the specified options will a value from the inherited options be used. - Transform functions are combined via chaining. If both inherited and specified options define a transform function, the transform function from the inherited options will be called first, and the transform function from the specified options will be called second. Signed-off-by: Joe Lanford --- pkg/cache/cache.go | 297 +++++++++++++++++-- pkg/cache/cache_test.go | 1 - pkg/cache/cache_unit_test.go | 454 +++++++++++++++++++++++++++++ pkg/cache/internal/transformers.go | 15 +- 4 files changed, 729 insertions(+), 38 deletions(-) create mode 100644 pkg/cache/cache_unit_test.go diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index 3ff41ffe63..965e4936e7 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -19,14 +19,18 @@ package cache import ( "context" "fmt" + "reflect" "time" "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" toolscache "k8s.io/client-go/tools/cache" + "sigs.k8s.io/controller-runtime/pkg/cache/internal" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" @@ -150,7 +154,7 @@ func New(config *rest.Config, opts Options) (Cache, error) { if err != nil { return nil, err } - selectorsByGVK, err := convertToSelectorsByGVK(opts.SelectorsByObject, opts.DefaultSelector, opts.Scheme) + selectorsByGVK, err := convertToByGVK(opts.SelectorsByObject, opts.DefaultSelector, opts.Scheme) if err != nil { return nil, err } @@ -158,16 +162,22 @@ func New(config *rest.Config, opts Options) (Cache, error) { if err != nil { return nil, err } - transformByGVK, err := convertToTransformByKindAndGVK(opts.TransformByObject, opts.DefaultTransform, opts.Scheme) + transformByGVK, err := convertToByGVK(opts.TransformByObject, opts.DefaultTransform, opts.Scheme) if err != nil { return nil, err } + transformByObj := internal.TransformFuncByObjectFromMap(transformByGVK) + + internalSelectorsByGVK := internal.SelectorsByGVK{} + for gvk, selector := range selectorsByGVK { + internalSelectorsByGVK[gvk] = internal.Selector(selector) + } - im := internal.NewInformersMap(config, opts.Scheme, opts.Mapper, *opts.Resync, opts.Namespace, selectorsByGVK, disableDeepCopyByGVK, transformByGVK) + im := internal.NewInformersMap(config, opts.Scheme, opts.Mapper, *opts.Resync, opts.Namespace, internalSelectorsByGVK, disableDeepCopyByGVK, transformByObj) return &informerCache{InformersMap: im}, nil } -// BuilderWithOptions returns a Cache constructor that will build the a cache +// 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 @@ -175,24 +185,211 @@ func New(config *rest.Config, opts Options) (Cache, error) { // WARNING: If 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, opts Options) (Cache, error) { - if options.Scheme == nil { - options.Scheme = opts.Scheme + return func(config *rest.Config, inherited Options) (Cache, error) { + var err error + inherited, err = defaultOpts(config, inherited) + if err != nil { + return nil, err } - if options.Mapper == nil { - options.Mapper = opts.Mapper + options, err = defaultOpts(config, options) + if err != nil { + return nil, err } - if options.Resync == nil { - options.Resync = opts.Resync + combined, err := options.inheritFrom(inherited) + if err != nil { + return nil, err } - if options.Namespace == "" { - options.Namespace = opts.Namespace + return New(config, *combined) + } +} + +func (options Options) inheritFrom(inherited Options) (*Options, error) { + var ( + combined Options + err error + ) + combined.Scheme = combineScheme(inherited.Scheme, options.Scheme) + combined.Mapper = selectMapper(inherited.Mapper, options.Mapper) + combined.Resync = selectResync(inherited.Resync, options.Resync) + combined.Namespace = selectNamespace(inherited.Namespace, options.Namespace) + combined.SelectorsByObject, combined.DefaultSelector, err = combineSelectors(inherited, options, combined.Scheme) + if err != nil { + return nil, err + } + combined.UnsafeDisableDeepCopyByObject, err = combineUnsafeDeepCopy(inherited, options, combined.Scheme) + if err != nil { + return nil, err + } + combined.TransformByObject, combined.DefaultTransform, err = combineTransforms(inherited, options, combined.Scheme) + if err != nil { + return nil, err + } + return &combined, nil +} + +func combineScheme(schemes ...*runtime.Scheme) *runtime.Scheme { + var out *runtime.Scheme + for _, sch := range schemes { + if sch == nil { + continue } - if opts.Resync == nil { - opts.Resync = options.Resync + for gvk, t := range sch.AllKnownTypes() { + if out == nil { + out = runtime.NewScheme() + } + out.AddKnownTypeWithName(gvk, reflect.New(t).Interface().(runtime.Object)) } + } + return out +} - return New(config, options) +func selectMapper(def, override meta.RESTMapper) meta.RESTMapper { + if override != nil { + return override + } + return def +} + +func selectResync(def, override *time.Duration) *time.Duration { + if override != nil { + return override + } + return def +} + +func selectNamespace(def, override string) string { + if override != "" { + return override + } + 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.SelectorsByObject, options.DefaultSelector, options.Scheme) + if err != nil { + return nil, ObjectSelector{}, err + } + inheritedSelectorsByGVK, err := convertToByGVK(inherited.SelectorsByObject, inherited.DefaultSelector, inherited.Scheme) + if err != nil { + return nil, ObjectSelector{}, err + } + + for gvk, inheritedSelector := range inheritedSelectorsByGVK { + optionsSelectorsByGVK[gvk] = combineSelector(inheritedSelector, optionsSelectorsByGVK[gvk]) + } + return convertToByObject(optionsSelectorsByGVK, scheme) +} + +func combineSelector(selectors ...ObjectSelector) ObjectSelector { + 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{ + Label: combineLabelSelectors(ls...), + Field: combineFieldSelectors(fs...), + } +} + +func combineLabelSelectors(ls ...labels.Selector) labels.Selector { + var combined labels.Selector + for _, l := range ls { + if l == nil { + continue + } + if combined == nil { + combined = labels.NewSelector() + } + reqs, _ := l.Requirements() + combined = combined.Add(reqs...) + } + return combined +} + +func combineFieldSelectors(fs ...fields.Selector) fields.Selector { + nonNil := fs[:0] + for _, f := range fs { + if f == nil { + continue + } + nonNil = append(nonNil, f) + } + if len(nonNil) == 0 { + return nil + } + if len(nonNil) == 1 { + return nonNil[0] + } + 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.UnsafeDisableDeepCopyByObject, options.Scheme) + if err != nil { + return nil, err + } + inheritedDisableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(inherited.UnsafeDisableDeepCopyByObject, 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.TransformByObject, options.DefaultTransform, options.Scheme) + if err != nil { + return nil, nil, err + } + inheritedTransformByGVK, err := convertToByGVK(inherited.TransformByObject, inherited.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 + } + if current == nil { + return inherited + } + return func(in interface{}) (interface{}, error) { + mid, err := inherited(in) + if err != nil { + return nil, err + } + return current(mid) } } @@ -219,17 +416,40 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) { return opts, nil } -func convertToSelectorsByGVK(selectorsByObject SelectorsByObject, defaultSelector ObjectSelector, scheme *runtime.Scheme) (internal.SelectorsByGVK, error) { - selectorsByGVK := internal.SelectorsByGVK{} - for object, selector := range selectorsByObject { +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 { gvk, err := apiutil.GVKForObject(object, scheme) if err != nil { return nil, err } - selectorsByGVK[gvk] = internal.Selector(selector) + 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 + } + 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 } - selectorsByGVK[schema.GroupVersionKind{}] = internal.Selector(defaultSelector) - return selectorsByGVK, nil + return byObject, def, nil } // DisableDeepCopyByObject associate a client.Object's GVK to disable DeepCopy during get or list from cache. @@ -259,17 +479,30 @@ func convertToDisableDeepCopyByGVK(disableDeepCopyByObject DisableDeepCopyByObje return disableDeepCopyByGVK, 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 - -func convertToTransformByKindAndGVK(t TransformByObject, defaultTransform toolscache.TransformFunc, scheme *runtime.Scheme) (internal.TransformFuncByObject, error) { - result := internal.NewTransformFuncByObject() - for obj, transformation := range t { - if err := result.Set(obj, scheme, transformation); err != nil { +func convertToDisableDeepCopyByObject(byGVK internal.DisableDeepCopyByGVK, scheme *runtime.Scheme) (DisableDeepCopyByObject, error) { + var byObject DisableDeepCopyByObject + for gvk, value := range byGVK { + if byObject == nil { + byObject = DisableDeepCopyByObject{} + } + if gvk == (schema.GroupVersionKind{}) { + byObject[ObjectAll{}] = value + continue + } + obj, err := scheme.New(gvk) + if err != nil { return nil, err } + cObj, ok := obj.(client.Object) + if !ok { + return nil, fmt.Errorf("object %T for GVK %q does not implement client.Object", obj, gvk) + } + + byObject[cObj] = value } - result.SetDefault(defaultTransform) - return result, nil + return byObject, 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 7f49cd85ec..e89e3a72de 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -25,7 +25,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" diff --git a/pkg/cache/cache_unit_test.go b/pkg/cache/cache_unit_test.go new file mode 100644 index 0000000000..a675cc341c --- /dev/null +++ b/pkg/cache/cache_unit_test.go @@ -0,0 +1,454 @@ +package cache + +import ( + "reflect" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" + "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" +) + +var _ = Describe("cache.inheritFrom", func() { + defer GinkgoRecover() + + var ( + inherited Options + specified Options + gv schema.GroupVersion + coreScheme *runtime.Scheme + ) + + BeforeEach(func() { + inherited = Options{} + specified = Options{} + gv = schema.GroupVersion{ + Group: "example.com", + Version: "v1alpha1", + } + coreScheme = runtime.NewScheme() + Expect(scheme.AddToScheme(coreScheme)).To(Succeed()) + }) + + Context("Scheme", func() { + It("is nil when specified and inherited are unset", func() { + Expect(checkError(specified.inheritFrom(inherited)).Scheme).To(BeNil()) + }) + It("is specified when only specified is set", func() { + specified.Scheme = runtime.NewScheme() + specified.Scheme.AddKnownTypes(gv, &unstructured.Unstructured{}) + Expect(specified.Scheme.KnownTypes(gv)).To(HaveLen(1)) + + Expect(checkError(specified.inheritFrom(inherited)).Scheme.KnownTypes(gv)).To(HaveLen(1)) + }) + It("is inherited when only inherited is set", func() { + inherited.Scheme = runtime.NewScheme() + inherited.Scheme.AddKnownTypes(gv, &unstructured.Unstructured{}) + Expect(inherited.Scheme.KnownTypes(gv)).To(HaveLen(1)) + + combined := checkError(specified.inheritFrom(inherited)) + Expect(combined.Scheme).NotTo(BeNil()) + Expect(combined.Scheme.KnownTypes(gv)).To(HaveLen(1)) + }) + It("is combined when both inherited and specified are set", func() { + specified.Scheme = runtime.NewScheme() + specified.Scheme.AddKnownTypes(gv, &unstructured.Unstructured{}) + Expect(specified.Scheme.AllKnownTypes()).To(HaveLen(1)) + + inherited.Scheme = runtime.NewScheme() + inherited.Scheme.AddKnownTypes(schema.GroupVersion{Group: "example.com", Version: "v1"}, &unstructured.Unstructured{}) + Expect(inherited.Scheme.AllKnownTypes()).To(HaveLen(1)) + + Expect(checkError(specified.inheritFrom(inherited)).Scheme.AllKnownTypes()).To(HaveLen(2)) + }) + }) + Context("Mapper", func() { + It("is nil when specified and inherited are unset", func() { + Expect(checkError(specified.inheritFrom(inherited)).Mapper).To(BeNil()) + }) + It("is unchanged when only specified is set", func() { + specified.Mapper = meta.NewDefaultRESTMapper(nil) + Expect(checkError(specified.inheritFrom(inherited)).Mapper).To(Equal(specified.Mapper)) + }) + It("is inherited when only inherited is set", func() { + inherited.Mapper = meta.NewDefaultRESTMapper(nil) + Expect(checkError(specified.inheritFrom(inherited)).Mapper).To(Equal(inherited.Mapper)) + }) + It("is unchanged when both inherited and specified are set", func() { + specified.Mapper = meta.NewDefaultRESTMapper(nil) + inherited.Mapper = meta.NewDefaultRESTMapper([]schema.GroupVersion{gv}) + Expect(checkError(specified.inheritFrom(inherited)).Mapper).To(Equal(specified.Mapper)) + }) + }) + Context("Resync", func() { + It("is nil when specified and inherited are unset", func() { + Expect(checkError(specified.inheritFrom(inherited)).Resync).To(BeNil()) + }) + It("is unchanged when only specified is set", func() { + specified.Resync = pointer.Duration(time.Second) + Expect(checkError(specified.inheritFrom(inherited)).Resync).To(Equal(specified.Resync)) + }) + It("is inherited when only inherited is set", func() { + inherited.Resync = pointer.Duration(time.Second) + Expect(checkError(specified.inheritFrom(inherited)).Resync).To(Equal(inherited.Resync)) + }) + It("is unchanged when both inherited and specified are set", func() { + specified.Resync = pointer.Duration(time.Second) + inherited.Resync = pointer.Duration(time.Minute) + Expect(checkError(specified.inheritFrom(inherited)).Resync).To(Equal(specified.Resync)) + }) + }) + Context("Namespace", func() { + It("is NamespaceAll when specified and inherited are unset", func() { + Expect(checkError(specified.inheritFrom(inherited)).Namespace).To(Equal(corev1.NamespaceAll)) + }) + It("is unchanged when only specified is set", func() { + specified.Namespace = "specified" + Expect(checkError(specified.inheritFrom(inherited)).Namespace).To(Equal(specified.Namespace)) + }) + It("is inherited when only inherited is set", func() { + inherited.Namespace = "inherited" + Expect(checkError(specified.inheritFrom(inherited)).Namespace).To(Equal(inherited.Namespace)) + }) + It("in unchanged when both inherited and specified are set", func() { + specified.Namespace = "specified" + inherited.Namespace = "inherited" + Expect(checkError(specified.inheritFrom(inherited)).Namespace).To(Equal(specified.Namespace)) + }) + }) + Context("SelectorsByObject", func() { + It("is unchanged when specified and inherited are unset", func() { + Expect(checkError(specified.inheritFrom(inherited)).SelectorsByObject).To(BeNil()) + }) + It("is unchanged when only specified is set", func() { + specified.Scheme = coreScheme + specified.SelectorsByObject = map[client.Object]ObjectSelector{&corev1.Pod{}: {}} + Expect(checkError(specified.inheritFrom(inherited)).SelectorsByObject).To(HaveLen(1)) + }) + It("is inherited when only inherited is set", func() { + inherited.Scheme = coreScheme + inherited.SelectorsByObject = map[client.Object]ObjectSelector{&corev1.ConfigMap{}: {}} + Expect(checkError(specified.inheritFrom(inherited)).SelectorsByObject).To(HaveLen(1)) + }) + It("is combined when both inherited and specified are set", func() { + specified.Scheme = coreScheme + inherited.Scheme = coreScheme + specified.SelectorsByObject = map[client.Object]ObjectSelector{&corev1.Pod{}: {}} + inherited.SelectorsByObject = map[client.Object]ObjectSelector{&corev1.ConfigMap{}: {}} + Expect(checkError(specified.inheritFrom(inherited)).SelectorsByObject).To(HaveLen(2)) + }) + It("combines selectors if specified and inherited specify selectors for the same object", func() { + specified.Scheme = coreScheme + inherited.Scheme = coreScheme + specified.SelectorsByObject = map[client.Object]ObjectSelector{&corev1.Pod{}: { + Label: labels.Set{"specified": "true"}.AsSelector(), + Field: fields.Set{"metadata.name": "specified"}.AsSelector(), + }} + inherited.SelectorsByObject = map[client.Object]ObjectSelector{&corev1.Pod{}: { + Label: labels.Set{"inherited": "true"}.AsSelector(), + Field: fields.Set{"metadata.namespace": "inherited"}.AsSelector(), + }} + combined := checkError(specified.inheritFrom(inherited)).SelectorsByObject + Expect(combined).To(HaveLen(1)) + var ( + obj client.Object + selector ObjectSelector + ) + for obj, selector = 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(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()) + }) + }) + Context("DefaultSelector", func() { + It("is unchanged when specified and inherited are unset", func() { + Expect(specified.DefaultSelector).To(Equal(ObjectSelector{})) + Expect(inherited.DefaultSelector).To(Equal(ObjectSelector{})) + Expect(checkError(specified.inheritFrom(inherited)).DefaultSelector).To(Equal(ObjectSelector{})) + }) + It("is unchanged when only specified is set", func() { + specified.DefaultSelector = ObjectSelector{Label: labels.Set{"specified": "true"}.AsSelector()} + Expect(checkError(specified.inheritFrom(inherited)).DefaultSelector).To(Equal(specified.DefaultSelector)) + }) + It("is inherited when only inherited is set", func() { + inherited.DefaultSelector = ObjectSelector{Label: labels.Set{"inherited": "true"}.AsSelector()} + Expect(checkError(specified.inheritFrom(inherited)).DefaultSelector).To(Equal(inherited.DefaultSelector)) + }) + It("is combined when both inherited and specified are set", func() { + specified.DefaultSelector = ObjectSelector{ + Label: labels.Set{"specified": "true"}.AsSelector(), + Field: fields.Set{"metadata.name": "specified"}.AsSelector(), + } + inherited.DefaultSelector = ObjectSelector{ + Label: labels.Set{"inherited": "true"}.AsSelector(), + Field: fields.Set{"metadata.namespace": "inherited"}.AsSelector(), + } + combined := checkError(specified.inheritFrom(inherited)).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)).UnsafeDisableDeepCopyByObject).To(BeNil()) + }) + It("is unchanged when only specified is set", func() { + specified.Scheme = coreScheme + specified.UnsafeDisableDeepCopyByObject = map[client.Object]bool{ObjectAll{}: true} + Expect(checkError(specified.inheritFrom(inherited)).UnsafeDisableDeepCopyByObject).To(HaveLen(1)) + }) + It("is inherited when only inherited is set", func() { + inherited.Scheme = coreScheme + inherited.UnsafeDisableDeepCopyByObject = map[client.Object]bool{ObjectAll{}: true} + Expect(checkError(specified.inheritFrom(inherited)).UnsafeDisableDeepCopyByObject).To(HaveLen(1)) + }) + It("is combined when both inherited and specified are set for different keys", func() { + specified.Scheme = coreScheme + inherited.Scheme = coreScheme + specified.UnsafeDisableDeepCopyByObject = map[client.Object]bool{&corev1.Pod{}: true} + inherited.UnsafeDisableDeepCopyByObject = map[client.Object]bool{&corev1.ConfigMap{}: true} + Expect(checkError(specified.inheritFrom(inherited)).UnsafeDisableDeepCopyByObject).To(HaveLen(2)) + }) + It("is true when inherited=false and specified=true for the same key", func() { + specified.Scheme = coreScheme + inherited.Scheme = coreScheme + specified.UnsafeDisableDeepCopyByObject = map[client.Object]bool{&corev1.Pod{}: true} + inherited.UnsafeDisableDeepCopyByObject = map[client.Object]bool{&corev1.Pod{}: false} + combined := checkError(specified.inheritFrom(inherited)).UnsafeDisableDeepCopyByObject + Expect(combined).To(HaveLen(1)) + + var ( + obj client.Object + disableDeepCopy bool + ) + for obj, disableDeepCopy = range combined { + } + Expect(obj).To(BeAssignableToTypeOf(&corev1.Pod{})) + Expect(disableDeepCopy).To(BeTrue()) + }) + It("is false when inherited=true and specified=false for the same key", func() { + specified.Scheme = coreScheme + inherited.Scheme = coreScheme + specified.UnsafeDisableDeepCopyByObject = map[client.Object]bool{&corev1.Pod{}: false} + inherited.UnsafeDisableDeepCopyByObject = map[client.Object]bool{&corev1.Pod{}: true} + combined := checkError(specified.inheritFrom(inherited)).UnsafeDisableDeepCopyByObject + Expect(combined).To(HaveLen(1)) + + var ( + obj client.Object + disableDeepCopy bool + ) + for obj, disableDeepCopy = range combined { + } + Expect(obj).To(BeAssignableToTypeOf(&corev1.Pod{})) + Expect(disableDeepCopy).To(BeFalse()) + }) + }) + Context("TransformByObject", func() { + type transformed struct { + podSpecified bool + podInherited bool + configmapSpecified bool + configmapInherited bool + } + var tx transformed + BeforeEach(func() { + tx = transformed{} + }) + It("is unchanged when specified and inherited are unset", func() { + Expect(checkError(specified.inheritFrom(inherited)).TransformByObject).To(BeNil()) + }) + It("is unchanged when only specified is set", func() { + specified.Scheme = coreScheme + specified.TransformByObject = 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)).TransformByObject + Expect(combined).To(HaveLen(1)) + for obj, fn := range combined { + Expect(obj).To(BeAssignableToTypeOf(&corev1.Pod{})) + out, _ := fn(tx) + Expect(out).To(And( + BeAssignableToTypeOf(tx), + WithTransform(func(i transformed) bool { return i.podSpecified }, BeTrue()), + WithTransform(func(i transformed) bool { return i.podInherited }, BeFalse()), + )) + } + }) + It("is inherited when only inherited is set", func() { + inherited.Scheme = coreScheme + inherited.TransformByObject = 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)).TransformByObject + Expect(combined).To(HaveLen(1)) + for obj, fn := range combined { + Expect(obj).To(BeAssignableToTypeOf(&corev1.Pod{})) + out, _ := fn(tx) + Expect(out).To(And( + BeAssignableToTypeOf(tx), + WithTransform(func(i transformed) bool { return i.podSpecified }, BeFalse()), + WithTransform(func(i transformed) bool { return i.podInherited }, BeTrue()), + )) + } + }) + It("is combined when both inherited and specified are set for different keys", func() { + specified.Scheme = coreScheme + inherited.Scheme = coreScheme + specified.TransformByObject = map[client.Object]cache.TransformFunc{&corev1.Pod{}: func(i interface{}) (interface{}, error) { + ti := i.(transformed) + ti.podSpecified = true + return ti, nil + }} + inherited.TransformByObject = 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)).TransformByObject + Expect(combined).To(HaveLen(2)) + for obj, fn := range combined { + out, _ := fn(tx) + if reflect.TypeOf(obj) == reflect.TypeOf(&corev1.Pod{}) { + Expect(out).To(And( + BeAssignableToTypeOf(tx), + WithTransform(func(i transformed) bool { return i.podSpecified }, BeTrue()), + WithTransform(func(i transformed) bool { return i.podInherited }, BeFalse()), + WithTransform(func(i transformed) bool { return i.configmapSpecified }, BeFalse()), + WithTransform(func(i transformed) bool { return i.configmapInherited }, BeFalse()), + )) + } + if reflect.TypeOf(obj) == reflect.TypeOf(&corev1.ConfigMap{}) { + Expect(out).To(And( + BeAssignableToTypeOf(tx), + WithTransform(func(i transformed) bool { return i.podSpecified }, BeFalse()), + WithTransform(func(i transformed) bool { return i.podInherited }, BeFalse()), + WithTransform(func(i transformed) bool { return i.configmapSpecified }, BeFalse()), + WithTransform(func(i transformed) bool { return i.configmapInherited }, BeTrue()), + )) + } + } + }) + 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.TransformByObject = map[client.Object]cache.TransformFunc{&corev1.Pod{}: func(i interface{}) (interface{}, error) { + ti := i.(transformed) + ti.podSpecified = true + return ti, nil + }} + inherited.TransformByObject = 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)).TransformByObject + Expect(combined).To(HaveLen(1)) + for obj, fn := range combined { + Expect(obj).To(BeAssignableToTypeOf(&corev1.Pod{})) + out, _ := fn(tx) + Expect(out).To(And( + BeAssignableToTypeOf(tx), + WithTransform(func(i transformed) bool { return i.podSpecified }, BeTrue()), + WithTransform(func(i transformed) bool { return i.podInherited }, BeTrue()), + WithTransform(func(i transformed) bool { return i.configmapSpecified }, BeFalse()), + WithTransform(func(i transformed) bool { return i.configmapInherited }, BeFalse()), + )) + } + }) + }) + Context("DefaultTransform", func() { + type transformed struct { + specified bool + inherited bool + } + var tx transformed + BeforeEach(func() { + tx = transformed{} + }) + It("is unchanged when specified and inherited are unset", func() { + Expect(checkError(specified.inheritFrom(inherited)).DefaultTransform).To(BeNil()) + }) + It("is unchanged when only specified is set", func() { + specified.DefaultTransform = func(i interface{}) (interface{}, error) { + ti := i.(transformed) + ti.specified = true + return ti, nil + } + combined := checkError(specified.inheritFrom(inherited)).DefaultTransform + out, _ := combined(tx) + Expect(out).To(And( + BeAssignableToTypeOf(tx), + WithTransform(func(i transformed) bool { return i.specified }, BeTrue()), + WithTransform(func(i transformed) bool { return i.inherited }, BeFalse()), + )) + }) + It("is inherited when only inherited is set", func() { + inherited.DefaultTransform = func(i interface{}) (interface{}, error) { + ti := i.(transformed) + ti.inherited = true + return ti, nil + } + combined := checkError(specified.inheritFrom(inherited)).DefaultTransform + out, _ := combined(tx) + Expect(out).To(And( + BeAssignableToTypeOf(tx), + WithTransform(func(i transformed) bool { return i.specified }, BeFalse()), + WithTransform(func(i transformed) bool { return i.inherited }, BeTrue()), + )) + }) + It("is combined when the transform function is defined in both inherited and specified", func() { + specified.DefaultTransform = func(i interface{}) (interface{}, error) { + ti := i.(transformed) + ti.specified = true + return ti, nil + } + inherited.DefaultTransform = func(i interface{}) (interface{}, error) { + ti := i.(transformed) + ti.inherited = true + return ti, nil + } + combined := checkError(specified.inheritFrom(inherited)).DefaultTransform + Expect(combined).NotTo(BeNil()) + out, _ := combined(tx) + Expect(out).To(And( + BeAssignableToTypeOf(tx), + WithTransform(func(i transformed) bool { return i.specified }, BeTrue()), + WithTransform(func(i transformed) bool { return i.inherited }, BeTrue()), + )) + }) + }) +}) + +func checkError[T any](v T, err error) T { + Expect(err).To(BeNil()) + return v +} diff --git a/pkg/cache/internal/transformers.go b/pkg/cache/internal/transformers.go index 8cf642c4bd..f69e02262a 100644 --- a/pkg/cache/internal/transformers.go +++ b/pkg/cache/internal/transformers.go @@ -4,6 +4,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/tools/cache" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" ) @@ -20,12 +21,16 @@ type transformFuncByGVK struct { transformers map[schema.GroupVersionKind]cache.TransformFunc } -// NewTransformFuncByObject creates a new TransformFuncByObject instance. -func NewTransformFuncByObject() TransformFuncByObject { - return &transformFuncByGVK{ - transformers: make(map[schema.GroupVersionKind]cache.TransformFunc), - defaultTransform: nil, +// TransformFuncByObjectFromMap creates a TransformFuncByObject from a map that +// maps GVKs to TransformFuncs. +func TransformFuncByObjectFromMap(in map[schema.GroupVersionKind]cache.TransformFunc) TransformFuncByObject { + byGVK := &transformFuncByGVK{} + if defaultFunc, hasDefault := in[schema.GroupVersionKind{}]; hasDefault { + byGVK.defaultTransform = defaultFunc } + delete(in, schema.GroupVersionKind{}) + byGVK.transformers = in + return byGVK } func (t *transformFuncByGVK) SetDefault(transformer cache.TransformFunc) {