Skip to content

Commit

Permalink
Reuse app domain for domain identity (#834)
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
lorenyu authored Jan 15, 2025
1 parent fb1e6e6 commit cb15833
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 16 deletions.
12 changes: 4 additions & 8 deletions infra/modules/notifications-email-domain/resources/dns.tf
Original file line number Diff line number Diff line change
@@ -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]
Expand All @@ -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"]
}
Expand 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"]
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion infra/{{app_name}}/service/identity_provider.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 7 additions & 5 deletions infra/{{app_name}}/service/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand All @@ -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
Expand Down
5 changes: 3 additions & 2 deletions infra/{{app_name}}/service/notifications.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit cb15833

Please sign in to comment.