Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

r/aws_db_parameter_group, r/aws_rds_cluster_parameter_group: Support resetting parameters #5728

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions aws/resource_aws_db_parameter_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,26 @@ func resourceAwsDbParameterGroupUpdate(d *schema.ResourceData, meta interface{})
}
d.SetPartial("parameter")
}

// Reset parameters that have been removed
parameters, err = expandParameters(os.Difference(ns).List())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing in the ResetDBParameterGroupInput SDK documentation here that there is a documented 20 item max, so it would probably be best to add that logic in here. There's an example of doing this with a for-loop here. 🙂

if err != nil {
return err
}
if len(parameters) > 0 {
parameterGroupName := d.Get("name").(string)
resetOpts := rds.ResetDBParameterGroupInput{
DBParameterGroupName: aws.String(parameterGroupName),
Parameters: parameters,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like by default, all parameters are reset so we should add this in to prevent that:

Suggested change
Parameters: parameters,
Parameters: parameters,
ResetAllParameters: aws.Bool(false),

}

log.Printf("[DEBUG] Reset DB Parameter Group: %s", resetOpts)
_, err = rdsconn.ResetDBParameterGroup(&resetOpts)
if err != nil {
return fmt.Errorf("Error resetting DB Parameter Group: %s", err)
}
d.SetPartial("parameter")
}
}

if err := setTagsRDS(rdsconn, d, d.Get("arn").(string)); err != nil {
Expand Down
48 changes: 48 additions & 0 deletions aws/resource_aws_db_parameter_group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,15 @@ func TestAccAWSDBParameterGroup_basic(t *testing.T) {
"aws_db_parameter_group.bar", "arn", regexp.MustCompile(fmt.Sprintf("^arn:[^:]+:rds:[^:]+:\\d{12}:pg:%s", groupName))),
),
},
{
Config: testAccAWSDBParameterGroupConfig(groupName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSDBParameterGroupExists("aws_db_parameter_group.bar", &v),
testAccCheckAWSDBParameterGroupAttributes(&v, groupName),
testAccCheckAWSDBParameterNotUserDefined("aws_db_parameter_group.bar", "collation_connection"),
testAccCheckAWSDBParameterNotUserDefined("aws_db_parameter_group.bar", "collation_server"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To add some additional checking to this, you could try adding something like:

Suggested change
testAccCheckAWSDBParameterNotUserDefined("aws_db_parameter_group.bar", "collation_server"),
testAccCheckAWSDBParameterNotUserDefined("aws_db_parameter_group.bar", "collation_server"),
testAccCheckNoResourceAttr("aws_db_parameter_group.bar", "parameter.2475805061.value"),

for additional testing to make sure the parameters were reset.

),
},
},
})
}
Expand Down Expand Up @@ -539,6 +548,45 @@ func testAccCheckAWSDBParameterGroupExists(n string, v *rds.DBParameterGroup) re
}
}

func testAccCheckAWSDBParameterNotUserDefined(n, paramName string) 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.ID == "" {
return fmt.Errorf("No DB Parameter Group ID is set")
}

conn := testAccProvider.Meta().(*AWSClient).rdsconn

opts := rds.DescribeDBParametersInput{
DBParameterGroupName: aws.String(rs.Primary.ID),
Source: aws.String("user"),
}
fmt.Printf("test: %s\n", rs.Primary.ID)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, we'd prefer not to have fmt.Printfs in the code. For potential debug information, a log.Printf could work, but if these were just testing messages, could you go ahead and remove them?


userDefined := false
conn.DescribeDBParametersPages(&opts, func(page *rds.DescribeDBParametersOutput, lastPage bool) bool {
for _, param := range page.Parameters {
fmt.Printf("param: %s %s\n", *param.ParameterName, *param.Source)
if *param.ParameterName == paramName {
userDefined = true
return false
}
}
return true
})

if userDefined {
return fmt.Errorf("DB Parameter is user defined")
}

return nil
}
}

func randomString(strlen int) string {
rand.Seed(time.Now().UTC().UnixNano())
const chars = "abcdefghijklmnopqrstuvwxyz"
Expand Down
20 changes: 20 additions & 0 deletions aws/resource_aws_rds_cluster_parameter_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,26 @@ func resourceAwsRDSClusterParameterGroupUpdate(d *schema.ResourceData, meta inte
}
d.SetPartial("parameter")
}

// Reset parameters that have been removed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you go ahead and add a test for this change as well? Thanks!

parameters, err = expandParameters(os.Difference(ns).List())
if err != nil {
return err
}
if len(parameters) > 0 {
parameterGroupName := d.Get("name").(string)
resetOpts := rds.ResetDBClusterParameterGroupInput{
DBClusterParameterGroupName: aws.String(parameterGroupName),
Parameters: parameters,
}

log.Printf("[DEBUG] Reset DB Cluster Parameter Group: %s", resetOpts)
_, err = rdsconn.ResetDBClusterParameterGroup(&resetOpts)
if err != nil {
return fmt.Errorf("Error resetting DB Cluster Parameter Group: %s", err)
}
d.SetPartial("parameter")
}
}

if err := setTagsRDS(rdsconn, d, d.Get("arn").(string)); err != nil {
Expand Down