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.

Sign up for GitHub

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

Feature/add enabled cloudwatch logs exports param for DB Instances #4111

Merged
merged 2 commits into from
Apr 13, 2018
Merged

Feature/add enabled cloudwatch logs exports param for DB Instances #4111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5a90bc0
Add enabled_cloudwatch_logs_exports attributes for aws_db_instance
teraken0509 Apr 7, 2018
df587f3
Update db_instance resources docs
teraken0509 Apr 7, 2018
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
85 changes: 85 additions & 0 deletions aws/resource_aws_db_instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/helper/validation"
)

func resourceAwsDbInstance() *schema.Resource {
Expand Down Expand Up @@ -351,6 +352,21 @@ func resourceAwsDbInstance() *schema.Resource {
Computed: true,
},

"enabled_cloudwatch_logs_exports": {
Type: schema.TypeList,
Computed: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nitpick: Computed: false is the default 👍

Optional: true,
Elem: &schema.Schema{
Type: schema.TypeString,
ValidateFunc: validation.StringInSlice([]string{
"audit",
"error",
"general",
"slowquery",
}, false),
},
},

"tags": tagsSchema(),
},
}
Expand Down Expand Up @@ -408,6 +424,10 @@ func resourceAwsDbInstanceCreate(d *schema.ResourceData, meta interface{}) error
opts.DBSubnetGroupName = aws.String(attr.(string))
}

if attr, ok := d.GetOk("enabled_cloudwatch_logs_exports"); ok && len(attr.([]interface{})) > 0 {
opts.EnableCloudwatchLogsExports = expandStringList(attr.([]interface{}))
}

if attr, ok := d.GetOk("kms_key_id"); ok {
opts.KmsKeyId = aws.String(attr.(string))
if arnParts := strings.Split(v.(string), ":"); len(arnParts) >= 4 {
Expand Down Expand Up @@ -462,6 +482,10 @@ func resourceAwsDbInstanceCreate(d *schema.ResourceData, meta interface{}) error
opts.DBSubnetGroupName = aws.String(attr.(string))
}

if attr, ok := d.GetOk("enabled_cloudwatch_logs_exports"); ok && len(attr.([]interface{})) > 0 {
opts.EnableCloudwatchLogsExports = expandStringList(attr.([]interface{}))
}

if attr, ok := d.GetOk("engine"); ok {
opts.Engine = aws.String(attr.(string))
}
Expand Down Expand Up @@ -628,6 +652,10 @@ func resourceAwsDbInstanceCreate(d *schema.ResourceData, meta interface{}) error
opts.DBSubnetGroupName = aws.String(attr.(string))
}

if attr, ok := d.GetOk("enabled_cloudwatch_logs_exports"); ok && len(attr.([]interface{})) > 0 {
opts.EnableCloudwatchLogsExports = expandStringList(attr.([]interface{}))
}

if attr, ok := d.GetOk("iops"); ok {
opts.Iops = aws.Int64(int64(attr.(int)))
}
Expand Down Expand Up @@ -775,6 +803,10 @@ func resourceAwsDbInstanceRead(d *schema.ResourceData, meta interface{}) error {
d.Set("monitoring_role_arn", v.MonitoringRoleArn)
}

if v.EnabledCloudwatchLogsExports != nil {
d.Set("enabled_cloudwatch_logs_exports", v.EnabledCloudwatchLogsExports)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is silently failing as d.Set() does not know how to handle a []*string directly. You can see this from the import test:

--- FAIL: TestAccAWSDBInstance_importBasic (535.11s)
    testing.go:518: Step 1 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.
        
        (map[string]string) {
        }
        
        
        (map[string]string) (len=3) {
         (string) (len=33) "enabled_cloudwatch_logs_exports.#": (string) (len=1) "2",
         (string) (len=33) "enabled_cloudwatch_logs_exports.0": (string) (len=5) "audit",
         (string) (len=33) "enabled_cloudwatch_logs_exports.1": (string) (len=5) "error"
        }

A general recommendation is to always perform error checking when setting non-scalar types. This can be caught adding an import test step when working with new attributes or using TF_SCHEMA_PANIC_ON_ERROR=1 while running the acceptance testing.

This should be something like:

if err := d.Set("enabled_cloudwatch_logs_exports", flattenStringList(v.EnabledCloudwatchLogsExports)); err != nil {
  return fmt.Errorf("error setting enabled_cloudwatch_logs_exports: %s", err)
}

I will fix this on merge 👍

}

