Skip to content

Commit

Permalink
resource/aws_organizations_account: Finish initial parent_id implem…
Browse files Browse the repository at this point in the history 
…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
```
  • Loading branch information
@bflad
bflad committed May 9, 2019
1 parent 072c303 commit ecf4653
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 86 deletions.
117 changes: 82 additions & 35 deletions aws/resource_aws_organizations_account.go
View file
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 @@ -36,7 +37,6 @@ func resourceAwsOrganizationsAccount() *schema.Resource {
Computed: true,
},
"parent_id": {
ForceNew: true,
Type: schema.TypeString,
Computed: true,
Optional: true,
Expand Down Expand Up @@ -90,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 @@ -137,28 +140,24 @@ func resourceAwsOrganizationsAccountCreate(d *schema.ResourceData, meta interfac
accountId := stateResp.(*organizations.CreateAccountStatus).AccountId
d.SetId(*accountId)

if newParentID, ok := d.GetOk("parent_id"); ok {
// move under an explicit parent
if v, ok := d.GetOk("parent_id"); ok {
newParentID := v.(string)

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

// this will be the root ID
existingParentID, err := resourceAwsOrganizationsGetParentID(conn, d.Id())
if err != nil {
return err
return fmt.Errorf("error getting AWS Organizations Account (%s) parent: %s", d.Id(), err)
}

newParentIdStr := newParentID.(string)
if newParentIdStr != existingParentID {
// TODO partial

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

_, err := conn.MoveAccount(moveOpts)
if err != nil {
return err
if _, err := conn.MoveAccount(input); err != nil {
return fmt.Errorf("error moving AWS Organizations Account (%s): %s", d.Id(), err)
}
}
}
Expand All @@ -172,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 @@ -188,20 +189,40 @@ 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)

parentId, err := resourceAwsOrganizationsGetParentID(conn, d.Id())
if err != nil {
return err
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)
}
}
d.Set("parent_id", parentId)

return nil
return resourceAwsOrganizationsAccountRead(d, meta)
}

func resourceAwsOrganizationsAccountDelete(d *schema.ResourceData, meta interface{}) error {
Expand Down Expand Up @@ -281,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
}
92 changes: 46 additions & 46 deletions aws/resource_aws_organizations_account_test.go
View file
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.")

var account organizations.Account

orgsEmailDomain, ok := os.LookupEnv("TEST_AWS_ORGANIZATION_ACCOUNT_EMAIL_DOMAIN")
Expand Down Expand Up @@ -51,7 +53,9 @@ func testAccAwsOrganizationsAccount_basic(t *testing.T) {
})
}

func testAccAwsOrganizationsAccount_parentRoot(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")
Expand All @@ -63,47 +67,32 @@ func testAccAwsOrganizationsAccount_parentRoot(t *testing.T) {
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: testAccAwsOrganizationsAccountConfigUnderRoot(name, email),
Config: testAccAwsOrganizationsAccountConfigParentId1(name, email),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsOrganizationsAccountExists("aws_organizations_account.test", &account),
resource.TestCheckResourceAttrSet("aws_organizations_account.test", "parent_id"),
testAccCheckAwsOrganizationsAccountExists(resourceName, &account),
resource.TestCheckResourceAttrPair(resourceName, "parent_id", parentIdResourceName1, "id"),
),
},
},
})
}

func testAccAwsOrganizationsAccount_parentOU(t *testing.T) {
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)

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAwsOrganizationsAccountDestroy,
Steps: []resource.TestStep{
{
Config: testAccAwsOrganizationsAccountConfigUnderOU(name, email),
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccAwsOrganizationsAccountConfigParentId2(name, email),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsOrganizationsAccountExists("aws_organizations_account.test", &account),
resource.TestCheckResourceAttrSet("aws_organizations_account.test", "parent_id"),
// TODO actually check that it lives under the parent
testAccCheckAwsOrganizationsAccountExists(resourceName, &account),
resource.TestCheckResourceAttrPair(resourceName, "parent_id", parentIdResourceName2, "id"),
),
},
},
Expand Down Expand Up @@ -178,35 +167,46 @@ resource "aws_organizations_account" "test" {
`, name, email)
}

func testAccAwsOrganizationsAccountConfigUnderRoot(name, email string) string {
func testAccAwsOrganizationsAccountConfigParentId1(name, email string) string {
return fmt.Sprintf(`
data "aws_organizations_unit" "root" {
root = true
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 = "%s"
email = "%s"
parent_id = "${data.aws_organizations_unit.root.id}"
name = %[1]q
email = %[2]q
parent_id = "${aws_organizations_organizational_unit.test1.id}"
}
`, name, email)
}

func testAccAwsOrganizationsAccountConfigUnderOU(name, email string) string {
func testAccAwsOrganizationsAccountConfigParentId2(name, email string) string {
return fmt.Sprintf(`
data "aws_organizations_unit" "root" {
root = true
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_unit" "test" {
parent_id = "${data.aws_organizations_unit.root.id}"
name = "%s"
resource "aws_organizations_organizational_unit" "test2" {
name = "test2"
parent_id = "${aws_organizations_organization.test.roots.0.id}"
}
resource "aws_organizations_account" "test" {
name = "%s"
email = "%s"
parent_id = "${aws_organizations_unit.test.id}"
name = %[1]q
email = %[2]q
parent_id = "${aws_organizations_organizational_unit.test2.id}"
}
`, name, name, email)
`, name, email)
}
5 changes: 2 additions & 3 deletions aws/resource_aws_organizations_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@ func TestAccAWSOrganizations(t *testing.T) {
"FeatureSet": testAccAwsOrganizationsOrganization_FeatureSet,
},
"Account": {
"basic": testAccAwsOrganizationsAccount_basic,
"parentRoot": testAccAwsOrganizationsAccount_parentRoot,
"parentOU": testAccAwsOrganizationsAccount_parentOU,
"basic": testAccAwsOrganizationsAccount_basic,
"ParentId": testAccAwsOrganizationsAccount_ParentId,
},
"OrganizationalUnit": {
"basic": testAccAwsOrganizationsOrganizationalUnit_basic,
Expand Down
3 changes: 1 addition & 2 deletions website/docs/r/organizations_account.html.markdown
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +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) The ID of the Organizational Unit or "root" the account should be under. Defaults to the root ID.
* `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

0 comments on commit ecf4653

Please sign in to comment.