cloudposse / terraform-aws-elasticache-redis

Terraform module to provision an ElastiCache Redis Cluster
https://cloudposse.com/accelerate
Apache License 2.0
141 stars 246 forks source link

feat: allow random auth token variable as an option #202

Closed travertischio closed 3 weeks ago

travertischio commented 1 year ago

what

why

Currently users of this module need to either handle the auth_token through something more secret outside of this module or commit the auth_token directly to their terraform variables. This random password path is a highly used pattern for modules like this one

goruha commented 1 year ago

@travertischio Terraform lint failed.

Could you please fix this issue

  Missing version constraint for provider "random" in `required_providers`

  Raw Output:
  main.tf:36:1: warning: Missing version constraint for provider "random" in `required_providers` ()
goruha commented 1 year ago

@Nuru @aknysh @Benbentwo @milldr What do you think about a random auth token for Redis module?

goruha commented 1 year ago

@travertischio May I ask you to update README by running the following commands on your local

make init
make readme

Thanks.

travertischio commented 12 months ago

@goruha I think this should be good now

travertischio commented 11 months ago

@goruha thank you, it looks like i'm now waiting on #204 before the CI will pass

travertischio commented 10 months ago

@goruha any movement on getting the README fixed in master?

travertischio commented 9 months ago

@goruha can you run the ci again? master was updated to unblock this pr

Gowiem commented 4 months ago

This is somewhat related to https://github.com/cloudposse/terraform-aws-elasticache-redis/pull/195. I believe if a consumer wants to use a random password for the auth_token value... they can provider that via their root module that consumes this child module. Then there is less going on in this module and it's less opinionated, but still supported. @goruha @travertischio Thoughts?

travertischio commented 4 months ago

@Gowiem this is only tangentially related to #195

There are many cases in which providing a secret as a value to a terraform module is not easy or possible at all in a secure manner. This PR actually makes this module less opinionated because it allows its use without forcing the user to create their own token and then pass it. This also has security benefits where users might default to something like "password12345678."

For me specifically, using terragrunt means that there is no "root module" consuming this module. I am just referencing this module, and I don't want to have an aditional secret that is managed outside of the terraform state just to create a token for this module. Having it all in the terraform state is simpler and the change here doesn't force anyone else to do anything different.

mergify[bot] commented 4 months ago

This pull request is now in conflict. Could you fix it @travertischio? 🙏

mergify[bot] commented 3 months ago

Thanks @travertischio for creating this pull request!

A maintainer will review your changes shortly. Please don't be discouraged if it takes a while.

While you wait, make sure to review our contributor guidelines.

[!TIP]

Need help or want to ask for a PR review to be expedited?

Join us on Slack in the #pr-reviews channel.

mergify[bot] commented 3 weeks ago

💥 This pull request now has conflicts. Could you fix it @travertischio? 🙏

mergify[bot] commented 3 weeks ago

This PR was closed due to inactivity and merge conflicts. 😭 Please resolve the conflicts and reopen if necessary.