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

ACM DNS validation record set not created for all domain names on Route53 #82

Closed
Rammohanemis opened this issue Apr 26, 2021 · 6 comments · Fixed by #89
Closed

ACM DNS validation record set not created for all domain names on Route53 #82

Rammohanemis opened this issue Apr 26, 2021 · 6 comments · Fixed by #89

Comments

@Rammohanemis
Copy link

Description

CNAME record not created for all distinct domain names on Route53

Reproduction

module "acm" {
  source = "../../"

  domain_name = "www.acm-test.com"
  zone_id     = coalescelist(data.aws_route53_zone.this.*.zone_id, aws_route53_zone.this.*.zone_id)[0]

  subject_alternative_names = [
    "acm-test.com",
    "*.acm-test.com",
    "www1.acm-test.com",
  ]

  wait_for_validation = true

  tags = {
    Name = "www.acm-test.com"
  }
}

My Domain list for ACM

here I am going to create ACM for four domains. For DNS validation we have to create four CNAME record in Route53 but three records is enough because "*.acm-test.com" and "acm-test.com" both having same record value so we will create record set for only "acm-test.com" on Route53.

locals {
  # Get distinct list of domains and SANs
  distinct_domain_names = distinct(
    [for s in concat([var.domain_name], var.subject_alternative_names) : replace(s, "*.", "")]
  )

  # Copy domain_validation_options for the distinct domain names
  validation_domains = var.create_certificate ? [for k, v in aws_acm_certificate.this[0].domain_validation_options : tomap(v) if contains(local.distinct_domain_names, replace(v.domain_name, "*.", ""))] : []
}

In above code distinct_domain_names will have below three value because we are skipping "*.acm-test.com"

we are getting domain_validation_options from ACM for all four domains so validation_domains have four value set

resource "aws_route53_record" "validation" {
  count = var.create_certificate && var.validation_method == "DNS" && var.validate_certificate ? length(local.distinct_domain_names) : 0

  zone_id = var.zone_id
  name    = element(local.validation_domains, count.index)["resource_record_name"]
  type    = element(local.validation_domains, count.index)["resource_record_type"]
  ttl     = var.dns_ttl

  records = [
    element(local.validation_domains, count.index)["resource_record_value"]
  ]

  allow_overwrite = var.validation_allow_overwrite_records

  depends_on = [aws_acm_certificate.this]
}

In above code length(local.distinct_domain_names) is 3 so it will create three Record. For record value we are getting from local.validation_domains list and it have 4 record set. Consider local.validation_domains have a list like below order

Based on the count we are creating Route53 record for first three record and in hosted zone we might have two CNAME record, because third record (*.acm-test.com) will overwrite second record (acm-test.com) value because both have same record value on ACM.

In above method we missed to create record for "www1.acm-test.com" domain on Route53 due to invalid logic.

FIX

To fix this issue we have to change the if condition logic on validation_domains local variable. Here we are checking whether ACM domain was there on distinct_domain_names after replacing "*." Obviously we have all the ACM domain name in distinct_domain_names so validation_domains will have four record set. So this part creating issue here.

 # Copy domain_validation_options for the distinct domain names
  validation_domains = var.create_certificate ? [for k, v in aws_acm_certificate.this[0].domain_validation_options : tomap(v) if contains(local.distinct_domain_names, replace(v.domain_name, "*.", ""))] : []

Instead of above code we have to use below code to resolve this issue.

 # Copy domain_validation_options for the distinct domain names
  validation_domains = var.create_certificate ? [for k, v in aws_acm_certificate.this[0].domain_validation_options : tomap(v) if contains(local.distinct_domain_names, v.domain_name)] : []

Here we are copying distinct domain name value so that validation_domains änd distinct_domain_names have same count and same domain name list

@antonbabenko
Copy link
Member

Please run code in examples as-is to confirm that this is a bug. ACM should not be validated if this is a bug.

@Rammohanemis
Copy link
Author

image

image

image

@flora-five
Copy link
Contributor

I have reproduced this issue. I'm using Terraform v0.14.11.

In this case distinct_domain_names has three items, and validation_domains has four items. The code inserts too many items into validation_domains, one for *.domain and one for domain, both referring to the same Route 53 validation record. Due to the one too many items, the last item from validation_domains is not used to create the validation record.

It can be fixed by inserting a single item instead.

> var.domain_name
"www.tf-dns-demo.tk"

> var.subject_alternative_names
tolist([
  "www1.tf-dns-demo.tk",
  "tf-dns-demo.tk",
  "*.tf-dns-demo.tk",
])

> local.distinct_domain_names
tolist([
  "www.tf-dns-demo.tk",
  "www1.tf-dns-demo.tk",
  "tf-dns-demo.tk",
])

