CustomRoles: remove only thouse previously configured in AKO (#1999)

Author: igor-karpukhin

pkg/controller/atlasproject/custom_roles.go

@@ -37,18 +37,39 @@ func hasSkippedCustomRoles(atlasProject *akov2.AtlasProject) (bool, error) {
 	return false, nil
 }
 
-func hasLastAppliedCustomRoles(atlasProject *akov2.AtlasProject) (bool, error) {
+func getLastAppliedCustomRoles(atlasProject *akov2.AtlasProject) ([]akov2.CustomRole, error) {
 	lastAppliedSpec := akov2.AtlasProjectSpec{}
 	lastAppliedSpecStr, ok := atlasProject.Annotations[customresource.AnnotationLastAppliedConfiguration]
 	if !ok {
-		return false, nil
+		return nil, nil
 	}
 
 	if err := json.Unmarshal([]byte(lastAppliedSpecStr), &lastAppliedSpec); err != nil {
-		return false, fmt.Errorf("failed to parse last applied configuration: %w", err)
+		return nil, fmt.Errorf("failed to parse last applied configuration: %w", err)
 	}
 
-	return len(lastAppliedSpec.CustomRoles) != 0, nil
+	return lastAppliedSpec.CustomRoles, nil
+}
+
+func findRolesToDelete(prevSpec, 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
+			}
+		}
+	}
+	return result
+}
+
+func convertToInternalRoles(roles []akov2.CustomRole) []customroles.CustomRole {
+	result := make([]customroles.CustomRole, 0, len(roles))
+	for i := range roles {
+		result = append(result, customroles.NewCustomRole(&roles[i]))
+	}
+	return result
 }
 
 func ensureCustomRoles(workflowCtx *workflow.Context, project *akov2.AtlasProject) workflow.Result {
@@ -63,7 +84,7 @@ func ensureCustomRoles(workflowCtx *workflow.Context, project *akov2.AtlasProjec
 		return workflow.OK()
 	}
 
-	hadPreviousCustomRoles, err := hasLastAppliedCustomRoles(project)
+	lastAppliedCustomRoles, err := getLastAppliedCustomRoles(project)
 	if err != nil {
 		return workflow.Terminate(workflow.Internal, err.Error())
 	}
@@ -87,8 +108,9 @@ func ensureCustomRoles(workflowCtx *workflow.Context, project *akov2.AtlasProjec
 	ops := calculateChanges(currentCustomRoles, akoRoles)
 
 	var deleteStatus map[string]status.CustomRole
-	if hadPreviousCustomRoles {
-		deleteStatus = r.deleteCustomRoles(workflowCtx, project.ID(), ops.Delete)
+	if len(lastAppliedCustomRoles) > 0 {
+		deleteStatus = r.deleteCustomRoles(workflowCtx, project.ID(),
+			findRolesToDelete(convertToInternalRoles(lastAppliedCustomRoles), currentCustomRoles))
 	}
 	updateStatus := r.updateCustomRoles(workflowCtx, project.ID(), ops.Update)
 	createStatus := r.createCustomRoles(workflowCtx, project.ID(), ops.Create)

pkg/controller/atlasproject/custom_roles_test.go

@@ -146,7 +146,7 @@ func TestEnsureCustomRoles(t *testing.T) {
 					d, _ := json.Marshal(&akov2.AtlasProjectSpec{
 						CustomRoles: []akov2.CustomRole{
 							{
-								Name: "test-role",
+								Name: "test-role-1",
 								InheritedRoles: []akov2.Role{
 									{Name: "role3", Database: "db1"},
 								},
@@ -164,7 +164,7 @@ func TestEnsureCustomRoles(t *testing.T) {
 					return string(d)
 				}(),
 			},
-			name: "Roles not in AKO but are in Atlas (Delete) if there were previous in AKO",
+			name: "Roles not in AKO but are in Atlas (Delete) if there were previous in AKO. Remove only those that were in AKO",
 			roleAPI: func() *mockadmin.CustomDatabaseRolesApi {
 				roleAPI := mockadmin.NewCustomDatabaseRolesApi(t)
 				roleAPI.EXPECT().ListCustomDatabaseRoles(context.Background(), "").
@@ -186,11 +186,25 @@ func TestEnsureCustomRoles(t *testing.T) {
 									},
 								},
 							},
+							{
+								RoleName: "test-role-1",
+								InheritedRoles: &[]admin.DatabaseInheritedRole{
+									{Role: "role3", Db: "db1"},
+								},
+								Actions: &[]admin.DatabasePrivilegeAction{
+									{
+										Action: "action1",
+										Resources: &[]admin.DatabasePermittedNamespaceResource{
+											{Db: "db2"},
+										},
+									},
+								},
+							},
 						},
 						&http.Response{},
 						nil,
 					)
-				roleAPI.EXPECT().DeleteCustomDatabaseRole(context.Background(), "", "test-role").
+				roleAPI.EXPECT().DeleteCustomDatabaseRole(context.Background(), "", "test-role-1").
 					Return(admin.DeleteCustomDatabaseRoleApiRequest{ApiService: roleAPI})
 				roleAPI.EXPECT().DeleteCustomDatabaseRoleExecute(mock.Anything).
 					Return(