New Resource: aws_organizations_account #3524

asedge · 2018-02-25T14:20:39Z

A new resource to implement AWS Organizations member accounts: #571.

I updated the branch based on feedback from #903. There's probably more to fix up because this branch hadn't been touched in quite a while. This is quite a bit more difficult to test because once you create a member account in an organization that organization cannot be deleted. The member accounts themselves cannot be deleted, only suspended, and each account creation consumes an email address that cannot be reused.

Originally attempted a PR at: hashicorp/terraform#14147.

CC: @bflad

bflad

Hi @asedge, thanks so much again! You rock. I left some initial feedback below. Same as before, if you do not have time to complete this or have any questions, please let me know! 👍

I think the biggest changes to this PR are reworking the deletion to call RemoveAccountFromOrganization and removing the import specific test (since its very likely we'll have lots of trouble acceptance testing this with AWS limits).

bflad · 2018-02-26T16:19:44Z

aws/resource_aws_organizations_account.go

+
+	var err error
+	var resp *organizations.CreateAccountOutput
+	err = resource.Retry(4*time.Minute, func() *resource.RetryError {


Out of curiosity, how was 4 minutes decided here?

I don't remember 100% because this code is nearly a year old. After looking at it for a little bit I think the issue was that there's no API action to check the status of CreateOrganization. I didn't realize this could be a problem until I wrote this resource, because I was creating an organization and immediately adding an account, so I put the poll here. Allowing organizations.ErrCodeFinalizingOrganizationException as a RetryableError seems to back that up.

Awesome information! If that's the case, maybe we (more like I 😄 ) should setup the organization resource to wait until the organization is finalized before returning that its done on creation. I'll make a note to check that myself.

If you create an issue and assign it to me, I will get it done.

bflad · 2018-02-26T16:21:12Z

aws/resource_aws_organizations_account.go

+		resp, err = conn.CreateAccount(createOpts)
+
+		if err != nil {
+			ec2err, ok := err.(awserr.Error)


We can drop this and just work with err below in the isAWSErr and log.Printf calls

This may have been a mistake to leave in.

bflad · 2018-02-26T16:21:53Z

aws/resource_aws_organizations_account.go

+
+		if err != nil {
+			ec2err, ok := err.(awserr.Error)
+			if !ok {


Can you please remove this? I don't think this is valid here

This may have been a mistake from when I was trying to clean up.

bflad · 2018-02-26T17:49:53Z

aws/import_aws_organizations_account_test.go

+				Config: testAccAwsOrganizationsAccountConfig(name, email),
+			},
+
+			{


Due to the likely issues we'll have with account number limits, can you please remove this explicit separate test and instead move this test step to the end of the _basic test? Thanks!

bflad · 2018-02-26T17:52:24Z

aws/resource_aws_organizations_account_test.go

+		t.Skip("'TEST_AWS_ORGANIZATION_ACCOUNT_EMAIL' not set, skipping test.")
+	}
+
+	name := "my_new_account"


We want to randomize this, which means maybe we'll want to accept just the email domain as an environment variable as well. e.g.

rInt := acctest.RandInt() name := fmt.Sprintf("tf_acctest_%s", rInt) email := fmt.Sprintf("tf-acctest+%d@%s", rInt, orgsEmailDomain)

For starters though, let's at least get the rInt bits in for now as we'll already need to do some other special changes to support repeatable acceptance testing.

bflad · 2018-02-26T18:05:00Z

website/aws.erb

@@ -1377,6 +1377,9 @@
                        <li<%= sidebar_current("docs-aws-resource-organizations-organization") %>>
                            <a href="/docs/providers/aws/r/organizations_organization.html">aws_organizations_organization</a>
                        </li>
+                        <li<%= sidebar_current("docs-aws-resource-organizations-account") %>>
+                            <a href="/docs/providers/aws/r/organizations_account.html">aws_organizations_account</a>


Minor nitpick: Can you sort this alphabetically please?

bflad · 2018-02-26T18:05:22Z

website/docs/r/organizations_account.html.markdown

+  Provides a resource to create a member account in the current AWS Organization.
+---
+
+# aws\_organizations\_account


Minor nitpick: Backslashes are no longer required in documentation titles

bflad · 2018-02-26T18:31:26Z

website/docs/r/organizations_account.html.markdown

+
+-> **Note:** Account creation must be done from the organization's master account.
+
+-> **Note:** AWS member accounts must be deleted manually by following these steps: 1) Perform a root account password recovery for the email address that was specified for the account in Organizations. 2) Login to the account as that root user. 3) Navigate to "My Organization" in the account menu top-right. 4) Leave the organization. 5) Once the account has successfully left the organization, delete the account as usual.


