From 12964502a46290bf2afa74edcd1eb5e996521d5d Mon Sep 17 00:00:00 2001 From: Jeremy Barlow Date: Thu, 20 Feb 2020 11:26:44 -0700 Subject: [PATCH] r/aws_rds_cluster_parameter_group and r/aws_db_parameter_group: Restore ability to change parameter values References: * https://github.com/terraform-providers/terraform-provider-aws/issues/11846 * https://github.com/terraform-providers/terraform-provider-aws/pull/11540 In #11540, support was added to the rds parameter group resources for resetting parameter values to AWS defaults when parameters are removed from config. A side-effect of these changes, however, was that parameters which remain in the config but whose values have been changed would also be reset to AWS defaults. This commit restores the ability for parameter values to be updated in the config while retaining the ability for parameters being removed from the config to be reset to AWS defaults. Output from acceptance testing: ``` make testacc TEST=./aws TESTARGS='-run=TestAccAWSDBClusterParameterGroup_' ... --- PASS: TestAccAWSDBClusterParameterGroup_disappears (18.01s) --- PASS: TestAccAWSDBClusterParameterGroup_namePrefix (23.28s) --- PASS: TestAccAWSDBClusterParameterGroup_only (23.40s) --- PASS: TestAccAWSDBClusterParameterGroup_generatedName (23.42s) --- PASS: TestAccAWSDBClusterParameterGroup_generatedName_Parameter (23.77s) --- PASS: TestAccAWSDBClusterParameterGroup_namePrefix_Parameter (23.79s) --- PASS: TestAccAWSDBClusterParameterGroup_withApplyMethod (23.89s) --- PASS: TestAccAWSDBClusterParameterGroup_basic (86.64s) --- PASS: TestAccAWSDBClusterParameterGroup_updateParameters (24.48s) ``` ``` make testacc TEST=./aws TESTARGS='-run=TestAccAWSDBParameterGroup_' ... --- PASS: TestAccAWSDBParameterGroup_Disappears (18.18s) --- PASS: TestAccAWSDBParameterGroup_generatedName (22.40s) --- PASS: TestAccAWSDBParameterGroup_namePrefix (22.69s) --- PASS: TestAccAWSDBParameterGroup_Only (23.40s) --- PASS: TestAccAWSDBParameterGroup_MatchDefault (24.19s) --- PASS: TestAccAWSDBParameterGroup_withApplyMethod (25.23s) --- PASS: TestAccAWSDBParameterGroup_limit (45.08s) --- PASS: TestAccAWSDBParameterGroup_basic (75.38s) --- PASS: TestAccAWSDBParameterGroup_updateParameters (30.10s) ``` --- aws/resource_aws_db_parameter_group.go | 19 +++- aws/resource_aws_db_parameter_group_test.go | 107 ++++++++++++++++++ ...esource_aws_rds_cluster_parameter_group.go | 29 ++++- ...ce_aws_rds_cluster_parameter_group_test.go | 107 ++++++++++++++++++ 4 files changed, 255 insertions(+), 7 deletions(-) diff --git a/aws/resource_aws_db_parameter_group.go b/aws/resource_aws_db_parameter_group.go index 050dbfcde790..197f93b3a146 100644 --- a/aws/resource_aws_db_parameter_group.go +++ b/aws/resource_aws_db_parameter_group.go @@ -277,8 +277,25 @@ func resourceAwsDbParameterGroupUpdate(d *schema.ResourceData, meta interface{}) } } + toRemove := map[string]*rds.Parameter{} + + for _, p := range expandParameters(os.List()) { + if p.ParameterName != nil { + toRemove[*p.ParameterName] = p + } + } + + for _, p := range expandParameters(ns.List()) { + if p.ParameterName != nil { + delete(toRemove, *p.ParameterName) + } + } + // Reset parameters that have been removed - resetParameters := expandParameters(os.Difference(ns).List()) + var resetParameters []*rds.Parameter + for _, v := range toRemove { + resetParameters = append(resetParameters, v) + } if len(resetParameters) > 0 { maxParams := 20 for resetParameters != nil { diff --git a/aws/resource_aws_db_parameter_group_test.go b/aws/resource_aws_db_parameter_group_test.go index b93e6f25dfe7..f08ed77840d3 100644 --- a/aws/resource_aws_db_parameter_group_test.go +++ b/aws/resource_aws_db_parameter_group_test.go @@ -688,6 +688,65 @@ func TestAccAWSDBParameterGroup_MatchDefault(t *testing.T) { }) } +func TestAccAWSDBParameterGroup_updateParameters(t *testing.T) { + var v rds.DBParameterGroup + resourceName := "aws_db_parameter_group.test" + groupName := fmt.Sprintf("parameter-group-test-terraform-%d", acctest.RandInt()) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSDBParameterGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSDBParameterGroupUpdateParametersInitialConfig(groupName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSDBParameterGroupExists(resourceName, &v), + testAccCheckAWSDBParameterGroupAttributes(&v, groupName), + resource.TestCheckResourceAttr(resourceName, "name", groupName), + resource.TestCheckResourceAttr(resourceName, "family", "mysql5.6"), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "parameter.*", map[string]string{ + "name": "character_set_results", + "value": "utf8", + }), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "parameter.*", map[string]string{ + "name": "character_set_server", + "value": "utf8", + }), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "parameter.*", map[string]string{ + "name": "character_set_client", + "value": "utf8", + }), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccAWSDBParameterGroupUpdateParametersUpdatedConfig(groupName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSDBParameterGroupExists(resourceName, &v), + testAccCheckAWSDBParameterGroupAttributes(&v, groupName), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "parameter.*", map[string]string{ + "name": "character_set_results", + "value": "ascii", + }), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "parameter.*", map[string]string{ + "name": "character_set_server", + "value": "ascii", + }), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "parameter.*", map[string]string{ + "name": "character_set_client", + "value": "utf8", + }), + ), + }, + }, + }) +} + func testAccCheckAWSDbParamaterGroupDisappears(v *rds.DBParameterGroup) resource.TestCheckFunc { return func(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).rdsconn @@ -1372,6 +1431,54 @@ resource "aws_db_parameter_group" "test" { `, n) } +func testAccAWSDBParameterGroupUpdateParametersInitialConfig(n string) string { + return fmt.Sprintf(` +resource "aws_db_parameter_group" "test" { + name = "%s" + family = "mysql5.6" + + parameter { + name = "character_set_server" + value = "utf8" + } + + parameter { + name = "character_set_client" + value = "utf8" + } + + parameter { + name = "character_set_results" + value = "utf8" + } +} +`, n) +} + +func testAccAWSDBParameterGroupUpdateParametersUpdatedConfig(n string) string { + return fmt.Sprintf(` +resource "aws_db_parameter_group" "test" { + name = "%s" + family = "mysql5.6" + + parameter { + name = "character_set_server" + value = "ascii" + } + + parameter { + name = "character_set_client" + value = "utf8" + } + + parameter { + name = "character_set_results" + value = "ascii" + } +} +`, n) +} + const testAccDBParameterGroupConfig_namePrefix = ` resource "aws_db_parameter_group" "test" { name_prefix = "tf-test-" diff --git a/aws/resource_aws_rds_cluster_parameter_group.go b/aws/resource_aws_rds_cluster_parameter_group.go index 97f53cf18dc5..9230ce9b4f43 100644 --- a/aws/resource_aws_rds_cluster_parameter_group.go +++ b/aws/resource_aws_rds_cluster_parameter_group.go @@ -221,16 +221,33 @@ func resourceAwsRDSClusterParameterGroupUpdate(d *schema.ResourceData, meta inte } } + toRemove := map[string]*rds.Parameter{} + + for _, p := range expandParameters(os.List()) { + if p.ParameterName != nil { + toRemove[*p.ParameterName] = p + } + } + + for _, p := range expandParameters(ns.List()) { + if p.ParameterName != nil { + delete(toRemove, *p.ParameterName) + } + } + // Reset parameters that have been removed - parameters = expandParameters(os.Difference(ns).List()) - if len(parameters) > 0 { - for parameters != nil { + var resetParameters []*rds.Parameter + for _, v := range toRemove { + resetParameters = append(resetParameters, v) + } + if len(resetParameters) > 0 { + for resetParameters != nil { parameterGroupName := d.Get("name").(string) var paramsToReset []*rds.Parameter - if len(parameters) <= rdsClusterParameterGroupMaxParamsBulkEdit { - paramsToReset, parameters = parameters[:], nil + if len(resetParameters) <= rdsClusterParameterGroupMaxParamsBulkEdit { + paramsToReset, resetParameters = resetParameters[:], nil } else { - paramsToReset, parameters = parameters[:rdsClusterParameterGroupMaxParamsBulkEdit], parameters[rdsClusterParameterGroupMaxParamsBulkEdit:] + paramsToReset, resetParameters = resetParameters[:rdsClusterParameterGroupMaxParamsBulkEdit], resetParameters[rdsClusterParameterGroupMaxParamsBulkEdit:] } resetOpts := rds.ResetDBClusterParameterGroupInput{ diff --git a/aws/resource_aws_rds_cluster_parameter_group_test.go b/aws/resource_aws_rds_cluster_parameter_group_test.go index b72788ceb707..30c214425cca 100644 --- a/aws/resource_aws_rds_cluster_parameter_group_test.go +++ b/aws/resource_aws_rds_cluster_parameter_group_test.go @@ -376,6 +376,65 @@ func TestAccAWSDBClusterParameterGroup_only(t *testing.T) { }) } +func TestAccAWSDBClusterParameterGroup_updateParameters(t *testing.T) { + var v rds.DBClusterParameterGroup + resourceName := "aws_rds_cluster_parameter_group.test" + groupName := fmt.Sprintf("cluster-parameter-group-test-tf-%d", acctest.RandInt()) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSDBClusterParameterGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSDBClusterParameterGroupUpdateParametersInitialConfig(groupName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSDBClusterParameterGroupExists(resourceName, &v), + testAccCheckAWSDBClusterParameterGroupAttributes(&v, groupName), + resource.TestCheckResourceAttr(resourceName, "name", groupName), + resource.TestCheckResourceAttr(resourceName, "family", "aurora5.6"), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "parameter.*", map[string]string{ + "name": "character_set_results", + "value": "utf8", + }), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "parameter.*", map[string]string{ + "name": "character_set_server", + "value": "utf8", + }), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "parameter.*", map[string]string{ + "name": "character_set_client", + "value": "utf8", + }), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccAWSDBClusterParameterGroupUpdateParametersUpdatedConfig(groupName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSDBClusterParameterGroupExists(resourceName, &v), + testAccCheckAWSDBClusterParameterGroupAttributes(&v, groupName), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "parameter.*", map[string]string{ + "name": "character_set_results", + "value": "ascii", + }), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "parameter.*", map[string]string{ + "name": "character_set_server", + "value": "ascii", + }), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "parameter.*", map[string]string{ + "name": "character_set_client", + "value": "utf8", + }), + ), + }, + }, + }) +} + func testAccCheckAWSDBClusterParameterGroupDestroy(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).rdsconn @@ -622,6 +681,54 @@ resource "aws_rds_cluster_parameter_group" "test" { `, name) } +func testAccAWSDBClusterParameterGroupUpdateParametersInitialConfig(name string) string { + return fmt.Sprintf(` +resource "aws_rds_cluster_parameter_group" "test" { + name = "%s" + family = "aurora5.6" + + parameter { + name = "character_set_server" + value = "utf8" + } + + parameter { + name = "character_set_client" + value = "utf8" + } + + parameter { + name = "character_set_results" + value = "utf8" + } +} +`, name) +} + +func testAccAWSDBClusterParameterGroupUpdateParametersUpdatedConfig(name string) string { + return fmt.Sprintf(` +resource "aws_rds_cluster_parameter_group" "test" { + name = "%s" + family = "aurora5.6" + + parameter { + name = "character_set_server" + value = "ascii" + } + + parameter { + name = "character_set_client" + value = "utf8" + } + + parameter { + name = "character_set_results" + value = "ascii" + } +} +`, name) +} + const testAccAWSDBClusterParameterGroupConfig_namePrefix = ` resource "aws_rds_cluster_parameter_group" "test" { name_prefix = "tf-test-"