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.

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

Use deployment_name in Random ID Resources #212

Closed
curtis-trynow-io opened this issue Sep 27, 2021 · 4 comments · Fixed by #227
Closed

Use deployment_name in Random ID Resources #212

curtis-trynow-io opened this issue Sep 27, 2021 · 4 comments · Fixed by #227
Labels
enhancement feature/terraform Missing feature from Terraform
Milestone

Comments

@curtis-trynow-io
Copy link

It would be nice if the resources that lean on a random_id included the deployment_name input in their prefix to make them more clearly identifiable. These would include, based on a search of what gets downloaded locally:

$ grep -r 'resource "random_id' .

./tf_next.next_image/main.tf:resource "random_id" "function_name" {
./tf_next.next_image/main.tf:resource "random_id" "policy_name" {
./tf_next/main.tf:resource "random_id" "function_name" {
./tf_next/main.tf:resource "random_id" "policy_name" {
./tf_next/modules/proxy/main.tf:resource "random_id" "function_name" {
./tf_next/modules/statics-deploy/main.tf:resource "random_id" "function_name" {

They currently look some of them either do not consider deployment_name or take it but then hex-encode it in the output which makes tracing resources rough without having to fall all the way back to:

terraform state show 'module.tf_next.aws_cloudwatch_log_group.this["__NEXT_PAGE_LAMBDA_0"]'

for example to actually know which log group you should look at. This is compounded with multiple NextJS apps and environments in a single AWS account. Same thing for the Lambda functions themselves.

@ofhouse
Copy link
Member

ofhouse commented Sep 28, 2021

Yes, that's a good point.
Using a random string is often times simpler because there are different restrictions for each AWS resource when it comes to naming them.

Will try to find a common regex and length restriction so that deployment_name can be added to the random ids.

@ofhouse ofhouse added feature/terraform Missing feature from Terraform enhancement labels Sep 28, 2021
@curtis-trynow-io
Copy link
Author

While it may not be the most elegant solution, consider a change like:

resource "aws_lambda_function" "this" {                                                                                                                               
  for_each = local.lambdas                                                                                                                                          
                                                                                       
  function_name = random_id.function_name[each.key].hex

to being:

resource "aws_lambda_function" "this" {                                                                                                                               
  for_each = local.lambdas                                                                                                                                          
                                                                                       
  function_name = var.deployment_name ? "${var.deployment_name}_${each.key}" : random_id.function_name[each.key].hex

or similar. Ignoring whether an underscore _ is acceptable in all cases, this would let you conditionally inject the deployment_name if it were defined and puts the responsibility of picking an appropriate deployment name on the developer using this code. Terraform is generally pretty good at reporting invalid resource names from the AWS APIs, so if you pick a name like great&deployment!is*sure^to%happen+++++, then that's your own problem to sort out.

@ofhouse
Copy link
Member

ofhouse commented Sep 28, 2021

I originally introduced the random suffix for development, since I was too lazy to change the deployment_name when testing the module with different configurations. 😅

So I agree that in a production environment this leads to more confusion than it helps and the deployment_name should be more predictable for the user.

While its true that the AWS API will eventually produce an error message when it gets a name with wrong length / pattern, it could be confusing for the user where the error comes from.
For example the integrated image optimization module also uses the deployment_name without modifying it, and creates additional Lambda & API-gateway from it with the same name as the main module.

So we will remove the random appendix altogether in the next version.

@ofhouse ofhouse added this to the v0.9.3 milestone Sep 29, 2021
@ofhouse ofhouse linked a pull request Oct 16, 2021 that will close this issue
@ofhouse ofhouse closed this as completed Oct 16, 2021
@ofhouse
Copy link
Member

ofhouse commented Oct 16, 2021

This has now been fixed in v0.10.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement feature/terraform Missing feature from Terraform
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants