feat(scim): replace brute-force member sync with members.value pagination#534
Merged
Conversation
…tion Closes #520. The AWS IAM Identity Center SCIM ListGroups endpoint now supports the `members.value eq "<user-id>"` filter together with cursor-based pagination, so a sync run no longer needs to issue one ListGroups call per (group, user) pair to reconstruct group membership. `internal/scim.GetGroupsMembers(ctx, groups, users)` now runs one paginated query per user (~O(users) calls) and inverts the result into the existing group → members map, replacing the previous O(groups × users) brute-force loop and its 10-150ms random sleep throttle. The concurrency cap is now exercised under Go 1.26's `testing/synctest` so the test asserts the true peak in-flight count using a virtual clock instead of waiting on a wall-clock sleep race. The legacy `GetGroupsMembers(ctx, gr)` and `GetGroupsMembersBruteForce` methods are removed without a compatibility shim. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Restructures the Unreleased entry for the SCIM members sync change to lead with an IMPORTANT callout and dedicated Security / Performance subsections, making the impact of closing #520 explicit for readers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes
Milestone: v0.45.0.
Summary
internal/scim.GetGroupsMembers(ctx, groups, users)now runs one cursor-paginatedListGroups?filter=members.value eq "<user-id>"request per user and inverts the response into the existing group → members map. The previous brute-forceO(N_groups × N_users)loop and its 10–150ms random sleep throttle are gone.pkg/aws.SCIMServicegainsListGroupsWithCursor(ctx, filter, cursor);pkg/aws.ListResponsegainsNextCursor.ListGroupsis unchanged for its remaining single-page callers.internal/core.SCIMService.GetGroupsMemberssignature updated to accept users;GetGroupsMembersBruteForceand the brokenGetGroupsMembers(ctx, gr)are deleted with no compatibility shim. Mocks regenerated viago generate ./....testing/synctest— virtual clock + bubble lets the test assert the true peak in-flight count deterministically.For an org with 200 groups and 500 users this drops the per-sync call count from ~100,000 to ~500 (roughly two orders of magnitude lighter on the AWS SCIM endpoint). Enabled by two AWS features documented in the SCIM ListGroups reference: the
members.valuefilter and cursor-based pagination (page cap raised from 50 to 100).Docs:
Test plan
go build ./...go vet ./...go test ./...(full suite green; new tests underpkg/awsandinternal/scim, integration mock ininternal/core/sync_test.gorewired to the new query shape)make go-fmt+make go-betteraligngolangci-lint run ./...— no new findings (remaining 15 errcheck warnings are all pre-existing in untouched files)🤖 Generated with Claude Code