Skip to content
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_account: Add parent_id argument (support moving accounts) #8583

Merged
merged 3 commits into from
May 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 102 additions & 15 deletions aws/resource_aws_organizations_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ func resourceAwsOrganizationsAccount() *schema.Resource {
return &schema.Resource{
Create: resourceAwsOrganizationsAccountCreate,
Read: resourceAwsOrganizationsAccountRead,
Update: resourceAwsOrganizationsAccountUpdate,
Delete: resourceAwsOrganizationsAccountDelete,
Importer: &schema.ResourceImporter{
State: schema.ImportStatePassthrough,
Expand All @@ -35,6 +36,12 @@ func resourceAwsOrganizationsAccount() *schema.Resource {
Type: schema.TypeString,
Computed: true,
},
"parent_id": {
Type: schema.TypeString,
Computed: true,
Optional: true,
ValidateFunc: validation.StringMatch(regexp.MustCompile("^(r-[0-9a-z]{4,32})|(ou-[0-9a-z]{4,32}-[a-z0-9]{8,32})$"), "see https://docs.aws.amazon.com/organizations/latest/APIReference/API_MoveAccount.html#organizations-MoveAccount-request-DestinationParentId"),
},
"status": {
Type: schema.TypeString,
Computed: true,
Expand Down Expand Up @@ -83,29 +90,32 @@ func resourceAwsOrganizationsAccountCreate(d *schema.ResourceData, meta interfac
createOpts.IamUserAccessToBilling = aws.String(iam_user.(string))
}

log.Printf("[DEBUG] Account create config: %#v", createOpts)
log.Printf("[DEBUG] Creating AWS Organizations Account: %s", createOpts)

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

resp, err = conn.CreateAccount(createOpts)

if err != nil {
if isAWSErr(err, organizations.ErrCodeFinalizingOrganizationException, "") {
log.Printf("[DEBUG] Trying to create account again: %q", err.Error())
return resource.RetryableError(err)
}
if isAWSErr(err, organizations.ErrCodeFinalizingOrganizationException, "") {
return resource.RetryableError(err)
}

if err != nil {
return resource.NonRetryableError(err)
}

return nil
})

if isResourceTimeoutError(err) {
resp, err = conn.CreateAccount(createOpts)
}

if err != nil {
return fmt.Errorf("Error creating account: %s", err)
}
log.Printf("[DEBUG] Account create response: %#v", resp)

requestId := *resp.CreateAccountStatus.Id

Expand All @@ -130,6 +140,28 @@ func resourceAwsOrganizationsAccountCreate(d *schema.ResourceData, meta interfac
accountId := stateResp.(*organizations.CreateAccountStatus).AccountId
d.SetId(*accountId)

if v, ok := d.GetOk("parent_id"); ok {
newParentID := v.(string)

existingParentID, err := resourceAwsOrganizationsAccountGetParentId(conn, d.Id())

if err != nil {
return fmt.Errorf("error getting AWS Organizations Account (%s) parent: %s", d.Id(), err)
}

if newParentID != existingParentID {
input := &organizations.MoveAccountInput{
AccountId: accountId,
SourceParentId: aws.String(existingParentID),
DestinationParentId: aws.String(newParentID),
}

if _, err := conn.MoveAccount(input); err != nil {
return fmt.Errorf("error moving AWS Organizations Account (%s): %s", d.Id(), err)
}
}
}

return resourceAwsOrganizationsAccountRead(d, meta)
}

Expand All @@ -139,13 +171,15 @@ func resourceAwsOrganizationsAccountRead(d *schema.ResourceData, meta interface{
AccountId: aws.String(d.Id()),
}
resp, err := conn.DescribeAccount(describeOpts)

if isAWSErr(err, organizations.ErrCodeAccountNotFoundException, "") {
log.Printf("[WARN] Account does not exist, removing from state: %s", d.Id())
d.SetId("")
return nil
}

if err != nil {
if isAWSErr(err, organizations.ErrCodeAccountNotFoundException, "") {
log.Printf("[WARN] Account does not exist, removing from state: %s", d.Id())
d.SetId("")
return nil
}
return err
return fmt.Errorf("error describing AWS Organizations Account (%s): %s", d.Id(), err)
}

account := resp.Account
Expand All @@ -155,15 +189,42 @@ func resourceAwsOrganizationsAccountRead(d *schema.ResourceData, meta interface{
return nil
}

parentId, err := resourceAwsOrganizationsAccountGetParentId(conn, d.Id())
if err != nil {
return fmt.Errorf("error getting AWS Organizations Account (%s) parent: %s", d.Id(), err)
}

d.Set("arn", account.Arn)
d.Set("email", account.Email)
d.Set("joined_method", account.JoinedMethod)
d.Set("joined_timestamp", account.JoinedTimestamp)
d.Set("name", account.Name)
d.Set("parent_id", parentId)
d.Set("status", account.Status)

return nil
}

func resourceAwsOrganizationsAccountUpdate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).organizationsconn

if d.HasChange("parent_id") {
o, n := d.GetChange("parent_id")

input := &organizations.MoveAccountInput{
AccountId: aws.String(d.Id()),
SourceParentId: aws.String(o.(string)),
DestinationParentId: aws.String(n.(string)),
}

if _, err := conn.MoveAccount(input); err != nil {
return fmt.Errorf("error moving AWS Organizations Account (%s): %s", d.Id(), err)
}
}

return resourceAwsOrganizationsAccountRead(d, meta)
}

func resourceAwsOrganizationsAccountDelete(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).organizationsconn

Expand Down Expand Up @@ -241,3 +302,29 @@ func validateAwsOrganizationsAccountRoleName(v interface{}, k string) (ws []stri

return
}

func resourceAwsOrganizationsAccountGetParentId(conn *organizations.Organizations, childId string) (string, error) {
input := &organizations.ListParentsInput{
ChildId: aws.String(childId),
}
var parents []*organizations.Parent

err := conn.ListParentsPages(input, func(page *organizations.ListParentsOutput, lastPage bool) bool {
parents = append(parents, page.Parents...)

return !lastPage
})

if err != nil {
return "", err
}

if len(parents) == 0 {
return "", nil
}

// assume there is only a single parent
// https://docs.aws.amazon.com/organizations/latest/APIReference/API_ListParents.html
parent := parents[0]
return aws.StringValue(parent.Id), nil
}
93 changes: 93 additions & 0 deletions aws/resource_aws_organizations_account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
)

func testAccAwsOrganizationsAccount_basic(t *testing.T) {
t.Skip("AWS Organizations Account testing is not currently automated due to manual account deletion steps.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to include a comment with a link back to this PR to show the verification steps taken as an example? Or maybe a comment with the link https://docs.aws.amazon.com/organizations/latest/userguide/orgs_manage_accounts_remove.html?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Including testing documentation in some form is probably not a bad idea. Let's discuss what form this should take (probably in a multi-line skip message) after we cut the release.

Luckily this is the first time we've needed to touch this resource since its creation so this awful manual process isn't required too much, but surely easy to forget!


var account organizations.Account

orgsEmailDomain, ok := os.LookupEnv("TEST_AWS_ORGANIZATION_ACCOUNT_EMAIL_DOMAIN")
Expand All @@ -36,6 +38,7 @@ func testAccAwsOrganizationsAccount_basic(t *testing.T) {
resource.TestCheckResourceAttrSet("aws_organizations_account.test", "arn"),
resource.TestCheckResourceAttrSet("aws_organizations_account.test", "joined_method"),
resource.TestCheckResourceAttrSet("aws_organizations_account.test", "joined_timestamp"),
resource.TestCheckResourceAttrSet("aws_organizations_account.test", "parent_id"),
resource.TestCheckResourceAttr("aws_organizations_account.test", "name", name),
resource.TestCheckResourceAttr("aws_organizations_account.test", "email", email),
resource.TestCheckResourceAttrSet("aws_organizations_account.test", "status"),
Expand All @@ -50,6 +53,52 @@ func testAccAwsOrganizationsAccount_basic(t *testing.T) {
})
}

func testAccAwsOrganizationsAccount_ParentId(t *testing.T) {
t.Skip("AWS Organizations Account testing is not currently automated due to manual account deletion steps.")

var account organizations.Account

orgsEmailDomain, ok := os.LookupEnv("TEST_AWS_ORGANIZATION_ACCOUNT_EMAIL_DOMAIN")

if !ok {
t.Skip("'TEST_AWS_ORGANIZATION_ACCOUNT_EMAIL_DOMAIN' not set, skipping test.")
}

rInt := acctest.RandInt()
name := fmt.Sprintf("tf_acctest_%d", rInt)
email := fmt.Sprintf("tf-acctest+%d@%s", rInt, orgsEmailDomain)
resourceName := "aws_organizations_account.test"
parentIdResourceName1 := "aws_organizations_organizational_unit.test1"
parentIdResourceName2 := "aws_organizations_organizational_unit.test2"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAwsOrganizationsAccountDestroy,
Steps: []resource.TestStep{
{
Config: testAccAwsOrganizationsAccountConfigParentId1(name, email),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsOrganizationsAccountExists(resourceName, &account),
resource.TestCheckResourceAttrPair(resourceName, "parent_id", parentIdResourceName1, "id"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccAwsOrganizationsAccountConfigParentId2(name, email),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsOrganizationsAccountExists(resourceName, &account),
resource.TestCheckResourceAttrPair(resourceName, "parent_id", parentIdResourceName2, "id"),
),
},
},
})
}

