feat: wildcard support in ignore-users and ignore-groups#4
Closed
kesor wants to merge 30 commits into
Closed
Conversation
…ressions (awslabs#303) ssue #, if available: awslabs#299 Description of changes: Refines the regex for rate: rate\((?:(?:\d{2,}+|[2-9]) (?:minutes|hours|days))|(?:1 (?:minute|hour|day))\) Allows for: rate(9 hours) rate(17 hours) rate(57 hours) rate(123 days) rate(1 hour) but rejects: rate(2 hour) rate(20 hour) rate(1 hours) By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.40.0 to 0.45.0. - [Commits](golang/crypto@v0.40.0...v0.45.0) --- updated-dependencies: - dependency-name: golang.org/x/crypto dependency-version: 0.45.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
* Feat: Delete users if inactive only * Update ScheduleExpression regex --------- Co-authored-by: Luis Moreira <luismendesmoreira@hotmail.com>
Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.74.2 to 1.79.3. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](grpc/grpc-go@v1.74.2...v1.79.3) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-version: 1.79.3 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [go.opentelemetry.io/otel](https://github.com/open-telemetry/opentelemetry-go) from 1.39.0 to 1.41.0. - [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases) - [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md) - [Commits](open-telemetry/opentelemetry-go@v1.39.0...v1.41.0) --- updated-dependencies: - dependency-name: go.opentelemetry.io/otel dependency-version: 1.41.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
* Edit the regex for the CloudFormation template for the PRECACHE_ORG_UNITS, to allow for a single OU path other than '/' * Updated handling of this environment variable so, if specified but and empty string or not set it disables precaching. * Updated handling of other optional comma separated string env_variables to have the same way.
* Edit the regex for the CloudFormation template for the PRECACHE_ORG_UNITS, to allow for a single OU path other than '/' * Updated handling of this environment variable so, if specified but and empty string or not set it disables precaching. * Updated handling of other optional comma separated string env_variables to have the same way.
Included in this change: Parameter PrecacheOrgUnits : When left empty will disable the pre-caching of both Groups and Users Parameter LogRetention : Added, config CloudWatch Log Group retention period, previously it defaulted to Indefinitely this default has been retain however, it is strongly recommended a more frugal option is selected. Parameter ScheduleExpression : When left empty will disable scheduling of the SSOSync lambda function. The default for this is unchanged Rate(15 minutes), where the lambda is being triggered by an external event or as part of CICD pipeline (such ac CodePipeline), this prevent concurrency limits being encountered. The Parameters page has also been re-grouped to be more intuitive. Quick Start templates: Added a simple template that can launched directly from CloudFormation, this can simply be launched from the repo. Currently, template creates a deployment in a single account with two nested stacks one for the secrets and the other for the lambda function. To update the deployment, download the latest version of the template and update the stack. * implement disabled schedule * Apply disable PreCache to Groups * Improve logging for precache activity * Remove Explicit RoleName * Added Log Retention Setting. * Tidy Up the Parameters page * Add QuickStart * Add job to update version strings in quickstart on release
This reverts commit 561a1c6.
Adjust SemanticVersion update sed regex Update node actions to latest versions
There was a problem hiding this comment.
Pull request overview
This PR adds wildcard (*) pattern support to --ignore-users and --ignore-groups, and extends ignore handling so matching AWS-only users/groups are also excluded from deletion operations.
Changes:
- Implemented a custom
*-only matcher (matchIgnorePattern) and cached, trimmed ignore patterns onsyncGSuite. - Updated
getUserOperations/getGroupOperationsto consult ignore predicates before scheduling AWS-only deletions. - Updated README documentation and added unit tests covering wildcard matching and the updated operations behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| README.md | Documents wildcard semantics and deletion-guard behavior for ignore lists. |
| internal/sync.go | Replaces exact-match ignore sets with cached pattern lists; adds * matcher and uses ignore predicates to prevent deletes. |
| internal/sync_test.go | Adds tests for wildcard matching, ignore trimming/empty handling, and delete-list filtering with ignore predicates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
868
to
+873
| for _, awsGroup := range awsGroups { | ||
| if _, found := googleMap[awsGroup.DisplayName]; !found { | ||
| if ignoreGroup != nil && ignoreGroup(awsGroup.DisplayName) { | ||
| log.WithField("group", awsGroup.DisplayName).Info("skip delete: ignore list") | ||
| continue | ||
| } |
Comment on lines
+194
to
+198
| Ignored users and groups are protected in both directions: they are skipped | ||
| when Google is enumerated, and they are also skipped when picking AWS | ||
| entries to delete. This means a pattern like `*@contractors.example.com` | ||
| acts as a deletion guard for accounts that exist only in AWS. | ||
|
|
Allow `*` in --ignore-users and --ignore-groups entries. Only `*` is special (matches any, possibly empty, substring); every other character including `?`, `[`, `]`, and `\` is matched literally. The matcher is hand-rolled rather than using path.Match so patterns cannot pick up extra metacharacter semantics. The ignore predicate is also consulted when picking AWS-only entries to delete, so a pattern like `*@contractors.example.com` protects matching AWS users from deletion even when they have no Google counterpart. Patterns are trimmed once and cached on the syncGSuite; empty entries are dropped. Inspired by awslabs#306, rewritten to avoid path.Match, drop the redundant double-check in the deletion loops, and remove per-call debug log spam.
6e912df to
7e0586d
Compare
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.
Summary
Allow
*in--ignore-usersand--ignore-groupsentries so patterns like*@contractors.example.comorAWS*can be used.*is special — it matches any (possibly empty) substring. Every other character, including?,[,], and\, is matched literally. The matcher is hand-rolled rather than usingpath.Match/filepath.Matchso patterns cannot pick up extra metacharacter semantics.getUserOperations/getGroupOperations), so a wildcard pattern protects matching AWS users/groups from deletion even when they have no Google counterpart.syncGSuite; empty entries are dropped.Inspired by awslabs#306, rewritten to:
path.Match(no?/[]/\semantics that could be misused),Checking wildcard patterndebug spam,Test plan
go test ./internal/ -run 'TestMatch|TestIgnore|TestGet'passes--ignore-users '*@contractors.example.com'against a Google + AWS pair and confirm matching AWS users are not deleted--ignore-groups 'AWS*'and confirm matching AWS groups are not deletedTests added
matchIgnorePattern: exact, leading / trailing / middle / multiple wildcards, edge cases (*matches empty,**), and that?/[]/\are literal.ignoreUser/ignoreGroup: whitespace trimming and empty-entry handling.getUserOperations/getGroupOperations: ignored AWS-only entries are excluded from the delete list; nil predicate is handled.Notes
The pre-existing
internal/aws/client_test.gobuild failure (missinginternal/mocks) is present onmasterand unrelated to this change.