-
-
Notifications
You must be signed in to change notification settings - Fork 356
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.
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
Feature/116 iam groups #364
Feature/116 iam groups #364
Conversation
Detach users from groups before attempting to remove the group
aws/iam_group.go
Outdated
return false | ||
} | ||
|
||
if excludeAfter.Before(*iamGroup.CreateDate) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer the AWS SDK convenience methods for dereferencing a pointer, i.e, aws.TimeValue
Looks like not just a terrific start, but ultimately pretty close to being merge-able! 👏 |
This is pretty non-intuitive, but every time you add a new resource to cloud-nuke, you need to go here and add the exact line So, in this case you'll need to add that line twice - otherwise you'll get a warning when trying to run tests about too few resources being defined in the config test. |
Did a bunch of smoke testing locally:
All have worked flawlessly. Well done 👍 |
"Version": "2012-10-17", | ||
"Statement": [ | ||
{ | ||
"Sid": "VisualEditor0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: Sid
is another place you could feel free to put some more debug / Grunt-facing info about this policy (as you did in the description), if you wanted: https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements_sid.html.
Might make it easier to visually identity what we're looking at once we're digging through an AWS account debugging.
@ellisonc Very nice work. This looks solid to me. Want to resolve the merge conflicts so I can approve? |
…roups # Conflicts: # config/config_test.go
@zackproser thanks! Just merged upstream back in to fix the conflict |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
Fixes #116.
Adds the ability to nuke IAM Groups
TODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
Added support for nuking IAM Groups and customer managed IAM Policies
Migration Guide