terraform-aws-modules / terraform-aws-acm

Terraform module to create AWS ACM resources 🇺🇦
https://registry.terraform.io/modules/terraform-aws-modules/acm/aws
Apache License 2.0
184 stars 230 forks source link

Cycle error using for_each with multiple providers example #146

Closed hobbsh closed 6 months ago

hobbsh commented 8 months ago

Description

Hi,

I am experiencing a cycle error as reported in three other issues (https://github.com/terraform-aws-modules/terraform-aws-acm/issues/138, https://github.com/terraform-aws-modules/terraform-aws-acm/issues/131, https://github.com/terraform-aws-modules/terraform-aws-acm/issues/121) but all conversations have been locked and closed without resolution. What has been glanced over is the fact that this module is being used in for_each which is a standard feature in terraform for modules, but when implementing the example for two providers, a cycle error is produced.

I understand why it's cycling (because both modules are dependent on the output from the other) but I'm wondering if there is a way to support this multi-provider use-case in the module itself so we don't need to use two separate ones which introduce this issue. Given there's been multiple issues about this, I feel that using separate DNS provider from the ACM provider is a common enough use-case.

Versions

Reproduction Code [Required]

Format for additional_wildcard_cert_domains

additional_wildcard_cert_domains = {
  "domain1" = "zone_id1"
  "domain2" = "zone_id2"
}
module "additional_legacy_acm" {
  source   = "terraform-aws-modules/acm/aws"
  version  = "~> 4.0"
  for_each = var.additional_wildcard_cert_domains

  providers = {
    aws = aws.acm
  }

  domain_name = each.key
  zone_id     = each.value

  # Hardcode wildcard
  subject_alternative_names = [
    "*.${each.key}"
  ]

  validation_method       = "DNS"
  create_route53_records  = false
  validation_record_fqdns = module.route53_records[each.key].validation_route53_record_fqdns

  tags = {
    Name = each.key
  }
}

module "route53_records" {
  source   = "terraform-aws-modules/acm/aws"
  version  = "~> 4.0"
  for_each = var.additional_wildcard_cert_domains

  providers = {
    aws = aws.legacy_dns
  }

  create_certificate          = false
  create_route53_records_only = true

  validation_method = "DNS"
  distinct_domain_names = module.additional_legacy_acm[each.key].distinct_domain_names
  zone_id               = each.value

  acm_certificate_domain_validation_options = module.additional_legacy_acm[each.key].acm_certificate_domain_validation_options
}

Expected behavior

I would expect to be able to use the example in a for_each or for the module to support multiple providers somehow. The use-case of passing in multiple domains is quite common, so not being able to use for_each is quite a burden.

Actual behavior

Error: Cycle: module.route53.module.route53_records.output.validation_domains (expand), module.route53.module.route53_records.output.distinct_domain_names (expand), module.route53.module.route53_records.aws_acm_certificate_validation.this, module.route53.module.route53_records.output.acm_certificate_arn (expand), module.route53.module.additional_legacy_acm.aws_acm_certificate_validation.this, module.route53.module.additional_legacy_acm.output.acm_certificate_arn (expand), module.route53.module.route53_records.var.acm_certificate_domain_validation_options (expand), module.route53.module.route53_records.local.validation_domains (expand), module.route53.module.route53_records.aws_route53_record.validation, module.route53.module.route53_records.output.validation_route53_record_fqdns (expand), module.route53.module.route53_records (close), module.route53.module.additional_legacy_acm.var.validation_record_fqdns (expand), module.route53.module.additional_legacy_acm (close), module.route53.module.route53_records.var.distinct_domain_names (expand), module.route53.module.route53_records.local.distinct_domain_names (expand)
hobbsh commented 8 months ago

I ended up just cloning this module to support multi provider, which involved setting an aws.acm provider alias for the acm resources and an aws.dns provider for the dns resource. Additionally adding a configuration_aliases block in the aws provider configuration in versions.tf, then I can instantiate the module like this:

locals {
  domains_zones = {
    "domain1"   = "zone1_id",
    "domain2" = "zone2_id"
  }
}

module "acm" {
  source   = "../../modules/acm"
  for_each = local.domains_zones

  providers = {
    aws.acm = aws.primary
    aws.dns = aws.legacy_dns
  }

  domain_name = each.key
  zone_id     = each.value

  subject_alternative_names = [
    "*.${each.key}",
  ]

  validation_method   = "DNS"
  wait_for_validation = true

  tags = {
    Name = each.key
  }
}
antonbabenko commented 8 months ago

It is great that you have found the solution. I don't think that we need to bring providers configuration settings into the module itself, but we can have it added as example 3 here - https://github.com/terraform-aws-modules/terraform-aws-acm/blob/master/examples/complete-dns-validation/main.tf#L56-L60

Would you be able to open a PR there?

hobbsh commented 8 months ago

It is great that you have found the solution. I don't think that we need to bring providers configuration settings into the module itself, but we can have it added as example 3 here - https://github.com/terraform-aws-modules/terraform-aws-acm/blob/master/examples/complete-dns-validation/main.tf#L56-L60

Would you be able to open a PR there?

Sure I'll open a PR, thanks!

billyshambrook commented 7 months ago

Another workaround we are using is to use aws_acm_certificate_validation resource outside of the module:

locals {
  additional_wildcard_cert_domains = {
    "domain1" = "zone_id1"
    "domain2" = "zone_id2"
  }
}

module "additional_legacy_acm" {
  source   = "terraform-aws-modules/acm/aws"
  version  = "~> 4.0"
  for_each = local.additional_wildcard_cert_domains

  providers = {
    aws = aws.acm
  }

  domain_name = each.key
  zone_id     = each.value

  # Hardcode wildcard
  subject_alternative_names = [
    "*.${each.key}"
  ]

  validation_method      = "DNS"
  create_route53_records = false

  # Validate outside of the module
  wait_for_validation = false

  tags = {
    Name = each.key
  }
}

module "route53_records" {
  source   = "terraform-aws-modules/acm/aws"
  version  = "~> 4.0"
  for_each = local.additional_wildcard_cert_domains

  providers = {
    aws = aws.legacy_dns
  }

  create_certificate          = false
  create_route53_records_only = true

  validation_method     = "DNS"
  distinct_domain_names = module.additional_legacy_acm[each.key].distinct_domain_names
  zone_id               = each.value

  acm_certificate_domain_validation_options = module.additional_legacy_acm[each.key].acm_certificate_domain_validation_options
}

resource "aws_acm_certificate_validation" "additional_legacy_acm" {
  for_each = local.additional_wildcard_cert_domains

  provider = aws.acm

  certificate_arn         = module.additional_legacy_acm[each.key].acm_certificate_arn
  validation_record_fqdns = module.route53_records[each.key].validation_route53_record_fqdns
}

The bit icky thing about this is you should reference the certificate_arn output from the aws_acm_certificate_validation to make sure the certificate isn't used before it's validated:

resource "aws_cloudfront_distribution" "s3_distribution" {
  ...

  viewer_certificate {
    acm_certificate_arn = aws_acm_certificate_validation.additional_legacy_acm["domain1"].certificate_arn
  }
}
github-actions[bot] commented 6 months ago

This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 10 days

github-actions[bot] commented 6 months ago

This issue was automatically closed because of stale in 10 days

github-actions[bot] commented 5 months ago

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.