-
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
resource/aws_organizations_organization: Add enabled_policy_types argument #8588
resource/aws_organizations_organization: Add enabled_policy_types argument #8588
Conversation
…ument Reference: #4545 The `aws_organizations_policy_attachment` acceptance testing was previously written to assume that the account running it was already in an Organization, manually had enabled Service Control Policies in the Root, and did not check Root or Organizational Unit policy attachments as the appropriate attributes/resources did not exist. The updates to those tests now verify the SCP attachment workflow end-to-end with creating a new Organization, enabling SCPs, and SCP attachments to an account, Root, and OU. Previous output from acceptance testing (before `enabled_policy_types` implementation): ``` --- FAIL: TestAccAWSOrganizations/PolicyAttachment (37.12s) --- FAIL: TestAccAWSOrganizations/PolicyAttachment/Account (14.00s) testing.go:568: Step 0 error: errors during apply: Error: error creating Organizations Policy Attachment: PolicyTypeNotEnabledException: This operation can be performed only for enabled policy types. status code: 400, request id: 381509e0-7225-11e9-b974-09edfb312bea on /var/folders/v0/_d108fkx1pbbg4_sh864_7740000gn/T/tf-test737240811/main.tf line 11: (source code not available) --- FAIL: TestAccAWSOrganizations/PolicyAttachment/OrganizationalUnit (10.85s) testing.go:568: Step 0 error: errors during apply: Error: error creating Organizations Policy Attachment: PolicyTypeNotEnabledException: This operation can be performed only for enabled policy types. status code: 400, request id: 3f587964-7225-11e9-96c5-1d623fb91cbf on /var/folders/v0/_d108fkx1pbbg4_sh864_7740000gn/T/tf-test570985045/main.tf line 16: (source code not available) --- FAIL: TestAccAWSOrganizations/PolicyAttachment/Root (12.27s) testing.go:568: Step 0 error: errors during apply: Error: error creating Organizations Policy Attachment: PolicyTypeNotEnabledException: This operation can be performed only for enabled policy types. status code: 400, request id: 46589efd-7225-11e9-b974-09edfb312bea on /var/folders/v0/_d108fkx1pbbg4_sh864_7740000gn/T/tf-test865604943/main.tf line 11: (source code not available) ``` Output from acceptance testing: ``` --- PASS: TestAccAWSOrganizations/Organization (79.29s) --- PASS: TestAccAWSOrganizations/Organization/basic (13.66s) --- PASS: TestAccAWSOrganizations/Organization/AwsServiceAccessPrincipals (24.59s) --- PASS: TestAccAWSOrganizations/Organization/EnabledPolicyTypes (30.29s) --- PASS: TestAccAWSOrganizations/Organization/FeatureSet (10.75s) --- PASS: TestAccAWSOrganizations/PolicyAttachment (58.58s) --- PASS: TestAccAWSOrganizations/PolicyAttachment/Account (21.28s) --- PASS: TestAccAWSOrganizations/PolicyAttachment/OrganizationalUnit (20.48s) --- PASS: TestAccAWSOrganizations/PolicyAttachment/Root (16.82s) ```
…s argument documentation
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.
A couple of minor things but this otherwise LGTM 👍
enabledPolicyTypes := make([]string, 0) | ||
|
||
for _, policyType := range roots[0].PolicyTypes { | ||
if aws.StringValue(policyType.Status) == organizations.PolicyTypeStatusEnabled || aws.StringValue(policyType.Status) == organizations.PolicyTypeStatusPendingEnable { |
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.
should we be returning that it's active when it's still potentially Pending here?
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.
A lot of the AWS resources currently treat pending statuses similar to their end state during read to account for scenarios where changes might have occurred outside Terraform or for eventual consistency reasons. This one was a holdout before I implemented the waiter functions as during my local testing of this it didn't appear to display any eventual consistency issues. I'll remove it and verify everything still looks okay. 👍
policyType := v.(string) | ||
input := &organizations.EnablePolicyTypeInput{ | ||
PolicyType: aws.String(policyType), | ||
RootId: aws.String(d.Get("roots.0.id").(string)), |
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.
minor we can use defaultRootID
here?
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.
Yes we can! Thanks!
Output from acceptance testing: ``` --- PASS: TestAccAWSOrganizations/Organization (84.63s) --- PASS: TestAccAWSOrganizations/Organization/EnabledPolicyTypes (34.08s) --- PASS: TestAccAWSOrganizations/Organization/FeatureSet (11.33s) --- PASS: TestAccAWSOrganizations/Organization/basic (13.29s) --- PASS: TestAccAWSOrganizations/Organization/AwsServiceAccessPrincipals (25.93s) ```
Output from acceptance testing after above minor adjustments:
Merging! |
This has been released in version 2.10.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
Community Note
Closes #4545
Release note for CHANGELOG:
The
aws_organizations_policy_attachment
acceptance testing was previously written to assume that the account running it was already in an Organization, manually had enabled Service Control Policies in the Root, and did not check Root or Organizational Unit policy attachments as the appropriate attributes/resources did not previously exist. The updates to those tests now verify the SCP attachment workflow end-to-end with creating a new Organization, enabling SCPs, and SCP attachments to an account, Root, and OU.Previous output from acceptance testing (before
enabled_policy_types
implementation):Output from acceptance testing: