cloudposse / terraform-aws-elasticache-redis

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

Default "zone_id" to empty string for "dns" module #135

Closed marcuz closed 2 years ago

marcuz commented 2 years ago

what

why

I am unsure why it doesn't simply skip this check because of length(var.zone_id) > 0 (which is []). :thinking:

My current workaround is setting zone_id = "" in the module resource.

references

nitrocode commented 2 years ago

It looks like the change in PR #133 was due to closing #82

The downstream https://github.com/cloudposse/terraform-aws-route53-cluster-hostname#input_zone_id requires a string and in this module, zone_id is using type any.

The issue with your PR is that it will skip inserting the var.zone_id unless it's a list.

cc: @Nuru , Could we simply change the variable default to null to get around this ?

or we could upgrade the https://github.com/cloudposse/terraform-aws-route53-cluster-hostname to take an input of a list

Nuru commented 2 years ago

@nitrocode Fixing route53-cluster-hostname to take a list is the right long-term solution, but we don't need to wait for that.