Skip to content

Commit

Permalink
Merge pull request #14247 from DrFaust92/r/config_configuration_aggre…
Browse files Browse the repository at this point in the history
…gator_lower
  • Loading branch information
gdavison committed Mar 25, 2021
2 parents 1d0d370 + 8be2d07 commit f3e15b5
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 60 deletions.
3 changes: 3 additions & 0 deletions .changelog/14247.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_config_configuration_aggregator: Allow name to have uppercase characters
```
44 changes: 23 additions & 21 deletions aws/resource_aws_config_configuration_aggregator.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"
"log"
"strings"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/configservice"
Expand Down Expand Up @@ -115,35 +114,33 @@ func resourceAwsConfigConfigurationAggregator() *schema.Resource {
func resourceAwsConfigConfigurationAggregatorPut(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).configconn

name := d.Get("name").(string)

req := &configservice.PutConfigurationAggregatorInput{
ConfigurationAggregatorName: aws.String(name),
ConfigurationAggregatorName: aws.String(d.Get("name").(string)),
Tags: keyvaluetags.New(d.Get("tags").(map[string]interface{})).IgnoreAws().ConfigserviceTags(),
}

account_aggregation_sources := d.Get("account_aggregation_source").([]interface{})
if len(account_aggregation_sources) > 0 {
req.AccountAggregationSources = expandConfigAccountAggregationSources(account_aggregation_sources)
if v, ok := d.GetOk("account_aggregation_source"); ok && len(v.([]interface{})) > 0 {
req.AccountAggregationSources = expandConfigAccountAggregationSources(v.([]interface{}))
}

organization_aggregation_sources := d.Get("organization_aggregation_source").([]interface{})
if len(organization_aggregation_sources) > 0 {
req.OrganizationAggregationSource = expandConfigOrganizationAggregationSource(organization_aggregation_sources[0].(map[string]interface{}))
if v, ok := d.GetOk("organization_aggregation_source"); ok && len(v.([]interface{})) > 0 {
req.OrganizationAggregationSource = expandConfigOrganizationAggregationSource(v.([]interface{})[0].(map[string]interface{}))
}

_, err := conn.PutConfigurationAggregator(req)
resp, err := conn.PutConfigurationAggregator(req)
if err != nil {
return fmt.Errorf("Error creating aggregator: %s", err)
return fmt.Errorf("error creating aggregator: %w", err)
}

d.SetId(strings.ToLower(name))
configAgg := resp.ConfigurationAggregator
d.SetId(aws.StringValue(configAgg.ConfigurationAggregatorName))

if !d.IsNewResource() && d.HasChange("tags") {
o, n := d.GetChange("tags")

if err := keyvaluetags.ConfigserviceUpdateTags(conn, d.Get("arn").(string), o, n); err != nil {
return fmt.Errorf("error updating Config Configuration Aggregator (%s) tags: %s", d.Get("arn").(string), err)
arn := aws.StringValue(configAgg.ConfigurationAggregatorArn)
if err := keyvaluetags.ConfigserviceUpdateTags(conn, arn, o, n); err != nil {
return fmt.Errorf("error updating Config Configuration Aggregator (%s) tags: %w", arn, err)
}
}

Expand Down Expand Up @@ -175,7 +172,8 @@ func resourceAwsConfigConfigurationAggregatorRead(d *schema.ResourceData, meta i
}

aggregator := res.ConfigurationAggregators[0]
d.Set("arn", aggregator.ConfigurationAggregatorArn)
arn := aws.StringValue(aggregator.ConfigurationAggregatorArn)
d.Set("arn", arn)
d.Set("name", aggregator.ConfigurationAggregatorName)

if err := d.Set("account_aggregation_source", flattenConfigAccountAggregationSources(aggregator.AccountAggregationSources)); err != nil {
Expand All @@ -186,14 +184,14 @@ func resourceAwsConfigConfigurationAggregatorRead(d *schema.ResourceData, meta i
return fmt.Errorf("error setting organization_aggregation_source: %s", err)
}

tags, err := keyvaluetags.ConfigserviceListTags(conn, d.Get("arn").(string))
tags, err := keyvaluetags.ConfigserviceListTags(conn, arn)

if err != nil {
return fmt.Errorf("error listing tags for Config Configuration Aggregator (%s): %s", d.Get("arn").(string), err)
return fmt.Errorf("error listing tags for Config Configuration Aggregator (%s): %w", arn, err)
}

if err := d.Set("tags", tags.IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()); err != nil {
return fmt.Errorf("error setting tags: %s", err)
return fmt.Errorf("error setting tags: %w", err)
}

return nil
Expand All @@ -206,10 +204,14 @@ func resourceAwsConfigConfigurationAggregatorDelete(d *schema.ResourceData, meta
ConfigurationAggregatorName: aws.String(d.Id()),
}
_, err := conn.DeleteConfigurationAggregator(req)

if isAWSErr(err, configservice.ErrCodeNoSuchConfigurationAggregatorException, "") {
return nil
}

if err != nil {
return err
return fmt.Errorf("error deleting Config Configuration Aggregator (%s): %w", d.Id(), err)
}

d.SetId("")
return nil
}
121 changes: 82 additions & 39 deletions aws/resource_aws_config_configuration_aggregator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package aws
import (
"fmt"
"log"
"regexp"
"testing"

"github.com/aws/aws-sdk-go/aws"
Expand Down Expand Up @@ -49,7 +50,8 @@ func testSweepConfigConfigurationAggregators(region string) error {
})

if err != nil {
return fmt.Errorf("Error deleting config configuration aggregator %s: %s", *agg.ConfigurationAggregatorName, err)
return fmt.Errorf("error deleting config configuration aggregator %s: %w",
aws.StringValue(agg.ConfigurationAggregatorName), err)
}
}

Expand All @@ -58,8 +60,9 @@ func testSweepConfigConfigurationAggregators(region string) error {

func TestAccAWSConfigConfigurationAggregator_account(t *testing.T) {
var ca configservice.ConfigurationAggregator
rName := acctest.RandomWithPrefix("tf-acc-test")
resourceName := "aws_config_configuration_aggregator.example"
//Name is upper case on purpose to test https:/terraform-providers/terraform-provider-aws/issues/8432
rName := acctest.RandomWithPrefix("Tf-acc-test")
resourceName := "aws_config_configuration_aggregator.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -73,11 +76,13 @@ func TestAccAWSConfigConfigurationAggregator_account(t *testing.T) {
testAccCheckAWSConfigConfigurationAggregatorExists(resourceName, &ca),
testAccCheckAWSConfigConfigurationAggregatorName(resourceName, rName, &ca),
resource.TestCheckResourceAttr(resourceName, "name", rName),
testAccMatchResourceAttrRegionalARN(resourceName, "arn", "config", regexp.MustCompile(`config-aggregator/config-aggregator-.+`)),
resource.TestCheckResourceAttr(resourceName, "account_aggregation_source.#", "1"),
resource.TestCheckResourceAttr(resourceName, "account_aggregation_source.0.account_ids.#", "1"),
testAccCheckResourceAttrAccountID(resourceName, "account_aggregation_source.0.account_ids.0"),
resource.TestCheckResourceAttr(resourceName, "account_aggregation_source.0.regions.#", "1"),
resource.TestCheckResourceAttrPair(resourceName, "account_aggregation_source.0.regions.0", "data.aws_region.current", "name"),
resource.TestCheckResourceAttr(resourceName, "tags.%", "0"),
),
},
{
Expand All @@ -92,7 +97,7 @@ func TestAccAWSConfigConfigurationAggregator_account(t *testing.T) {
func TestAccAWSConfigConfigurationAggregator_organization(t *testing.T) {
var ca configservice.ConfigurationAggregator
rName := acctest.RandomWithPrefix("tf-acc-test")
resourceName := "aws_config_configuration_aggregator.example"
resourceName := "aws_config_configuration_aggregator.test"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccOrganizationsAccountPreCheck(t) },
Expand All @@ -107,7 +112,7 @@ func TestAccAWSConfigConfigurationAggregator_organization(t *testing.T) {
testAccCheckAWSConfigConfigurationAggregatorName(resourceName, rName, &ca),
resource.TestCheckResourceAttr(resourceName, "name", rName),
resource.TestCheckResourceAttr(resourceName, "organization_aggregation_source.#", "1"),
resource.TestCheckResourceAttrPair(resourceName, "organization_aggregation_source.0.role_arn", "aws_iam_role.example", "arn"),
resource.TestCheckResourceAttrPair(resourceName, "organization_aggregation_source.0.role_arn", "aws_iam_role.test", "arn"),
resource.TestCheckResourceAttr(resourceName, "organization_aggregation_source.0.all_regions", "true"),
),
},
Expand All @@ -122,7 +127,7 @@ func TestAccAWSConfigConfigurationAggregator_organization(t *testing.T) {

func TestAccAWSConfigConfigurationAggregator_switch(t *testing.T) {
rName := acctest.RandomWithPrefix("tf-acc-test")
resourceName := "aws_config_configuration_aggregator.example"
resourceName := "aws_config_configuration_aggregator.test"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccOrganizationsAccountPreCheck(t) },
Expand Down Expand Up @@ -151,7 +156,7 @@ func TestAccAWSConfigConfigurationAggregator_switch(t *testing.T) {
func TestAccAWSConfigConfigurationAggregator_tags(t *testing.T) {
var ca configservice.ConfigurationAggregator
rName := acctest.RandomWithPrefix("tf-acc-test")
resourceName := "aws_config_configuration_aggregator.example"
resourceName := "aws_config_configuration_aggregator.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -160,25 +165,22 @@ func TestAccAWSConfigConfigurationAggregator_tags(t *testing.T) {
CheckDestroy: testAccCheckAWSConfigConfigurationAggregatorDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSConfigConfigurationAggregatorConfig_tags(rName, "foo", "bar", "fizz", "buzz"),
Config: testAccAWSConfigConfigurationAggregatorConfigTags1(rName, "key1", "value1"),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSConfigConfigurationAggregatorExists(resourceName, &ca),
testAccCheckAWSConfigConfigurationAggregatorName(resourceName, rName, &ca),
resource.TestCheckResourceAttr(resourceName, "tags.%", "3"),
resource.TestCheckResourceAttr(resourceName, "tags.Name", rName),
resource.TestCheckResourceAttr(resourceName, "tags.foo", "bar"),
resource.TestCheckResourceAttr(resourceName, "tags.fizz", "buzz"),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"),
),
},
{
Config: testAccAWSConfigConfigurationAggregatorConfig_tags(rName, "foo", "bar2", "fizz2", "buzz2"),
Config: testAccAWSConfigConfigurationAggregatorConfigTags2(rName, "key1", "value1updated", "key2", "value2"),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSConfigConfigurationAggregatorExists(resourceName, &ca),
testAccCheckAWSConfigConfigurationAggregatorName(resourceName, rName, &ca),
resource.TestCheckResourceAttr(resourceName, "tags.%", "3"),
resource.TestCheckResourceAttr(resourceName, "tags.Name", rName),
resource.TestCheckResourceAttr(resourceName, "tags.foo", "bar2"),
resource.TestCheckResourceAttr(resourceName, "tags.fizz2", "buzz2"),
resource.TestCheckResourceAttr(resourceName, "tags.%", "2"),
resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"),
resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"),
),
},
{
Expand All @@ -187,25 +189,48 @@ func TestAccAWSConfigConfigurationAggregator_tags(t *testing.T) {
ImportStateVerify: true,
},
{
Config: testAccAWSConfigConfigurationAggregatorConfig_account(rName),
Config: testAccAWSConfigConfigurationAggregatorConfigTags1(rName, "key2", "value2"),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSConfigConfigurationAggregatorExists(resourceName, &ca),
testAccCheckAWSConfigConfigurationAggregatorName(resourceName, rName, &ca),
resource.TestCheckResourceAttr(resourceName, "tags.%", "0"),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"),
),
},
},
})
}

func TestAccAWSConfigConfigurationAggregator_disappears(t *testing.T) {
var ca configservice.ConfigurationAggregator
rName := acctest.RandomWithPrefix("tf-acc-test")
resourceName := "aws_config_configuration_aggregator.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSConfigConfigurationAggregatorDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSConfigConfigurationAggregatorConfig_account(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSConfigConfigurationAggregatorExists(resourceName, &ca),
testAccCheckResourceDisappears(testAccProvider, resourceAwsConfigConfigurationAggregator(), resourceName),
),
ExpectNonEmptyPlan: true,
},
},
})
}

func testAccCheckAWSConfigConfigurationAggregatorName(n, desired string, obj *configservice.ConfigurationAggregator) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
if !ok {
return fmt.Errorf("Not found: %s", n)
}
if rs.Primary.Attributes["name"] != *obj.ConfigurationAggregatorName {
return fmt.Errorf("Expected name: %q, given: %q", desired, *obj.ConfigurationAggregatorName)
if rs.Primary.Attributes["name"] != aws.StringValue(obj.ConfigurationAggregatorName) {
return fmt.Errorf("expected name: %q, given: %q", desired, aws.StringValue(obj.ConfigurationAggregatorName))
}
return nil
}
Expand Down Expand Up @@ -254,7 +279,7 @@ func testAccCheckAWSConfigConfigurationAggregatorDestroy(s *terraform.State) err

if err == nil {
if len(resp.ConfigurationAggregators) != 0 &&
*resp.ConfigurationAggregators[0].ConfigurationAggregatorName == rs.Primary.Attributes["name"] {
aws.StringValue(resp.ConfigurationAggregators[0].ConfigurationAggregatorName) == rs.Primary.Attributes["name"] {
return fmt.Errorf("config configuration aggregator still exists: %s", rs.Primary.Attributes["name"])
}
}
Expand All @@ -265,19 +290,18 @@ func testAccCheckAWSConfigConfigurationAggregatorDestroy(s *terraform.State) err

func testAccAWSConfigConfigurationAggregatorConfig_account(rName string) string {
return fmt.Sprintf(`
data "aws_caller_identity" "current" {}
data "aws_region" "current" {}
resource "aws_config_configuration_aggregator" "example" {
name = %q
resource "aws_config_configuration_aggregator" "test" {
name = %[1]q
account_aggregation_source {
account_ids = [data.aws_caller_identity.current.account_id]
regions = [data.aws_region.current.name]
}
}
data "aws_caller_identity" "current" {
}
`, rName)
}

Expand All @@ -287,20 +311,20 @@ resource "aws_organizations_organization" "test" {
aws_service_access_principals = ["config.${data.aws_partition.current.dns_suffix}"]
}
resource "aws_config_configuration_aggregator" "example" {
depends_on = [aws_iam_role_policy_attachment.example, aws_organizations_organization.test]
resource "aws_config_configuration_aggregator" "test" {
depends_on = [aws_iam_role_policy_attachment.test, aws_organizations_organization.test]
name = %[1]q
organization_aggregation_source {
all_regions = true
role_arn = aws_iam_role.example.arn
role_arn = aws_iam_role.test.arn
}
}
data "aws_partition" "current" {}
resource "aws_iam_role" "example" {
resource "aws_iam_role" "test" {
name = %[1]q
assume_role_policy = <<EOF
Expand All @@ -320,18 +344,20 @@ resource "aws_iam_role" "example" {
EOF
}
resource "aws_iam_role_policy_attachment" "example" {
role = aws_iam_role.example.name
resource "aws_iam_role_policy_attachment" "test" {
role = aws_iam_role.test.name
policy_arn = "arn:${data.aws_partition.current.partition}:iam::aws:policy/service-role/AWSConfigRoleForOrganizations"
}
`, rName)
}

func testAccAWSConfigConfigurationAggregatorConfig_tags(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string {
func testAccAWSConfigConfigurationAggregatorConfigTags1(rName, tagKey1, tagValue1 string) string {
return fmt.Sprintf(`
data "aws_caller_identity" "current" {}
data "aws_region" "current" {}
resource "aws_config_configuration_aggregator" "example" {
resource "aws_config_configuration_aggregator" "test" {
name = %[1]q
account_aggregation_source {
Expand All @@ -340,13 +366,30 @@ resource "aws_config_configuration_aggregator" "example" {
}
tags = {
Name = %[1]q
%[2]s = %[3]q
%[4]s = %[5]q
%[2]q = %[3]q
}
}
`, rName, tagKey1, tagValue1)
}

func testAccAWSConfigConfigurationAggregatorConfigTags2(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string {
return fmt.Sprintf(`
data "aws_caller_identity" "current" {}
data "aws_region" "current" {}
resource "aws_config_configuration_aggregator" "test" {
name = %[1]q
account_aggregation_source {
account_ids = [data.aws_caller_identity.current.account_id]
regions = [data.aws_region.current.name]
}
tags = {
%[2]q = %[3]q
%[4]q = %[5]q
}
}
`, rName, tagKey1, tagValue1, tagKey2, tagValue2)
}

0 comments on commit f3e15b5

Please sign in to comment.