Merge pull request #2808 from k8s-infra-cherrypick-robot/cherry-pick-…

…2805-to-release-0.18 [release-0.18] 🐛 Cache: Fix label defaulting of byObject when namespaces are configured
kubernetes-sigs · May 1, 2024 · d6feaae · d6feaae
2 parents ed81fa6 + 87cae4c
commit d6feaae
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 13 deletions.
diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go
@@ -434,19 +434,6 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) {
 		}
 	}
 
-	for namespace, cfg := range opts.DefaultNamespaces {
-		cfg = defaultConfig(cfg, optionDefaultsToConfig(&opts))
-		if namespace == metav1.NamespaceAll {
-			cfg.FieldSelector = fields.AndSelectors(
-				appendIfNotNil(
-					namespaceAllSelector(maps.Keys(opts.DefaultNamespaces)),
-					cfg.FieldSelector,
-				)...,
-			)
-		}
-		opts.DefaultNamespaces[namespace] = cfg
-	}
-
 	for obj, byObject := range opts.ByObject {
 		isNamespaced, err := apiutil.IsObjectNamespaced(obj, opts.Scheme, opts.Mapper)
 		if err != nil {
@@ -500,6 +487,22 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) {
 		opts.ByObject[obj] = byObject
 	}
 
+	// Default namespaces after byObject has been defaulted, otherwise a namespace without selectors
+	// will get the `Default` selectors, then get copied to byObject and then not get defaulted from
+	// byObject, as it already has selectors.
+	for namespace, cfg := range opts.DefaultNamespaces {
+		cfg = defaultConfig(cfg, optionDefaultsToConfig(&opts))
+		if namespace == metav1.NamespaceAll {
+			cfg.FieldSelector = fields.AndSelectors(
+				appendIfNotNil(
+					namespaceAllSelector(maps.Keys(opts.DefaultNamespaces)),
+					cfg.FieldSelector,
+				)...,
+			)
+		}
+		opts.DefaultNamespaces[namespace] = cfg
+	}
+
 	// Default the resync period to 10 hours if unset
 	if opts.SyncPeriod == nil {
 		opts.SyncPeriod = &defaultSyncPeriod

diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go
@@ -1630,6 +1630,26 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
 					}},
 					expectedPods: []string{"test-pod-4"},
 				}),
+				Entry("namespaces configured, type-level label selector matches everything, overrides global selector", selectorsTestCase{
+					options: cache.Options{
+						DefaultNamespaces: map[string]cache.Config{testNamespaceOne: {}},
+						ByObject: map[client.Object]cache.ByObject{
+							&corev1.Pod{}: {Label: labels.Everything()},
+						},
+						DefaultLabelSelector: labels.SelectorFromSet(map[string]string{"does-not": "match-anything"}),
+					},
+					expectedPods: []string{"test-pod-1", "test-pod-5"},
+				}),
+				Entry("namespaces configured, global selector is used", selectorsTestCase{
+					options: cache.Options{
+						DefaultNamespaces: map[string]cache.Config{testNamespaceTwo: {}},
+						ByObject: map[client.Object]cache.ByObject{
+							&corev1.Pod{}: {},
+						},
+						DefaultLabelSelector: labels.SelectorFromSet(map[string]string{"common-label": "common"}),
+					},
+					expectedPods: []string{"test-pod-3"},
+				}),
 				Entry("global label selector matches one pod", selectorsTestCase{
 					options: cache.Options{
 						DefaultLabelSelector: labels.SelectorFromSet(map[string]string{