diff --git a/.github/workflows/go-lint.yml b/.github/workflows/go-lint.yml index 0c02d74..b196e3b 100644 --- a/.github/workflows/go-lint.yml +++ b/.github/workflows/go-lint.yml @@ -36,9 +36,9 @@ jobs: # Checkout the code base # ########################## - name: Checkout Code - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 - - uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5.0.2 + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 with: go-version-file: 'go.mod' cache: true @@ -47,10 +47,19 @@ jobs: # Run Linter against code base # ################################ - name: golangci-lint - uses: golangci/golangci-lint-action@9fae48acfc02a90574d7c304a1758ef9895495fa # v7.0.1 + uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a # v9.3.0 with: # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version - version: latest + # This selects the golangci-lint binary and is independent of the action version above, + # which was upgraded to v9 to use the Node.js 24 runtime. + # Keep v2.11.4 pinned because it is the last known-good version for this codebase. + # Starting with v2.12.0, the upgraded goconst linter performs extended detection and + # reports 77 pre-existing repeated-string findings, mainly in test fixtures. + # Do not change this to `latest`, as that makes CI behavior change without repository changes. + # Upgrade the linter in a dedicated PR after reviewing the v2.12+ changelog and deciding + # whether to fix the findings or explicitly configure goconst (for example, ignore-tests). + # Changelog: https://golangci-lint.run/docs/product/changelog/#2120 + version: v2.11.4 # Optional: working directory, useful for monorepos # working-directory: somedir diff --git a/deployment/managementgroup.go b/deployment/managementgroup.go index daa1d65..ea96f9f 100644 --- a/deployment/managementgroup.go +++ b/deployment/managementgroup.go @@ -626,6 +626,15 @@ func (mg *HierarchyManagementGroup) generatePolicyAssignmentAdditionalRoleAssign continue } + + // An assignPermissions parameter whose effective value is empty (e.g. a member + // policy added by an upstream version bump whose parameter defaults to "" and is + // not overridden by the assignment) has no scope to grant permissions on. + // Skip it — consistent with the standalone policy definition branch above. + if scopeStr == "" { + continue + } + // The value should be an ARM resource ID. resid, err := arm.ParseResourceID(scopeStr) if err != nil { diff --git a/deployment/managementgroup_test.go b/deployment/managementgroup_test.go index 359f4d5..8ac7c07 100644 --- a/deployment/managementgroup_test.go +++ b/deployment/managementgroup_test.go @@ -58,6 +58,7 @@ func TestGeneratePolicyAssignmentAdditionalRoleAssignments(t *testing.T) { Identity: &armpolicy.Identity{Type: to.Ptr(armpolicy.ResourceIdentityTypeSystemAssigned)}, Properties: &armpolicy.AssignmentProperties{ PolicyDefinitionID: to.Ptr("/providers/Microsoft.Authorization/policySetDefinitions/test-policy-set-definition"), + // setparameter3 is intentionally omitted so the policy set's empty default is used. Parameters: map[string]*armpolicy.ParameterValuesValue{ "setparameter1": {Value: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/my-rg"}, "setparameter2": {Value: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/my-rg2"}, @@ -76,6 +77,10 @@ func TestGeneratePolicyAssignmentAdditionalRoleAssignments(t *testing.T) { "setparameter2": { Type: to.Ptr(armpolicy.ParameterTypeString), }, + "setparameter3": { + Type: to.Ptr(armpolicy.ParameterTypeString), + DefaultValue: "", + }, }, PolicyDefinitions: []*armpolicy.DefinitionReference{ { @@ -84,6 +89,7 @@ func TestGeneratePolicyAssignmentAdditionalRoleAssignments(t *testing.T) { Parameters: map[string]*armpolicy.ParameterValuesValue{ "parameter1": {Value: "[parameters('setparameter1')]"}, "parameter2": {Value: "[parameters('setparameter2')]"}, + "parameter3": {Value: "[parameters('setparameter3')]"}, }, }, }, @@ -161,6 +167,12 @@ func TestGeneratePolicyAssignmentAdditionalRoleAssignments(t *testing.T) { AssignPermissions: to.Ptr(false), }, }, + "parameter3": { + Type: to.Ptr(armpolicy.ParameterTypeString), + Metadata: &armpolicy.ParameterDefinitionsValueMetadata{ + AssignPermissions: to.Ptr(true), + }, + }, }, }, }) @@ -192,6 +204,10 @@ func TestGeneratePolicyAssignmentAdditionalRoleAssignments(t *testing.T) { // check that the additional role assignments were generated correctly. assert.Equal(t, 4, mg.policyRoleAssignments.Cardinality()) + for roleAssignment := range mg.policyRoleAssignments.Iter() { + assert.NotEmpty(t, roleAssignment.Scope) + } + assert.True(t, mg.policyRoleAssignments.Contains(PolicyRoleAssignment{ AssignmentName: *paDef.Name, RoleDefinitionID: pd1.Properties.PolicyRule.(map[string]any)["then"].(map[string]any)["details"].(map[string]any)["roleDefinitionIds"].([]any)[0].(string), //nolint:forcetypeassert