diff --git a/pkg/reftracker/ref_tracker.go b/pkg/reftracker/ref_tracker.go index 26a2c0101..0861ff487 100644 --- a/pkg/reftracker/ref_tracker.go +++ b/pkg/reftracker/ref_tracker.go @@ -28,18 +28,27 @@ func (a *AppRefTracker) AppsForRef(refKey RefKey) (map[RefKey]struct{}, error) { return nil, fmt.Errorf("could not find ref %s", refKey.Description()) } - return apps, nil + return cloneRefKeySet(apps), nil } func (a *AppRefTracker) RefsForApp(appKey RefKey) (map[RefKey]struct{}, error) { a.lock.Lock() defer a.lock.Unlock() - if a.appsToRefs[appKey] == nil { + refs := a.appsToRefs[appKey] + if refs == nil { return nil, fmt.Errorf("could not find refs for App %s", appKey.RefName()) } - return a.appsToRefs[appKey], nil + return cloneRefKeySet(refs), nil +} + +func cloneRefKeySet(in map[RefKey]struct{}) map[RefKey]struct{} { + out := make(map[RefKey]struct{}, len(in)) + for k := range in { + out[k] = struct{}{} + } + return out } func (a *AppRefTracker) RemoveRef(refKey RefKey) { diff --git a/pkg/reftracker/ref_tracker_test.go b/pkg/reftracker/ref_tracker_test.go index 9ff9c9b6b..fadfded65 100644 --- a/pkg/reftracker/ref_tracker_test.go +++ b/pkg/reftracker/ref_tracker_test.go @@ -59,3 +59,55 @@ func Test_RemoveAppFromAllRefs_RemovesApp(t *testing.T) { t.Fatalf("expected app to be removed from appRefTracker after deletion") } } + +func Test_AppsForRef_ReturnsSnapshot(t *testing.T) { + // Mutating the map returned by AppsForRef must not affect the tracker's + // internal state (regression test for concurrent-map panic fix). + appRefTracker := reftracker.NewAppRefTracker() + + refKey := reftracker.NewSecretKey("secretName", "default") + appKey := reftracker.NewAppKey("app", "default") + appRefTracker.ReconcileRefs(map[reftracker.RefKey]struct{}{refKey: {}}, appKey) + + apps, err := appRefTracker.AppsForRef(refKey) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Mutate the returned map. + delete(apps, appKey) + + // The tracker should still know about the app. + apps2, err := appRefTracker.AppsForRef(refKey) + if err != nil { + t.Fatalf("unexpected error on second call: %v", err) + } + if _, ok := apps2[appKey]; !ok { + t.Fatalf("mutating the returned snapshot corrupted internal tracker state") + } +} + +func Test_RefsForApp_ReturnsSnapshot(t *testing.T) { + appRefTracker := reftracker.NewAppRefTracker() + + refKey := reftracker.NewSecretKey("secretName", "default") + appKey := reftracker.NewAppKey("app", "default") + appRefTracker.ReconcileRefs(map[reftracker.RefKey]struct{}{refKey: {}}, appKey) + + refs, err := appRefTracker.RefsForApp(appKey) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Mutate the returned map. + delete(refs, refKey) + + // The tracker should still know about the ref. + refs2, err := appRefTracker.RefsForApp(appKey) + if err != nil { + t.Fatalf("unexpected error on second call: %v", err) + } + if _, ok := refs2[refKey]; !ok { + t.Fatalf("mutating the returned snapshot corrupted internal tracker state") + } +}