⚠️ Remove cache.BuilderWithOptions, and inheritance funcs

The function was previously created and used to override the Manager's NewCache function. Now that the Manager embeds cache options we can remove this function and all of the inheritance logic which is highly error prone. Signed-off-by: Vince Prignano <vincepri@redhat.com>
kubernetes-sigs · May 4, 2023 · 54e84fc · 54e84fc
1 parent 57674c8
commit 54e84fc
Show file tree

Hide file tree

Showing 3 changed files with 6 additions and 775 deletions.
diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go
@@ -20,7 +20,6 @@ import (
 	"context"
 	"fmt"
 	"net/http"
-	"reflect"
 	"time"
 
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -233,183 +232,6 @@ func New(config *rest.Config, opts Options) (Cache, error) {
 	}, nil
 }
 
-// BuilderWithOptions returns a Cache constructor that will build a cache
-// honoring the options argument, this is useful to specify options like
-// ByObjects, DefaultSelector, DefaultTransform, etc.
-// WARNING: If ByObject selectors are specified, filtered out resources are not
-// returned.
-// 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) {
-		var err error
-		inherited, err = defaultOpts(config, inherited)
-		if err != nil {
-			return nil, err
-		}
-		options, err = defaultOpts(config, options)
-		if err != nil {
-			return nil, err
-		}
-		combined, err := options.inheritFrom(inherited)
-		if err != nil {
-			return nil, err
-		}
-		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.SyncPeriod = selectResync(inherited.SyncPeriod, options.SyncPeriod)
-	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
-	}
-	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
-		}
-		for gvk, t := range sch.AllKnownTypes() {
-			if out == nil {
-				out = runtime.NewScheme()
-			}
-			out.AddKnownTypeWithName(gvk, reflect.New(t).Interface().(runtime.Object))
-		}
-	}
-	return out
-}
-
-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 selectNamespaces(def, override []string) []string {
-	if len(override) > 0 {
-		return override
-	}
-	return def
-}
-
-func combineByObject(inherited, options Options, scheme *runtime.Scheme) (map[client.Object]ByObject, error) {
-	optionsByGVK, err := convertToInformerOptsByGVK(options.ByObject, scheme)
-	if err != nil {
-		return nil, err
-	}
-	inheritedByGVK, err := convertToInformerOptsByGVK(inherited.ByObject, scheme)
-	if err != nil {
-		return nil, err
-	}
-	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(optionsByGVK, scheme)
-}
-
-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 internal.Selector{
-		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 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)
-	}
-}
-
 func defaultOpts(config *rest.Config, opts Options) (Options, error) {
 	logger := log.WithName("setup")
 
@@ -466,28 +288,3 @@ func convertToInformerOptsByGVK(in map[client.Object]ByObject, scheme *runtime.S
 	}
 	return out, nil
 }
-
-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{}) {
-			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)
-		}
-		cObj.GetObjectKind().SetGroupVersionKind(gvk)
-		out[cObj] = ByObject{
-			Field:                 opts.Selector.Field,
-			Label:                 opts.Selector.Label,
-			Transform:             opts.Transform,
-			UnsafeDisableDeepCopy: opts.UnsafeDisableDeepCopy,
-		}
-	}
-	return out, nil
-}
diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go
@@ -1223,17 +1223,14 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
 			}
 			DescribeTable(" and cache with selectors", func(tc selectorsTestCase) {
 				By("creating the cache")
-				builder := cache.BuilderWithOptions(
-					cache.Options{
-						ByObject: map[client.Object]cache.ByObject{
-							&corev1.Pod{}: {
-								Label: labels.Set(tc.labelSelectors).AsSelector(),
-								Field: fields.Set(tc.fieldSelectors).AsSelector(),
-							},
+				informer, err := cache.New(cfg, cache.Options{
+					ByObject: map[client.Object]cache.ByObject{
+						&corev1.Pod{}: {
+							Label: labels.Set(tc.labelSelectors).AsSelector(),
+							Field: fields.Set(tc.fieldSelectors).AsSelector(),
 						},
 					},
-				)
-				informer, err := builder(cfg, cache.Options{})
+				})
 				Expect(err).NotTo(HaveOccurred())
 
 				By("running the cache and waiting for it to sync")