cloudposse / terraform-aws-rds

Terraform module to provision AWS RDS instances
https://cloudposse.com/accelerate
Apache License 2.0
153 stars 180 forks source link

add feature Master Passwords via Secrets Manager #157

Closed ByJacob closed 10 months ago

ByJacob commented 1 year ago

what

why

references

adamantike commented 1 year ago

This would be really useful to have, and could potentially avoid the need for #118 for most users, while also not exposing the password in the state file. Can this be reviewed?

adamantike commented 10 months ago

@joe-niland, @Gowiem, gentle ping. Can this be reviewed? It will be really useful to have this option merged, as this new feature gains more traction.

Gowiem commented 10 months ago

/terratest

Gowiem commented 10 months ago

@adamantike thanks for the ping! I've seen you do that on a few PRs and it's appreciated when there are good fixes + enhancements like this one that fall through the cracks. Though I think we'd all like to do better, there are so many modules that the squeaky wheels get the grease. That said, I believe Erik + team are always looking for more maintainers so if you're interested in helping us get things reviewed + merged, please reach out to me or Erik via Slack and we can likely make that happen.

adamantike commented 10 months ago

@ByJacob, if needed, I can tackle the VPC and subnet module upgrade in the examples, for you to rebase your changes after that is done.

ByJacob commented 10 months ago

Thanks for message @adamantike. I forgot about this PR. Changes are added.

Gowiem commented 10 months ago

/terratest

ByJacob commented 10 months ago

/terratest

@Gowiem I fixed tflint and Readme

Gowiem commented 10 months ago

/terratest

Gowiem commented 10 months ago

/terratest

Gowiem commented 10 months ago

@ByJacob Tests are failing on the following: CleanShot 2023-11-09 at 10 53 42

ByJacob commented 10 months ago

@ByJacob Tests are failing on the following: CleanShot 2023-11-09 at 10 53 42

Id was change. I fix it and add identifier parameter

https://github.com/hashicorp/terraform-provider-aws/commit/a7cb14bfc969b896e6d7d1290529b572a016b46a

Gowiem commented 10 months ago

/terratest

ByJacob commented 10 months ago

Solid work @ByJacob -- Thanks for working through the test failures!

It was enough to tighten the main branch, there it was fixed :D

Gowiem commented 10 months ago

@ByJacob release of your changes are blocked on some internal issue that I'm fixing in #164. Once that ships, a new release will be cut for v1.1.0