Skip to content

Commit

Permalink
Merge pull request #3182 from terraform-providers/paddy_593
Browse files Browse the repository at this point in the history
Store all config-specified DB params in state.
  • Loading branch information
paddycarver authored Jan 29, 2018
2 parents 6b2a6c3 + 8e0df3b commit 2ef79ee
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 4 deletions.
70 changes: 66 additions & 4 deletions aws/resource_aws_db_parameter_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,18 +151,80 @@ func resourceAwsDbParameterGroupRead(d *schema.ResourceData, meta interface{}) e
d.Set("family", describeResp.DBParameterGroups[0].DBParameterGroupFamily)
d.Set("description", describeResp.DBParameterGroups[0].Description)

// Only include user customized parameters as there's hundreds of system/default ones
configParams := d.Get("parameter").(*schema.Set)
describeParametersOpts := rds.DescribeDBParametersInput{
DBParameterGroupName: aws.String(d.Id()),
Source: aws.String("user"),
}
if configParams.Len() < 1 {
// if we don't have any params in the ResourceData already, two possibilities
// first, we don't have a config available to us. Second, we do, but it has
// no parameters. We're going to assume the first, to be safe. In this case,
// we're only going to ask for the user-modified values, because any defaults
// the user may have _also_ set are indistinguishable from the hundreds of
// defaults AWS sets. If the user hasn't set any parameters, this will return
// an empty list anyways, so we just make some unnecessary requests. But in
// the more common case (I assume) of an import, this will make fewer requests
// and "do the right thing".
describeParametersOpts.Source = aws.String("user")
}

describeParametersResp, err := rdsconn.DescribeDBParameters(&describeParametersOpts)
var parameters []*rds.Parameter
err = rdsconn.DescribeDBParametersPages(&describeParametersOpts,
func(describeParametersResp *rds.DescribeDBParametersOutput, lastPage bool) bool {
parameters = append(parameters, describeParametersResp.Parameters...)
return !lastPage
})
if err != nil {
return err
}

d.Set("parameter", flattenParameters(describeParametersResp.Parameters))
var userParams []*rds.Parameter
if configParams.Len() < 1 {
// if we have no config/no parameters in config, we've already asked for only
// user-modified values, so we can just use the entire response.
userParams = parameters
} else {
// if we have a config available to us, we have two possible classes of value
// in the config. On the one hand, the user could have specified a parameter
// that _actually_ changed things, in which case its Source would be set to
// user. On the other, they may have specified a parameter that coincides with
// the default value. In that case, the Source will be set to "system" or
// "engine-default". We need to set the union of all "user" Source parameters
// _and_ the "system"/"engine-default" Source parameters _that appear in the
// config_ in the state, or the user gets a perpetual diff. See
// terraform-providers/terraform-provider-aws#593 for more context and details.
confParams, err := expandParameters(configParams.List())
if err != nil {
return err
}
for _, param := range parameters {
if param.Source == nil || param.ParameterName == nil {
continue
}
if *param.Source == "user" {
userParams = append(userParams, param)
continue
}
var paramFound bool
for _, cp := range confParams {
if cp.ParameterName == nil {
continue
}
if *cp.ParameterName == *param.ParameterName {
userParams = append(userParams, param)
break
}
}
if !paramFound {
log.Printf("[DEBUG] Not persisting %s to state, as its source is %q and it isn't in the config", *param.ParameterName, *param.Source)
}
}
}

err = d.Set("parameter", flattenParameters(userParams))
if err != nil {
return fmt.Errorf("error setting 'parameter' in state: %#v", err)
}

paramGroup := describeResp.DBParameterGroups[0]
arn, err := buildRDSPGARN(d.Id(), meta.(*AWSClient).partition, meta.(*AWSClient).accountid, meta.(*AWSClient).region)
Expand Down
37 changes: 37 additions & 0 deletions aws/resource_aws_db_parameter_group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,29 @@ func TestAccAWSDBParameterGroup_Only(t *testing.T) {
})
}

func TestAccAWSDBParameterGroup_MatchDefault(t *testing.T) {
var v rds.DBParameterGroup

groupName := fmt.Sprintf("parameter-group-test-terraform-%d", acctest.RandInt())
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSDBParameterGroupDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: testAccAWSDBParameterGroupIncludeDefaultConfig(groupName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSDBParameterGroupExists("aws_db_parameter_group.bar", &v),
resource.TestCheckResourceAttr(
"aws_db_parameter_group.bar", "name", groupName),
resource.TestCheckResourceAttr(
"aws_db_parameter_group.bar", "family", "postgres9.4"),
),
},
},
})
}

func TestResourceAWSDBParameterGroupName_validation(t *testing.T) {
cases := []struct {
Value string
Expand Down Expand Up @@ -752,6 +775,20 @@ resource "aws_db_parameter_group" "large" {
}`, n)
}

func testAccAWSDBParameterGroupIncludeDefaultConfig(n string) string {
return fmt.Sprintf(`
resource "aws_db_parameter_group" "bar" {
name = "%s"
family = "postgres9.4"
parameter {
name = "client_encoding"
value = "UTF8"
apply_method = "pending-reboot"
}
}`, n)
}

const testAccDBParameterGroupConfig_namePrefix = `
resource "aws_db_parameter_group" "test" {
name_prefix = "tf-test-"
Expand Down

0 comments on commit 2ef79ee

Please sign in to comment.