[WIP] support setting the parent for organization accounts #4405

afeld · 2018-05-01T06:41:52Z

Part of #571. Builds on #4207 - diff.

afeld · 2018-05-01T07:04:01Z

Getting some errors I don't understand; could use another set of eyes on this.

$ TEST_AWS_ORGANIZATION_ACCOUNT_EMAIL_DOMAIN=gmail.com make testacc TEST=./aws TESTARGS="-run TestAccAWSOrganizations/Account/basic"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run TestAccAWSOrganizations/Account/basic -timeout 120m
=== RUN   TestAccAWSOrganizations
=== RUN   TestAccAWSOrganizations/Account
=== RUN   TestAccAWSOrganizations/Account/basic
--- FAIL: TestAccAWSOrganizations (18.84s)
    --- FAIL: TestAccAWSOrganizations/Account (18.83s)
        --- FAIL: TestAccAWSOrganizations/Account/basic (18.83s)
        	testing.go:518: Step 0 error: Check failed: Check 4/8 error: aws_organizations_account.test: Attribute 'joined_timestamp' expected to be set
        	testing.go:579: Error destroying resource! WARNING: Dangling resources
        		may exist. The full state and error is shown below.

        		Error: Error applying: 1 error(s) occurred:

        		* aws_organizations_account.test (destroy): 1 error(s) occurred:

        		* aws_organizations_account.test: ConstraintViolationException: The member account must be configured with a payment method, such as a credit card.
        			status code: 400, request id: XXXXXXX

        		State: aws_organizations_account.test:
        		  ID = XXXXXXX
        		  provider = provider.aws
        		  arn = arn:aws:organizations::XXXXXXXX:account/o-XXXXXXX/XXXXXXXX
        		  email = tf-acctest+XXXXXXXXXX@gmail.com
        		  joined_method = CREATED
        		  name = tf_acctest_XXXXXXXXXX
        		  parent_id = r-XXXX
        		  status = ACTIVE

Attribute 'joined_timestamp' expected to be set

Why would this be missing, of all things?

Error destroying resource!

The error is what I would expect if Terraform were trying to delete the account, ~~but this error isn't happening on other branches, so somehow I'm causing a change in the cleanup behavior~~. Anyone have ideas?

afeld · 2018-05-01T07:14:04Z

Correction: I am seeing those same errors on master. @asedge Is there a trick to running these tests?

asedge · 2018-05-01T14:08:37Z

@afeld This is the error:

ConstraintViolationException: The member account must be configured with a payment method, such as a credit card.

AWS Organization member accounts are a real pain to try to clean up.

asedge · 2018-05-01T14:12:10Z

Here was the discussion around trying to do the delete: #3524 (comment). Prior, I just had printed a message that said you need to do the delete from the Web UI or something along those lines - which wasn't a great solution.

bflad · 2018-05-01T16:32:47Z

This resource is a real pain to acceptance test due to how Organizations/AWS accounts in general work. Basically, its not really doable in an automated fashion since AWS requires converting accounts to standalone (requiring EULA agreement, credit card information, phone verification, etc.) and there is no simple DeleteAccount API call.

I'm not sure we want to implement a flag attribute to leave the account dangling in the organization during resource deletion just for the acceptance testing either (also doesn't seem like a valid regular use case when there is state rm). 🙁

We could switch this to service/organizations/iface for the acceptance testing, but that also has its own problems with keeping it appropriately up to date.

In short, I'm not sure there is a good way to handle testing for this resource 😅 -- I wanted to get the resource merged as it worked as expected with manual testing. I would highly recommend opening an AWS support case if you have a support plan that asks for more automated ways to handle deleting an account (either the missing DeleteAccount API call or APIs for the billing/EULA pieces. Personally, I'd accept a PR with manual testing for now. 👍

breser · 2018-05-01T18:10:45Z

I have Enterprise Support at work and I'm starting to use this stuff for work. So I'll start a discussion with them around this.

afeld · 2018-05-01T23:46:52Z

@asedge Right, the error makes sense to me... Are you saying that the basic acceptance test is known to fail on master?

I have Enterprise Support at work and I'm starting to use this stuff for work. So I'll start a discussion with them around this.

Yep, I already have as well, but couldn't hurt for them to hear it again. They are understandably reluctant to make it possible to delete all of your infrastructure with one API call.

breser · 2018-05-03T02:56:31Z

@afeld They added us to the list of ~40 customers that have asked for this.

afeld · 2018-05-06T00:48:33Z

Repeating my question from above: is the basic acceptance test is known to fail on master?

bflad · 2018-05-18T13:52:10Z

Repeating my question from above: is the basic acceptance test is known to fail on master?

Yes, see my comment above about the various issues encountered trying to implement the automated testing. I would accept a manually tested PR in this rare scenario. Adding automated tests with t.Skip() would be nice as well in case there is a good workaround in the future.

