From 8bb4c2a6de19206c981bc68bc43d5165a035be67 Mon Sep 17 00:00:00 2001 From: Yingjie Date: Tue, 14 Jul 2026 16:13:04 +0800 Subject: [PATCH 1/4] Skip empty scopes in the policy set branch to align the behavior with the single definition branch --- deployment/managementgroup.go | 9 +++++++++ 1 file changed, 9 insertions(+) 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 { From 6d9be17553caa09343be8f236f4790002b7b9e3d Mon Sep 17 00:00:00 2001 From: Yingjie Date: Tue, 14 Jul 2026 16:33:21 +0800 Subject: [PATCH 2/4] Upate due to Node.js 20 is deprecated --- .github/workflows/go-lint.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/go-lint.yml b/.github/workflows/go-lint.yml index 0c02d74..240c983 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,7 +47,7 @@ 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 From e34b92cfbb7f55e463c925e64cd5197f5f2d149e Mon Sep 17 00:00:00 2001 From: Yingjie Date: Tue, 14 Jul 2026 17:47:01 +0800 Subject: [PATCH 3/4] Fix the golangci-lint version issue --- .github/workflows/go-lint.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/go-lint.yml b/.github/workflows/go-lint.yml index 240c983..b196e3b 100644 --- a/.github/workflows/go-lint.yml +++ b/.github/workflows/go-lint.yml @@ -50,7 +50,16 @@ jobs: 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 From 02092c8299aff06611bbcf03aae3ca543d1060f2 Mon Sep 17 00:00:00 2001 From: Yingjie Date: Fri, 17 Jul 2026 14:37:11 +0800 Subject: [PATCH 4/4] test(deployment): cover empty policy set permission scopes Reproduce the issue 4190 path where an assignPermissions parameter resolves to an empty policy set default. Verify role assignment generation succeeds and never emits an empty scope. Refs Azure/Azure-Landing-Zones#4190 --- deployment/managementgroup_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) 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