-
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
RDS: Added two CW log export values to support postgresql #6829
Conversation
The new values are supported by PostgreSQL after version PostgreSQL 9.6.6. PostgreSQL doesn't support the existing values in the log export list.
The new values are supported by PostgreSQL after version PostgreSQL 9.6.6. PostgreSQL doesn't support the existing values in the log export list.
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.
LGTM, thanks @tylersmith34! 🚀
I'm adding an acceptance test on merge just to prevent future regressions. 👍
--- PASS: TestAccAWSDBInstance_EnabledCloudwatchLogsExports_Postgresql (599.82s)
func TestAccAWSDBInstance_EnabledCloudwatchLogsExports_Postgresql(t *testing.T) {
var dbInstance rds.DBInstance
rName := acctest.RandomWithPrefix("tf-acc-test")
resourceName := "aws_db_instance.test"
resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSDBInstanceDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSDBInstanceConfig_EnabledCloudwatchLogsExports_Postgresql(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSDBInstanceExists(resourceName, &dbInstance),
resource.TestCheckResourceAttr(resourceName, "enabled_cloudwatch_logs_exports.#", "2"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"password"},
},
},
})
}
func testAccAWSDBInstanceConfig_EnabledCloudwatchLogsExports_Postgresql(rName string) string {
return fmt.Sprintf(`
resource "aws_db_instance" "test" {
allocated_storage = 10
enabled_cloudwatch_logs_exports = ["postgresql", "upgrade"]
engine = "postgres"
identifier = %q
instance_class = "db.t2.micro"
password = "avoid-plaintext-passwords"
username = "tfacctest"
skip_final_snapshot = true
}
`, rName)
}
@bflad Do you want me to add that acceptance test and submit a PR? |
I already added it. |
This has been released in version 1.52.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
* hashicorp#6829 Adds "postgresql" and "upgrade" as valid options.
I guess these should be allowed in UPDATE |
It appears that it does now, so similar changes need to be made to Specifically, just
|
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! |
This change allows for CloudWatch log exports to be enabled for PostgreSQL databases v9.6.6 and newer. Older versions use the existing log exports but newer versions have their own list. This PR adds those new values to the list and to the documentation.
Changes proposed in this pull request:
aws_db_instance
codeNote, it's in two commits because my company blocks access to GitHub so I had to use the GUI to make the changes.
Output from acceptance testing:
I didn't run the acceptance tests, I don't have an account where I can incur charges and I only added two items to a list.