We should link to the AWS Organizations documentation for this information, e.g.

~> **Note:** Deleting this Terraform resource will only remove an AWS account from an organization. Terraform will not close the account. The member account must be prepared to be a standalone account beforehand. See the [AWS Organizations documentation](https://docs.aws.amazon.com/organizations/latest/userguide/orgs_manage_accounts_remove.html) for more information.

bflad · 2018-02-26T18:32:21Z

website/docs/r/organizations_account.html.markdown

+
+# aws\_organizations\_account
+
+-> **Note:** Account creation must be done from the organization's master account.


We should imply this for all account management, e.g.

~> **Note:** Account management must be done from the organization's master account.

bflad · 2018-02-26T18:42:29Z

aws/resource_aws_organizations_account.go

+	return nil
+}
+
+func resourceAwsOrganizationsAccountDelete(d *schema.ResourceData, meta interface{}) error {


Due to how the AWS Organizations service and closing AWS accounts in general works, we should probably set up the Terraform deletion function here to call RemoveAccountFromOrganization and pass along the error to the Terraform user. I put a review comment to update the documentation appropriately as well. e.g.

func resourceAwsOrganizationsAccountDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).organizationsconn input := &organizations.RemoveAccountFromOrganizationInput{ AccountId: aws.String(d.Id()), } log.Printf("[DEBUG] Removing AWS account from organization: %s", input) _, err := conn.RemoveAccountFromOrganization(input) if err != nil { if isAWSErr(err, organizations.ErrCodeAccountNotFoundException, "") { return nil } return err } return nil }

OK, I can do that. In fact, since you wrote the code feel free to update the PR. Let me know either way, thanks.

…esource will be challenging.

…ions documentation rather than spell it out.

… accidentally left in the old awserr check.

…user rather than issuing a pointless warning.

…ion.

…yed.

asedge · 2018-03-02T15:10:36Z

@bflad I took care of nearly all the feedback you provided. I provided a response about the 4 minute retry timeout but haven't made any changes on it just yet.

bflad · 2018-03-02T22:06:59Z

@asedge thank you so much again for your work here! You've really gone above and beyond here. 😄

At this point, I need to temporarily put this PR on hold while I work with AWS support on how we can regularly acceptance test this in a designated AWS account. No clue what their response will be like yet 😂.

If people are feeling brave, maybe they could try this out themselves in a custom build? terraform state rm is always an option to back out the account management with Terraform. At the current time, I don't think there will be much in the line of "breaking changes" to the resource (e.g. attribute changes), but I cannot make a 100% promise yet until I get more details from AWS.

lorengordon · 2018-03-09T22:26:16Z

I might be able to play with this. Would be awesome to be able to manage accounts as resources (and data sources) through Terraform. Would someone be able to create a Windows binary from this branch?

jeffnappi · 2018-03-26T21:04:21Z

Thanks for the hard work on this, looking forward to seeing it released!

…event crash from empty account result

… warning and other minor formatting fixes

bflad

Bringing this in after I changed three things:

Add d.Set("email", resp.Account.Email) to fix import
Prevent crash if resp.Account is nil for some reason
Upgraded the deletion message in the docs to a nice red WARNING

Verified manually using my personal AWS accounts 😉 .

aws_organizations_account.bflad-dev2: Creating...
  arn:              "" => "<computed>"
  email:            "" => "--OMITTED--"
  joined_method:    "" => "<computed>"
  joined_timestamp: "" => "<computed>"
  name:             "" => "bflad-dev2"
  status:           "" => "<computed>"
aws_organizations_account.bflad-dev2: Still creating... (10s elapsed)
aws_organizations_account.bflad-dev2: Creation complete after 11s (ID: --OMITTED--)
# Complete all the "sign up steps" required to make account standalone
aws_organizations_account.bflad-dev2: Destroying... (ID: --OMITTED--)
aws_organizations_account.bflad-dev2: Destruction complete after 4s
# Able to invite back and accept invite in AWS Console, then reimport into Terraform

asedge · 2018-04-06T11:01:56Z

@bflad Thanks for the review and merge!

bflad · 2018-04-06T19:00:21Z

This has been released in version 1.14.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

Flowman · 2018-04-11T06:56:21Z

Can we extend this with add account to specific OU?

ghost · 2020-04-06T17:52:26Z

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!

