From 8df40c3da00258481f6d6e12c8ae4fe16efc11b6 Mon Sep 17 00:00:00 2001 From: Ryan Kennedy Date: Mon, 12 Oct 2020 12:55:10 -0700 Subject: [PATCH 1/4] adding tags to aws_organizations_organizational_unit and tests --- ..._aws_organizations_organizational_units.go | 6 ++ ...organizations_organizational_units_test.go | 13 +++- ...e_aws_organizations_organizational_unit.go | 37 +++++++-- ..._organizations_organizational_unit_test.go | 78 +++++++++++++++++++ 4 files changed, 126 insertions(+), 8 deletions(-) diff --git a/aws/data_source_aws_organizations_organizational_units.go b/aws/data_source_aws_organizations_organizational_units.go index 08d49df5674..0e9884cb4fd 100644 --- a/aws/data_source_aws_organizations_organizational_units.go +++ b/aws/data_source_aws_organizations_organizational_units.go @@ -7,6 +7,7 @@ import ( "github.com/aws/aws-sdk-go/service/organizations" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) func dataSourceAwsOrganizationsOrganizationalUnits() *schema.Resource { @@ -38,6 +39,7 @@ func dataSourceAwsOrganizationsOrganizationalUnits() *schema.Resource { }, }, }, + "tags": tagsSchemaComputed(), }, } } @@ -69,6 +71,10 @@ func dataSourceAwsOrganizationsOrganizationalUnitsRead(d *schema.ResourceData, m return fmt.Errorf("Error setting children: %s", err) } + if err := d.Set("tags", keyvaluetags.OrganizationsKeyValueTags(volume.Tags).IgnoreAws().IgnoreConfig(client.IgnoreTagsConfig).Map()); err != nil { + return fmt.Errorf("error setting tags: %s", err) + } + return nil } diff --git a/aws/data_source_aws_organizations_organizational_units_test.go b/aws/data_source_aws_organizations_organizational_units_test.go index 9301852dcb8..8be911cb133 100644 --- a/aws/data_source_aws_organizations_organizational_units_test.go +++ b/aws/data_source_aws_organizations_organizational_units_test.go @@ -9,6 +9,7 @@ import ( func testAccDataSourceAwsOrganizationsOrganizationalUnits_basic(t *testing.T) { resourceName := "aws_organizations_organizational_unit.test" dataSourceName := "data.aws_organizations_organizational_units.test" + rInt := acctest.RandIntRange(0, 256) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) @@ -17,7 +18,7 @@ func testAccDataSourceAwsOrganizationsOrganizationalUnits_basic(t *testing.T) { Providers: testAccProviders, Steps: []resource.TestStep{ { - Config: testAccDataSourceAwsOrganizationsOrganizationalUnitsConfig, + Config: testAccDataSourceAwsOrganizationsOrganizationalUnitsConfig(rInt), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(dataSourceName, "children.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "name", dataSourceName, "children.0.name"), @@ -29,15 +30,21 @@ func testAccDataSourceAwsOrganizationsOrganizationalUnits_basic(t *testing.T) { }) } -const testAccDataSourceAwsOrganizationsOrganizationalUnitsConfig = ` +func testAccDataSourceAwsOrganizationsOrganizationalUnitsConfig(rInt int) string { + return fmt.Sprintf(` resource "aws_organizations_organization" "test" {} resource "aws_organizations_organizational_unit" "test" { name = "test" parent_id = aws_organizations_organization.test.roots[0].id + + tags = { + TestIdentifierSet = "testAccDataSourceAwsEbsVolumes-%d" + } } data "aws_organizations_organizational_units" "test" { parent_id = aws_organizations_organizational_unit.test.parent_id } -` +`, rInt) +} diff --git a/aws/resource_aws_organizations_organizational_unit.go b/aws/resource_aws_organizations_organizational_unit.go index 70ab43c4ba7..4441c012c98 100644 --- a/aws/resource_aws_organizations_organizational_unit.go +++ b/aws/resource_aws_organizations_organizational_unit.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) func resourceAwsOrganizationsOrganizationalUnit() *schema.Resource { @@ -63,6 +64,7 @@ func resourceAwsOrganizationsOrganizationalUnit() *schema.Resource { Required: 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_CreateOrganizationalUnit.html#organizations-CreateOrganizationalUnit-request-ParentId"), }, + "tags": tagsSchema(), }, } } @@ -85,7 +87,7 @@ func resourceAwsOrganizationsOrganizationalUnitCreate(d *schema.ResourceData, me if err != nil { if isAWSErr(err, organizations.ErrCodeFinalizingOrganizationException, "") { - log.Printf("[DEBUG] Trying to create organizational unit again: %q", err.Error()) + log.Printf("[DEBUG] Trying to create Organizational Unit again: %q", err.Error()) return resource.RetryableError(err) } @@ -99,7 +101,7 @@ func resourceAwsOrganizationsOrganizationalUnitCreate(d *schema.ResourceData, me } if err != nil { - return fmt.Errorf("Error creating organizational unit: %s", err) + return fmt.Errorf("Error creating Organizational Unit: %s", err) } log.Printf("[DEBUG] Organizational Unit create response: %#v", resp) @@ -107,6 +109,12 @@ func resourceAwsOrganizationsOrganizationalUnitCreate(d *schema.ResourceData, me ouId := resp.OrganizationalUnit.Id d.SetId(*ouId) + if v := d.Get("tags").(map[string]interface{}); len(v) > 0 { + if err := keyvaluetags.OrganizationsUpdateTags(conn, d.Id(), nil, v); err != nil { + return fmt.Errorf("Error adding Organizational Unit (%s) tags: %s", d.Id(), err) + } + } + return resourceAwsOrganizationsOrganizationalUnitRead(d, meta) } @@ -134,7 +142,7 @@ func resourceAwsOrganizationsOrganizationalUnitRead(d *schema.ResourceData, meta parentId, err := resourceAwsOrganizationsOrganizationalUnitGetParentId(conn, d.Id()) if err != nil { - log.Printf("[WARN] Unable to find parent organizational unit, removing from state: %s", d.Id()) + log.Printf("[WARN] Unable to find parent Organizational Unit, removing from state: %s", d.Id()) d.SetId("") return nil } @@ -163,6 +171,17 @@ func resourceAwsOrganizationsOrganizationalUnitRead(d *schema.ResourceData, meta d.Set("arn", ou.Arn) d.Set("name", ou.Name) d.Set("parent_id", parentId) + + tags, err := keyvaluetags.OrganizationsListTags(conn, d.Id()) + + if err != nil { + return fmt.Errorf("error listing tags for AWS Organizational Unit (%s): %s", d.Id(), err) + } + + if err := d.Set("tags", tags.IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()); err != nil { + return fmt.Errorf("error setting tags: %s", err) + } + return nil } @@ -178,11 +197,19 @@ func resourceAwsOrganizationsOrganizationalUnitUpdate(d *schema.ResourceData, me log.Printf("[DEBUG] Organizational Unit update config: %#v", updateOpts) resp, err := conn.UpdateOrganizationalUnit(updateOpts) if err != nil { - return fmt.Errorf("Error creating organizational unit: %s", err) + return fmt.Errorf("Error updating Organizational Unit: %s", err) } log.Printf("[DEBUG] Organizational Unit update response: %#v", resp) } + if d.HasChange("tags") { + o, n := d.GetChange("tags") + + if err := keyvaluetags.OrganizationsUpdateTags(conn, d.Id(), o, n); err != nil { + return fmt.Errorf("error updating Organizational Unit (%s) tags: %s", d.Id(), err) + } + } + return nil } @@ -192,7 +219,7 @@ func resourceAwsOrganizationsOrganizationalUnitDelete(d *schema.ResourceData, me input := &organizations.DeleteOrganizationalUnitInput{ OrganizationalUnitId: aws.String(d.Id()), } - log.Printf("[DEBUG] Removing AWS organizational unit from organization: %s", input) + log.Printf("[DEBUG] Removing AWS Organizational Unit from organization: %s", input) _, err := conn.DeleteOrganizationalUnit(input) if err != nil { if isAWSErr(err, organizations.ErrCodeOrganizationalUnitNotFoundException, "") { diff --git a/aws/resource_aws_organizations_organizational_unit_test.go b/aws/resource_aws_organizations_organizational_unit_test.go index 2d6f35afee6..e92a28cef32 100644 --- a/aws/resource_aws_organizations_organizational_unit_test.go +++ b/aws/resource_aws_organizations_organizational_unit_test.go @@ -30,6 +30,7 @@ func testAccAwsOrganizationsOrganizationalUnit_basic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "accounts.#", "0"), testAccMatchResourceAttrGlobalARN(resourceName, "arn", "organizations", regexp.MustCompile(`ou/o-.+/ou-.+`)), resource.TestCheckResourceAttr(resourceName, "name", name), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), ), }, { @@ -77,6 +78,52 @@ func testAccAwsOrganizationsOrganizationalUnit_Name(t *testing.T) { }) } +func testAccAwsOrganizationsOrganizationalUnit_Tags(t *testing.T) { + var unit organizations.OrganizationalUnit + + rInt := acctest.RandInt() + name := fmt.Sprintf("tf_outest_%d", rInt) + resourceName := "aws_organizations_organizational_unit.test" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + PreCheck: func() { testAccPreCheck(t); testAccOrganizationsAccountPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAwsOrganizationsOrganizationalUnitDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAwsOrganizationsOrganizationalUnitConfigTags1(name, "key1", "value1"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsOrganizationsOrganizationalUnitExists(resourceName, &unit), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccAwsOrganizationsOrganizationalUnitConfigTags2(name, "key1", "value1updated", "key2", "value2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsOrganizationsOrganizationalUnitExists(resourceName, &unit), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), + ), + }, + { + Config: testAccAwsOrganizationsOrganizationalUnitConfig(name), + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsOrganizationsOrganizationalUnitExists(resourceName, &unit), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + ), + }, + }, + }) +} + func testAccCheckAwsOrganizationsOrganizationalUnitDestroy(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).organizationsconn @@ -151,3 +198,34 @@ resource "aws_organizations_organizational_unit" "test" { } `, name) } + +func testAccAwsOrganizationsOrganizationalUnitConfigTags1(name, tagKey1, tagValue1 string) string { + return fmt.Sprintf(` +resource "aws_organizations_organization" "test" {} + +resource "aws_organizations_organizational_unit" "test" { + name = %[1]q + parent_id = aws_organizations_organization.test.roots[0].id + + tags = { + %[3]q = %[4]q + } +} +`, name, tagKey1, tagValue1) +} + +func testAccAwsOrganizationsOrganizationalUnitConfigTags2(name, tagKey1, tagValue1, tagKey2, tagValue2 string) string { + return fmt.Sprintf(` +resource "aws_organizations_organization" "test" {} + +resource "aws_organizations_organizational_unit" "test" { + name = %[1]q + parent_id = aws_organizations_organization.test.roots[0].id + + tags = { + %[3]q = %[4]q + %[5]q = %[6]q + } +} +`, name, tagKey1, tagValue1, tagKey2, tagValue2) +} From c766cbb7902bd911c8f359b9832e37c605eddf16 Mon Sep 17 00:00:00 2001 From: Ryan Kennedy Date: Mon, 12 Oct 2020 22:31:31 -0700 Subject: [PATCH 2/4] Adding some fixes to resource_aws_organizations_organizational_unit Reverting changes to data_source_aws_organizations_organizational_units* --- ...source_aws_organizations_organizational_units.go | 6 ------ ...e_aws_organizations_organizational_units_test.go | 13 +++---------- ...esource_aws_organizations_organizational_unit.go | 6 ++++-- 3 files changed, 7 insertions(+), 18 deletions(-) diff --git a/aws/data_source_aws_organizations_organizational_units.go b/aws/data_source_aws_organizations_organizational_units.go index 0e9884cb4fd..08d49df5674 100644 --- a/aws/data_source_aws_organizations_organizational_units.go +++ b/aws/data_source_aws_organizations_organizational_units.go @@ -7,7 +7,6 @@ import ( "github.com/aws/aws-sdk-go/service/organizations" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) func dataSourceAwsOrganizationsOrganizationalUnits() *schema.Resource { @@ -39,7 +38,6 @@ func dataSourceAwsOrganizationsOrganizationalUnits() *schema.Resource { }, }, }, - "tags": tagsSchemaComputed(), }, } } @@ -71,10 +69,6 @@ func dataSourceAwsOrganizationsOrganizationalUnitsRead(d *schema.ResourceData, m return fmt.Errorf("Error setting children: %s", err) } - if err := d.Set("tags", keyvaluetags.OrganizationsKeyValueTags(volume.Tags).IgnoreAws().IgnoreConfig(client.IgnoreTagsConfig).Map()); err != nil { - return fmt.Errorf("error setting tags: %s", err) - } - return nil } diff --git a/aws/data_source_aws_organizations_organizational_units_test.go b/aws/data_source_aws_organizations_organizational_units_test.go index 8be911cb133..9301852dcb8 100644 --- a/aws/data_source_aws_organizations_organizational_units_test.go +++ b/aws/data_source_aws_organizations_organizational_units_test.go @@ -9,7 +9,6 @@ import ( func testAccDataSourceAwsOrganizationsOrganizationalUnits_basic(t *testing.T) { resourceName := "aws_organizations_organizational_unit.test" dataSourceName := "data.aws_organizations_organizational_units.test" - rInt := acctest.RandIntRange(0, 256) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) @@ -18,7 +17,7 @@ func testAccDataSourceAwsOrganizationsOrganizationalUnits_basic(t *testing.T) { Providers: testAccProviders, Steps: []resource.TestStep{ { - Config: testAccDataSourceAwsOrganizationsOrganizationalUnitsConfig(rInt), + Config: testAccDataSourceAwsOrganizationsOrganizationalUnitsConfig, Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(dataSourceName, "children.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "name", dataSourceName, "children.0.name"), @@ -30,21 +29,15 @@ func testAccDataSourceAwsOrganizationsOrganizationalUnits_basic(t *testing.T) { }) } -func testAccDataSourceAwsOrganizationsOrganizationalUnitsConfig(rInt int) string { - return fmt.Sprintf(` +const testAccDataSourceAwsOrganizationsOrganizationalUnitsConfig = ` resource "aws_organizations_organization" "test" {} resource "aws_organizations_organizational_unit" "test" { name = "test" parent_id = aws_organizations_organization.test.roots[0].id - - tags = { - TestIdentifierSet = "testAccDataSourceAwsEbsVolumes-%d" - } } data "aws_organizations_organizational_units" "test" { parent_id = aws_organizations_organizational_unit.test.parent_id } -`, rInt) -} +` diff --git a/aws/resource_aws_organizations_organizational_unit.go b/aws/resource_aws_organizations_organizational_unit.go index 4441c012c98..47a01be979e 100644 --- a/aws/resource_aws_organizations_organizational_unit.go +++ b/aws/resource_aws_organizations_organizational_unit.go @@ -120,6 +120,8 @@ func resourceAwsOrganizationsOrganizationalUnitCreate(d *schema.ResourceData, me func resourceAwsOrganizationsOrganizationalUnitRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).organizationsconn + ignoreTagsConfig := meta.(*AWSClient).IgnoreTagsConfig + describeOpts := &organizations.DescribeOrganizationalUnitInput{ OrganizationalUnitId: aws.String(d.Id()), } @@ -186,9 +188,9 @@ func resourceAwsOrganizationsOrganizationalUnitRead(d *schema.ResourceData, meta } func resourceAwsOrganizationsOrganizationalUnitUpdate(d *schema.ResourceData, meta interface{}) error { - if d.HasChange("name") { - conn := meta.(*AWSClient).organizationsconn + conn := meta.(*AWSClient).organizationsconn + if d.HasChange("name") { updateOpts := &organizations.UpdateOrganizationalUnitInput{ Name: aws.String(d.Get("name").(string)), OrganizationalUnitId: aws.String(d.Id()), From e5425e0d6820f3ead9ec18d6fa0d301dbaa95de6 Mon Sep 17 00:00:00 2001 From: Ryan Kennedy Date: Tue, 13 Oct 2020 00:00:06 -0700 Subject: [PATCH 3/4] fixing a few issues and adding the OU tags to the service suite. --- aws/resource_aws_organizations_organizational_unit_test.go | 7 +++---- aws/resource_aws_organizations_test.go | 1 + 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/aws/resource_aws_organizations_organizational_unit_test.go b/aws/resource_aws_organizations_organizational_unit_test.go index e92a28cef32..02f8214cab8 100644 --- a/aws/resource_aws_organizations_organizational_unit_test.go +++ b/aws/resource_aws_organizations_organizational_unit_test.go @@ -86,7 +86,6 @@ func testAccAwsOrganizationsOrganizationalUnit_Tags(t *testing.T) { resourceName := "aws_organizations_organizational_unit.test" resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, PreCheck: func() { testAccPreCheck(t); testAccOrganizationsAccountPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckAwsOrganizationsOrganizationalUnitDestroy, @@ -208,7 +207,7 @@ resource "aws_organizations_organizational_unit" "test" { parent_id = aws_organizations_organization.test.roots[0].id tags = { - %[3]q = %[4]q + %[2]q = %[3]q } } `, name, tagKey1, tagValue1) @@ -223,8 +222,8 @@ resource "aws_organizations_organizational_unit" "test" { parent_id = aws_organizations_organization.test.roots[0].id tags = { - %[3]q = %[4]q - %[5]q = %[6]q + %[2]q = %[3]q + %[4]q = %[5]q } } `, name, tagKey1, tagValue1, tagKey2, tagValue2) diff --git a/aws/resource_aws_organizations_test.go b/aws/resource_aws_organizations_test.go index 409a9d1b42b..c2e9a72963b 100644 --- a/aws/resource_aws_organizations_test.go +++ b/aws/resource_aws_organizations_test.go @@ -21,6 +21,7 @@ func TestAccAWSOrganizations_serial(t *testing.T) { "OrganizationalUnit": { "basic": testAccAwsOrganizationsOrganizationalUnit_basic, "Name": testAccAwsOrganizationsOrganizationalUnit_Name, + "Tags": testAccAwsOrganizationsOrganizationalUnit_Tags, }, "OrganizationalUnits": { "DataSource": testAccDataSourceAwsOrganizationsOrganizationalUnits_basic, From 6ef6001ffc7df938c0a21f7e2afad0e531bd47b4 Mon Sep 17 00:00:00 2001 From: Ryan Kennedy Date: Tue, 13 Oct 2020 00:17:40 -0700 Subject: [PATCH 4/4] adding docs --- website/docs/r/organizations_organizational_unit.html.markdown | 1 + 1 file changed, 1 insertion(+) diff --git a/website/docs/r/organizations_organizational_unit.html.markdown b/website/docs/r/organizations_organizational_unit.html.markdown index 39acb76ef7e..d83081a40a8 100644 --- a/website/docs/r/organizations_organizational_unit.html.markdown +++ b/website/docs/r/organizations_organizational_unit.html.markdown @@ -25,6 +25,7 @@ The following arguments are supported: * `name` - The name for the organizational unit * `parent_id` - ID of the parent organizational unit, which may be the root +* `tags` - (Optional) Key-value map of resource tags. ## Attributes Reference