Skip to content

Commit

Permalink
bug: Cache: Fix label defaulting of byObject when namespaces are conf…
Browse files Browse the repository at this point in the history
…igured

Currently, if there are global namespaces configured and no per-object
namesapces, while there is both a global and a per-object labelSelector,
we will:
1. Default the namespaces labelSelector from `DefaultLabelSelector`
2. Copy the namespaces including config into `byObject.Namespaces`

And thus end up with the global labelSelector overriding the per-object
one, this change fixes that by swapping step 1 and 2.
  • Loading branch information
alvaroaleman authored and k8s-infra-cherrypick-robot committed May 1, 2024
1 parent ed81fa6 commit 87cae4c
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 13 deletions.
29 changes: 16 additions & 13 deletions pkg/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions pkg/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down

0 comments on commit 87cae4c

Please sign in to comment.