func testAccCheckAwsOrganizationsAccountDestroy(s *terraform.State) error {
conn := testAccProvider.Meta().(*AWSClient).organizationsconn

Expand Down Expand Up @@ -117,3 +166,47 @@ resource "aws_organizations_account" "test" {
}
`, name, email)
}

func testAccAwsOrganizationsAccountConfigParentId1(name, email string) string {
return fmt.Sprintf(`
resource "aws_organizations_organization" "test" {}

resource "aws_organizations_organizational_unit" "test1" {
name = "test1"
parent_id = "${aws_organizations_organization.test.roots.0.id}"
}

resource "aws_organizations_organizational_unit" "test2" {
name = "test2"
parent_id = "${aws_organizations_organization.test.roots.0.id}"
}

resource "aws_organizations_account" "test" {
name = %[1]q
email = %[2]q
parent_id = "${aws_organizations_organizational_unit.test1.id}"
}
`, name, email)
}

func testAccAwsOrganizationsAccountConfigParentId2(name, email string) string {
return fmt.Sprintf(`
resource "aws_organizations_organization" "test" {}

resource "aws_organizations_organizational_unit" "test1" {
name = "test1"
parent_id = "${aws_organizations_organization.test.roots.0.id}"
}

resource "aws_organizations_organizational_unit" "test2" {
name = "test2"
parent_id = "${aws_organizations_organization.test.roots.0.id}"
}

resource "aws_organizations_account" "test" {
name = %[1]q
email = %[2]q
parent_id = "${aws_organizations_organizational_unit.test2.id}"
}
`, name, email)
}
3 changes: 2 additions & 1 deletion aws/resource_aws_organizations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ func TestAccAWSOrganizations(t *testing.T) {
"FeatureSet": testAccAwsOrganizationsOrganization_FeatureSet,
},
"Account": {
"basic": testAccAwsOrganizationsAccount_basic,
"basic": testAccAwsOrganizationsAccount_basic,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️

"ParentId": testAccAwsOrganizationsAccount_ParentId,
},
"OrganizationalUnit": {
"basic": testAccAwsOrganizationsOrganizationalUnit_basic,
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/organizations_account.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ The following arguments are supported:
* `name` - (Required) A friendly name for the member account.
* `email` - (Required) The email address of the owner to assign to the new member account. This email address must not already be associated with another AWS account.
* `iam_user_access_to_billing` - (Optional) If set to `ALLOW`, the new account enables IAM users to access account billing information if they have the required permissions. If set to `DENY`, then only the root user of the new account can access account billing information.
* `parent_id` - (Optional) Parent Organizational Unit ID or Root ID for the account. Defaults to the Organization default Root ID. A configuration must be present for this argument to perform drift detection.
* `role_name` - (Optional) The name of an IAM role that Organizations automatically preconfigures in the new member account. This role trusts the master account, allowing users in the master account to assume the role, as permitted by the master account administrator. The role has administrator permissions in the new member account.

## Attributes Reference

In addition to all arguments above, the following attributes are exported:

* `arn` - The ARN for this account.

* `id` - The AWS account id

## Import
Expand Down