mirror of
https://github.com/gravitational/teleport.git
synced 2026-09-24 16:17:11 +08:00
Simplify Azure matchers from dynamic configs (#66219)
* Simplify Azure matchers from dynamic configs Dedup, normalize of Azure matchers for dynamic config. * Stop mutating caller's regions in Azure matcher SimplifyAzureMatchers now normalizes regions into a freshly allocated slice rather than writing back through the caller's input slice, since Deduplicate may return its input unchanged for short inputs. * Reuse existing util for Azure region normalization * Make test parallel Co-authored-by: Marek Smoliński <marek@goteleport.com> * Dedupe Azure regions after normalization --------- Co-authored-by: Marek Smoliński <marek@goteleport.com>
This commit is contained in:
co-authored by
Marek Smoliński
parent
353d840acd
commit
c55406bcdd
@@ -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
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user