Implement support for Macie member accounts #323
Conversation
zackproser
requested review from
yorinasub17,
rhoboat and
denis256
as code owners
zackproser
force-pushed
the
feature/macie
branch
from
49c7be3 to
8b19907
Compare
zackproser
changed the title
Co-authored-by: Rho <[email protected]>
|
Thanks for reviews @yorinasub17 and @rhoboat - looks like we're actually running cloud-nuke against phxdevops, so I updated our test invite and account ID and the Macie test is now passing. Current failure is a single known flaky test, so I think this is ready for another look. |
| "github.com/gruntwork-io/go-commons/errors" | ||
| ) | ||
|
|
||
| func getAllMacieMemberAccounts(session *session.Session, excludeAfter time.Time, configObj config.Config) ([]string, error) { |
There was a problem hiding this comment.
Noticed that excludeAfter and configObj aren't used now or this is for future usage?
There was a problem hiding this comment.
Yeah - unfortunately we can't really implement this pattern with Macie member accounts. Since account membership is determined by the invitation acceptance status, and since the API doesn't expose when an account last accepted an invite, we don't have much to go off.
There was a problem hiding this comment.
Perhaps remove it from the arg list and add a comment indicating that we can't support it?
aws/macie_test.go
Outdated
| // Macie is not very conducive to programmatic testing. In order to make this test work, we maintain a standing invite | ||
| // from our phxdevops test account to our nuclear-wasteland account. We can continuously "nuke" our membership because | ||
| // Macie supports a member account *that was invited* to remove its own association at any time. Meanwhile, disassociating | ||
| // in this manner does not destroy or invalidate the original invitation, which allows us to to continually re-accept it | ||
| // from our nuclear-wasteland account (where cloud-nuke tests are run), just so that we can nuke it again | ||
| // | ||
| // Macie is also regional, so for the purposes of cost-savings and lower admin overhead, we're initially only testing this | ||
| // in the one hardcoded region |
There was a problem hiding this comment.
Does this have risk of breaking our macie test in the other repos? E.g., in CIS service catalog: https://github.com/gruntwork-io/terraform-aws-cis-service-catalog/blob/master/test/security/macie_test.go
There was a problem hiding this comment.
As far as I can tell, that test is using a very similar pattern:
- The Sandbox account is the administrator account, which invites phxdevops
- Phxdevops accepts the invite, to become a member account of Sandbox
However, Macie is regional, and the CIS service catalog test is using regions in the EU, whereas this cloud-nuke Macie test is currently hardcoded to us-east-1.
I think that will prevent conflict - so long as neither test has its testing regions expanded.
There was a problem hiding this comment.
Added more context in 7438eea
|
Looks like the current test failure is |
| "github.com/gruntwork-io/go-commons/errors" | ||
| ) | ||
|
|
||
| func getAllMacieMemberAccounts(session *session.Session, excludeAfter time.Time, configObj config.Config) ([]string, error) { |
There was a problem hiding this comment.
Perhaps remove it from the arg list and add a comment indicating that we can't support it?
|
Thanks for review! I'll merge this PR now and open a follow-up to make the improvement you suggested. |
Description
Fixes #322 by implementing support for "nuking" Macie accounts that were created by Invitation only. (Macie member accounts created via AWS Organizations do not have sufficient permissions to disassociate themselves from the Org).
Programmatic testing with Macie is pretty awkward. I found that you can continuously accept an invite from another account to become a member account, then disassociate yourself (nuke), then re-use the same standing invitation to re-associate yourself as a member account. I'm currently using this workaround to leave a "standing" invite from our phxdevops account in
us-east-1for sanity.We could expand our test coverage out to other regions, but that would just require maintaining a map of regions to standing invitation IDs.
Furthermore, Macie member accounts don't expose anything useful to us in terms of filtering resources by their creation time, so the usual semantics of
excludeAfterwon't work here. As a result, I just include all found member Macie accounts as nuke-able.TODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
Added / Removed / Updated [X].
Migration Guide
Implement support for inspecting and nuking Macie member accounts that were created via Invitation only. (Macie member accounts created via AWS Organizations are not able to disassociate themselves).