fix(deployment): ALZ role definitions replaced by new deployment (#174)

Fixes #173

deployment/managementgroup.go

@@ -506,7 +506,9 @@ func updatePolicyAsignments(mg *HierarchyManagementGroup, pd2mg, psd2mg map[stri
 func updateRoleDefinitions(alzmg *HierarchyManagementGroup) {
 	for _, roledef := range alzmg.roleDefinitions {
 		u := uuidV5(alzmg.id, *roledef.Name)
+		roledef.Name = to.Ptr(u.String())
 		roledef.ID = to.Ptr(fmt.Sprintf(RoleDefinitionIdFmt, alzmg.id, u))
+		roledef.Properties.RoleName = to.Ptr(fmt.Sprintf("%s (%s)", *roledef.Properties.RoleName, alzmg.id))
 		if len(roledef.Properties.AssignableScopes) == 0 {
 			roledef.Properties.AssignableScopes = make([]*string, 1)
 		}

deployment/managementgroup_test.go

@@ -608,6 +608,7 @@ func TestModifyRoleDefinitions(t *testing.T) {
 				Name: to.Ptr("role1"),
 				Properties: &armauthorization.RoleDefinitionProperties{
 					AssignableScopes: []*string{},
+					RoleName:         to.Ptr("role1"),
 				},
 			}),
 		},
@@ -627,12 +628,14 @@ func TestModifyRoleDefinitions(t *testing.T) {
 				Name: to.Ptr("role1"),
 				Properties: &armauthorization.RoleDefinitionProperties{
 					AssignableScopes: []*string{},
+					RoleName:         to.Ptr("role1"),
 				},
 			}),
 			"rd2": assets.NewRoleDefinition(armauthorization.RoleDefinition{
 				Name: to.Ptr("role2"),
 				Properties: &armauthorization.RoleDefinitionProperties{
 					AssignableScopes: []*string{},
+					RoleName:         to.Ptr("role2"),
 				},
 			}),
 		},

integrationtest/alzlib_test.go

@@ -68,3 +68,17 @@ func TestInitSimpleExistingMg(t *testing.T) {
 	mg := h.ManagementGroup("simple")
 	assert.True(t, mg.Exists())
 }
+
+func TestInitMultipleRoleDefinitions(t *testing.T) {
+	az := alzlib.NewAlzLib(nil)
+	lib := alzlib.NewCustomLibraryReference("./testdata/multipleroledefinitions")
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	require.NoError(t, az.Init(ctx, lib))
+	h := deployment.NewHierarchy(az)
+	err := h.FromArchitecture(ctx, "test", "00000000-0000-0000-0000-000000000000", "testlocation")
+	require.NoError(t, err)
+	mg1 := h.ManagementGroup("test1")
+	mg2 := h.ManagementGroup("test2")
+	assert.NotEqual(t, mg1.RoleDefinitionsMap()["test-role-definition"].Name, mg2.RoleDefinitionsMap()["test-role-definition"].Name)
+}

integrationtest/testdata/multipleroledefinitions/test-role-definition.alz_role_definition.json

@@ -0,0 +1,28 @@
+{
+  "name": "c9a07a05-a1fc-53fe-a565-5eed25597c03",
+  "type": "Microsoft.Authorization/roleDefinitions",
+  "apiVersion": "2018-01-01-preview",
+  "properties": {
+    "roleName": "test-role-definition",
+    "description": "Test role definition",
+    "type": "CustomRole",
+    "permissions": [
+      {
+        "actions": [
+          "*"
+        ],
+        "notActions": [
+          "Microsoft.Authorization/*/write",
+          "Microsoft.Network/publicIPAddresses/write",
+          "Microsoft.Network/virtualNetworks/write",
+          "Microsoft.KeyVault/locations/deletedVaults/purge/action"
+        ],
+        "dataActions": [],
+        "notDataActions": []
+      }
+    ],
+    "assignableScopes": [
+      "${current_scope_resource_id}"
+    ]
+  }
+}

integrationtest/testdata/multipleroledefinitions/test.alz_archetype_definition.yml

@@ -0,0 +1,3 @@
+name: test
+role_definitions:
+  - test-role-definition

integrationtest/testdata/multipleroledefinitions/test.alz_architecture_definition.yml

@@ -0,0 +1,14 @@
+management_groups:
+  - archetypes:
+      - test
+    display_name: test1
+    exists: false
+    id: test1
+    parent_id: null
+  - archetypes:
+      - test
+    display_name: test2
+    exists: false
+    id: test2
+    parent_id: null
+name: test

internal/processor/processor.go

@@ -326,17 +326,18 @@ func processPolicySetDefinition(res *Result, unmar unmarshaler) error {
 }
 
 // processRoleDefinition is a processFunc that reads the role_definition
-// bytes, processes, then adds the created armpolicy.SetDefinition to the result.
+// bytes, processes, then adds the created armauthorization.RoleDefinition to the result.
+// We use Properties.RoleName as the key in the result map, as the GUID must be unique and a role definition may be deployed at multiple scopes.
 func processRoleDefinition(res *Result, unmar unmarshaler) error {
 	rd := new(armauthorization.RoleDefinition)
 	if err := unmar.unmarshal(rd); err != nil {
 		return fmt.Errorf("processRoleDefinition: error unmarshalling: %w", err)
 	}
-	if rd.Name == nil || *rd.Name == "" {
-		return fmt.Errorf("processRoleDefinition: policy set definition name is empty or not present")
+	if rd.Properties == nil || rd.Properties.RoleName == nil || *rd.Properties.RoleName == "" {
+		return fmt.Errorf("processRoleDefinition: role definition role name is empty or not present")
 	}
-	if _, exists := res.PolicySetDefinitions[*rd.Name]; exists {
-		return fmt.Errorf("processRoleDefinition: policy set definition with name `%s` already exists", *rd.Name)
+	if _, exists := res.PolicySetDefinitions[*rd.Properties.RoleName]; exists {
+		return fmt.Errorf("processRoleDefinition: role definition with role name `%s` already exists", *rd.Properties.RoleName)
 	}
 	// Use roleName here not the name, which is a GUID
 	res.RoleDefinitions[*rd.Properties.RoleName] = rd