// list tags for resource
// set tags
conn := meta.(*AWSClient).rdsconn
Expand Down Expand Up @@ -1020,6 +1052,12 @@ func resourceAwsDbInstanceUpdate(d *schema.ResourceData, meta interface{}) error
requestUpdate = true
}

if d.HasChange("enabled_cloudwatch_logs_exports") && !d.IsNewResource() {
d.SetPartial("enabled_cloudwatch_logs_exports")
req.CloudwatchLogsExportConfiguration = buildCloudwatchLogsExportConfiguration(d)
requestUpdate = true
}

if d.HasChange("iam_database_authentication_enabled") {
req.EnableIAMDatabaseAuthentication = aws.Bool(d.Get("iam_database_authentication_enabled").(bool))
requestUpdate = true
Expand Down Expand Up @@ -1151,10 +1189,55 @@ func resourceAwsDbInstanceStateRefreshFunc(id string, conn *rds.RDS) resource.St
}
}

func buildRDSARN(identifier, partition, accountid, region string) (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This likely snuck back in during a rebase. I can remove it on merge 👍

if partition == "" {
return "", fmt.Errorf("Unable to construct RDS ARN because of missing AWS partition")
}
if accountid == "" {
return "", fmt.Errorf("Unable to construct RDS ARN because of missing AWS Account ID")
}
arn := fmt.Sprintf("arn:%s:rds:%s:%s:db:%s", partition, region, accountid, identifier)
return arn, nil
}

func buildCloudwatchLogsExportConfiguration(d *schema.ResourceData) *rds.CloudwatchLogsExportConfiguration {

oraw, nraw := d.GetChange("enabled_cloudwatch_logs_exports")
o := oraw.([]interface{})
n := nraw.([]interface{})

create, disable := diffCloudwatchLogsExportConfiguration(o, n)

return &rds.CloudwatchLogsExportConfiguration{
EnableLogTypes: expandStringList(create),
DisableLogTypes: expandStringList(disable),
}
}

func diffCloudwatchLogsExportConfiguration(old, new []interface{}) ([]interface{}, []interface{}) {
create := make([]interface{}, 0)
disable := make([]interface{}, 0)

for _, n := range new {
if _, contains := sliceContainsString(old, n.(string)); !contains {
create = append(create, n)
}
}

for _, o := range old {
if _, contains := sliceContainsString(new, o.(string)); !contains {
disable = append(disable, o)
}
}

return create, disable
}

// Database instance status: http://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/Overview.DBInstance.Status.html
var resourceAwsDbInstanceCreatePendingStates = []string{
"backing-up",
"configuring-enhanced-monitoring",
"configuring-log-exports",
"creating",
"maintenance",
"modifying",
Expand All @@ -1170,6 +1253,7 @@ var resourceAwsDbInstanceDeletePendingStates = []string{
"available",
"backing-up",
"configuring-enhanced-monitoring",
"configuring-log-exports",
"creating",
"deleting",
"incompatible-parameters",
Expand All @@ -1183,6 +1267,7 @@ var resourceAwsDbInstanceDeletePendingStates = []string{
var resourceAwsDbInstanceUpdatePendingStates = []string{
"backing-up",
"configuring-enhanced-monitoring",
"configuring-log-exports",
"creating",
"maintenance",
"modifying",
Expand Down
Loading