> aws_acm_certificate.this[0].domain_validation_options
toset([
  {
    "domain_name" = "*.tf-dns-demo.tk"
    "resource_record_name" = "_29d70008ded6be1c238b3cc11f6035f1.tf-dns-demo.tk."
    "resource_record_type" = "CNAME"
    "resource_record_value" = "_de65906e23186f56bd0369e182a0dac9.gwpjclltnz.acm-validations.aws."
  },
  {
    "domain_name" = "tf-dns-demo.tk"
    "resource_record_name" = "_29d70008ded6be1c238b3cc11f6035f1.tf-dns-demo.tk."
    "resource_record_type" = "CNAME"
    "resource_record_value" = "_de65906e23186f56bd0369e182a0dac9.gwpjclltnz.acm-validations.aws."
  },
  {
    "domain_name" = "www.tf-dns-demo.tk"
    "resource_record_name" = "_6398f2852e0309e9942133c93d857822.www.tf-dns-demo.tk."
    "resource_record_type" = "CNAME"
    "resource_record_value" = "_8a2122384d886016b02b12fb27080d97.gwpjclltnz.acm-validations.aws."
  },
  {
    "domain_name" = "www1.tf-dns-demo.tk"
    "resource_record_name" = "_cac0c32f110869a026b327edbb83ca10.www1.tf-dns-demo.tk."
    "resource_record_type" = "CNAME"
    "resource_record_value" = "_9b9298a5eb180efd90d4affb3f042779.gwpjclltnz.acm-validations.aws."
  },
])

> [for k, v in aws_acm_certificate.this[0].domain_validation_options : tomap(v) if contains(local.distinct_domain_names, replace(v.domain_name, "*.", ""))]
[
  tomap({
    "domain_name" = "*.tf-dns-demo.tk"
    "resource_record_name" = "_29d70008ded6be1c238b3cc11f6035f1.tf-dns-demo.tk."
    "resource_record_type" = "CNAME"
    "resource_record_value" = "_de65906e23186f56bd0369e182a0dac9.gwpjclltnz.acm-validations.aws."
  }),
  tomap({
    "domain_name" = "tf-dns-demo.tk"
    "resource_record_name" = "_29d70008ded6be1c238b3cc11f6035f1.tf-dns-demo.tk."
    "resource_record_type" = "CNAME"
    "resource_record_value" = "_de65906e23186f56bd0369e182a0dac9.gwpjclltnz.acm-validations.aws."
  }),
  tomap({
    "domain_name" = "www.tf-dns-demo.tk"
    "resource_record_name" = "_6398f2852e0309e9942133c93d857822.www.tf-dns-demo.tk."
    "resource_record_type" = "CNAME"
    "resource_record_value" = "_8a2122384d886016b02b12fb27080d97.gwpjclltnz.acm-validations.aws."
  }),
  tomap({
    "domain_name" = "www1.tf-dns-demo.tk"
    "resource_record_name" = "_cac0c32f110869a026b327edbb83ca10.www1.tf-dns-demo.tk."
    "resource_record_type" = "CNAME"
    "resource_record_value" = "_9b9298a5eb180efd90d4affb3f042779.gwpjclltnz.acm-validations.aws."
  }),
]

With the suggested fix:

> distinct([for v in aws_acm_certificate.this[0].domain_validation_options : merge(tomap(v), { domain_name = replace(v.domain_name, "*.", "") }) if contains(local.distinct_domain_names, replace(v.domain_name, "*.", ""))])
tolist([
  {
    "domain_name" = "tf-dns-demo.tk"
    "resource_record_name" = "_29d70008ded6be1c238b3cc11f6035f1.tf-dns-demo.tk."
    "resource_record_type" = "CNAME"
    "resource_record_value" = "_de65906e23186f56bd0369e182a0dac9.gwpjclltnz.acm-validations.aws."
  },
  {
    "domain_name" = "www.tf-dns-demo.tk"
    "resource_record_name" = "_6398f2852e0309e9942133c93d857822.www.tf-dns-demo.tk."
    "resource_record_type" = "CNAME"
    "resource_record_value" = "_8a2122384d886016b02b12fb27080d97.gwpjclltnz.acm-validations.aws."
  },
  {
    "domain_name" = "www1.tf-dns-demo.tk"
    "resource_record_name" = "_cac0c32f110869a026b327edbb83ca10.www1.tf-dns-demo.tk."
    "resource_record_type" = "CNAME"
    "resource_record_value" = "_9b9298a5eb180efd90d4affb3f042779.gwpjclltnz.acm-validations.aws."
  },
])

@antonbabenko
Copy link
Member

Thanks for opening this issue.

I have managed to reproduce the issue and completed PR #89 created by @flora-five .

@Rammohanemis There is a potential problem with the code:

  domain_name = "www.acm-test.com"  # <- this should be zone name like `acm-test.com` because all SAN are subdomains

  subject_alternative_names = [
    "acm-test.com",
    "*.acm-test.com",
    "www1.acm-test.com",
  ]

If you want to create an ACM certificate for a custom set of domains and SANs then you need to set wait_for_validation = false and optionally validate_certificate = false to manage records outside (often using different AWS credentials, for e.g).

@antonbabenko
Copy link
Member

Please give a try to v3.1.0 that has been just released.

@github-actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants