Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Reestablish old behavior of okta_group_memberships resource #1161

Merged
merged 1 commit into from
Jun 9, 2022
Merged

Reestablish old behavior of okta_group_memberships resource #1161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
198 changes: 143 additions & 55 deletions okta/resource_okta_group_memberships.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ func resourceGroupMemberships() *schema.Resource {
Description: "The list of Okta user IDs which the group should have membership managed for.",
Elem: &schema.Schema{Type: schema.TypeString},
},
"track_all_users": {
Type: schema.TypeBool,
Optional: true,
Default: false,
Description: "The resource concerns itself with all users added/deleted to the group; even those managed outside of the resource.",
},
},
}
}
Expand All @@ -60,6 +66,7 @@ func resourceGroupMembershipsCreate(ctx context.Context, d *schema.ResourceData,
// adding users to a group. Use a backoff to wait for at list one user to be
// associated with the group.
err = backoff.Retry(func() error {
// TODO, should we wait for all users to be added to the group?
ok, err := checkIfGroupHasUsers(ctx, client, groupId, users)
if err != nil {
return backoff.Permanent(err)
Expand All @@ -77,36 +84,35 @@ func resourceGroupMembershipsCreate(ctx context.Context, d *schema.ResourceData,
}

func resourceGroupMembershipsRead(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
groupId := d.Get("group_id").(string)
client := getOktaClientFromMetadata(m)
expected_users := convertInterfaceToStringSetNullable(d.Get("users"))
groupId := d.Get("group_id").(string)
oldUsers := convertInterfaceToStringSetNullable(d.Get("users"))
trackAllUsers := d.Get("track_all_users").(bool)

// handle import edge case
if len(expected_users) == 0 {
logger(m).Info("reading group membership", "id", d.Id())
userIDList, err := listGroupUserIDs(ctx, m, d.Id())
// New behavior, tracking all users.
if trackAllUsers {
changed, newUserIDs, err := checkIfUsersHaveChanged(ctx, client, groupId, &oldUsers)
if err != nil {
return diag.Errorf("unable to return membership for group (%s) from API", d.Id())
return diag.Errorf("An error occured checking user ids for group %q, error: %+v", groupId, err)
}
d.Set("group_id", d.Id())
d.Set("users", convertStringSliceToSet(userIDList))
if changed {
// set the new user ids if users have changed
d.Set("users", convertStringSliceToSet(*newUserIDs))
}

return nil
}

ok, err := checkIfGroupHasUsers(ctx, client, groupId, expected_users)
// Legacy behavior is just to check if any users have left the group.
changed, newUserIDs, err := checkIfUsersHaveBeenRemoved(ctx, client, groupId, &oldUsers)
if err != nil {
return diag.Errorf("unable to complete group check for user: %v", err)
return diag.Errorf("An error occured checking user ids for group %q, error: %+v", groupId, err)
}
if ok {
d.Set("group_id", d.Id())
userIDList, _ := listGroupUserIDs(ctx, m, d.Id())
d.Set("users", convertStringSliceToSet(userIDList))
return nil
} else {
d.SetId("")
logger(m).Info("group (%s) did not have expected memberships or did not exist", groupId)
return nil
if changed {
// The user list has changed, set the new user ids to the users value.
d.Set("users", convertStringSliceToSet(*newUserIDs))
}
return nil
}

func resourceGroupMembershipsDelete(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
Expand Down Expand Up @@ -145,60 +151,142 @@ func resourceGroupMembershipsUpdate(ctx context.Context, d *schema.ResourceData,
return nil
}

// checkIfGroupHasUsers firstly checks if the group has users given an immediate
// API call to list group users. It will additionally compare the current list
// of users found in the API call with a slice of known users passed in to the
// function. This second comparison is a stateful comparison.
func checkIfGroupHasUsers(ctx context.Context, client *okta.Client, groupId string, users []string) (bool, error) {
// TODO: This method should be renamed and/or refactored. Its name implies
// it is only checking for users but it can return false to signal that
// users have changed.
// checkIfUsersHaveChanged If the function returns true then users have been
// changed and the returned user ids should be considered the new set of users.
// Returns error for API errors. Returns false if no users have changed and the
// slice of returned strings will be empty.
func checkIfUsersHaveChanged(ctx context.Context, client *okta.Client, groupId string, users *[]string) (bool, *[]string, error) {
noop := []string{}
if users == nil || len(*users) == 0 {
return false, &noop, nil
}

// We are using the old users map as a ledger to find users that have been
// removed from the user list.
oldUsers := toStrIndexedMap(users)
changed := false
// Collect all user ids that are returned from the API
usersFromAPI := []string{}

groupUsers, resp, err := client.Group.ListGroupUsers(ctx, groupId, &query.Params{Limit: defaultPaginationLimit})
if err := suppressErrorOn404(resp, err); err != nil {
return false, fmt.Errorf("unable to return membership for group (%s) from API", groupId)
return false, &noop, fmt.Errorf("unable to list users for group (%s) from API, error: %+v", groupId, err)
}

for _, user := range groupUsers {
// if the new user id is not in the old users map then the list of users has changed
if _, found := (*oldUsers)[user.Id]; !found {
changed = true
}
usersFromAPI = append(usersFromAPI, user.Id)
}

for resp.HasNextPage() {
var additionalUsers []*okta.User
resp, err = resp.Next(context.Background(), &additionalUsers)
groupUsers = nil
resp, err = resp.Next(context.Background(), &groupUsers)
if err != nil {
return false, fmt.Errorf("unable to return membership for group (%s) from API", groupId)
return false, &noop, fmt.Errorf("unable to list users for group (%s) from API, error: %+v", groupId, err)
}
groupUsers = append(groupUsers, additionalUsers...)

for _, user := range groupUsers {
// if the new user id is not in the old users map then the list of users has changed
if _, found := (*oldUsers)[user.Id]; !found {
changed = true
}
usersFromAPI = append(usersFromAPI, user.Id)
}
}
if len(*oldUsers) != len(usersFromAPI) {
changed = true
}

// We need to return false if there isn't any users present. For eventual
// consistency issues create has a backoff/retry when we return false
// without an error here. Read occurs in the future and so it doesn't guard
// against eventual consistency.
if len(groupUsers) == 0 {
return false, nil
var result *[]string = &noop
if changed {
result = &usersFromAPI
}

return changed, result, nil
}

// checkIfUsersHaveBeenRemoved If the function returns true then users have been
// removed and the subset of returned user ids should be considered the new set
// of users. Returns error for API errors. Returns false if no users have been
// removed and the slice of returned strings will be empty.
func checkIfUsersHaveBeenRemoved(ctx context.Context, client *okta.Client, groupId string, users *[]string) (bool, *[]string, error) {
noop := []string{}
if users == nil || len(*users) == 0 {
return false, &noop, nil
}

// Create a set to compare the users slice passed into the check to compare
// with what the api returns from list group users API call.
expectedUserSet := make(map[string]bool)
// We are using the old users map as a ledger to find users that have been
// removed from the user list. If it ever becomes sized 0 then we've found
// all of our user ids and no longer have to make API calls.
oldUsers := toStrIndexedMap(users)

// We train the set with false values for our previously known users.
for _, user := range users {
expectedUserSet[user] = false
groupUsers, resp, err := client.Group.ListGroupUsers(ctx, groupId, &query.Params{Limit: defaultPaginationLimit})
if err := suppressErrorOn404(resp, err); err != nil {
return false, &noop, fmt.Errorf("unable to list users for group (%s) from API, error: %+v", groupId, err)
}

// We confirm the latest user ids from list group users API call are still
// present in the set.
for _, user := range groupUsers {
if _, ok := expectedUserSet[user.Id]; ok {
expectedUserSet[user.Id] = true
// Deleting user from API from the old users map
delete(*oldUsers, user.Id)
if len(*oldUsers) == 0 {
// All old users have been accounted for.
return false, &noop, nil
}
}

for resp.HasNextPage() {
groupUsers = nil
resp, err = resp.Next(context.Background(), &groupUsers)
if err != nil {
return false, &noop, fmt.Errorf("unable to list users for group (%s) from API, error: %+v", groupId, err)
}
for _, user := range groupUsers {
// Deleting user from API from the old users map
delete(*oldUsers, user.Id)
if len(*oldUsers) == 0 {
// All old users have been accounted for.
return false, &noop, nil
}
}
}

// If one of the known users in the call to the check function is no longer
// in the list group users API call we return false.
for _, state := range expectedUserSet {
if !state {
return false, nil
// Any old users left are the IDs that have been removed from group.

// This loop keeps the returned new user list in the same order as the users
// list passed in minus the now missing user IDs.
newUsers := []string{}
for _, userId := range *users {
// Any single user id found in old users should not be appended to the
// new users result.
if _, found := (*oldUsers)[userId]; !found {
newUsers = append(newUsers, userId)
}
}

return true, nil
return true, &newUsers, nil
}

func checkIfGroupHasUsers(ctx context.Context, client *okta.Client, groupId string, users []string) (bool, error) {
groupUsers, resp, err := client.Group.ListGroupUsers(ctx, groupId, &query.Params{Limit: defaultPaginationLimit})
if err := suppressErrorOn404(resp, err); err != nil {
return false, fmt.Errorf("unable to return membership for group (%s) from API", groupId)
}
return (len(groupUsers) > 0), nil
}

func toStrIndexedMap(strs *[]string) *map[string]int {
result := map[string]int{}
if strs == nil {
return &result
}

length := len(*strs)
for i := 0; i < length; i++ {
result[(*strs)[i]] = i
}

return &result
}
Loading