diff --git a/lib/services/matchers.go b/lib/services/matchers.go index ad1d5ed3932..50ad74423cb 100644 --- a/lib/services/matchers.go +++ b/lib/services/matchers.go @@ -30,6 +30,7 @@ import ( "github.com/gravitational/teleport/api/types" apiutils "github.com/gravitational/teleport/api/utils" azureutils "github.com/gravitational/teleport/api/utils/azure" + libslices "github.com/gravitational/teleport/lib/utils/slices" "github.com/gravitational/teleport/lib/utils/typical" ) @@ -82,7 +83,6 @@ func SimplifyAzureMatchers(matchers []types.AzureMatcher) []types.AzureMatcher { for _, m := range matchers { subs := apiutils.Deduplicate(m.Subscriptions) groups := apiutils.Deduplicate(m.ResourceGroups) - regions := apiutils.Deduplicate(m.Regions) ts := apiutils.Deduplicate(m.Types) if len(subs) == 0 || slices.Contains(subs, types.Wildcard) { subs = []string{types.Wildcard} @@ -90,12 +90,13 @@ func SimplifyAzureMatchers(matchers []types.AzureMatcher) []types.AzureMatcher { if len(groups) == 0 || slices.Contains(groups, types.Wildcard) { groups = []string{types.Wildcard} } - if len(regions) == 0 || slices.Contains(regions, types.Wildcard) { + var regions []string + if len(m.Regions) == 0 || slices.Contains(m.Regions, types.Wildcard) { regions = []string{types.Wildcard} } else { - for i, region := range regions { - regions[i] = azureutils.NormalizeLocation(region) - } + // Normalize before dedup so case-variant inputs ("East US", "eastus") collapse. + // The fresh slice also keeps m.Regions safe from the watcher's concurrent IsEqual reads. + regions = apiutils.Deduplicate(libslices.Map(m.Regions, azureutils.NormalizeLocation)) } elem := m elem.Subscriptions = subs diff --git a/lib/services/matchers_test.go b/lib/services/matchers_test.go index 40e01dc8241..48e2f036cf3 100644 --- a/lib/services/matchers_test.go +++ b/lib/services/matchers_test.go @@ -196,6 +196,77 @@ func TestSimplifyAzureMatchers(t *testing.T) { require.Equal(t, want, simplified) } +// TestSimplifyAzureMatchersDoesNotMutateInput guards against a regression where +// SimplifyAzureMatchers normalized regions through an aliased input slice, +// mutating the caller's matcher and racing with concurrent readers. +func TestSimplifyAzureMatchersDoesNotMutateInput(t *testing.T) { + t.Parallel() + matchers := []types.AzureMatcher{ + { + Subscriptions: []string{"sub-1"}, + ResourceGroups: []string{"rg-1"}, + // "East US" normalizes to "eastus", so any in-place write is observable by value comparison. + Regions: []string{"East US"}, + Types: []string{"vm"}, + }, + } + regionsBackingArray := matchers[0].Regions + + simplified := SimplifyAzureMatchers(matchers) + + require.Equal(t, []string{"East US"}, matchers[0].Regions, + "input regions slice must not be mutated") + require.Equal(t, "East US", regionsBackingArray[0], + "input regions backing array must not be mutated") + require.Equal(t, []string{"eastus"}, simplified[0].Regions, + "output regions must be normalized") +} + +// TestSimplifyAzureMatchersDeduplicatesNormalizedRegions guards against a +// regression where deduplication ran before normalization, leaving case-variant +// inputs like "East US" and "eastus" as distinct entries in the output. +func TestSimplifyAzureMatchersDeduplicatesNormalizedRegions(t *testing.T) { + t.Parallel() + matchers := []types.AzureMatcher{ + { + Subscriptions: []string{"sub-1"}, + ResourceGroups: []string{"rg-1"}, + // Four inputs, three of which normalize to "eastus", one to "eastus2". + // Byte-equality dedup before normalization would let all four through. + Regions: []string{"East US", "eastus", "EASTUS", "East US 2"}, + Types: []string{"vm"}, + }, + } + + simplified := SimplifyAzureMatchers(matchers) + + require.ElementsMatch(t, []string{"eastus", "eastus2"}, simplified[0].Regions, + "case-variant regions must collapse to one normalized entry per location") +} + +// TestSimplifyAzureMatchersCollapsesRegionsWildcard locks in the Regions +// wildcard branch: when any region entry is the wildcard, the output must +// collapse to a single wildcard rather than keep the explicit entries +// alongside it. Without this test the wildcard collapse for Regions has no +// fixture coverage (existing tests only exercise the wildcard branch via an +// empty Regions slice). +func TestSimplifyAzureMatchersCollapsesRegionsWildcard(t *testing.T) { + t.Parallel() + matchers := []types.AzureMatcher{ + { + Subscriptions: []string{"sub-1"}, + ResourceGroups: []string{"rg-1"}, + Regions: []string{"East US", types.Wildcard, "westus"}, + Types: []string{"vm"}, + }, + } + + simplified := SimplifyAzureMatchers(matchers) + + require.Equal(t, []string{types.Wildcard}, simplified[0].Regions, + "any wildcard in regions must collapse the slice to a single wildcard") +} + func TestMatchResourceByFilters_Helper(t *testing.T) { t.Parallel() diff --git a/lib/srv/discovery/discovery.go b/lib/srv/discovery/discovery.go index fb53efa32db..927cfcbbaf4 100644 --- a/lib/srv/discovery/discovery.go +++ b/lib/srv/discovery/discovery.go @@ -1590,7 +1590,7 @@ func (s *Server) startAzureServerDiscovery() { // in case of API calls, e.g., to expand subscription wildcard. dynamicConfigs := make(map[string][]types.AzureMatcher, len(s.dynamicDiscoveryConfig)) for _, config := range s.dynamicDiscoveryConfig { - dynamicConfigs[config.GetName()] = config.Spec.Azure + dynamicConfigs[config.GetName()] = services.SimplifyAzureMatchers(config.Spec.Azure) } s.dynamicDiscoveryConfigMu.RUnlock()