CLOUDP-293113: Fixed unintended removals of custom roles

internal/controller/atlasproject/custom_roles.go

@@ -50,16 +50,27 @@ func getLastAppliedCustomRoles(atlasProject *akov2.AtlasProject) ([]akov2.Custom
 	return lastAppliedSpec.CustomRoles, nil
 }
 
-func findRolesToDelete(prevSpec, atlasRoles []customroles.CustomRole) map[string]customroles.CustomRole {
+func findRolesToDelete(prevSpec, akoRoles, atlasRoles []customroles.CustomRole) map[string]customroles.CustomRole {
 	result := map[string]customroles.CustomRole{}
-	for atlasRoleIdx := range atlasRoles {
-		for specRoleIdx := range prevSpec {
-			if atlasRoles[atlasRoleIdx].Name == prevSpec[specRoleIdx].Name {
-				result[prevSpec[specRoleIdx].Name] = prevSpec[specRoleIdx]
-				continue
-			}
+	deletionCandidates := map[string]customroles.CustomRole{}
+	atlasRolesMap := mapCustomRolesByName(atlasRoles)
+
+	// Get roles from the previous spec
+	for _, customRole := range prevSpec {
+		deletionCandidates[customRole.Name] = customRole
+	}
+	// Get roles from the current spec and remove the ones that are in the previous spec
+	for _, customRole := range akoRoles {
+		delete(deletionCandidates, customRole.Name)
+	}
+
+	// Compare combinedAkoRoles with the current Atlas roles
+	for _, role := range deletionCandidates {
+		if _, exists := atlasRolesMap[role.Name]; exists {
+			result[role.Name] = role
 		}
 	}
+
 	return result
 }
 
@@ -94,7 +105,7 @@ func ensureCustomRoles(workflowCtx *workflow.Context, project *akov2.AtlasProjec
 		service: customroles.NewCustomRoles(workflowCtx.SdkClient.CustomDatabaseRolesApi),
 	}
 
-	currentCustomRoles, err := r.service.List(r.ctx.Context, r.project.ID())
+	currentAtlasCustomRoles, err := r.service.List(r.ctx.Context, r.project.ID())
 	if err != nil {
 		return workflow.Terminate(workflow.ProjectCustomRolesReady, err.Error())
 	}
@@ -104,12 +115,12 @@ func ensureCustomRoles(workflowCtx *workflow.Context, project *akov2.AtlasProjec
 		akoRoles[i] = customroles.NewCustomRole(&project.Spec.CustomRoles[i])
 	}
 
-	ops := calculateChanges(currentCustomRoles, akoRoles)
+	ops := calculateChanges(currentAtlasCustomRoles, akoRoles)
 
 	var deleteStatus map[string]status.CustomRole
 	if len(lastAppliedCustomRoles) > 0 {
 		deleteStatus = r.deleteCustomRoles(workflowCtx, project.ID(),
-			findRolesToDelete(convertToInternalRoles(lastAppliedCustomRoles), currentCustomRoles))
+			findRolesToDelete(convertToInternalRoles(lastAppliedCustomRoles), akoRoles, currentAtlasCustomRoles))
 	}
 	updateStatus := r.updateCustomRoles(workflowCtx, project.ID(), ops.Update)
 	createStatus := r.createCustomRoles(workflowCtx, project.ID(), ops.Create)

internal/controller/atlasproject/custom_roles_test.go

@@ -214,6 +214,137 @@ func TestEnsureCustomRoles(t *testing.T) {
 			}(),
 			isOK: true,
 		},
+		{
+			name: "Roles in AKO and in last applied config. Delete only those that were deleted from the spec",
+			projectAnnotations: map[string]string{
+				customresource.AnnotationLastAppliedConfiguration: func() string {
+					d, _ := json.Marshal(&akov2.AtlasProjectSpec{
+						CustomRoles: []akov2.CustomRole{
+							{
+								Name: "test-role",
+								InheritedRoles: []akov2.Role{
+									{Name: "role", Database: "db"},
+								},
+								Actions: []akov2.Action{
+									{
+										Name: "action",
+										Resources: []akov2.Resource{
+											{
+												Database:   pointer.MakePtr("db"),
+												Cluster:    pointer.MakePtr(true),
+												Collection: pointer.MakePtr("test-collection"),
+											},
+										},
+									},
+								},
+							},
+							{
+								Name: "test-role-2",
+								InheritedRoles: []akov2.Role{
+									{Name: "role2", Database: "db2"},
+								},
+								Actions: []akov2.Action{
+									{
+										Name: "action2",
+										Resources: []akov2.Resource{
+											{Database: pointer.MakePtr("db2")},
+										},
+									},
+								},
+							},
+						},
+					})
+					return string(d)
+				}(),
+			},
+			roles: []akov2.CustomRole{
+				{
+					Name: "test-role",
+					InheritedRoles: []akov2.Role{
+						{Name: "role", Database: "db"},
+					},
+					Actions: []akov2.Action{
+						{
+							Name: "action",
+							Resources: []akov2.Resource{
+								{
+									Database:   pointer.MakePtr("db"),
+									Cluster:    pointer.MakePtr(true),
+									Collection: pointer.MakePtr("test-collection"),
+								},
+							},
+						},
+					},
+				},
+			},
+			roleAPI: func() *mockadmin.CustomDatabaseRolesApi {
+				roleAPI := mockadmin.NewCustomDatabaseRolesApi(t)
+				roleAPI.EXPECT().ListCustomDatabaseRoles(context.Background(), "").
+					Return(admin.ListCustomDatabaseRolesApiRequest{ApiService: roleAPI})
+				roleAPI.EXPECT().ListCustomDatabaseRolesExecute(mock.Anything).
+					Return(
+						[]admin.UserCustomDBRole{
+							{
+								RoleName: "test-role",
+								InheritedRoles: &[]admin.DatabaseInheritedRole{
+									{Role: "role", Db: "db"},
+								},
+								Actions: &[]admin.DatabasePrivilegeAction{
+									{
+										Action: "action",
+										Resources: &[]admin.DatabasePermittedNamespaceResource{
+											{
+												Db:         "db",
+												Collection: "test-collection",
+												Cluster:    true,
+											},
+										},
+									},
+								},
+							},
+							{
+								RoleName: "test-role-1",
+								InheritedRoles: &[]admin.DatabaseInheritedRole{
+									{Role: "role1", Db: "db1"},
+								},
+								Actions: &[]admin.DatabasePrivilegeAction{
+									{
+										Action: "action1",
+										Resources: &[]admin.DatabasePermittedNamespaceResource{
+											{Db: "db1"},
+										},
+									},
+								},
+							},
+							{
+								RoleName: "test-role-2",
+								InheritedRoles: &[]admin.DatabaseInheritedRole{
+									{Role: "role2", Db: "db2"},
+								},
+								Actions: &[]admin.DatabasePrivilegeAction{
+									{
+										Action: "action2",
+										Resources: &[]admin.DatabasePermittedNamespaceResource{
+											{Db: "db2"},
+										},
+									},
+								},
+							},
+						},
+						&http.Response{},
+						nil,
+					)
+				roleAPI.EXPECT().DeleteCustomDatabaseRole(context.Background(), "", "test-role-2").
+					Return(admin.DeleteCustomDatabaseRoleApiRequest{ApiService: roleAPI})
+				roleAPI.EXPECT().DeleteCustomDatabaseRoleExecute(mock.Anything).
+					Return(
+						&http.Response{},
+						nil,
+					)
+				return roleAPI
+			}(),
+			isOK: true,
+		},
 		{
 			name: "Roles not in AKO but are in Atlas (Do not Delete) and NO previous in AKO",
 			roleAPI: func() *mockadmin.CustomDatabaseRolesApi {