Add organizations ou child and descendant accounts data source #1

ahublersos · 2022-04-18T15:55:19Z

This PR adds two data sources that allow the retrieval of a list of account IDs based on an AWS organizations Organizational Unit ID.

The aws_organizations_organizational_unit_child_accounts data source provides the immediate child accounts while the aws_organizations_organizational_unit_descendant_accounts provides all generations of child account IDs under the given OU.

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Output from acceptance testing:
I'm still having trouble getting acceptance tests to run. I'm sure it's something simple I'm missing with the test framework. (Open to any suggestions!)
Cleanup from testing is tricky since all the accounts need to be manually deleted.

❯ make testacc TESTS=testAccOrganizationalUnitChildAccountsDataSource_basic PKG=organizations
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/organizations/... -v -count 1 -parallel 20 -run='testAccOrganizationalUnitChildAccountsDataSource_basic'  -timeout 180m
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/organizations	3.254s [no tests to run]

❯ make testacc TESTS=testAccOrganizationalUnitDescendantAccountsDataSource_basic PKG=organizations
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/organizations/... -v -count 1 -parallel 20 -run='testAccOrganizationalUnitDescendantAccountsDataSource_basic'  -timeout 180m
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/organizations	3.162s [no tests to run]

My test org for manual testing looks like this:

Root OU
- Account "test0" (283961987728)
- Account "ahubler-provider-test-mgmt" (831556657578)
- OU "TestA"
- OU "Test0"
- - Account "test1" (574221610224)
- - OU "Test1"
- - - Account "test2" (137602874675)

Output from some manual testing:

root_children = {
  "accounts" = tolist([
    {
      "arn" = "arn:aws:organizations::831556657578:account/o-ux49ku5z71/831556657578"
      "email" = "ahubler@rapidsos.com"
      "id" = "831556657578"
      "joined_method" = "INVITED"
      "joined_timestamp" = "2022-04-20 14:46:44.35 +0000 UTC"
      "name" = "ahubler-provider-test-mgmt"
      "status" = "ACTIVE"
    },
    {
      "arn" = "arn:aws:organizations::831556657578:account/o-ux49ku5z71/283961987728"
      "email" = "ahubler+test0@rapidsos.com"
      "id" = "283961987728"
      "joined_method" = "CREATED"
      "joined_timestamp" = "2022-04-20 17:23:24.888 +0000 UTC"
      "name" = "test0"
      "status" = "ACTIVE"
    },
  ])
  "id" = "r-eca0"
  "parent_id" = "r-eca0"
}
root_descendant_ids = tolist([
  "831556657578",
  "283961987728",
  "574221610224",
  "137602874675",
])
test0_children = {
  "accounts" = tolist([
    {
      "arn" = "arn:aws:organizations::831556657578:account/o-ux49ku5z71/574221610224"
      "email" = "ahubler+test1@rapidsos.com"
      "id" = "574221610224"
      "joined_method" = "CREATED"
      "joined_timestamp" = "2022-04-20 17:22:56.755 +0000 UTC"
      "name" = "test1"
      "status" = "ACTIVE"
    },
  ])
  "id" = "ou-eca0-cno43299"
  "parent_id" = "ou-eca0-cno43299"
}
test0_descendants = {
  "accounts" = tolist([
    {
      "arn" = "arn:aws:organizations::831556657578:account/o-ux49ku5z71/574221610224"
      "email" = "ahubler+test1@rapidsos.com"
      "id" = "574221610224"
      "joined_method" = "CREATED"
      "joined_timestamp" = "2022-04-20 17:22:56.755 +0000 UTC"
      "name" = "test1"
      "status" = "ACTIVE"
    },
    {
      "arn" = "arn:aws:organizations::831556657578:account/o-ux49ku5z71/137602874675"
      "email" = "ahubler+test2@rapidsos.com"
      "id" = "137602874675"
      "joined_method" = "CREATED"
      "joined_timestamp" = "2022-04-20 17:26:55.225 +0000 UTC"
      "name" = "test2"
      "status" = "ACTIVE"
    },
  ])
  "id" = "ou-eca0-cno43299"
  "parent_id" = "ou-eca0-cno43299"
}
testA_children = {
  "accounts" = tolist([])
  "id" = "ou-eca0-f8gxkt9v"
  "parent_id" = "ou-eca0-f8gxkt9v"
}
testA_descendants = {
  "accounts" = tolist([])
  "id" = "ou-eca0-f8gxkt9v"
  "parent_id" = "ou-eca0-f8gxkt9v"
}

bharvillsos · 2022-04-21T14:25:04Z

internal/service/organizations/organizational_unit_child_accounts.go

