hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.74k stars 9.09k forks source link

for_each not working well when creating aws_route53_record #14447

Open mchinatang opened 4 years ago

mchinatang commented 4 years ago

The aws_acm_certificate.cert.domain_validation_options.0.resource_record_name works in 2.70.0, but this doesn't work in 3.0.0. Checking the latest 3.0.0 document, now terraform use the for_each expression to iterate the value, but when I use this new approach to create aws_route53_record, I got the following error and can't create resources.

The "for_each" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many instances will be created. To work around this, use the -target argument to first apply only the resources that the for_each depends on.

For a workaround, I have to run the following commands to finish the creation, terraform init terraform plan -target=aws_acm_certificate.cert -out=tfplanout terraform apply "tfplanout" terraform plan terraform apply

falktan commented 4 years ago

Workaround

Here is another workaround that may be more convenient, as it avoids running apply twice. This only works in cases where there is only one validation option. I guess this should also work (I did not test this) in cases where there are multiple validation options (e.g. due to multiple SANs), with a known fixed number (add a count and replace [0] by [count.index]).

resource "aws_route53_record" "cert_validation_entry" {
  name            = aws_acm_certificate.this.domain_validation_options.*.resource_record_name[0]
  records         = [aws_acm_certificate.this.domain_validation_options.*.resource_record_value[0]]
  type            = aws_acm_certificate.this.domain_validation_options.*.resource_record_type[0]
  zone_id         = aws_route53_zone.this.zone_id
  ttl             = 60
}
hpohl commented 4 years ago

I think this should be tagged with service/acm instead of service/route53 since the domain_validation_options are the issue here.

wjam commented 4 years ago

From the comment in the tests for aws_acm_certificate_validation, it looks like it might be fixed by the upgrade to SDKv2? Either that, or no one could get it to work... https://github.com/terraform-providers/terraform-provider-aws/blob/master/aws/resource_aws_acm_certificate_validation_test.go#L212-L245

bflad commented 4 years ago

@wjam that comment was because the older Terraform Plugin SDK version 1 testing framework only emulated early 0.12 Terraform CLI functionality (and did not receive updates for for_each support). It is unrelated to how the released code works. The mention of Terraform Plugin SDK version 2 is because it uses real Terraform binaries for testing rather than that emulation, so it properly supports anything real Terraform binaries can support.

simonc-613 commented 4 years ago

I've found similar issues and hopefully this will help with the debugging.

Using the following to create the validation records in both cases

resource "aws_route53_record" "existing" {
  for_each = {
    for dvo in aws_acm_certificate.existing.domain_validation_options : dvo.domain_name => {
      name   = dvo.resource_record_name
      record = dvo.resource_record_value
      type   = dvo.resource_record_type
    }
  }

  allow_overwrite = true
  name            = each.value.name
  records         = [each.value.record]
  ttl             = 60
  type            = each.value.type
  zone_id         = var.zone_id
}

