fix: Support update of usernames in mongodbatlas_team resource by ensuring team is never empty (#2669)

* include new update logic with acceptance test and unit testing

* adding changelog entry

* fmt fix

* define additional username env variable in CI, use same values for dev and qa

* remove redundant initialization

Author: AgustinBettati

.changelog/2669.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+resource/mongodbatlas_team: Fixes update logic of `usernames` attribute ensuring team is never emptied
+```

.github/workflows/acceptance-tests-runner.yml

@@ -38,9 +38,6 @@ on:
       mongodb_atlas_teams_ids:
         type: string
         required: true  
-      mongodb_atlas_username:
-        type: string
-        required: true 
       azure_atlas_app_id:
         type: string
         required: true
@@ -503,7 +500,8 @@ jobs:
       - name: Acceptance Tests
         env:
           MONGODB_ATLAS_PROJECT_OWNER_ID: ${{ inputs.mongodb_atlas_project_owner_id }}
-          MONGODB_ATLAS_USERNAME: ${{ inputs.mongodb_atlas_username }}
+          MONGODB_ATLAS_USERNAME: ${{ vars.MONGODB_ATLAS_USERNAME }}
+          MONGODB_ATLAS_USERNAME_2: ${{ vars.MONGODB_ATLAS_USERNAME_2 }}
           AZURE_ATLAS_APP_ID: ${{ inputs.azure_atlas_app_id }}
           AZURE_SERVICE_PRINCIPAL_ID: ${{ inputs.azure_service_principal_id }}
           AZURE_TENANT_ID: ${{ inputs.azure_tenant_id }}

.github/workflows/acceptance-tests.yml

@@ -97,7 +97,6 @@ jobs:
       mongodb_atlas_base_url: ${{ inputs.atlas_cloud_env == 'qa' && vars.MONGODB_ATLAS_BASE_URL_QA || vars.MONGODB_ATLAS_BASE_URL }}
       mongodb_atlas_project_owner_id: ${{ inputs.atlas_cloud_env == 'qa' && vars.MONGODB_ATLAS_PROJECT_OWNER_ID_QA || vars.MONGODB_ATLAS_PROJECT_OWNER_ID }}
       mongodb_atlas_teams_ids: ${{ inputs.atlas_cloud_env == 'qa' && vars.MONGODB_ATLAS_TEAMS_IDS_QA || vars.MONGODB_ATLAS_TEAMS_IDS }}
-      mongodb_atlas_username: ${{ inputs.atlas_cloud_env == 'qa' && vars.MONGODB_ATLAS_USERNAME_CLOUD_QA || vars.MONGODB_ATLAS_USERNAME_CLOUD_DEV }}
       azure_atlas_app_id: ${{ inputs.atlas_cloud_env == 'qa' && vars.AZURE_ATLAS_APP_ID_QA || vars.AZURE_ATLAS_APP_ID }}
       azure_service_principal_id: ${{ inputs.atlas_cloud_env == 'qa' && vars.AZURE_SERVICE_PRINCIPAL_ID_QA || vars.AZURE_SERVICE_PRINCIPAL_ID }}
       azure_tenant_id: ${{ inputs.atlas_cloud_env == 'qa' && vars.AZURE_TENANT_ID_QA || vars.AZURE_TENANT_ID }}

internal/service/team/resource_team.go

@@ -148,63 +148,15 @@ func resourceUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.
 	}
 
 	if d.HasChange("usernames") {
-		users, _, err := connV2.TeamsApi.ListTeamUsers(ctx, orgID, teamID).Execute()
-
+		existingUsers, _, err := connV2.TeamsApi.ListTeamUsers(ctx, orgID, teamID).Execute()
 		if err != nil {
 			return diag.FromErr(fmt.Errorf(errorTeamRead, err))
 		}
+		newUsernames := conversion.ExpandStringList(d.Get("usernames").(*schema.Set).List())
 
-		index := make(map[string]admin.CloudAppUser)
-		for i := range users.GetResults() {
-			index[users.GetResults()[i].GetUsername()] = users.GetResults()[i]
-		}
-
-		cleanUsers := func() error {
-			for i := range users.GetResults() {
-				_, err := connV2.TeamsApi.RemoveTeamUser(ctx, orgID, teamID, users.GetResults()[i].GetId()).Execute()
-				if err != nil {
-					return fmt.Errorf("error deleting Atlas User (%s) information: %s", teamID, err)
-				}
-			}
-			return nil
-		}
-
-		var newUsers []admin.AddUserToTeam
-
-		for _, username := range d.Get("usernames").(*schema.Set).List() {
-			user, _, err := connV2.MongoDBCloudUsersApi.GetUserByUsername(ctx, username.(string)).Execute()
-
-			updatedUserData := user
-
-			if err != nil {
-				if !strings.Contains(err.Error(), "401") {
-					return diag.FromErr(fmt.Errorf("error getting Atlas User (%s) information: %s", username, err))
-				}
-
-				log.Printf("[WARN] error fetching information user for (%s): %s\n", username, err)
-				if user == nil {
-					log.Printf("[WARN] there is no runtime information to fetch, checking in the existing users")
-
-					cached, ok := index[username.(string)]
-
-					if !ok {
-						log.Printf("[WARN] no information in cached for (%s)", username)
-						return diag.FromErr(fmt.Errorf("error getting Atlas User (%s) information: %s", username, err))
-					}
-					updatedUserData = &cached
-				}
-			}
-			newUsers = append(newUsers, admin.AddUserToTeam{Id: updatedUserData.GetId()})
-		}
-
-		err = cleanUsers()
-		if err != nil {
-			return diag.FromErr(err)
-		}
-
-		_, _, err = connV2.TeamsApi.AddTeamUser(ctx, orgID, teamID, &newUsers).Execute()
+		err = UpdateTeamUsers(connV2.TeamsApi, connV2.MongoDBCloudUsersApi, existingUsers, newUsernames, orgID, teamID)
 		if err != nil {
-			return diag.FromErr(fmt.Errorf(errorTeamAddUsers, err))
+			return diag.FromErr(fmt.Errorf("error when updating usernames in team: %s", err))
 		}
 	}
 

internal/service/team/resource_team_test.go

@@ -65,6 +65,58 @@ func TestAccConfigRSTeam_basic(t *testing.T) {
 	})
 }
 
+func TestAccConfigRSTeam_updatingUsernames(t *testing.T) {
+	var (
+		resourceName          = "mongodbatlas_team.test"
+		orgID                 = os.Getenv("MONGODB_ATLAS_ORG_ID")
+		firstUser             = os.Getenv("MONGODB_ATLAS_USERNAME")
+		secondUser            = os.Getenv("MONGODB_ATLAS_USERNAME_2")
+		usernames             = []string{firstUser}
+		updatedSingleUsername = []string{secondUser}
+		updatedBothUsername   = []string{firstUser, secondUser}
+		name                  = acc.RandomName()
+	)
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { acc.PreCheckAtlasUsernames(t) },
+		ProtoV6ProviderFactories: acc.TestAccProviderV6Factories,
+		CheckDestroy:             acc.CheckDestroyTeam,
+		Steps: []resource.TestStep{
+			{
+				Config: configBasic(orgID, name, usernames),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					checkExists(resourceName),
+					resource.TestCheckResourceAttrSet(resourceName, "org_id"),
+					resource.TestCheckResourceAttr(resourceName, "name", name),
+					resource.TestCheckResourceAttr(resourceName, "usernames.#", "1"),
+					resource.TestCheckTypeSetElemAttr(resourceName, "usernames.*", usernames[0]),
+				),
+			},
+			{
+				Config: configBasic(orgID, name, updatedSingleUsername),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					checkExists(resourceName),
+					resource.TestCheckResourceAttrSet(resourceName, "org_id"),
+					resource.TestCheckResourceAttr(resourceName, "name", name),
+					resource.TestCheckResourceAttr(resourceName, "usernames.#", "1"),
+					resource.TestCheckTypeSetElemAttr(resourceName, "usernames.*", updatedSingleUsername[0]),
+				),
+			},
+			{
+				Config: configBasic(orgID, name, updatedBothUsername),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					checkExists(resourceName),
+					resource.TestCheckResourceAttrSet(resourceName, "org_id"),
+					resource.TestCheckResourceAttr(resourceName, "name", name),
+					resource.TestCheckResourceAttr(resourceName, "usernames.#", "2"),
+					resource.TestCheckTypeSetElemAttr(resourceName, "usernames.*", updatedBothUsername[0]),
+					resource.TestCheckTypeSetElemAttr(resourceName, "usernames.*", updatedBothUsername[1]),
+				),
+			},
+		},
+	})
+}
+
 func TestAccConfigRSTeam_legacyName(t *testing.T) {
 	var (
 		resourceName   = "mongodbatlas_teams.test"

internal/service/team/update_user.go

@@ -0,0 +1,84 @@
+package team
+
+import (
+	"context"
+
+	"go.mongodb.org/atlas-sdk/v20240805004/admin"
+)
+
+func UpdateTeamUsers(teamsAPI admin.TeamsApi, usersAPI admin.MongoDBCloudUsersApi, existingTeamUsers *admin.PaginatedApiAppUser, newUsernames []string, orgID, teamID string) error {
+	validNewUsers, err := ValidateUsernames(usersAPI, newUsernames)
+	if err != nil {
+		return err
+	}
+	usersToAdd, usersToRemove, err := GetChangesForTeamUsers(existingTeamUsers.GetResults(), validNewUsers)
+	if err != nil {
+		return err
+	}
+
+	// Avoid leaving the team empty with no users by first making new additions, ensuring no API validation errors
+
+	var userToAddModels []admin.AddUserToTeam
+	for i := range usersToAdd {
+		userToAddModels = append(userToAddModels, admin.AddUserToTeam{Id: usersToAdd[i]})
+	}
+	// save all users to add
+	if len(userToAddModels) > 0 {
+		_, _, err = teamsAPI.AddTeamUser(context.Background(), orgID, teamID, &userToAddModels).Execute()
+		if err != nil {
+			return err
+		}
+	}
+
+	for i := range usersToRemove {
+		// remove user from team
+		_, err := teamsAPI.RemoveTeamUser(context.Background(), orgID, teamID, usersToRemove[i]).Execute()
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func ValidateUsernames(c admin.MongoDBCloudUsersApi, usernames []string) ([]admin.CloudAppUser, error) {
+	var validUsers []admin.CloudAppUser
+	for _, elem := range usernames {
+		userToAdd, _, err := c.GetUserByUsername(context.Background(), elem).Execute()
+		if err != nil {
+			return nil, err
+		}
+		validUsers = append(validUsers, *userToAdd)
+	}
+	return validUsers, nil
+}
+
+func GetChangesForTeamUsers(currentUsers, newUsers []admin.CloudAppUser) (toAdd, toDelete []string, err error) {
+	// Create two sets to store the elements in A and B
+	currentUsersSet := InitUserSet(currentUsers)
+	newUsersSet := InitUserSet(newUsers)
+
+	// Iterate over the elements in B and add them to the toAdd array if they are not in A
+	for elem := range newUsersSet {
+		if _, ok := currentUsersSet[elem]; !ok {
+			toAdd = append(toAdd, elem)
+		}
+	}
+
+	// Iterate over the elements in A and add them to the toDelete array if they are not in B
+	for elem := range currentUsersSet {
+		if _, ok := newUsersSet[elem]; !ok {
+			toDelete = append(toDelete, elem)
+		}
+	}
+
+	return toAdd, toDelete, nil
+}
+
+func InitUserSet(users []admin.CloudAppUser) map[string]interface{} {
+	usersSet := make(map[string]interface{}, len(users))
+	for i := 0; i < len(users); i++ {
+		usersSet[users[i].GetId()] = true
+	}
+	return usersSet
+}