fix(iam): support setting multiple users and groups with appropriate endpoint

* use SetGroupMembers

* use expo retry for setting the group membership as the API needs a bit of time to converge

Author: Gnoale

internal/services/iam/group_membership.go

@@ -3,16 +3,27 @@ package iam
 import (
 	"context"
 	"fmt"
+	"slices"
+	"sort"
 	"strings"
+	"time"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	iam "github.com/scaleway/scaleway-sdk-go/api/iam/v1alpha1"
 	"github.com/scaleway/scaleway-sdk-go/scw"
 	"github.com/scaleway/terraform-provider-scaleway/v2/internal/httperrors"
+	"github.com/scaleway/terraform-provider-scaleway/v2/internal/transport"
 	"github.com/scaleway/terraform-provider-scaleway/v2/internal/types"
 )
 
+type EntityKind string
+
+const (
+	EntityKindUser        EntityKind = "user"
+	EntityKindApplication EntityKind = "application"
+)
+
 func ResourceGroupMembership() *schema.Resource {
 	return &schema.Resource{
 		CreateContext: resourceIamGroupMembershipCreate,
@@ -23,18 +34,20 @@ func ResourceGroupMembership() *schema.Resource {
 		},
 		SchemaVersion: 0,
 		Schema: map[string]*schema.Schema{
-			"user_id": {
-				Type:         schema.TypeString,
+			"user_ids": {
+				Type:         schema.TypeList,
+				Elem:         &schema.Schema{Type: schema.TypeString},
 				Optional:     true,
-				Description:  "The ID of the user",
-				ExactlyOneOf: []string{"application_id"},
+				ExactlyOneOf: []string{"application_ids"},
+				Description:  "The IDs of the users",
 				ForceNew:     true,
 			},
-			"application_id": {
-				Type:         schema.TypeString,
+			"application_ids": {
+				Type:         schema.TypeList,
+				Elem:         &schema.Schema{Type: schema.TypeString},
 				Optional:     true,
-				Description:  "The ID of the user",
-				ExactlyOneOf: []string{"user_id"},
+				ExactlyOneOf: []string{"user_ids"},
+				Description:  "The IDs of the applications",
 				ForceNew:     true,
 			},
 			"group_id": {
@@ -50,27 +63,27 @@ func ResourceGroupMembership() *schema.Resource {
 func resourceIamGroupMembershipCreate(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
 	api := NewAPI(m)
 
-	userID := types.ExpandStringPtr(d.Get("user_id"))
-	applicationID := types.ExpandStringPtr(d.Get("application_id"))
+	userIDs := types.ExpandStrings(d.Get("user_ids"))
+	applicationIDs := types.ExpandStrings(d.Get("application_ids"))
 
-	group, err := api.AddGroupMember(&iam.AddGroupMemberRequest{
-		GroupID:       d.Get("group_id").(string),
-		UserID:        userID,
-		ApplicationID: applicationID,
-	}, scw.WithContext(ctx))
+	group, err := MakeSetGroupMembershipRequest(ctx, api, &iam.SetGroupMembersRequest{
+		GroupID:        d.Get("group_id").(string),
+		UserIDs:        userIDs,
+		ApplicationIDs: applicationIDs,
+	})
 	if err != nil {
 		return diag.FromErr(err)
 	}
 
-	d.SetId(GroupMembershipID(group.ID, userID, applicationID))
+	d.SetId(SetGroupMembershipResourceID(group.ID, userIDs, applicationIDs))
 
 	return resourceIamGroupMembershipRead(ctx, d, m)
 }
 
 func resourceIamGroupMembershipRead(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
 	api := NewAPI(m)
 
-	groupID, userID, applicationID, err := ExpandGroupMembershipID(d.Id())
+	groupID, entityKind, entityIDs, err := ExpandGroupMembershipResourceID(d.Id())
 	if err != nil {
 		return diag.FromErr(err)
 	}
@@ -88,58 +101,46 @@ func resourceIamGroupMembershipRead(ctx context.Context, d *schema.ResourceData,
 		return diag.FromErr(err)
 	}
 
-	foundInGroup := false
-
-	if userID != "" {
-		for _, groupUserID := range group.UserIDs {
-			if groupUserID == userID {
-				foundInGroup = true
+	foundEntityIDs := make([]bool, len(entityIDs))
 
-				break
+	if entityKind == EntityKindUser {
+		for i, groupUserID := range group.UserIDs {
+			if slices.Contains(entityIDs, groupUserID) {
+				foundEntityIDs[i] = true
 			}
 		}
-	} else if applicationID != "" {
-		for _, groupApplicationID := range group.ApplicationIDs {
-			if groupApplicationID == applicationID {
-				foundInGroup = true
-
-				break
+	} else if entityKind == EntityKindApplication {
+		for i, groupApplicationID := range group.ApplicationIDs {
+			if slices.Contains(entityIDs, groupApplicationID) {
+				foundEntityIDs[i] = true
 			}
 		}
 	}
 
-	if !foundInGroup {
+	if slices.Contains(foundEntityIDs, false) {
 		d.SetId("")
 
 		return nil
 	}
 
 	_ = d.Set("group_id", groupID)
-	_ = d.Set("user_id", userID)
-	_ = d.Set("application_id", applicationID)
+	_ = d.Set(fmt.Sprintf("%s_ids", entityKind), entityIDs)
 
 	return nil
 }
 
 func resourceIamGroupMembershipDelete(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
 	api := NewAPI(m)
 
-	groupID, userID, applicationID, err := ExpandGroupMembershipID(d.Id())
+	groupID, _, _, err := ExpandGroupMembershipResourceID(d.Id())
 	if err != nil {
 		return diag.FromErr(err)
 	}
 
-	req := &iam.RemoveGroupMemberRequest{
+	_, err = MakeSetGroupMembershipRequest(ctx, api, &iam.SetGroupMembersRequest{
 		GroupID: groupID,
-	}
-
-	if userID != "" {
-		req.UserID = &userID
-	} else if applicationID != "" {
-		req.ApplicationID = &applicationID
-	}
+	})
 
-	_, err = api.RemoveGroupMember(req, scw.WithContext(ctx))
 	if err != nil {
 		if httperrors.Is404(err) {
 			d.SetId("")
@@ -153,28 +154,60 @@ func resourceIamGroupMembershipDelete(ctx context.Context, d *schema.ResourceDat
 	return nil
 }
 
-func GroupMembershipID(groupID string, userID *string, applicationID *string) string {
-	if userID != nil {
-		return fmt.Sprintf("%s/user/%s", groupID, *userID)
+func SetGroupMembershipResourceID(groupID string, userIDs []string, applicationIDs []string) (resourceID string) {
+	sort.Strings(userIDs)
+	sort.Strings(applicationIDs)
+
+	if len(userIDs) > 0 {
+		resourceID = fmt.Sprintf("%s/%s/%s", groupID, EntityKindUser, strings.Join(userIDs, ","))
+	} else if len(applicationIDs) > 0 {
+		resourceID = fmt.Sprintf("%s/%s/%s", groupID, EntityKindApplication, strings.Join(applicationIDs, ","))
 	}
 
-	return fmt.Sprintf("%s/app/%s", groupID, *applicationID)
+	return
 }
 
-func ExpandGroupMembershipID(id string) (groupID string, userID string, applicationID string, err error) {
+func ExpandGroupMembershipResourceID(id string) (groupID string, kind EntityKind, entityIDs []string, err error) {
 	elems := strings.Split(id, "/")
 	if len(elems) != 3 {
-		return "", "", "", fmt.Errorf("invalid group member id format, expected {groupID}/{type}/{memberID}, got: %s", id)
+		return "", "", []string{}, fmt.Errorf("invalid group membership id format, expected {groupID}/{entityKind}/{entityIDs}, got: %s", id)
 	}
 
 	groupID = elems[0]
-
-	switch elems[1] {
-	case "user":
-		userID = elems[2]
-	case "app":
-		applicationID = elems[2]
+	kind = EntityKind(elems[1])
+	if kind != EntityKindUser && kind != EntityKindApplication {
+		return "", "", []string{}, fmt.Errorf("invalid entity kind, expected %s or %s, got: %s", EntityKindUser, EntityKindApplication, kind)
 	}
+	entityIDs = strings.Split(elems[2], ",")
 
 	return
 }
+
+func MakeSetGroupMembershipRequest(ctx context.Context, api *iam.API, request *iam.SetGroupMembersRequest) (*iam.Group, error) {
+	retryInterval := 250 * time.Millisecond
+	maxRetries := 10
+
+	if transport.DefaultWaitRetryInterval != nil {
+		retryInterval = *transport.DefaultWaitRetryInterval
+	}
+
+	var response *iam.Group
+	var err error
+
+	// exponential backoff
+	for i := 0; i < maxRetries; i++ {
+		response, err = api.SetGroupMembers(request, scw.WithContext(ctx))
+		if err != nil {
+			if httperrors.Is409(err) && strings.Contains(err.Error(), fmt.Sprintf("resource group with ID %s is in a transient state: updating", request.GroupID)) {
+				time.Sleep(retryInterval * time.Duration(i))
+				continue
+			}
+
+			return nil, err
+		}
+
+		return response, nil
+	}
+
+	return nil, fmt.Errorf("failed to set group membership after %d retries", maxRetries)
+}

internal/services/iam/group_membership_test.go

@@ -36,7 +36,7 @@ func TestAccGroupMembership_Basic(t *testing.T) {
 
 					resource scaleway_iam_group_membership main {
 						group_id = scaleway_iam_group.main.id
-						application_id = scaleway_iam_application.main.id
+						application_ids = [scaleway_iam_application.main.id]
 					}
 				`,
 				Check: resource.ComposeTestCheckFunc(
@@ -57,12 +57,12 @@ func TestAccGroupMembership_Basic(t *testing.T) {
 
 					resource scaleway_iam_group_membership main {
 						group_id = scaleway_iam_group.main.id
-						application_id = scaleway_iam_application.main.id
+						application_ids = [scaleway_iam_application.main.id]
 					}
 
 					resource scaleway_iam_group_membership import {
 						group_id = scaleway_iam_group.main.id
-						application_id = scaleway_iam_application.main.id
+						application_ids = [scaleway_iam_application.main.id]
 					}
 				`,
 				ImportState:  true,
@@ -71,7 +71,7 @@ func TestAccGroupMembership_Basic(t *testing.T) {
 					groupID := state.RootModule().Resources["scaleway_iam_group.main"].Primary.ID
 					applicationID := state.RootModule().Resources["scaleway_iam_application.main"].Primary.ID
 
-					return iam.GroupMembershipID(groupID, nil, &applicationID), nil
+					return iam.SetGroupMembershipResourceID(groupID, nil, []string{applicationID}), nil
 				},
 				ImportStatePersist: true,
 			},
@@ -88,12 +88,12 @@ func TestAccGroupMembership_Basic(t *testing.T) {
 
 					resource scaleway_iam_group_membership main {
 						group_id = scaleway_iam_group.main.id
-						application_id = scaleway_iam_application.main.id
+						application_ids = [scaleway_iam_application.main.id]
 					}
 
 					resource scaleway_iam_group_membership import {
 						group_id = scaleway_iam_group.main.id
-						application_id = scaleway_iam_application.main.id
+						application_ids = [scaleway_iam_application.main.id]
 					}
 				`,
 				PlanOnly: true,
@@ -154,12 +154,12 @@ func testAccCheckIamGroupMembershipApplicationInGroup(tt *acctest.TestTools, n s
 
 		api := iam.NewAPI(tt.Meta)
 
-		groupID, _, applicationID, err := iam.ExpandGroupMembershipID(rs.Primary.ID)
+		groupID, _, applicationID, err := iam.ExpandGroupMembershipResourceID(rs.Primary.ID)
 		if err != nil {
 			return err
 		}
 
-		if applicationID != expectedApplicationID {
+		if applicationID[0] != expectedApplicationID {
 			return fmt.Errorf("group membership id does not contain expected application id, expected %s, got %s", expectedApplicationID, applicationID)
 		}
 
@@ -173,7 +173,7 @@ func testAccCheckIamGroupMembershipApplicationInGroup(tt *acctest.TestTools, n s
 		foundInGroup := false
 
 		for _, groupApplicationID := range group.ApplicationIDs {
-			if groupApplicationID == applicationID {
+			if groupApplicationID == applicationID[0] {
 				foundInGroup = true
 			}
 		}
@@ -202,12 +202,12 @@ func testAccCheckIamGroupMembershipUserInGroup(tt *acctest.TestTools, n string,
 
 		api := iam.NewAPI(tt.Meta)
 
-		groupID, userID, _, err := iam.ExpandGroupMembershipID(rs.Primary.ID)
+		groupID, _, userID, err := iam.ExpandGroupMembershipResourceID(rs.Primary.ID)
 		if err != nil {
 			return err
 		}
 
-		if userID != expectedUserID {
+		if userID[0] != expectedUserID {
 			return fmt.Errorf("group membership id does not contain expected user id, expected %s, got %s", expectedUserID, userID)
 		}
 
@@ -221,7 +221,7 @@ func testAccCheckIamGroupMembershipUserInGroup(tt *acctest.TestTools, n string,
 		foundInGroup := false
 
 		for _, groupUserID := range group.UserIDs {
-			if groupUserID == userID {
+			if groupUserID == userID[0] {
 				foundInGroup = true
 			}
 		}