Add resource to create an AWS Organizations member account.

823388a

ghost added the size/L Managed by automation to categorize the size of a PR. label Feb 25, 2018

asedge added 2 commits February 25, 2018 13:42

Must set ForceNew:true since there is no update method defined.

046ecef

Fix up some conditions in the tests based on previous feedback.

ecbf076

bflad added new-resource Introduces a new resource. service/organizations Issues and PRs that pertain to the organizations service. labels Feb 26, 2018

bflad reviewed Feb 26, 2018

View reviewed changes

asedge added 8 commits March 2, 2018 10:01

Remove separate import test since running acceptance tests for this r…

158741f

…esource will be challenging.

Alphabetize navigation items.

df173f8

Provide clearer notes based on review feedback. Link to AWS Organizat…

187ed10

…ions documentation rather than spell it out.

Fix refactoring gone wrong. Should have just used isAWSErr helper but…

911290d

… accidentally left in the old awserr check.

Add in a Delete method so that the AWS error can be presented to the …

c381d4c

…user rather than issuing a pointless warning.

Randomize the name and email inputs for acceptance test account creat…

0c58762

…ion.

Test that all the resources are set and that some are set correctly.

ea7573f

Return nil instead of an error if the account has already been destro…

e607c48

…yed.

ghost added the size/L Managed by automation to categorize the size of a PR. label Mar 2, 2018

bflad self-assigned this Mar 2, 2018

bflad added the upstream Addresses functionality related to the cloud provider. label Mar 2, 2018

bflad added 2 commits April 5, 2018 21:57

resource/aws_organization_account: Ensure email is set on read and pr…

9a434eb

…event crash from empty account result

docs/resource/aws_organizations_account: Increase deletion message to…

d132b6c

… warning and other minor formatting fixes

ghost added the size/L Managed by automation to categorize the size of a PR. label Apr 6, 2018

bflad added this to the v1.14.0 milestone Apr 6, 2018

bflad approved these changes Apr 6, 2018

View reviewed changes

bflad merged commit 4ae5dc1 into hashicorp:master Apr 6, 2018

bflad added a commit that referenced this pull request Apr 6, 2018

Update CHANGELOG for #3524

0e8d0b6

asedge deleted the resource_aws_organizations_account branch April 10, 2018 16:38

asedge mentioned this pull request May 1, 2018

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

Closed

ghost locked and limited conversation to collaborators Apr 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: aws_organizations_account #3524

New Resource: aws_organizations_account #3524

asedge commented Feb 25, 2018

bflad left a comment

bflad Feb 26, 2018

asedge Feb 27, 2018 •

edited

Loading

bflad Mar 2, 2018

asedge Mar 2, 2018

bflad Feb 26, 2018

asedge Feb 27, 2018

bflad Feb 26, 2018

asedge Feb 27, 2018

bflad Feb 26, 2018

bflad Feb 26, 2018

bflad Feb 26, 2018

bflad Feb 26, 2018

bflad Feb 26, 2018

bflad Feb 26, 2018

bflad Feb 26, 2018

asedge Feb 27, 2018

asedge commented Mar 2, 2018

bflad commented Mar 2, 2018

lorengordon commented Mar 9, 2018

jeffnappi commented Mar 26, 2018

bflad left a comment

asedge commented Apr 6, 2018

bflad commented Apr 6, 2018

Flowman commented Apr 11, 2018

ghost commented Apr 6, 2020


		-> Note: Account creation must be done from the organization's master account.

		-> Note: AWS member accounts must be deleted manually by following these steps: 1) Perform a root account password recovery for the email address that was specified for the account in Organizations. 2) Login to the account as that root user. 3) Navigate to "My Organization" in the account menu top-right. 4) Leave the organization. 5) Once the account has successfully left the organization, delete the account as usual.


		# aws\_organizations\_account

		-> Note: Account creation must be done from the organization's master account.

New Resource: aws_organizations_account #3524

New Resource: aws_organizations_account #3524

Conversation

asedge commented Feb 25, 2018

bflad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asedge Feb 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asedge commented Mar 2, 2018

bflad commented Mar 2, 2018

lorengordon commented Mar 9, 2018

jeffnappi commented Mar 26, 2018

bflad left a comment

Choose a reason for hiding this comment

asedge commented Apr 6, 2018

bflad commented Apr 6, 2018

Flowman commented Apr 11, 2018

ghost commented Apr 6, 2020

asedge Feb 27, 2018 •

edited

Loading