fix: Fixed the for_each value depends on resource attributes that can…

…not be determined until apply (#72)
terraform-aws-modules · Jun 19, 2022 · e6fe5ba · e6fe5ba
1 parent 0b40ca0
commit e6fe5ba
Show file tree

Hide file tree

Showing 6 changed files with 64 additions and 92 deletions.
diff --git a/examples/complete/README.md b/examples/complete/README.md
@@ -39,12 +39,10 @@ Note that this example may create resources which cost money. Run `terraform des
 | <a name="module_disabled_records"></a> [disabled\_records](#module\_disabled\_records) | ../../modules/records | n/a |
 | <a name="module_records"></a> [records](#module\_records) | ../../modules/records | n/a |
 | <a name="module_records_with_full_names"></a> [records\_with\_full\_names](#module\_records\_with\_full\_names) | ../../modules/records | n/a |
-| <a name="module_records_with_lists"></a> [records\_with\_lists](#module\_records\_with\_lists) | ../../modules/records | n/a |
-| <a name="module_records_with_terragrunt"></a> [records\_with\_terragrunt](#module\_records\_with\_terragrunt) | ../../modules/records | n/a |
-| <a name="module_records_with_terragrunt_with_lists"></a> [records\_with\_terragrunt\_with\_lists](#module\_records\_with\_terragrunt\_with\_lists) | ../../modules/records | n/a |
 | <a name="module_resolver_rule_associations"></a> [resolver\_rule\_associations](#module\_resolver\_rule\_associations) | ../../modules/resolver-rule-associations | n/a |
 | <a name="module_s3_bucket"></a> [s3\_bucket](#module\_s3\_bucket) | terraform-aws-modules/s3-bucket/aws | n/a |
-| <a name="module_vpc"></a> [vpc](#module\_vpc) | terraform-aws-modules/vpc/aws | n/a |
+| <a name="module_terragrunt"></a> [terragrunt](#module\_terragrunt) | ../../modules/records | n/a |
+| <a name="module_vpc1"></a> [vpc1](#module\_vpc1) | terraform-aws-modules/vpc/aws | n/a |
 | <a name="module_vpc2"></a> [vpc2](#module\_vpc2) | terraform-aws-modules/vpc/aws | n/a |
 | <a name="module_zones"></a> [zones](#module\_zones) | ../../modules/zones | n/a |
 

diff --git a/examples/complete/main.tf b/examples/complete/main.tf
@@ -4,23 +4,23 @@ provider "aws" {
 
 locals {
   zone_name = sort(keys(module.zones.route53_zone_zone_id))[0]
-  #  zone_id = module.zones.route53_zone_zone_id["app.terraform-aws-modules-example.com"]
+  #  zone_id = module.zones.route53_zone_zone_id["terraform-aws-modules-example.com"]
 }
 
 module "zones" {
   source = "../../modules/zones"
 
   zones = {
     "terraform-aws-modules-example.com" = {
-      comment           = "terraform-aws-modules-example.com (production)"
-      delegation_set_id = module.delegation_sets.route53_delegation_set_id.main
+      comment = "terraform-aws-modules-example.com (production)"
       tags = {
         Name = "terraform-aws-modules-example.com"
       }
     }
 
     "app.terraform-aws-modules-example.com" = {
-      comment = "app.terraform-aws-modules-example.com"
+      comment           = "app.terraform-aws-modules-example.com"
+      delegation_set_id = module.delegation_sets.route53_delegation_set_id.main
       tags = {
         Name = "app.terraform-aws-modules-example.com"
       }
@@ -32,7 +32,7 @@ module "zones" {
       comment     = "private-vpc.terraform-aws-modules-example.com"
       vpc = [
         {
-          vpc_id = module.vpc.vpc_id
+          vpc_id = module.vpc1.vpc_id
         },
         {
           vpc_id = module.vpc2.vpc_id
@@ -65,7 +65,8 @@ module "records" {
       ]
     },
     {
-      name = "s3-bucket"
+      key  = "s3-bucket"
+      name = "s3-bucket-${module.s3_bucket.s3_bucket_hosted_zone_id}"
       type = "A"
       alias = {
         name    = module.s3_bucket.s3_bucket_website_domain
@@ -148,44 +149,15 @@ module "records" {
   depends_on = [module.zones]
 }
 
-module "records_with_lists" {
+module "terragrunt" {
   source = "../../modules/records"
 
   zone_name = local.zone_name
-  #  zone_id = local.zone_id
-
-  records = [
-    {
-      name = "tf-list1"
-      type = "A"
-      ttl  = 3600
-      records = [
-        "10.10.10.10",
-      ]
-    },
-    {
-      name = "tf-list2"
-      type = "A"
-      ttl  = 3600
-      records = [
-        "20.10.10.10",
-        "30.10.10.10",
-      ]
-    },
-  ]
-
-  depends_on = [module.zones]
-}
-
-module "records_with_terragrunt" {
-  source = "../../modules/records"
-
-  zone_name = local.zone_name
-  #  zone_id = local.zone_id
 
   # Terragrunt has a bug (https://github.com/gruntwork-io/terragrunt/issues/1211) that requires `records` to be wrapped with `jsonencode()`
   records_jsonencoded = jsonencode([
     {
+      key  = "new A"
       name = "new"
       type = "A"
       ttl  = 3600
@@ -194,42 +166,25 @@ module "records_with_terragrunt" {
       ]
     },
     {
-      name = "s3-bucket-new"
-      type = "A"
-      alias = {
-        name    = module.s3_bucket.s3_bucket_website_domain
-        zone_id = module.s3_bucket.s3_bucket_hosted_zone_id
-      }
-    }
-  ])
-
-  depends_on = [module.zones]
-}
-
-module "records_with_terragrunt_with_lists" {
-  source = "../../modules/records"
-
-  zone_name = local.zone_name
-  #  zone_id = local.zone_id
-
-  # Terragrunt has a bug (https://github.com/gruntwork-io/terragrunt/issues/1211) that requires `records` to be wrapped with `jsonencode()`
-  records_jsonencoded = jsonencode([
-    {
-      name = "tg-list1"
+      name = "new2"
       type = "A"
       ttl  = 3600
       records = [
-        "10.10.10.10",
+        "10.10.10.11",
+        "10.10.10.12",
       ]
     },
     {
-      name = "tg-list2"
+      name = "s3-bucket-terragrunt"
       type = "A"
-      ttl  = 3600
-      records = [
-        "20.10.10.10",
-        "30.10.10.10",
-      ]
+      alias = {
+        # In Terragrunt code the values may depend on the outputs of modules:
+        # name    = dependency.s3_bucket.outputs.s3_bucket_website_domain
+        # zone_id = dependency.s3_bucket.outputs.s3_bucket_hosted_zone_id
+        # Terragrunt passes known values to the module:
+        name    = "s3-website-eu-west-1.amazonaws.com"
+        zone_id = "Z1BKCTXD74EZPE"
+      }
     }
   ])
 
@@ -257,6 +212,7 @@ module "records_with_full_names" {
       ttl  = 3600
       records = [
         "10.10.10.11",
+        "10.10.10.12",
       ]
     },
   ]
@@ -275,7 +231,7 @@ module "delegation_sets" {
 module "resolver_rule_associations" {
   source = "../../modules/resolver-rule-associations"
 
-  vpc_id = module.vpc.vpc_id
+  vpc_id = module.vpc1.vpc_id
 
   resolver_rule_associations = {
     example = {
@@ -289,7 +245,6 @@ module "resolver_rule_associations" {
   }
 }
 
-
 module "disabled_records" {
   source = "../../modules/records"
 
@@ -342,7 +297,7 @@ module "cloudfront" {
   }
 }
 
-module "vpc" {
+module "vpc1" {
   source = "terraform-aws-modules/vpc/aws"
 
   name = "my-vpc-for-private-route53-zone"

diff --git a/modules/records/README.md b/modules/records/README.md
@@ -57,7 +57,7 @@ No modules.
 |------|-------------|------|---------|:--------:|
 | <a name="input_create"></a> [create](#input\_create) | Whether to create DNS records | `bool` | `true` | no |
 | <a name="input_private_zone"></a> [private\_zone](#input\_private\_zone) | Whether Route53 zone is private or public | `bool` | `false` | no |
-| <a name="input_records"></a> [records](#input\_records) | List of maps of DNS records | `any` | `[]` | no |
+| <a name="input_records"></a> [records](#input\_records) | List of objects of DNS records | `any` | `[]` | no |
 | <a name="input_records_jsonencoded"></a> [records\_jsonencoded](#input\_records\_jsonencoded) | List of map of DNS records (stored as jsonencoded string, for terragrunt) | `string` | `null` | no |
 | <a name="input_zone_id"></a> [zone\_id](#input\_zone\_id) | ID of DNS zone | `string` | `null` | no |
 | <a name="input_zone_name"></a> [zone\_name](#input\_zone\_name) | Name of DNS zone | `string` | `null` | no |

diff --git a/modules/records/main.tf b/modules/records/main.tf
@@ -1,23 +1,10 @@
 locals {
-  # terragrunt users have to provide `records` as jsonencode()'d string.
+  # Terragrunt users have to provide `records_jsonencoded` as jsonencode()'d string.
   # See details: https://github.com/gruntwork-io/terragrunt/issues/1211
-  records = try(jsondecode(var.records_jsonencoded), var.records)
+  records = concat(var.records, try(jsondecode(var.records_jsonencoded), []))
 
   # Convert `records` from list to map with unique keys
-  #
-  # A list of `records` values is jsonencode()'d to a string to solve issue:
-  # The true result value has the wrong type:
-  # element types must all match for conversion to map.
-  # Ref:
-  # https://github.com/terraform-aws-modules/terraform-aws-route53/issues/47
-  # https://github.com/terraform-aws-modules/terraform-aws-route53/pull/39
-
-  recordsets = {
-    for rs in local.records :
-    join(" ", compact(["${rs.name} ${rs.type}", lookup(rs, "set_identifier", "")])) => merge(rs, {
-      records = jsonencode(try(rs.records, null))
-    })
-  }
+  recordsets = { for rs in local.records : try(rs.key, join(" ", compact(["${rs.name} ${rs.type}", try(rs.set_identifier, "")]))) => rs }
 }
 
 data "aws_route53_zone" "this" {
@@ -36,7 +23,7 @@ resource "aws_route53_record" "this" {
   name                             = each.value.name != "" ? (lookup(each.value, "full_name_override", false) ? each.value.name : "${each.value.name}.${data.aws_route53_zone.this[0].name}") : data.aws_route53_zone.this[0].name
   type                             = each.value.type
   ttl                              = lookup(each.value, "ttl", null)
-  records                          = jsondecode(each.value.records)
+  records                          = try(each.value.records, null)
   set_identifier                   = lookup(each.value, "set_identifier", null)
   health_check_id                  = lookup(each.value, "health_check_id", null)
   multivalue_answer_routing_policy = lookup(each.value, "multivalue_answer_routing_policy", null)

diff --git a/modules/records/variables.tf b/modules/records/variables.tf
@@ -23,7 +23,7 @@ variable "private_zone" {
 }
 
 variable "records" {
-  description = "List of maps of DNS records"
+  description = "List of objects of DNS records"
   type        = any
   default     = []
 }

diff --git a/terragrunt_59.md b/terragrunt_59.md
@@ -0,0 +1,32 @@
+# Notes for issue 59 and PR 72
+
+https://github.com/terraform-aws-modules/terraform-aws-route53/pull/72
+https://github.com/terraform-aws-modules/terraform-aws-route53/pull/64
+
+```
+########################################################################################
+# IF YOU ARE READING THIS AND WANT TO TRY FIXING IT, PLEASE INCREASE THE COUNTER BELOW!
+########################################################################################
+# number_of_hours_spent_on_issue_59 = 22
+########################################################################################
+```
+
+## Here is the short explanation of the problem.
+This module can be called by TF and TG, so it has to support these 3 cases:
+1. Terraform with "known values" (static values)
+2. Terraform with "(known after apply)" (module.s3_bucket, module.cloudfront, etc)
+3. Terragrunt with "known values" - (TG always resolves values before calling Terraform). TG wraps values with `jsonencode()`.
+4. Terragrunt with "unknown values"/"(known after apply)" - this is not possible with TG.
+
+## Considerations:
+
+1. Try not to change variable types very much. `type = any` is prefered.
+2. Keep in mind different behaviour when `records` has different length, `records` or/and `alias` is specified.
+3. The same name of resources (aws_route53_record.this) should be used for both TF and TG.
+4. If necessary, use Terraform 1.0 as a minimum version (not newer). Terraform 0.13 can be a history, if necessary for this fix to work.
+
+## Success criteria:
+
+1. `terraform apply` in `examples/complete` should work without `-target`
+2. `records` should include records, alias, references to `module.s3_bucket` in `name`.
+3. At least, modules `zones`, `records`, and `terragrunt` should be enabled (not commented)