+func flattenAccounts(accounts []*organizations.Account) []map[string]interface{} {
+	if len(accounts) == 0 {
+		return nil
+	}
+	var result []map[string]interface{}
+	for _, account := range accounts {
+		result = append(result, map[string]interface{}{
+			"arn":              aws.StringValue(account.Arn),
+			"email":            aws.StringValue(account.Email),
+			"id":               aws.StringValue(account.Id),
+			"joined_method":    aws.StringValue(account.JoinedMethod),
+			"joined_timestamp": aws.TimeValue(account.JoinedTimestamp).String(),
+			"name":             aws.StringValue(account.Name),
+			"status":           aws.StringValue(account.Status),
+		})
+	}
+	return result
+}


What's the point of this function? Isn't the input already in this format? https://docs.aws.amazon.com/sdk-for-go/api/service/organizations/#Account

I believe it's main point is to get the account info into a format that fits the schema defined in DataSourceOrganizationalUnitChildAccounts. organizational_unit_child_accounts.go is based on organizational_units_data_source.go which has a similar function.

Huh, wonder if there's a typing thing I missed in the AWS SDK. Or maybe go does something weird and requires this, which wouldn't be the first time I've seen it (had to do some odd stuff to get terraform plan to be usable in tests).

internal/service/organizations/organizational_unit_descendant_accounts.go

bharvillsos · 2022-04-21T14:32:05Z

internal/service/organizations/organizational_unit_descendant_accounts.go

+	}
+
+	//Base Case. ParentId has no children.
+	return nil, nil


Since you could have some results at this point, could you return those and then ditch the entire internal/service/organizations/organizational_unit_child_accounts.go file?

organizational_unit_child_accounts.go is needed so that we can have two separate data sources. In the case where an OU has no child OUs both data sources should return the same data.

Right, so if both data sources return the same thing in that case, then we don't need the other one. Right? If you can stick all the logic into a single data source, then consumers don't need to know their organization in order to call the right one.

I had it in my head that there would be a valid use case where I had an OU that contained both child OUs and immediate child accounts, and I wanted just those immediate child accounts and not the other "generations" of children. If there was only one data source you would only be able to get all generations of child accounts.

I think that's still a valid use case, but i guess it's debatable whether it's a common use case.

bharvillsos · 2022-04-21T14:34:14Z

internal/service/organizations/organizational_unit_child_accounts_test.go

@@ -0,0 +1,51 @@
+package organizations_test


Can you document how we run these? Is it just go test?

There's some documentation here on provider acceptance testing. Just under that section is instructions for using the provider locally for manual testing. That's how I did all my testing so far. The automated tests are tricky b/c you can't easily destroy the account resources after creating them so in practicality you get one run before you have to change the tests.

bharvillsos · 2022-04-21T14:38:29Z

internal/service/organizations/organizational_unit_descendant_accounts_test.go

+	"github.com/hashicorp/terraform-provider-aws/internal/acctest"
+)
+
+func testAccOrganizationalUnitDescendantAccountsDataSource_basic(t *testing.T) {


Not sure if the Terraform source tests allow for this behavior, but I assume you want to check all them. If possible, you can try the t.Run method with 0, 1, 2, 3 for that resource.TestCheckResourceAttr(dataSourceName, "accounts.#", "3"), line. https://go.dev/blog/subtests

ahublersos added 9 commits April 18, 2022 11:52

Add ou child accounts data source

6cae0a5

Add ou descendant accounts data source

7d80386

formatting

8ac0cf3

Fix timestamp type

f93b14c

Fix replace ou variable with new accounts var

76e6b51

convert time type to string

d49d0a5

add new data sources to provider.go

fca0b2b

update test emails

f4bd37b

Add documentation

3e9c8ff

bharvillsos suggested changes Apr 21, 2022

View reviewed changes

ahublersos added 10 commits April 21, 2022 08:46

remove dupe flatten_accounts function

58a4e69

remove commented out declaration

d28dca6

rename data source files

53b4a16

add checks for each account in acc test

8249b16

Use random email addresses in acc test

413b076

pass random addresses to test config

1a2729d

formatting

347e5d9

add fmt package

231fcc8

correct test config function name

c6f97c6

Update child account tests to use random email.

7e6eeb2

ahublersos closed this Apr 21, 2022

ahublersos deleted the add_organizations_ou_child_and_descendant_accounts_data_source branch April 21, 2022 21:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add organizations ou child and descendant accounts data source #1

Add organizations ou child and descendant accounts data source #1

ahublersos commented Apr 18, 2022 •

edited

Loading

bharvillsos Apr 21, 2022

ahublersos Apr 21, 2022

bharvillsos Apr 22, 2022

bharvillsos Apr 21, 2022

ahublersos Apr 21, 2022

bharvillsos Apr 22, 2022

ahublersos Apr 22, 2022

bharvillsos Apr 21, 2022

ahublersos Apr 21, 2022

bharvillsos Apr 21, 2022

Add organizations ou child and descendant accounts data source #1

Add organizations ou child and descendant accounts data source #1

Conversation

ahublersos commented Apr 18, 2022 • edited Loading

Community Note

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

ahublersos commented Apr 18, 2022 •

edited

Loading