fix #46 - character point in VPC name #47
Conversation
There was a problem hiding this comment.
I recommend verifying if the proposed change affects other resources.
In this case, the vpc_name local is used to substitute also descriptions.
When it is modified globally before, you will eventually finish with descriptions mentioning improper VPC names and that can lead to unnecessary confusions.
I know it may seem minor, but imagine a case when a newly hired engineer starts working with an infrastructure and the resource's description points them to different VPC.
If the VPC doesn't exist, it is confusing what is going on.
If both - dotted any hyphened VPCs exist - they would be confused even more.
| @@ -3,7 +3,7 @@ data "aws_vpc" "vpc" { | |||
| } | |||
|
|
|||
| locals { | |||
| vpc_name = lookup(data.aws_vpc.vpc.tags, "Name", var.vpc_id) | |||
| vpc_name = replace(lookup(data.aws_vpc.vpc.tags, "Name", var.vpc_id), ".", "-") | |||
There was a problem hiding this comment.
I would suggest adding a separate local and perform the replace for that local, then use the new local in L46:
name = replace(format("%.255s", lower(replace("tf-redis-${var.name}-${var.env}-${local.vpc_name}-${random_id.salt.hex}", "_", "-"))), "/\\s/", "-")
as vpc_name is used for other parts and such change would make the descriptions unclear.
And maybe the replace would be honestly better directly in the name, like:
name = replace(format("%.255s", lower(replace("tf-redis-${var.name}-${var.env}-replace(${local.vpc_name}, ".", "-")-${random_id.salt.hex}", "_", "-"))), "/\\s/", "-")
or some sort of combination, it is worth to play up a bit with it.
There was a problem hiding this comment.
note: I haven't tested that my proposal is syntactically correct
Fix the issue #46