This will be more future-proof, allowing support for non-root organizational units.

…ations_unit data source

NissesSenap · 2019-03-14T14:52:05Z

Any new status on this pull request?
I can understand all the billing stuff is a pain, would a possible way forward be to split up the easier stuff like aws_organizations_unit be split up to a separate pull request so it gets merged?

afeld · 2019-03-14T15:01:20Z

split up the easier stuff like aws_organizations_unit

See #4207.

bflad · 2019-05-08T20:56:51Z

@afeld would you be okay if I cherry-picked the following two of your commits into a new pull request?

Now that we have Organizational Unit support would love to use your work as a starting point for this enhancement without the messy rebase. Thanks!

afeld · 2019-05-09T01:19:36Z

Go for it!

…entation References: * #4405 * #8281 Please note that automated acceptance testing is not currently possible with this resource, due to manual steps required to remove an account from an organization: https://docs.aws.amazon.com/organizations/latest/userguide/orgs_manage_accounts_remove.html These changes were manually verified via the following. Given an existing configuration, previously applied with version 2.9.0 of the Terraform AWS Provider: ```hcl resource "aws_organizations_organization" "organization" { feature_set = "ALL" } resource "aws_organizations_account" "bflad-dev1" { name = "bflad-dev1" email = "--OMITTED--" } resource "aws_organizations_account" "bflad-dev2" { name = "bflad-dev2" email = "--OMITTED--" } ``` Overwrite Terraform AWS Provider binary including this changeset, ensure plan shows no changes, and ensure `parent_id` is properly written to Terraform state: ```console $ cp ~/go/bin/terraform-provider-aws .terraform/plugins/darwin_amd64/terraform-provider-aws_v2.9.0_x4 $ terraform init ... $ terraform plan ... aws_organizations_organization.organization: Refreshing state... (ID: o-p687o6l073) aws_organizations_account.bflad-dev2: Refreshing state... (ID: --OMITTED--) aws_organizations_account.bflad-dev1: Refreshing state... (ID: --OMITTED--) ------------------------------------------------------------------------ No changes. Infrastructure is up-to-date. $ terraform refresh ... $ terraform state show aws_organizations_account.bflad-dev1 | grep parent_id parent_id = r-cg2b ``` Add organizational unit to configuration and add `parent_id` to an existing account pointing to it: ```hcl resource "aws_organizations_organization" "organization" { feature_set = "ALL" } resource "aws_organizations_organizational_unit" "test1" { name = "test1" parent_id = "${aws_organizations_organization.organization.roots.0.id}" } resource "aws_organizations_account" "bflad-dev1" { name = "bflad-dev1" email = "--OMITTED--" parent_id = "${aws_organizations_organizational_unit.test1.id}" } resource "aws_organizations_account" "bflad-dev2" { name = "bflad-dev2" email = "--OMITTED--" } ``` Verifying `Update` functionality: ``` $ terraform apply ... An execution plan has been generated and is shown below. Resource actions are indicated with the following symbols: + create ~ update in-place Terraform will perform the following actions: ~ aws_organizations_account.bflad-dev1 parent_id: "r-cg2b" => "${aws_organizations_organizational_unit.test1.id}" + aws_organizations_organizational_unit.test1 id: <computed> arn: <computed> name: "test1" parent_id: "r-cg2b" Plan: 1 to add, 1 to change, 0 to destroy. ... aws_organizations_organizational_unit.test1: Creating... arn: "" => "<computed>" name: "" => "test1" parent_id: "" => "r-cg2b" aws_organizations_organizational_unit.test1: Creation complete after 0s (ID: ou-cg2b-7aa8b56k) aws_organizations_account.bflad-dev1: Modifying... (ID: --OMITTED--) parent_id: "r-cg2b" => "ou-cg2b-7aa8b56k" aws_organizations_account.bflad-dev1: Modifications complete after 1s (ID: --OMITTED--) $ terraform state show aws_organizations_account.bflad-dev1 | grep parent_id parent_id = ou-cg2b-7aa8b56k ``` Add account with `parent_id` to configuration: ```hcl resource "aws_organizations_organization" "organization" { feature_set = "ALL" } resource "aws_organizations_organizational_unit" "test1" { name = "test1" parent_id = "${aws_organizations_organization.organization.roots.0.id}" } resource "aws_organizations_account" "bflad-dev1" { name = "bflad-dev1" email = "--OMITTED--" parent_id = "${aws_organizations_organizational_unit.test1.id}" } resource "aws_organizations_account" "bflad-dev2" { name = "bflad-dev2" email = "--OMITTED--" } resource "aws_organizations_account" "bflad-dev3" { name = "bflad-dev3" email = "--OMITTED--" parent_id = "${aws_organizations_organizational_unit.test1.id}" } ``` Verifying `Create` functionality: ``` $ terraform apply ... An execution plan has been generated and is shown below. Resource actions are indicated with the following symbols: + create Terraform will perform the following actions: + aws_organizations_account.bflad-dev3 id: <computed> arn: <computed> email: "--OMITTED--" joined_method: <computed> joined_timestamp: <computed> name: "bflad-dev3" parent_id: "ou-cg2b-7aa8b56k" status: <computed> Plan: 1 to add, 0 to change, 0 to destroy. ... aws_organizations_account.bflad-dev3: Creating... arn: "" => "<computed>" email: "" => "--OMITTED--" joined_method: "" => "<computed>" joined_timestamp: "" => "<computed>" name: "" => "bflad-dev3" parent_id: "" => "ou-cg2b-7aa8b56k" status: "" => "<computed>" aws_organizations_account.bflad-dev3: Still creating... (10s elapsed) aws_organizations_account.bflad-dev3: Creation complete after 12s (ID: --OMITTED--) $ terraform state show aws_organizations_account.bflad-dev3 | grep parent_id parent_id = ou-cg2b-7aa8b56k ```

bflad · 2019-05-09T06:02:43Z

Thanks so much, @afeld! Including those two commits as the start, I was able to finish the implementation here: #8583

Since the new pull request now supersedes this one, closing this. Please follow that new pull request (or its tracking feature request #8281) for further updates. 👍

ghost · 2020-03-30T17:24:13Z

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!

ghost added the size/XL Managed by automation to categorize the size of a PR. label May 1, 2018

afeld force-pushed the account-parent branch from 298bfc0 to 6880fba Compare May 1, 2018 06:45

ghost added the size/XL Managed by automation to categorize the size of a PR. label May 1, 2018

afeld force-pushed the account-parent branch from 6880fba to 3f94f3f Compare May 1, 2018 06:52

ghost added the size/XL Managed by automation to categorize the size of a PR. label May 1, 2018

bflad added new-resource Introduces a new resource. new-data-source Introduces a new data source. service/organizations Issues and PRs that pertain to the organizations service. labels May 1, 2018

afeld mentioned this pull request May 9, 2018

Feature request: AWS Organizations support #571

Closed

bflad mentioned this pull request May 30, 2018

[WIP] New Data Sources: aws_organizations_* #4688

Closed

afeld added 8 commits December 27, 2018 21:39

add an aws_organizations_root data source

b28e84a

switch aws_organizations_root to aws_organizations_unit

a4286f1

This will be more future-proof, allowing support for non-root organizational units.

update aws_organizations_policy_attachment example to use aws_organiz…

20959bf

…ations_unit data source

add an aws_organization_unit resource

3edc42d

add support for changing the name of an aws_organizations_unit

80ea22d

add import test for aws_organizations_unit

9e468dc

compute the parent_id for the aws_organizations_account

d7f007c

allow the parent_id to be specified on an aws_organizations_account

3b071fd

afeld force-pushed the account-parent branch from 3f94f3f to 3b071fd Compare December 28, 2018 03:04

ghost added the documentation Introduces or discusses updates to documentation. label Dec 28, 2018

ghost added provider Pertains to the provider itself, rather than any interaction with AWS. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Dec 28, 2018

bflad mentioned this pull request May 9, 2019

resource/aws_organizations_account: Add parent_id argument (support moving accounts) #8583

Merged

bflad closed this May 9, 2019

bflad mentioned this pull request May 9, 2019

AWS Organizations Move Account support #8281

Closed

ghost locked and limited conversation to collaborators Mar 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] support setting the parent for organization accounts #4405

[WIP] support setting the parent for organization accounts #4405

afeld commented May 1, 2018

afeld commented May 1, 2018 •

edited

Loading

afeld commented May 1, 2018

asedge commented May 1, 2018

asedge commented May 1, 2018

bflad commented May 1, 2018

breser commented May 1, 2018

afeld commented May 1, 2018

breser commented May 3, 2018

afeld commented May 6, 2018

bflad commented May 18, 2018

NissesSenap commented Mar 14, 2019

afeld commented Mar 14, 2019

bflad commented May 8, 2019

afeld commented May 9, 2019

bflad commented May 9, 2019

ghost commented Mar 30, 2020

[WIP] support setting the parent for organization accounts #4405

[WIP] support setting the parent for organization accounts #4405

Conversation

afeld commented May 1, 2018

afeld commented May 1, 2018 • edited Loading

afeld commented May 1, 2018

asedge commented May 1, 2018

asedge commented May 1, 2018

bflad commented May 1, 2018

breser commented May 1, 2018

afeld commented May 1, 2018

breser commented May 3, 2018

afeld commented May 6, 2018

bflad commented May 18, 2018

NissesSenap commented Mar 14, 2019

afeld commented Mar 14, 2019

bflad commented May 8, 2019

afeld commented May 9, 2019

bflad commented May 9, 2019

ghost commented Mar 30, 2020

afeld commented May 1, 2018 •

edited

Loading