If we use pre-defined values for the domain_name and subject_alternative_names as below it all works to the point of a plan being generated (haven't tried an apply for various reasons)

resource "aws_acm_certificate" "cert" {
  domain_name               = "dn.example.com"
  subject_alternative_names = ["san1.example.com", "san2.example.com"]
  validation_method         = "DNS"
}

However we generate the DNS names as part of the same stack so rather than building FQDNs which is our case becomes quite complex (multiple zones, different domain name formatting options etc) we use the outputs from the Route53 records. This causes the 'attributes that cannot be determined' error from above. This works fine when we were using the previous count method such as in the terraform-aws-acm module

resource "aws_acm_certificate" "cert" {
  domain_name               = aws_route53_record.live.fqdn
  subject_alternative_names = concat([aws_route53_record.pending.fqdn], aws_route53_record.additional_names.*.fqdn)
  validation_method         = "DNS"
}

I'll post a workaround if I find one

simonc-613 commented 4 years ago

Dug a bit deeper into this and tried doing some for_each logic on the Route53 FQDN's which throws the same depends on resource attributes that cannot be determined error so I don't know if this is what is causing others issues as well. I even tried it with a single record (i.e. no splat operator) and got the same results so I guess the bug in our case lies in between the Route53 record and the plan not being able to determine how many resources will be created.

resource "aws_route53_record" "cert_validation" {
  for_each = {
    for fqdn in [aws_route53_record.live.fqdn] : fqdn => {
      name = fqdn
    }
  }
....
}

Fails with

The "for_each" value depends on resource attributes that cannot be determined
until apply, so Terraform cannot predict how many instances will be created.
To work around this, use the -target argument to first apply only the
resources that the for_each depends on.

Yet

resource "aws_route53_record" "cert_validation" {
  for_each = {
    for fqdn in ["aws_route53_record.live.fqdn"] : fqdn => {
      name = fqdn
    }
  }
....
}

Succeeds

countless-integers commented 4 years ago

Yet

....

Succeeds

In the first one you're iterating over a terraform resource's property that's apparently unknown at plan stage and in the second over a string which is determined. So it seems like terraform bails whenever the iterable's count is unknown or is consists of values unknown at plan stage. Only I don't get why it doesn't follow the same behaviour of implicit dependency tracking to just wait with execution until the unknown is known... Even adding explicit dependency doesn't seem to help.

simonc-613 commented 4 years ago

I think the above is a bug in for_each itself, granted the value of aws_route53_record.live.fqdn isn't know at plan time as it's computed but given it's type is schema.TypeString the number of instances is predictable so I'd have expected Terraform to be able to work this out! https://github.com/terraform-providers/terraform-provider-aws/blob/master/aws/resource_aws_route53_record.go#L52-L55

Given the Domain Validation Options is a set of 'unknown length' and converting it to a list makes the ordering arbitrary again I think the only way I'm going to get round this one is to reengineer the module to build the domains as inputs to the ACM Cert :(

simonc-613 commented 4 years ago

Did a little more delving into this and worked out exactly what the problem is here. In the CustomizeDiff function for the domain_validation_options it initiates the []interface using the domain_name from the resource itself validating that a string is provided. In this case I assume it is not considered a string or is an empty string and this is what causes the for_each error. https://github.com/terraform-providers/terraform-provider-aws/blob/master/aws/resource_aws_acm_certificate.go#L166-L171 So the issue is domain_validation_options is an empty set and thus for_each errors due to the lack of computed value.

The slight worry of this is that so long as aws_acm_certificate.this.domain_name is an actual string then Terraform will be able to apply a plan even if aws_acm_certificate.this.subject_alternative_names depend on computed outputs from other resources. Not sure if this is a bug or just one of those things.

Also not sure if I'm really contributing to the original issue at this point or this is a separate issue!

simonc-613 commented 4 years ago

Decided that this is a bug that could cause problems, but not really sure how to handle it.

If the certificate is to be created only then all is well at the moment. However if the certificate is later validated (as has been the case in this thread) then not all validation domains will be created if one of the SANs has not been computed pre-plan as some would be missing. The aws_acm_certificate_validation would then sit and wait for 45m (or more see #9338) until eventually failing the apply due to the missing DNS records.

Tried taking a look at the code, failing the diff due to domain_name or one of subject_alternative_names not being a string is pretty simple, the complication comes when trying to only fail it if validation is enabled as if this isn't then it really doesn't matter. I guess a warn would work but not sure how to warn.

dn, ok := diff.Get("domain_name").(string)

if !ok {
    return fmt.Errorf("error setting new domain_validation_options due to unknown 'domain_name'")
}
domainValidationOptionsList := []interface{}{map[string]interface{}{
    // AWS Provider 3.0 -- plan-time validation prevents "domain_name"
    // argument to accept a string with trailing period; thus, trim of trailing period
    // no longer required here
    "domain_name": dn,
}}

and changing if !ok {continue} to an error in the SAN range

sidcarter commented 4 years ago

Same here. I actually got this snippet from the provider upgrade for aws and now plan fails everytime.

https://registry.terraform.io/providers/hashicorp/aws/latest/docs/guides/version-3-upgrade

Would have at least liked a working snippet that didn't break plans :facepalm:

babuamudala commented 3 years ago

Is above bug resolved? just curious, if it is resolved which aws provider version we have to upgrade?

jangrewe commented 3 years ago

I'm hitting the same problem, using TF 0.13.4 and aws 3.11.0, with this code:

resource "aws_acm_certificate" "alb" {
  domain_name       = format("${var.role}.%s", data.aws_route53_zone.current.name)
  validation_method = "DNS"
}

resource "aws_route53_record" "acm_certificate_validation" {
  for_each = {
    for dvo in aws_acm_certificate.alb.domain_validation_options :
    dvo.domain_name => {
      name   = dvo.resource_record_name
      record = dvo.resource_record_value
      type   = dvo.resource_record_type
    }
  }

  allow_overwrite = true
  name            = each.value.name
  records         = [each.value.record]
  ttl             = 60
  type            = each.value.type
  zone_id         = data.aws_route53_zone.current.zone_id
}

i get this error:

Error: Invalid for_each argument

  on ../green-staging/modules/alb/acm.tf line 18, in resource "aws_route53_record" "acm_certificate_validation":
  18:   for_each = {
  19:     for dvo in aws_acm_certificate.alb.domain_validation_options :
  20:     dvo.domain_name => {
  21:       name   = dvo.resource_record_name
  22:       record = dvo.resource_record_value
  23:       type   = dvo.resource_record_type
  24:     }
  25:   }

The "for_each" value depends on resource attributes that cannot be determined
until apply, so Terraform cannot predict how many instances will be created.
To work around this, use the -target argument to first apply only the
resources that the for_each depends on.

I have migrated a different TF plan from 0.12 to 0.13 with this resource definition (take straight from the docs), but for whatever reason it doesn't work in this plan.

Update: I am aware how ugly this is, but at least it doesn't error out:


resource "aws_route53_record" "acm_certificate_validation" {
  zone_id         = data.aws_route53_zone.current.zone_id
  name            = tolist(aws_acm_certificate.alb.domain_validation_options)[0].resource_record_name
  type            = tolist(aws_acm_certificate.alb.domain_validation_options)[0].resource_record_type
  records         = [ tolist(aws_acm_certificate.alb.domain_validation_options)[0].resource_record_value ]
  allow_overwrite = true
  ttl             = 60
}
grimm26 commented 3 years ago

@jangrewe it's funny, that works fine for me. The only real difference I see in my code is that I am not doing a data lookup inside the domain_name variable for aws_acm_certificate. I have a couple extra little bits in there to check if I don't need records created in route53 (or at all):

resource "aws_acm_certificate" "c" {
  provider                  = aws.this
  domain_name               = var.domain_name
  subject_alternative_names = var.subject_alternative_names
  validation_method         = "DNS"

  tags = var.tags

  lifecycle {
    create_before_destroy = true
  }
}

resource "aws_route53_record" "cert_validation" {
  provider = aws.this
  for_each = {
    for dvo in aws_acm_certificate.c.domain_validation_options : dvo.domain_name => {
      name   = replace(dvo.resource_record_name, "/\\.$/", "")
      record = replace(dvo.resource_record_value, "/\\.$/", "")
      type   = dvo.resource_record_type
    }
    if ! var.validation_records_already_exist && var.domain_zone_lookup[dvo.domain_name]["type"] == "route53"
  }
  name    = each.value.name
  type    = each.value.type
  zone_id = var.domain_zone_lookup[each.key]["zone"]
  records = [each.value.record]
  ttl     = 300
}
babuamudala commented 3 years ago

@grimm26 Are you sure for_each is working for creating new record, i know above condition only works for me if it is already created. It fails with same error what @jangrewe gets when i try to create new record.

simonc-613 commented 3 years ago

i know above condition only works for me if it is already created

That was the exact problem I had cause I was depending on the output of the R53 creation which isn't known until it has been created. You need to make sure domain_name is a known string which is what it looks like @jangrewe is using.

@jangrewe I assume there is nothing that the data resource is depending on before fetching it's data? I know that mine works in a similar fashion, the main difference is that instead of using the format function I'm just using string interpolation so yours would look like domain_name = "${var.role}.${data.aws_route53_zone.current.name}", think that would be clutching at straws a little but worth a try?

@babuamudala can you share your code snippet?

Can you also share which version of the AWS provider you are both using?

jangrewe commented 3 years ago

@simonc-613 yes, the data source gets a variable passed into the module, that's all for dependencies.

I also noticed that the original method from the docs works fine if the record already exists, that's why i initially didn't get this error when i was doing a migration from 0.12 to 0.13. But when i then tested my module after destroying all resources, the error showed up.

PS: ugh, i have no clue why i'm using a format there, there's really no reason for it. i guess that's the result of re-using code from other modules that had a requirement for this... i'll change it to normal interpolation :-)

babuamudala commented 3 years ago

@simonc-613 As you requested code snippet, This works for already existing record, doesn't works for creating new record.

referred this https://registry.terraform.io/providers/hashicorp/aws/latest/docs/guides/version-3-upgrade#resource-aws_acm_certificate

resource "aws_acm_certificate" "cert" { domain_name = test.contoso.com validation_method = "DNS" subject_alternative_names = var.subject_alternative_names

lifecycle { create_before_destroy = true } }

resource "infoblox_record_cname" "www" { for_each = { for dvo in aws_acm_certificate.cert.domain_validation_options : dvo.domain_name => { name = dvo.resource_record_name canonical = dvo.resource_record_value } } name = replace( each.value.name, "mln.com.", "mln.com", ) canonical = replace( each.value.canonical, ".aws.", ".aws", ) comment = "${var.comment != "" ? var.comment : each.value.name} ACM Validation" view = "External"

lifecycle { ignore_changes = [ comment, ttl, ] } }

resource "aws_acm_certificate_validation" "cert" { certificate_arn = aws_acm_certificate.cert.arn }

babuamudala commented 3 years ago

@simonc-613 My bad, above code totally work fine with aws provider 3.0.0, it is not working with 2.66.0 . For workaround used tolist()func.

simonc-613 commented 3 years ago

@babuamudala as in it works if test.contoso.com has an A/CNAME record added to your DNS provider but not if it hasn't?

I honestly have no idea other, in our stack we create the R53 record which terraform seems to do earlier than creating the cert. We did have issues when relying on the output of the R53 resource but that's cause the output wasn't known at plan time.

I've looked through the code for the diff again and it doesn't do any DNS look ups or anything so I'm beyond confused here.

jsipprell commented 2 years ago

What's the status of this? I'm in the same scenario that other are in, I have to target state on bootstrapping.

philipwigg commented 2 years ago

Hey @jsipprell, I hit this very same issue today and upgrading to version 3 of the AWS provider fixed my issue. They mention it in the upgrade guide too.

I did have to upgrade some other modules I was using first because the modules had provider constraints that meant I only pulled down the version 2.70.0 of the AWS provider even when I did a terraform init -upgrade.

jsipprell commented 2 years ago

Hey @jsipprell, I hit this very same issue today and upgrading to version 3 of the AWS provider fixed my issue. They mention it in the upgrade guide too.

I did have to upgrade some other modules I was using first because the modules had provider constraints that meant I only pulled down the version 2.70.0 of the AWS provider even when I did a terraform init -upgrade.

@philipwigg, I'm using version >= 3.36.0 already.

GerardoGR commented 2 years ago

I also encountered this error in version v3.69.0 but as pointed out by @jangrewe (ref) the solution using the tolist works as expected. The full terraform configuration I'm using is:

resource "aws_acm_certificate_validation" "main" {
  certificate_arn         = aws_acm_certificate.main.arn
  validation_record_fqdns = [aws_route53_record.acm_dns_validation.fqdn]
}

resource "aws_acm_certificate" "main" {
  domain_name       = local.domain_name
  validation_method = "DNS"
}

resource "aws_route53_record" "acm_dns_validation" {
  allow_overwrite = true
  zone_id         = data.aws_route53_zone.main.zone_id
  name            = local.main_acm_domain_validation_option.resource_record_name
  type            = local.main_acm_domain_validation_option.resource_record_type
  records         = [local.main_acm_domain_validation_option.resource_record_value]
  ttl             = 60
}

data "aws_route53_zone" "main" {
  name         = local.zone_name
  private_zone = false
}

locals {
  main_acm_domain_validation_option = tolist(aws_acm_certificate.main.domain_validation_options)[0]
  domain_name                       = "${lower(random_string.suffix.result)}.${local.zone_name}"
  zone_name                         = "example.com"
}

resource "random_string" "suffix" {
  length  = 8
  special = false
}

The misleading part with this issue is that one need to be sure that the "unknown-until-apply" part, in my case the domain_name, was not applied/created already. That might explain cases like @philipwigg 's

While the example from the docs do work for domain names known in advance, I think it would make sense to include in the example a dynamically created domain name.

ip-sf commented 2 years ago

This is also affecting me. When i use a r53 zone that is created outside of the templates and use a data block to load the zone using a static string for the ID, it works fine.

Exact same logic but create a new zone within the template, and pass that ID to the data block to load it, and I get the same kind of error:

│ Error: Invalid for_each argument
│
│   on aws/modules/certificate-tls-public/main.tf line 46, in resource "aws_route53_record" "cert_validation":
│   46:   for_each = {
│   47:     for dvo in aws_acm_certificate.this.domain_validation_options : dvo.domain_name => {
│   48:       name   = dvo.resource_record_name
│   49:       record = dvo.resource_record_value
│   50:       type   = dvo.resource_record_type
│   51:     }
│   52:   }
│     ├────────────────
│     │ aws_acm_certificate.this.domain_validation_options is a set of object, known only after apply
mission-lblaney commented 1 year ago

Note sure if this is related but a data source also fails?

data "aws_route53_zone" "app" {
  for_each = toset(var.zone_ids)

  id = each.key
}

Error is Error: Invalid or unknown key

FalconerTC commented 1 year ago

Still running into this issue with AWS provider 5.1.0. I'm using static domain names too, following the Terraform example.

resource "aws_acm_certificate" "this" {
  domain_name               = "test.net"
  subject_alternative_names = []
  validation_method         = "DNS"

  lifecycle {
    create_before_destroy = true
  }
}

resource "aws_route53_record" "this" {
  for_each = {
    for dvo in aws_acm_certificate.this.domain_validation_options : dvo.domain_name => {
      name    = dvo.resource_record_name
      record  = dvo.resource_record_value
      type    = dvo.resource_record_type
      zone_id = var.zone_id
    }
  }

  allow_overwrite = true
  name            = each.value.name
  records         = [each.value.record]
  ttl             = 60
  type            = each.value.type
  zone_id         = var.zone_id
}

Yields

Error: Invalid for_each argument
│
│   on modules/acm/main.tf line 19, in resource "aws_route53_record" "this":
│   19:   for_each = {
│   20:     for dvo in aws_acm_certificate.this.domain_validation_options : dvo.domain_name => {
│   21:       name    = dvo.resource_record_name
│   22:       record  = dvo.resource_record_value
│   23:       type    = dvo.resource_record_type
│   24:       zone_id = var.zone_id
│   25:     }
│   26:   }
│     ├────────────────
│     │ aws_acm_certificate.this.domain_validation_options is a set of object, known only after apply
│     │ var.zone_id is "XXXX"
│
│ The "for_each" map includes keys derived from resource attributes that cannot be determined until apply, and so Terraform cannot determine the full set of keys that will identify the instances of this resource.
│
│ When working with unknown values in for_each, it's better to define the map keys statically in your configuration and place apply-time results only in the map values.
│
│ Alternatively, you could use the -target planning option to first apply only the resources that the for_each value depends on, and then apply a second time to fully converge.
brittandeyoung commented 1 year ago

I was starting to look at this issue and was attempting to reproduce the error and am currently unable to using the following config and provider version 5.2.0 and terraform version 1.4.6.


resource "aws_route53_zone" "this" {
  name = "test.net"
}

resource "aws_acm_certificate" "this" {
  domain_name               = "test.net"
  subject_alternative_names = []
  validation_method         = "DNS"

  lifecycle {
    create_before_destroy = true
  }
}

resource "aws_route53_record" "this" {

  for_each = {
    for dvo in aws_acm_certificate.this.domain_validation_options : dvo.domain_name => {
      name    = dvo.resource_record_name
      record  = dvo.resource_record_value
      type    = dvo.resource_record_type
      zone_id = aws_route53_zone.this.zone_id
    }
  }

  allow_overwrite = true
  name            = each.value.name
  records         = [each.value.record]
  ttl             = 60
  type            = each.value.type
  zone_id         = aws_route53_zone.this.zone_id
}

Is a second person able to verify that they either can or cannot reproduce this error on the latest provider version?

shines1011000 commented 1 year ago

I was starting to look at this issue and was attempting to reproduce the error and am currently unable to using the following config and provider version 5.2.0 and terraform version 1.4.6.


resource "aws_route53_zone" "this" {
  name = "test.net"
}

resource "aws_acm_certificate" "this" {
  domain_name               = "test.net"
  subject_alternative_names = []
  validation_method         = "DNS"

  lifecycle {
    create_before_destroy = true
  }
}

resource "aws_route53_record" "this" {

  for_each = {
    for dvo in aws_acm_certificate.this.domain_validation_options : dvo.domain_name => {
      name    = dvo.resource_record_name
      record  = dvo.resource_record_value
      type    = dvo.resource_record_type
      zone_id = aws_route53_zone.this.zone_id
    }
  }

  allow_overwrite = true
  name            = each.value.name
  records         = [each.value.record]
  ttl             = 60
  type            = each.value.type
  zone_id         = aws_route53_zone.this.zone_id
}

Is a second person able to verify that they either can or cannot reproduce this error on the latest provider version?

I can confirm I'm seeing this issue all of a sudden with aws provider version 4.66.1 and terraform 1.4.6

iamgeef commented 1 year ago

I can confirm I have just started seeing this issue, provider version: 4.58.0 and terraform version 1.4.6.

Same provider version and terraform version were working on June 9th.

brittandeyoung commented 1 year ago

@iamgeef I am unable to replicate with the config i posted earlier on that same terraform and provider version. Can you provide me the code you are able to replicate with?

DwayneNickels-tyler commented 8 months ago

We encountered the same issue.

data "aws_route53_zone" "this" {
  name = var.public_hosted_zone
}

resource "aws_acm_certificate" "this" {
  domain_name       = "${local.region}-${var.environment}.${data.aws_route53_zone.this.name}"
  validation_method = "DNS"

  subject_alternative_names = [
    "*.${data.aws_route53_zone.this.name}",
    "*.api.${data.aws_route53_zone.this.name}",
    "${local.api_domain}"
  ]

  lifecycle {
    create_before_destroy = true
  }
}

resource "aws_route53_record" "certificate_validation" {
  for_each = {
    for dvo in aws_acm_certificate.this.domain_validation_options : dvo.domain_name => {
      name   = dvo.resource_record_name
      record = dvo.resource_record_value
      type   = dvo.resource_record_type
    }
  }

  name            = each.value.name
  type            = each.value.type
  zone_id         = data.aws_route53_zone.this.zone_id
  records         = [each.value.record]
  ttl             = "60"
  allow_overwrite = true
}
eranshapira commented 7 months ago

i'm encountering the same issue, would appreciate a response here.