Skip to content

Commit

Permalink
cache: clone maps to prevent data race when concurrently creating cac…
Browse files Browse the repository at this point in the history
…hes using the same options
  • Loading branch information
chrischdi committed Jan 20, 2025
1 parent 2aa9459 commit a1adc6b
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
5 changes: 4 additions & 1 deletion pkg/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,8 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) {
}
}

opts.ByObject = maps.Clone(opts.ByObject)
opts.DefaultNamespaces = maps.Clone(opts.DefaultNamespaces)
for obj, byObject := range opts.ByObject {
isNamespaced, err := apiutil.IsObjectNamespaced(obj, opts.Scheme, opts.Mapper)
if err != nil {
Expand All @@ -480,14 +482,15 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) {

if isNamespaced && byObject.Namespaces == nil {
byObject.Namespaces = maps.Clone(opts.DefaultNamespaces)
} else {
byObject.Namespaces = maps.Clone(byObject.Namespaces)
}

// Default the namespace-level configs first, because they need to use the undefaulted type-level config
// to be able to potentially fall through to settings from DefaultNamespaces.
for namespace, config := range byObject.Namespaces {
// 1. Default from the undefaulted type-level config
config = defaultConfig(config, byObjectToConfig(byObject))

// 2. Default from the namespace-level config. This was defaulted from the global default config earlier, but
// might not have an entry for the current namespace.
if defaultNamespaceSettings, hasDefaultNamespace := opts.DefaultNamespaces[namespace]; hasDefaultNamespace {
Expand Down
29 changes: 29 additions & 0 deletions pkg/cache/defaulting_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package cache

import (
"reflect"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -432,6 +433,34 @@ func TestDefaultOpts(t *testing.T) {
}
}

func TestDefaultOptsRace(t *testing.T) {
opts := Options{
Mapper: &fakeRESTMapper{},
ByObject: map[client.Object]ByObject{
&corev1.Pod{}: {
Label: labels.SelectorFromSet(map[string]string{"from": "pod"}),
Namespaces: map[string]Config{"default": {
LabelSelector: labels.SelectorFromSet(map[string]string{"from": "pod"}),
}},
},
},
DefaultNamespaces: map[string]Config{"default": {}},
}

// Start go routines which re-use the above options struct.
wg := sync.WaitGroup{}
for range 2 {
wg.Add(1)
go func() {
_, _ = defaultOpts(&rest.Config{}, opts)
wg.Done()
}()
}

// Wait for the go routines to finish.
wg.Wait()
}

type fakeRESTMapper struct {
meta.RESTMapper
}
Expand Down

0 comments on commit a1adc6b

Please sign in to comment.