-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Add enabled_cloudwatch_logs_exports to rds cluster resource #4875
Add enabled_cloudwatch_logs_exports to rds cluster resource #4875
Conversation
92412d8
to
d8b359d
Compare
aws/resource_aws_rds_cluster.go
Outdated
@@ -807,6 +838,10 @@ func flattenAwsRdsClusterResource(d *schema.ResourceData, meta interface{}, dbc | |||
d.Set("iam_database_authentication_enabled", dbc.IAMDatabaseAuthenticationEnabled) | |||
d.Set("hosted_zone_id", dbc.HostedZoneId) | |||
|
|||
if dbc.EnabledCloudwatchLogsExports != nil { | |||
d.Set("enabled_cloudwatch_logs_exports", dbc.EnabledCloudwatchLogsExports) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is currently failing acceptance testing:
=== RUN TestAccAWSRDSCluster_importBasic
--- FAIL: TestAccAWSRDSCluster_importBasic (112.23s)
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"
}
FAIL
We prefer to add error checking when handling aggregate attribute types, which should generate an error here that the types don't match:
if err := d.Set("enabled_cloudwatch_logs_exports", dbc.EnabledCloudwatchLogsExports); err != nil {
return fmt.Errorf("error setting enabled_cloudwatch_logs_exports: %s", err)
}
To fix the error you'll need to convert dbc.EnabledCloudwatchLogsExports
(a []*string
) to []string
or []interface{}
, which can be accomplished with flattenStringList()
. This should cover the correct behavior:
if err := d.Set("enabled_cloudwatch_logs_exports", flattenStringList(dbc.EnabledCloudwatchLogsExports)); err != nil {
return fmt.Errorf("error setting enabled_cloudwatch_logs_exports: %s", err)
}
e2c9a8f
to
9259922
Compare
Thank you for the feedback! I hope I fixed it. Unfortunately I don't have an easy way to run the tests against an actual AWS account myself currently |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this @FaHeymann! LGTM 🚀
18 tests passed (all tests)
=== RUN TestAccAWSRDSCluster_missingUserNameCausesError
--- PASS: TestAccAWSRDSCluster_missingUserNameCausesError (3.88s)
=== RUN TestAccAWSRDSCluster_iamAuth
--- PASS: TestAccAWSRDSCluster_iamAuth (98.96s)
=== RUN TestAccAWSRDSCluster_namePrefix
--- PASS: TestAccAWSRDSCluster_namePrefix (105.10s)
=== RUN TestAccAWSRDSCluster_encrypted
--- PASS: TestAccAWSRDSCluster_encrypted (109.17s)
=== RUN TestAccAWSRDSCluster_importBasic
--- PASS: TestAccAWSRDSCluster_importBasic (110.26s)
=== RUN TestAccAWSRDSCluster_EngineVersion
--- PASS: TestAccAWSRDSCluster_EngineVersion (114.79s)
=== RUN TestAccAWSRDSCluster_generatedName
--- PASS: TestAccAWSRDSCluster_generatedName (114.87s)
=== RUN TestAccAWSRDSCluster_BacktrackWindow
--- PASS: TestAccAWSRDSCluster_BacktrackWindow (115.69s)
=== RUN TestAccAWSRDSCluster_backupsUpdate
--- PASS: TestAccAWSRDSCluster_backupsUpdate (118.14s)
=== RUN TestAccAWSRDSCluster_kmsKey
--- PASS: TestAccAWSRDSCluster_kmsKey (119.25s)
=== RUN TestAccAWSRDSCluster_updateCloudwatchLogsExports
--- PASS: TestAccAWSRDSCluster_updateCloudwatchLogsExports (124.42s)
=== RUN TestAccAWSRDSCluster_takeFinalSnapshot
--- PASS: TestAccAWSRDSCluster_takeFinalSnapshot (125.23s)
=== RUN TestAccAWSRDSCluster_updateIamRoles
--- PASS: TestAccAWSRDSCluster_updateIamRoles (128.85s)
=== RUN TestAccAWSRDSCluster_updateTags
--- PASS: TestAccAWSRDSCluster_updateTags (129.01s)
=== RUN TestAccAWSRDSCluster_basic
--- PASS: TestAccAWSRDSCluster_basic (143.67s)
=== RUN TestAccAWSRDSCluster_Port
--- PASS: TestAccAWSRDSCluster_Port (201.77s)
=== RUN TestAccAWSRDSCluster_s3Restore
--- PASS: TestAccAWSRDSCluster_s3Restore (1483.77s)
=== RUN TestAccAWSRDSCluster_EncryptedCrossRegionReplication
--- PASS: TestAccAWSRDSCluster_EncryptedCrossRegionReplication (2410.50s)
This has been released in version 1.24.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
Based on the implementation for the
db_instance
resource this is supposed to add theenabled_cloudwatch_logs_exports
field to cluster resources as well.