Choosing the right zone for each SAN when attaching validation records

[apanzerj]

Found a bug? Maybe our Slack Community can help.

Describe the Bug

When you have two SANs that belong to different zones, the module tries to add validation records to the incorrect zone.

Expected Behavior

It should add validation records to zones:

foo.baz.bar.com and bar.com

Steps to Reproduce

Steps to reproduce the behavior:
Say you have these two zones:

zone 1: bar.com
zone 2: foo.baz.bar.com

You want a cert that allows you to use both zones so you do this:

module "acm_request_certificate_east_coast" {
  source = "cloudposse/acm-request-certificate/aws"

  domain_name                       = "foo.baz.bar.com"
  process_domain_validation_options = true
  ttl                               = "300"
  subject_alternative_names         = ["*.foo.baz.bar.com", "*.bar.com"]

  providers = {
    aws = aws.use1
  }
}

When I terraform apply, the module does a data lookup for the zone:

• baz.bar.com

The expectation is that the zones it should look up:

• foo.baz.bar.com
• bar.com

Screenshots

N/A

Environment (please complete the following information):

Mac OS

Additional Notes

In chatting on Slack I suggested this:

Instead of trying to guess what zone to put each SAN in, just have the user specify it manually:

module "cert_request" {
  subject_alternative_names = [
    {
      zone_to_lookup = "foo.baz.com",
      names          = ["a.foo.baz.com", "b.foo.baz.com"]
    },
   {
      zone_to_lookup = "*.baz.com",
      names          = ["bob.baz.com", "alice.baz.com"]
    }
  ]
  # etc etc

[nitrocode]

For now, you may want to set the zone_id or zone_name to keep all the verification records added to the same zone. This should work for you.

If that doesn't work, you can also set process_domain_validation_options = false and do the validation from outside the module.

[nitrocode]

Ive been thinking of the interface. How's this one?

cc @apanzerj @jamengual

   domains = [
     {
       domain    = "google.com"
       zone_name = "google.com"
       san       = false
     },
     {
       domain    = "*.xyz.google.com"
       zone_name = "google.com"
       san       = true
     },
     {
       domain    = "*.abc.google.com"
       zone_name = "abc.google.com"
       san       = true
     },
   ]

[jamengual]

it makes it much easier to read and configure +1

[nitrocode]

Or option 2

  # same interface for these top inputs
  domain_name   = "google.com"
  zone_name     = "google.com"
  # or zone_id
  process_domain_validation_options = true

  ttl = "300"

  # this one can become a list(object)
  subject_alternative_names = [
    {
      domain    = "*.xyz.google.com"
      zone_name = "google.com"
      # or zone_id
    },
    {
      domain    = "*.abc.google.com"
      zone_name = "abc.google.com"
      # or zone_id
    },
  ]

[apanzerj]

Both look good to me. Testing alternative suggestions now

[apanzerj]

What's odd to me is that 0.16.2 worked. It created validation records in all the right places.

[vzaitsev77]

What's the status? 0.17.0 is broken in the multi-domain case (even worse if domains are very different in our case)

[nitrocode]

@vzaitsev77 @apanzerj @jamengual while we come up with an implementation that works for everyone, have you tried overriding the zone_id or zone_name suggested above?

#62 (comment)

[nitrocode]

For now, I've set 0.17.0 as a pre-release until we can validate that zone_id or zone_name can be overridden without reverting the PR or introducing breaking changes.

e.g.

module "acm_request_certificate_east_coast" {
  source = "cloudposse/acm-request-certificate/aws"
  # Cloud Posse recommends pinning every module to a specific version
  version = "0.17.0"

  # explicit zone_name should be applied to each SANs' validation
  zone_name                         = "baz.bar.com"
  domain_name                       = "foo.baz.bar.com"
  process_domain_validation_options = true
  ttl                               = "300"
  subject_alternative_names         = ["*.foo.baz.bar.com", "*.bar.com"]
}

If we cannot verify the above works as expected with 0.17.0,, then we can merge this PR #66 to release 0.18.0 to go back to 0.16.2 release behavior.