CLOUDP-293113: Fixed unintended removals of custom roles

api/v1/atlascustomrole_types.go

@@ -17,7 +17,6 @@ func init() {
 // +kubebuilder:object:root=true
 // +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status`
 // +kubebuilder:printcolumn:name="Name",type=string,JSONPath=`.spec.role.name`
-// +kubebuilder:printcolumn:name="Project ID",type=string,JSONPath=`.spec.projectIDRef.id`
 // +kubebuilder:subresource:status
 // +groupName:=atlas.mongodb.com
 // +kubebuilder:resource:categories=atlas,shortName=acr

internal/controller/atlasproject/custom_roles.go

@@ -50,16 +50,20 @@ 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
-			}
+	lastAppliedRolesMap := mapCustomRolesByName(prevSpec)
+	akoRolesMap := mapCustomRolesByName(akoRoles)
+	atlasRolesMap := mapCustomRolesByName(atlasRoles)
+
+	for atlasName, atlasRole := range atlasRolesMap {
+		_, inAKO := akoRolesMap[atlasName]
+		_, inLastApplied := lastAppliedRolesMap[atlasName]
+		if !inAKO && inLastApplied {
+			result[atlasName] = atlasRole
 		}
 	}
+
 	return result
 }
 
@@ -94,7 +98,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 +108,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)
@@ -218,22 +222,13 @@ func mapCustomRolesByName(customRoles []customroles.CustomRole) map[string]custo
 type CustomRolesOperations struct {
 	Create map[string]customroles.CustomRole
 	Update map[string]customroles.CustomRole
-	Delete map[string]customroles.CustomRole
 }
 
 func calculateChanges(currentCustomRoles []customroles.CustomRole, desiredCustomRoles []customroles.CustomRole) CustomRolesOperations {
 	currentCustomRolesByName := mapCustomRolesByName(currentCustomRoles)
-	desiredCustomRolesByName := mapCustomRolesByName(desiredCustomRoles)
 	ops := CustomRolesOperations{
 		Create: map[string]customroles.CustomRole{},
 		Update: map[string]customroles.CustomRole{},
-		Delete: map[string]customroles.CustomRole{},
-	}
-
-	for _, currentCustomRole := range currentCustomRoles {
-		if _, ok := desiredCustomRolesByName[currentCustomRole.Name]; !ok {
-			ops.Delete[currentCustomRole.Name] = currentCustomRole
-		}
 	}
 
 	for _, desiredCustomRole := range desiredCustomRoles {

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 {