From cb15833ca914af837d380ceec3db9aae432dc811 Mon Sep 17 00:00:00 2001 From: Loren Yu Date: Wed, 15 Jan 2025 09:56:39 -0800 Subject: [PATCH] Reuse app domain for domain identity (#834) - Use service_config.domain_name for notifications-email-domain module - DRY up usage of domain_name and hosted_zone_id in service root module by defining local variables ## Context We originally thought that the domain for the SES domain identity we create had to be the same as the root of the hosted zone domain rather than a subdomain, but we were wrong. This change causes the domain identity to just reuse the same domain as the app's configured domain, which simplifies things and also removes the chance of conflict between two apps that enable notifications. --- .../notifications-email-domain/resources/dns.tf | 12 ++++-------- .../resources/variables.tf | 6 ++++++ infra/{{app_name}}/service/identity_provider.tf | 2 +- infra/{{app_name}}/service/main.tf | 12 +++++++----- infra/{{app_name}}/service/notifications.tf | 5 +++-- 5 files changed, 21 insertions(+), 16 deletions(-) diff --git a/infra/modules/notifications-email-domain/resources/dns.tf b/infra/modules/notifications-email-domain/resources/dns.tf index d877817ee..516df2cca 100644 --- a/infra/modules/notifications-email-domain/resources/dns.tf +++ b/infra/modules/notifications-email-domain/resources/dns.tf @@ -1,15 +1,11 @@ -data "aws_route53_zone" "zone" { - name = var.domain_name -} - resource "aws_route53_record" "dkim" { count = 3 allow_overwrite = true ttl = 60 type = "CNAME" - zone_id = data.aws_route53_zone.zone.zone_id - name = "${aws_sesv2_email_identity.sender_domain.dkim_signing_attributes[0].tokens[count.index]}._domainkey" + zone_id = var.hosted_zone_id + name = "${aws_sesv2_email_identity.sender_domain.dkim_signing_attributes[0].tokens[count.index]}._domainkey.${var.domain_name}" records = ["${aws_sesv2_email_identity.sender_domain.dkim_signing_attributes[0].tokens[count.index]}.dkim.amazonses.com"] depends_on = [aws_sesv2_email_identity.sender_domain] @@ -19,7 +15,7 @@ resource "aws_route53_record" "spf_mail_from" { allow_overwrite = true ttl = "600" type = "TXT" - zone_id = data.aws_route53_zone.zone.zone_id + zone_id = var.hosted_zone_id name = aws_sesv2_email_identity_mail_from_attributes.sender_domain.mail_from_domain records = ["v=spf1 include:amazonses.com ~all"] } @@ -29,6 +25,6 @@ resource "aws_route53_record" "mx_receive" { type = "MX" ttl = "600" name = local.mail_from_domain - zone_id = data.aws_route53_zone.zone.zone_id + zone_id = var.hosted_zone_id records = ["10 feedback-smtp.${data.aws_region.current.name}.amazonaws.com"] } diff --git a/infra/modules/notifications-email-domain/resources/variables.tf b/infra/modules/notifications-email-domain/resources/variables.tf index 01af377be..4d324a1a1 100644 --- a/infra/modules/notifications-email-domain/resources/variables.tf +++ b/infra/modules/notifications-email-domain/resources/variables.tf @@ -2,3 +2,9 @@ variable "domain_name" { description = "The domain name to configure SES, also used as the resource names" type = string } + +variable "hosted_zone_id" { + type = string + description = "The Route53 hosted zone id for the domain" + default = null +} diff --git a/infra/{{app_name}}/service/identity_provider.tf b/infra/{{app_name}}/service/identity_provider.tf index e2af836b5..84871292c 100644 --- a/infra/{{app_name}}/service/identity_provider.tf +++ b/infra/{{app_name}}/service/identity_provider.tf @@ -22,7 +22,7 @@ module "identity_provider" { temporary_password_validity_days = local.identity_provider_config.password_policy.temporary_password_validity_days verification_email_message = local.identity_provider_config.verification_email.verification_email_message verification_email_subject = local.identity_provider_config.verification_email.verification_email_subject - domain_name = local.network_config.domain_config.hosted_zone + domain_name = local.domain_name domain_identity_arn = local.notifications_config == null ? null : local.domain_identity_arn sender_email = local.notifications_config == null ? null : local.notifications_config.sender_email sender_display_name = local.notifications_config == null ? null : local.notifications_config.sender_display_name diff --git a/infra/{{app_name}}/service/main.tf b/infra/{{app_name}}/service/main.tf index a29399c8d..65c2ba2c4 100644 --- a/infra/{{app_name}}/service/main.tf +++ b/infra/{{app_name}}/service/main.tf @@ -50,7 +50,9 @@ locals { network_config = module.project_config.network_configs[local.environment_config.network_name] - service_name = "${local.prefix}${local.service_config.service_name}" + service_name = "${local.prefix}${local.service_config.service_name}" + domain_name = local.service_config.domain_name + hosted_zone_id = local.domain_name != null ? data.aws_route53_zone.zone[0].zone_id : null } terraform { @@ -119,11 +121,11 @@ data "aws_security_groups" "aws_services" { data "aws_acm_certificate" "certificate" { count = local.service_config.enable_https ? 1 : 0 - domain = local.service_config.domain_name + domain = local.domain_name } data "aws_route53_zone" "zone" { - count = local.service_config.domain_name != null ? 1 : 0 + count = local.domain_name != null ? 1 : 0 name = local.network_config.domain_config.hosted_zone } @@ -140,8 +142,8 @@ module "service" { public_subnet_ids = data.aws_subnets.public.ids private_subnet_ids = data.aws_subnets.private.ids - domain_name = local.service_config.domain_name - hosted_zone_id = local.service_config.domain_name != null ? data.aws_route53_zone.zone[0].zone_id : null + domain_name = local.domain_name + hosted_zone_id = local.hosted_zone_id certificate_arn = local.service_config.enable_https ? data.aws_acm_certificate.certificate[0].arn : null cpu = local.service_config.cpu diff --git a/infra/{{app_name}}/service/notifications.tf b/infra/{{app_name}}/service/notifications.tf index 82aab8f5c..8d3aad2c1 100644 --- a/infra/{{app_name}}/service/notifications.tf +++ b/infra/{{app_name}}/service/notifications.tf @@ -18,7 +18,8 @@ module "notifications_email_domain" { count = local.notifications_config != null && !local.is_temporary ? 1 : 0 source = "../../modules/notifications-email-domain/resources" - domain_name = local.network_config.domain_config.hosted_zone + domain_name = local.domain_name + hosted_zone_id = local.hosted_zone_id } # If the app has `enable_notifications` set to true AND this *is* a temporary @@ -27,7 +28,7 @@ module "existing_notifications_email_domain" { count = local.notifications_config != null && local.is_temporary ? 1 : 0 source = "../../modules/notifications-email-domain/data" - domain_name = local.network_config.domain_config.hosted_zone + domain_name = local.domain_name } # If the app has `enable_notifications` set to true, create a new email notification