-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
New resource for Macie2 Organization Admin Account #19303
Conversation
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.
One change to skip the tests if they're not being run in an Organization management account, otherwise looks good
PreCheck: func() { testAccPreCheck(t) }, | ||
ProviderFactories: testAccProviderFactories, | ||
CheckDestroy: testAccCheckAwsMacie2OrganizationAdminAccountDestroy, | ||
ErrorCheck: testAccErrorCheck(t, macie2.EndpointsID), |
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.
The Organization Admin Account tests fail if they are run in an AWS Organization member (non management) account with the error AccessDeniedException: The request failed because you must be a user of the management account for your AWS organization to perform this operation
.
You can add a custom ErrorCheck
function for this resource that tests for that error message, e.g.
ErrorCheck: testAccErrorCheck(t, macie2.EndpointsID), | |
ErrorCheck: testAccErrorCheckSkipMacie2OrganizationAdminAccount(t), |
and
func testAccErrorCheckSkipMacie2OrganizationAdminAccount(t *testing.T) resource.ErrorCheckFunc {
return testAccErrorCheckSkipMessagesContaining(t,
"AccessDeniedException: The request failed because you must be a user of the management account for your AWS organization to perform this operation",
)
}
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.
Understood, thank you for the suggestion, graham 👍
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.
I missed a step in the test configuration: it needs to create an Organization
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.
One last change
resourceName := "aws_macie2_organization_admin_account.test" | ||
|
||
resource.Test(t, resource.TestCase{ | ||
PreCheck: func() { testAccPreCheck(t) }, |
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.
Because we're creating an Organization, we have to add a PreCheck
that ensures we're not already in an Organization account
PreCheck: func() { testAccPreCheck(t) }, | |
PreCheck: func() { | |
testAccPreCheck(t) | |
testAccOrganizationsAccountPreCheck(t) | |
}, |
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.
Looks good 🚀
Acceptance test results in an Organization main account
--- PASS: TestAccAWSMacie2_serial (563.77s)
...
--- PASS: TestAccAWSMacie2_serial/OrganizationAdminAccount (39.88s)
--- PASS: TestAccAWSMacie2_serial/OrganizationAdminAccount/basic (21.10s)
--- PASS: TestAccAWSMacie2_serial/OrganizationAdminAccount/disappears (18.78s)
Acceptance test results in a non Organization account
--- PASS: TestAccAWSMacie2_serial (563.77s)
...
--- PASS: TestAccAWSMacie2_serial/OrganizationAdminAccount (0.74s)
--- SKIP: TestAccAWSMacie2_serial/OrganizationAdminAccount/basic (0.36s)
--- SKIP: TestAccAWSMacie2_serial/OrganizationAdminAccount/disappears (0.38s)
This has been released in version 3.41.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Added a new resource, docs and tests for Macie Organization Admin Account called
aws_macie2_organization_admin_account
Community Note
Relates #13432
Output from acceptance testing: