From 512cbf723ef449c762a81fc8a1e1bc1adca6ce3d Mon Sep 17 00:00:00 2001 From: DrFaust92 Date: Sun, 19 Jul 2020 23:40:43 +0300 Subject: [PATCH 1/5] fix setting id some refactor to tests and resource added disappears test remove hardcoded region from tests --- ...rce_aws_config_configuration_aggregator.go | 45 +++---- ...ws_config_configuration_aggregator_test.go | 110 ++++++++++++------ 2 files changed, 100 insertions(+), 55 deletions(-) diff --git a/aws/resource_aws_config_configuration_aggregator.go b/aws/resource_aws_config_configuration_aggregator.go index 71afa91648f..6ae318e814a 100644 --- a/aws/resource_aws_config_configuration_aggregator.go +++ b/aws/resource_aws_config_configuration_aggregator.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "log" - "strings" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/configservice" @@ -115,35 +114,34 @@ 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) + log.Printf("[DEBUG] Putting Config Configuration Aggregator: %#v", 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) } } @@ -175,7 +173,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 { @@ -186,14 +185,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 @@ -206,10 +205,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 } diff --git a/aws/resource_aws_config_configuration_aggregator_test.go b/aws/resource_aws_config_configuration_aggregator_test.go index db238e97c15..c20a4e80e1b 100644 --- a/aws/resource_aws_config_configuration_aggregator_test.go +++ b/aws/resource_aws_config_configuration_aggregator_test.go @@ -3,6 +3,7 @@ package aws import ( "fmt" "log" + "regexp" "testing" "github.com/aws/aws-sdk-go/aws" @@ -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) } } @@ -59,7 +61,7 @@ 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" + resourceName := "aws_config_configuration_aggregator.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -72,11 +74,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"), ), }, { @@ -91,7 +95,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.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t); testAccOrganizationsAccountPreCheck(t) }, @@ -120,7 +124,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.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t); testAccOrganizationsAccountPreCheck(t) }, @@ -148,7 +152,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) }, @@ -156,25 +160,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"), ), }, { @@ -183,12 +184,35 @@ 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, }, }, }) @@ -200,8 +224,8 @@ func testAccCheckAWSConfigConfigurationAggregatorName(n, desired string, obj *co 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 } @@ -250,7 +274,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"]) } } @@ -261,19 +285,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) } @@ -281,14 +304,14 @@ func testAccAWSConfigConfigurationAggregatorConfig_organization(rName string) st return fmt.Sprintf(` resource "aws_organizations_organization" "test" {} -resource "aws_config_configuration_aggregator" "example" { - depends_on = [aws_iam_role_policy_attachment.example] +resource "aws_config_configuration_aggregator" "test" { + depends_on = [aws_iam_role_policy_attachment.test] name = %[1]q organization_aggregation_source { all_regions = true - role_arn = aws_iam_role.example.arn + role_arn = aws_iam_role.test.arn } } @@ -315,17 +338,19 @@ EOF } resource "aws_iam_role_policy_attachment" "example" { - role = aws_iam_role.example.name + 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 { @@ -334,13 +359,30 @@ resource "aws_config_configuration_aggregator" "example" { } tags = { - Name = %[1]q - %[2]s = %[3]q - %[4]s = %[5]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]s = %[3]q + %[4]s = %[5]q + } +} `, rName, tagKey1, tagValue1, tagKey2, tagValue2) } From 786addd5090febce3c99a593588b47080c08b756 Mon Sep 17 00:00:00 2001 From: DrFaust92 Date: Thu, 27 Aug 2020 23:37:53 +0300 Subject: [PATCH 2/5] upper case test --- aws/resource_aws_config_configuration_aggregator_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/resource_aws_config_configuration_aggregator_test.go b/aws/resource_aws_config_configuration_aggregator_test.go index c20a4e80e1b..e09591d8d11 100644 --- a/aws/resource_aws_config_configuration_aggregator_test.go +++ b/aws/resource_aws_config_configuration_aggregator_test.go @@ -60,7 +60,7 @@ func testSweepConfigConfigurationAggregators(region string) error { func TestAccAWSConfigConfigurationAggregator_account(t *testing.T) { var ca configservice.ConfigurationAggregator - rName := acctest.RandomWithPrefix("tf-acc-test") + rName := acctest.RandomWithPrefix("Tf-acc-test") resourceName := "aws_config_configuration_aggregator.test" resource.ParallelTest(t, resource.TestCase{ From 5cf57bdb09869ce325663928c013377d071791f0 Mon Sep 17 00:00:00 2001 From: DrFaust92 Date: Sat, 29 Aug 2020 00:20:41 +0300 Subject: [PATCH 3/5] add comment --- aws/resource_aws_config_configuration_aggregator_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/aws/resource_aws_config_configuration_aggregator_test.go b/aws/resource_aws_config_configuration_aggregator_test.go index e09591d8d11..5bf3ddb0a36 100644 --- a/aws/resource_aws_config_configuration_aggregator_test.go +++ b/aws/resource_aws_config_configuration_aggregator_test.go @@ -60,6 +60,7 @@ func testSweepConfigConfigurationAggregators(region string) error { func TestAccAWSConfigConfigurationAggregator_account(t *testing.T) { var ca configservice.ConfigurationAggregator + //Name is upper case on purpose to test https://github.com/terraform-providers/terraform-provider-aws/issues/8432 rName := acctest.RandomWithPrefix("Tf-acc-test") resourceName := "aws_config_configuration_aggregator.test" From 9c179d121574a8683a9d4119b579864f946f7b85 Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Wed, 30 Sep 2020 10:40:23 +0300 Subject: [PATCH 4/5] lint --- aws/resource_aws_config_configuration_aggregator_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/aws/resource_aws_config_configuration_aggregator_test.go b/aws/resource_aws_config_configuration_aggregator_test.go index 5bf3ddb0a36..184bc42785e 100644 --- a/aws/resource_aws_config_configuration_aggregator_test.go +++ b/aws/resource_aws_config_configuration_aggregator_test.go @@ -360,7 +360,7 @@ resource "aws_config_configuration_aggregator" "test" { } tags = { - %[2]s = %[3]q + %[2]q = %[3]q } } `, rName, tagKey1, tagValue1) @@ -381,8 +381,8 @@ resource "aws_config_configuration_aggregator" "test" { } tags = { - %[2]s = %[3]q - %[4]s = %[5]q + %[2]q = %[3]q + %[4]q = %[5]q } } `, rName, tagKey1, tagValue1, tagKey2, tagValue2) From 8be2d07264c2a4fd405cbed316de8ba4b52af937 Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Fri, 12 Feb 2021 09:57:06 +0200 Subject: [PATCH 5/5] changelog --- .changelog/14247.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/14247.txt diff --git a/.changelog/14247.txt b/.changelog/14247.txt new file mode 100644 index 00000000000..93eb2aa3832 --- /dev/null +++ b/.changelog/14247.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_config_configuration_aggregator: Allow name to have uppercase characters +```