diff --git a/assets/policyAssignment.go b/assets/policyAssignment.go index bdabe16..28a853f 100644 --- a/assets/policyAssignment.go +++ b/assets/policyAssignment.go @@ -7,6 +7,7 @@ import ( "fmt" "unicode/utf8" + "github.com/Azure/alzlib/internal/parametername" "github.com/Azure/alzlib/to" "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armpolicy" @@ -63,12 +64,22 @@ func (pa *PolicyAssignment) ReferencedPolicyDefinitionResourceIDAndVersion() (*a } // ParameterValueAsString returns the value of a policy assignment parameter. +// The parameter name is matched exactly first, then case-insensitively, as Azure Resource Manager +// treats parameter names as case-insensitive. // We always expect the value to be a string as it's used in calculating the additional role // assignments for // policy parameters with the assignPermissions metadata set to true. // Therefore the value should be an ARM resourceId. func (pa *PolicyAssignment) ParameterValueAsString(paramName string) (string, error) { - paParamVal, ok := pa.Properties.Parameters[paramName] + match, ok, err := parametername.Resolve(pa.Properties.Parameters, paramName) + if err != nil { + return "", fmt.Errorf( + "PolicyAssignment.ParameterValueAsString: policy assignment %s: %w", + *pa.Name, + err, + ) + } + if !ok { return "", fmt.Errorf( "PolicyAssignment.ParameterValueAsString: parameter %s not found in policy assignment %s", @@ -77,7 +88,8 @@ func (pa *PolicyAssignment) ParameterValueAsString(paramName string) (string, er ) } - if paParamVal.Value == nil { + paParamVal := match.Value + if paParamVal == nil || paParamVal.Value == nil { return "", fmt.Errorf( "PolicyAssignment.ParameterValueAsString: parameter %s value field in policy assignment %s is nil", paramName, diff --git a/assets/policyDefinition.go b/assets/policyDefinition.go index 2ca494e..ba679e2 100644 --- a/assets/policyDefinition.go +++ b/assets/policyDefinition.go @@ -9,6 +9,7 @@ import ( "fmt" "unicode/utf8" + "github.com/Azure/alzlib/internal/parametername" "github.com/Azure/alzlib/to" "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armpolicy" @@ -242,6 +243,7 @@ func (pd *PolicyDefinition) AssignPermissionsParameterNames() ([]string, error) } // ParameterIsOptional checks if the parameter with the given name is optional in the policy definition. +// The name is matched case-insensitively if there is no exact match. func (pd *PolicyDefinition) ParameterIsOptional(name string) (bool, error) { if pd == nil || pd.Properties == nil || pd.Properties.Parameters == nil { return false, errors.New( @@ -249,8 +251,12 @@ func (pd *PolicyDefinition) ParameterIsOptional(name string) (bool, error) { ) } - param, ok := pd.Properties.Parameters[name] - if !ok { + _, param, found, err := pd.ResolveParameter(name) + if err != nil { + return false, err + } + + if !found { return false, fmt.Errorf( "PolicyDefinition.ParameterIsOptional: parameter %s not found in policy definition", name, @@ -264,30 +270,45 @@ func (pd *PolicyDefinition) ParameterIsOptional(name string) (bool, error) { return true, nil } -// Parameter returns the parameter with the given name from the policy definition. -func (pd *PolicyDefinition) Parameter(name string) *armpolicy.ParameterDefinitionsValue { +// ResolveParameter returns the canonical name and value of the parameter matching the given name. +// Names are matched exactly first, then case-insensitively, as Azure Resource Manager treats +// parameter names as case-insensitive. +// The boolean return value reports whether a matching parameter was declared. An error is returned +// if the name matches more than one parameter when compared case-insensitively. +func (pd *PolicyDefinition) ResolveParameter( + name string, +) (string, *armpolicy.ParameterDefinitionsValue, bool, error) { if pd == nil || pd.Properties == nil || pd.Properties.Parameters == nil { - return nil + return "", nil, false, nil } - ret, ok := pd.Properties.Parameters[name] - if !ok { + match, found, err := parametername.Resolve(pd.Properties.Parameters, name) + if err != nil { + return "", nil, false, fmt.Errorf("PolicyDefinition.ResolveParameter: %w", err) + } + + return match.Key, match.Value, found, nil +} + +// Parameter returns the parameter with the given name from the policy definition. +// The name is matched case-insensitively if there is no exact match. +// It returns nil if the parameter is not found, or if the name is ambiguous. +func (pd *PolicyDefinition) Parameter(name string) *armpolicy.ParameterDefinitionsValue { + _, param, found, err := pd.ResolveParameter(name) + if err != nil || !found { return nil } - return ret + return param } // SetAssignPermissionsOnParameter sets the AssignPermissions metadata field to true for the // parameter with the given // name. +// The name is matched case-insensitively if there is no exact match. func (pd *PolicyDefinition) SetAssignPermissionsOnParameter(parameterName string) { - if pd == nil || pd.Properties == nil || pd.Properties.Parameters == nil { - return - } - - param, ok := pd.Properties.Parameters[parameterName] - if !ok { + param := pd.Parameter(parameterName) + if param == nil { return } @@ -300,17 +321,10 @@ func (pd *PolicyDefinition) SetAssignPermissionsOnParameter(parameterName string // UnsetAssignPermissionsOnParameter removes the AssignPermissions metadata field for the parameter // with the given name. +// The name is matched case-insensitively if there is no exact match. func (pd *PolicyDefinition) UnsetAssignPermissionsOnParameter(parameterName string) { - if pd == nil || pd.Properties == nil || pd.Properties.Parameters == nil { - return - } - - param, ok := pd.Properties.Parameters[parameterName] - if !ok { - return - } - - if param.Metadata == nil { + param := pd.Parameter(parameterName) + if param == nil || param.Metadata == nil { return } diff --git a/assets/policyDefinition_test.go b/assets/policyDefinition_test.go index a22746f..d8d309a 100644 --- a/assets/policyDefinition_test.go +++ b/assets/policyDefinition_test.go @@ -85,6 +85,100 @@ func TestUnsetAssignPermissionsOnParameter(t *testing.T) { assert.Nil(t, pd.Properties.Parameters["test"].Metadata.AssignPermissions) } +// TestResolveParameterCaseInsensitive covers the built-in Windows AMA/DCR shape, where the +// initiative passes `DcrResourceId` to a definition declaring `dcrResourceId`. +func TestResolveParameterCaseInsensitive(t *testing.T) { + pd := &PolicyDefinition{ + Definition: armpolicy.Definition{ + Properties: &armpolicy.DefinitionProperties{ + Parameters: map[string]*armpolicy.ParameterDefinitionsValue{ + "dcrResourceId": {Type: to.Ptr(armpolicy.ParameterTypeString)}, + }, + }, + }, + } + + name, param, found, err := pd.ResolveParameter("dcrResourceId") + require.NoError(t, err) + assert.True(t, found) + assert.Equal(t, "dcrResourceId", name) + assert.NotNil(t, param) + + name, param, found, err = pd.ResolveParameter("DcrResourceId") + require.NoError(t, err) + assert.True(t, found) + assert.Equal(t, "dcrResourceId", name) + assert.NotNil(t, param) + assert.NotNil(t, pd.Parameter("DcrResourceId")) + + _, _, found, err = pd.ResolveParameter("minPort") + require.NoError(t, err) + assert.False(t, found) + assert.Nil(t, pd.Parameter("minPort")) +} + +func TestResolveParameterAmbiguous(t *testing.T) { + pd := &PolicyDefinition{ + Definition: armpolicy.Definition{ + Properties: &armpolicy.DefinitionProperties{ + Parameters: map[string]*armpolicy.ParameterDefinitionsValue{ + "dcrResourceId": {Type: to.Ptr(armpolicy.ParameterTypeString)}, + "DCRRESOURCEID": {Type: to.Ptr(armpolicy.ParameterTypeString)}, + }, + }, + }, + } + + _, _, found, err := pd.ResolveParameter("DcrResourceId") + require.Error(t, err) + assert.False(t, found) + require.ErrorContains(t, err, "is ambiguous") + assert.Nil(t, pd.Parameter("DcrResourceId")) +} + +// TestAssignPermissionsAndOptionalParameterCaseInsensitive asserts that the parameter-name based +// helpers agree with Parameter() on casing, so a case-only variant does not silently no-op. +func TestAssignPermissionsAndOptionalParameterCaseInsensitive(t *testing.T) { + pd := &PolicyDefinition{ + Definition: armpolicy.Definition{ + Properties: &armpolicy.DefinitionProperties{ + Parameters: map[string]*armpolicy.ParameterDefinitionsValue{ + "dcrResourceId": {Type: to.Ptr(armpolicy.ParameterTypeString)}, + }, + }, + }, + } + + optional, err := pd.ParameterIsOptional("DcrResourceId") + require.NoError(t, err) + assert.False(t, optional) + + pd.SetAssignPermissionsOnParameter("DcrResourceId") + require.NotNil(t, pd.Properties.Parameters["dcrResourceId"].Metadata) + assert.True(t, *pd.Properties.Parameters["dcrResourceId"].Metadata.AssignPermissions) + + pd.UnsetAssignPermissionsOnParameter("DCRRESOURCEID") + assert.Nil(t, pd.Properties.Parameters["dcrResourceId"].Metadata.AssignPermissions) + + _, err = pd.ParameterIsOptional("minPort") + require.ErrorContains(t, err, "not found in policy definition") +} + +func TestResolveParameterNilReceiverAndProperties(t *testing.T) { + var pd *PolicyDefinition + + _, _, found, err := pd.ResolveParameter("dcrResourceId") + require.NoError(t, err) + assert.False(t, found) + assert.Nil(t, pd.Parameter("dcrResourceId")) + + pd = &PolicyDefinition{Definition: armpolicy.Definition{}} + + _, _, found, err = pd.ResolveParameter("dcrResourceId") + require.NoError(t, err) + assert.False(t, found) +} + func TestNewPolicyDefinitionFromVersionSuccess(t *testing.T) { versionID := "/subscriptions/00000000-0000-0000-0000-000000000000/providers/" + "Microsoft.Authorization/policyDefinitions/myPolicy/versions/1.0.0" diff --git a/assets/policySetDefinition.go b/assets/policySetDefinition.go index 3b6031e..d7a31e2 100644 --- a/assets/policySetDefinition.go +++ b/assets/policySetDefinition.go @@ -9,6 +9,7 @@ import ( "fmt" "unicode/utf8" + "github.com/Azure/alzlib/internal/parametername" "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armpolicy" ) @@ -152,17 +153,19 @@ func (psd *PolicySetDefinition) PolicyDefinitionReferences() []*armpolicy.Defini } // Parameter returns the parameter definition for the given name. +// The name is matched case-insensitively if there is no exact match. +// It returns nil if the parameter is not found, or if the name is ambiguous. func (psd *PolicySetDefinition) Parameter(name string) *armpolicy.ParameterDefinitionsValue { if psd == nil || psd.Properties == nil || psd.Properties.Parameters == nil { return nil } - ret, ok := psd.Properties.Parameters[name] - if !ok { + match, found, err := parametername.Resolve(psd.Properties.Parameters, name) + if err != nil || !found { return nil } - return ret + return match.Value } // GetVersion returns the version of the policy definition, if it exists. diff --git a/deployment/hierarchy.go b/deployment/hierarchy.go index 3b67a34..72e8c54 100644 --- a/deployment/hierarchy.go +++ b/deployment/hierarchy.go @@ -432,7 +432,20 @@ func (h *Hierarchy) addManagementGroup( } for param := range rf.Parameters { - if pd.Parameter(param) == nil { + _, _, found, err := pd.ResolveParameter(param) + if err != nil { + return nil, fmt.Errorf( + "Hierarchy.AddManagementGroup(): parameter `%s` in policy set definition `%s` "+ + "is ambiguous in referenced definition `%s` in management group `%s`: %w", + param, + *psd.Name, + alzlib.JoinNameAndVersion(*pd.Name, rf.DefinitionVersion), + req.id, + err, + ) + } + + if !found { return nil, fmt.Errorf( "Hierarchy.AddManagementGroup(): parameter `%s` in policy set definition `%s` "+ "does not match a parameter in referenced definition `%s` in management group `%s`", diff --git a/deployment/managementgroup.go b/deployment/managementgroup.go index ea96f9f..2a92a6f 100644 --- a/deployment/managementgroup.go +++ b/deployment/managementgroup.go @@ -11,6 +11,7 @@ import ( "github.com/Azure/alzlib" "github.com/Azure/alzlib/assets" + "github.com/Azure/alzlib/internal/parametername" "github.com/Azure/alzlib/to" "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armpolicy" @@ -547,32 +548,6 @@ func (mg *HierarchyManagementGroup) generatePolicyAssignmentAdditionalRoleAssign !*paramVal.Metadata.AssignPermissions { continue } - // get the parameter value from the policy reference within the set definition. - if _, ok := pd.Properties.Parameters[paramName]; !ok { - if errs == nil { - errs = NewPolicyRoleAssignmentErrors() - } - - errs.Add( - NewPolicyRoleAssignmentError( - paName, - mg.id, - paramName, - *pdRef.PolicyDefinitionReferenceID, - rdids, - fmt.Errorf( - "ManagementGroup.GeneratePolicyAssignmentAdditionalRoleAssignments: "+ - "assignment `%s` for policy set `%s`, parameter `%s` not found in refernced policy definition `%s`", - paName, - *psd.Name, - paramName, - *pd.Name, - ), - ), - ) - - continue - } // use goarmfunctions to evaluate the ARM expression in the parameter value in the set // definition reference. scope, err := parseArmFunctionInPolicySetParameter( @@ -743,13 +718,24 @@ func WithParameters(parameters map[string]*armpolicy.ParameterValuesValue) Modif ) } - pa.Properties.Parameters[k] = v + pa.Properties.Parameters[canonicalAssignmentParameterName(pa, k)] = v } return nil } } +// canonicalAssignmentParameterName returns the casing already used by the assignment for name, so +// that a case-only variant does not add a second parameter that Azure would treat as a duplicate. +func canonicalAssignmentParameterName(pa *assets.PolicyAssignment, name string) string { + match, found, err := parametername.Resolve(pa.Properties.Parameters, name) + if err != nil || !found { + return name + } + + return match.Key +} + // WithEnforcementMode sets the enforcement mode for the policy assignment. func WithEnforcementMode(enforcementMode *armpolicy.EnforcementMode) ModifyPolicyAssignmentOption { return func(mg *HierarchyManagementGroup, name string) error { @@ -884,7 +870,15 @@ func parseArmFunctionInPolicySetParameter( } for k, v := range ass.Properties.Parameters { - resultantParams[k] = v.Value + // Assignment parameter names are case-insensitive, so map them onto the canonical set + // definition parameter name, otherwise the default value would not be overridden. + key := k + if match, found, err := parametername.Resolve(setDef.Properties.Parameters, k); err == nil && + found { + key = match.Key + } + + resultantParams[key] = v.Value } var toParse string @@ -894,8 +888,17 @@ func parseArmFunctionInPolicySetParameter( continue } - p, ok := def.Parameters[paramName] - if !ok { + match, found, err := parametername.Resolve(def.Parameters, paramName) + if err != nil { + return nil, fmt.Errorf( + "parseArmFunctionInPolicySetParameter: paramName %s in %s: %w", + paramName, + *def.PolicyDefinitionReferenceID, + err, + ) + } + + if !found { return nil, fmt.Errorf( "parseArmFunctionInPolicySetParameter: paramName %s not found in %s", paramName, @@ -903,7 +906,12 @@ func parseArmFunctionInPolicySetParameter( ) } - pStr, ok := p.Value.(string) + var value any + if match.Value != nil { + value = match.Value.Value + } + + pStr, ok := value.(string) if !ok { return nil, fmt.Errorf( "parseArmFunctionInPolicySetParameter: paramName %s in %s is not a string", diff --git a/deployment/parameterNameCasing_test.go b/deployment/parameterNameCasing_test.go new file mode 100644 index 0000000..1f188cb --- /dev/null +++ b/deployment/parameterNameCasing_test.go @@ -0,0 +1,292 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package deployment + +import ( + "context" + "testing" + + "github.com/Azure/alzlib" + "github.com/Azure/alzlib/assets" + "github.com/Azure/alzlib/to" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armpolicy" + mapset "github.com/deckarep/golang-set/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The following fixtures reproduce the shape of the official Azure built-in Windows AMA/DCR +// initiative `9575b8b7-78ab-4281-b53b-d3c1ace2260b`, which passes `DcrResourceId` to the policy +// definition `eab1f514-22e3-42e3-9a1f-e1dc9199355c` that declares `dcrResourceId`. +// See https://github.com/Azure/Azure-Landing-Zones/issues/4248. +const ( + dcrMemberDefinitionName = "eab1f514-22e3-42e3-9a1f-e1dc9199355c" + dcrInitiativeName = "9575b8b7-78ab-4281-b53b-d3c1ace2260b" + dcrAssignmentName = "Deploy-VM-Monitoring" + dcrMemberParameterName = "dcrResourceId" + dcrInitiativeParamName = "DcrResourceId" + dcrRoleDefinitionID = "/providers/Microsoft.Authorization/roleDefinitions/749f88d5-cbae-40b8-bcfc-e573ddc772fa" + dcrResourceID = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/rg-ama/" + + "providers/Microsoft.Insights/dataCollectionRules/dcr-windows" +) + +// newDcrMemberDefinition returns a policy definition declaring memberParamName with +// assignPermissions set to true. +func newDcrMemberDefinition(memberParamName string) *assets.PolicyDefinition { + return assets.NewPolicyDefinition(armpolicy.Definition{ + Name: to.Ptr(dcrMemberDefinitionName), + Type: to.Ptr("Microsoft.Authorization/policyDefinitions"), + Properties: &armpolicy.DefinitionProperties{ + PolicyRule: map[string]any{ + "then": map[string]any{ + "details": map[string]any{ + "roleDefinitionIds": []any{dcrRoleDefinitionID}, + }, + }, + }, + Parameters: map[string]*armpolicy.ParameterDefinitionsValue{ + memberParamName: { + Type: to.Ptr(armpolicy.ParameterTypeString), + Metadata: &armpolicy.ParameterDefinitionsValueMetadata{ + AssignPermissions: to.Ptr(true), + }, + }, + }, + }, + }) +} + +// newDcrInitiative returns an initiative that passes refParamName to the member definition. +func newDcrInitiative(refParamName string) *assets.PolicySetDefinition { + return assets.NewPolicySetDefinition(armpolicy.SetDefinition{ + Name: to.Ptr(dcrInitiativeName), + Type: to.Ptr("Microsoft.Authorization/policySetDefinitions"), + Properties: &armpolicy.SetDefinitionProperties{ + Parameters: map[string]*armpolicy.ParameterDefinitionsValue{ + dcrInitiativeParamName: {Type: to.Ptr(armpolicy.ParameterTypeString)}, + }, + PolicyDefinitions: []*armpolicy.DefinitionReference{ + { + PolicyDefinitionReferenceID: to.Ptr("WindowsDcrAssociation"), + PolicyDefinitionID: to.Ptr( + "/providers/Microsoft.Authorization/policyDefinitions/" + dcrMemberDefinitionName, + ), + Parameters: map[string]*armpolicy.ParameterValuesValue{ + refParamName: {Value: "[parameters('" + dcrInitiativeParamName + "')]"}, + }, + }, + }, + }, + }) +} + +// newDcrAssignment returns an assignment of the initiative supplying assignmentParamName. +func newDcrAssignment(assignmentParamName string) *assets.PolicyAssignment { + return assets.NewPolicyAssignment(armpolicy.Assignment{ + Name: to.Ptr(dcrAssignmentName), + Type: to.Ptr("Microsoft.Authorization/policyAssignments"), + Identity: &armpolicy.Identity{Type: to.Ptr(armpolicy.ResourceIdentityTypeSystemAssigned)}, + Properties: &armpolicy.AssignmentProperties{ + PolicyDefinitionID: to.Ptr( + "/providers/Microsoft.Authorization/policySetDefinitions/" + dcrInitiativeName, + ), + Parameters: map[string]*armpolicy.ParameterValuesValue{ + assignmentParamName: {Value: dcrResourceID}, + }, + }, + }) +} + +// newDcrArchetype returns an archetype referencing the initiative assignment. +func newDcrArchetype() *alzlib.Archetype { + arch := alzlib.NewArchetype("dcr") + arch.PolicyAssignments.Add(dcrAssignmentName) + + return arch +} + +func newDcrManagementGroupAddRequest() managementGroupAddRequest { + return managementGroupAddRequest{ + id: "mg1", + displayName: "mg1", + parentID: "external", + parentIsExternal: true, + archetypes: []*alzlib.Archetype{newDcrArchetype()}, + location: eastUSLocation, + } +} + +// TestAddManagementGroupParameterNameCasing asserts that the hierarchy accepts an initiative that +// passes a case-only variant of a member definition parameter name, as Azure does, while a +// genuinely missing parameter still fails. +func TestAddManagementGroupParameterNameCasing(t *testing.T) { + t.Parallel() + + tcs := []struct { + name string + memberParamName string + refParamName string + errContains string + }{ + { + name: "exact match", + memberParamName: dcrMemberParameterName, + refParamName: dcrMemberParameterName, + }, + { + name: "case only difference", + memberParamName: dcrMemberParameterName, + refParamName: dcrInitiativeParamName, + }, + { + name: "genuinely missing parameter", + memberParamName: "minPort", + refParamName: dcrInitiativeParamName, + errContains: "does not match a parameter in referenced definition", + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + az := alzlib.NewAlzLib(nil) + require.NoError(t, az.AddPolicyDefinitions(newDcrMemberDefinition(tc.memberParamName))) + require.NoError(t, az.AddPolicySetDefinitions(newDcrInitiative(tc.refParamName))) + require.NoError(t, az.AddPolicyAssignments(newDcrAssignment(dcrInitiativeParamName))) + + h := NewHierarchy(az) + + mg, err := h.addManagementGroup(context.Background(), newDcrManagementGroupAddRequest()) + if tc.errContains != "" { + require.Error(t, err) + assert.ErrorContains(t, err, tc.errContains) + + return + } + + require.NoError(t, err) + require.NotNil(t, mg) + assert.Contains(t, mg.policyAssignments, dcrAssignmentName) + }) + } +} + +// TestAddManagementGroupAmbiguousParameterName asserts that a parameter name matching more than one +// member definition parameter case-insensitively is rejected deterministically. +func TestAddManagementGroupAmbiguousParameterName(t *testing.T) { + t.Parallel() + + pd := newDcrMemberDefinition(dcrMemberParameterName) + pd.Properties.Parameters["DCRRESOURCEID"] = &armpolicy.ParameterDefinitionsValue{ + Type: to.Ptr(armpolicy.ParameterTypeString), + } + + az := alzlib.NewAlzLib(nil) + require.NoError(t, az.AddPolicyDefinitions(pd)) + require.NoError(t, az.AddPolicySetDefinitions(newDcrInitiative(dcrInitiativeParamName))) + require.NoError(t, az.AddPolicyAssignments(newDcrAssignment(dcrInitiativeParamName))) + + h := NewHierarchy(az) + + mg, err := h.addManagementGroup(context.Background(), newDcrManagementGroupAddRequest()) + require.Error(t, err) + assert.Nil(t, mg) + assert.ErrorContains(t, err, "is ambiguous") +} + +// TestWithParametersDoesNotDuplicateCaseVariantKeys asserts that supplying a case-only variant of a +// parameter the assignment already sets overwrites it, rather than emitting two parameters that +// Azure would treat as duplicates. +func TestWithParametersDoesNotDuplicateCaseVariantKeys(t *testing.T) { + t.Parallel() + + az := alzlib.NewAlzLib(nil) + require.NoError(t, az.AddPolicyDefinitions(newDcrMemberDefinition(dcrMemberParameterName))) + require.NoError(t, az.AddPolicySetDefinitions(newDcrInitiative(dcrMemberParameterName))) + + pa := newDcrAssignment(dcrInitiativeParamName) + require.NoError(t, az.AddPolicyAssignments(pa)) + + h := NewHierarchy(az) + mg := &HierarchyManagementGroup{ + id: "mg1", + hierarchy: h, + policyAssignments: map[string]*assets.PolicyAssignment{*pa.Name: pa}, + } + h.mgs["mg1"] = mg + + const overriddenID = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/rg-ama/" + + "providers/Microsoft.Insights/dataCollectionRules/dcr-other" + + require.NoError(t, mg.ModifyPolicyAssignment(*pa.Name, WithParameters( + map[string]*armpolicy.ParameterValuesValue{"dcrresourceid": {Value: overriddenID}}, + ))) + + assert.Len(t, pa.Properties.Parameters, 1) + require.Contains(t, pa.Properties.Parameters, dcrInitiativeParamName) + assert.Equal(t, overriddenID, pa.Properties.Parameters[dcrInitiativeParamName].Value) +} + +// TestPolicyRoleAssignmentsParameterNameCasing asserts that the additional role assignment for the +// assignPermissions parameter is still scoped to the supplied data collection rule when the +// initiative and the member definition disagree on parameter name casing. +func TestPolicyRoleAssignmentsParameterNameCasing(t *testing.T) { + t.Parallel() + + tcs := []struct { + name string + refParamName string + assignmentParamName string + }{ + { + name: "exact match", + refParamName: dcrMemberParameterName, + assignmentParamName: dcrInitiativeParamName, + }, + { + name: "reference key differs in case", + refParamName: dcrInitiativeParamName, + assignmentParamName: dcrInitiativeParamName, + }, + { + name: "assignment key differs in case", + refParamName: dcrInitiativeParamName, + assignmentParamName: "dcrresourceid", + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + az := alzlib.NewAlzLib(nil) + require.NoError(t, az.AddPolicyDefinitions(newDcrMemberDefinition(dcrMemberParameterName))) + require.NoError(t, az.AddPolicySetDefinitions(newDcrInitiative(tc.refParamName))) + + pa := newDcrAssignment(tc.assignmentParamName) + require.NoError(t, az.AddPolicyAssignments(pa)) + + h := NewHierarchy(az) + h.mgs["mg1"] = &HierarchyManagementGroup{ + id: "mg1", + hierarchy: h, + policyRoleAssignments: mapset.NewThreadUnsafeSet[PolicyRoleAssignment](), + policyDefinitions: make(map[string]*assets.PolicyDefinition), + policySetDefinitions: make(map[string]*assets.PolicySetDefinition), + policyAssignments: map[string]*assets.PolicyAssignment{*pa.Name: pa}, + } + + res, err := h.PolicyRoleAssignments(context.Background()) + require.NoError(t, err) + + assert.True(t, res.Contains(PolicyRoleAssignment{ + AssignmentName: dcrAssignmentName, + RoleDefinitionID: dcrRoleDefinitionID, + Scope: dcrResourceID, + ManagementGroupID: "mg1", + }), "expected a role assignment scoped to the supplied data collection rule, got %v", res.ToSlice()) + }) + } +} diff --git a/internal/parametername/resolve.go b/internal/parametername/resolve.go new file mode 100644 index 0000000..3d6b2e1 --- /dev/null +++ b/internal/parametername/resolve.go @@ -0,0 +1,60 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +// Package parametername resolves Azure Policy parameter names against the maps that contain them. +// +// Azure Resource Manager treats template function parameters as case-insensitive and Azure Policy +// artifacts rely on this, e.g. the built-in Windows AMA/DCR initiative passes `DcrResourceId` to a +// policy definition that declares `dcrResourceId`. Go maps are case-sensitive, so an exact key +// lookup rejects artifacts that Azure itself accepts. +package parametername + +import ( + "fmt" + "slices" + "strings" +) + +// Match is a successfully resolved parameter. +// Key is the canonical key as declared by the source artifact, which may differ in case from the +// requested name. +type Match[V any] struct { + Key string + Value V +} + +// Resolve looks up name in values, preferring an exact match and falling back to a unique +// case-insensitive match. The boolean return value reports whether a match was found. +// +// An error is returned when the case-insensitive fallback matches more than one key, as the +// result would otherwise depend on Go map iteration order. +func Resolve[V any](values map[string]V, name string) (Match[V], bool, error) { + if value, ok := values[name]; ok { + return Match[V]{Key: name, Value: value}, true, nil + } + + var candidates []string + + for key := range values { + if strings.EqualFold(key, name) { + candidates = append(candidates, key) + } + } + + switch len(candidates) { + case 0: + return Match[V]{}, false, nil + case 1: + key := candidates[0] + + return Match[V]{Key: key, Value: values[key]}, true, nil + } + + slices.Sort(candidates) + + return Match[V]{}, false, fmt.Errorf( + "parameter name `%s` is ambiguous, it matches `%s` when compared case-insensitively", + name, + strings.Join(candidates, "`, `"), + ) +} diff --git a/internal/parametername/resolve_test.go b/internal/parametername/resolve_test.go new file mode 100644 index 0000000..b5830d5 --- /dev/null +++ b/internal/parametername/resolve_test.go @@ -0,0 +1,87 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package parametername + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestResolveExactMatch(t *testing.T) { + t.Parallel() + + values := map[string]int{"dcrResourceId": 1, "DcrResourceId": 2} + + match, found, err := Resolve(values, "DcrResourceId") + + require.NoError(t, err) + assert.True(t, found) + assert.Equal(t, "DcrResourceId", match.Key) + assert.Equal(t, 2, match.Value) +} + +func TestResolveCaseInsensitiveFallback(t *testing.T) { + t.Parallel() + + values := map[string]int{"dcrResourceId": 1} + + match, found, err := Resolve(values, "DcrResourceId") + + require.NoError(t, err) + assert.True(t, found) + assert.Equal(t, "dcrResourceId", match.Key, "the canonical key of the source artifact is returned") + assert.Equal(t, 1, match.Value) +} + +func TestResolveNotFound(t *testing.T) { + t.Parallel() + + values := map[string]int{"dcrResourceId": 1} + + match, found, err := Resolve(values, "minPort") + + require.NoError(t, err) + assert.False(t, found) + assert.Empty(t, match.Key) +} + +func TestResolveNilAndEmptyMap(t *testing.T) { + t.Parallel() + + for name, values := range map[string]map[string]int{ + "nil": nil, + "empty": {}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + _, found, err := Resolve(values, "dcrResourceId") + + require.NoError(t, err) + assert.False(t, found) + }) + } +} + +func TestResolveAmbiguousIsDeterministic(t *testing.T) { + t.Parallel() + + values := map[string]int{"dcrResourceId": 1, "DCRRESOURCEID": 2} + + // Repeat to make sure the outcome does not depend on map iteration order. + for range 100 { + _, found, err := Resolve(values, "DcrResourceId") + + require.Error(t, err) + assert.False(t, found) + assert.Equal( + t, + "parameter name `DcrResourceId` is ambiguous, "+ + "it matches `DCRRESOURCEID`, `dcrResourceId` when compared case-insensitively", + err.Error(), + ) + } +}