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

Route53 records optional? #62

Closed
blastik opened this issue Nov 30, 2020 · 6 comments · Fixed by #65
Closed

Route53 records optional? #62

blastik opened this issue Nov 30, 2020 · 6 comments · Fixed by #65

Comments

@blastik
Copy link

blastik commented Nov 30, 2020

Hello,

First of all thank you for this module.
I have an issue which is that sometimes you are dealing with the Route53 somewhere else and records already exist so it fails to create SES Route53 verification records because they are already in place. Is there any possibility you can update this module to either conditionally disable the creation of the records or have the possibility of using lifecycle to ignore changes?

Thanks!

@blastik
Copy link
Author

blastik commented Nov 30, 2020

BTW tested with

  enable_spf_record = false
  enable_verification = false

without luck since i have some _amazonses values already.
And of course cannot use extra_ses_records because as i said this is being handled within several terraform states

@dynamike
Copy link
Contributor

dynamike commented Dec 8, 2020

I think I get what you asking but to be sure could you write up the example of how you're using the module. I can try to reproduce it locally.

@blastik
Copy link
Author

blastik commented Dec 9, 2020

I think I get what you asking but to be sure could you write up the example of how you're using the module. I can try to reproduce it locally.

module "ses-domain" {
  source  = "trussworks/ses-domain/aws"
  version = "2.0.6"

  domain_name           = "whatever.com"
  from_addresses        = var.from_addresses
  mail_from_domain      = var.mail_from_domain
  dmarc_rua             = var.dmarc_rua
  route53_zone_id       = data.aws_route53_zone.ses_domain.zone_id
  ses_rule_set          = var.ses_domain_name
  enable_incoming_email = false
  extra_ses_records     = var.extra_ses_records
  enable_spf_record     = var.enable_spf_record
}

resource "aws_ses_receipt_rule_set" "ses_rule_set" {
  rule_set_name = var.ses_domain_name
}

data "aws_route53_zone" "ses_domain" {
  name = var.ses_domain_name
}

but it does not matter really. the thing is that the module tries to create a new txt entry in _amazonses but this record already exists

@dynamike
Copy link
Contributor

dynamike commented Dec 9, 2020

Ah so this is specifically related to the Route53 domain verification record.

resource "aws_route53_record" "ses_verification" {                                                                                                                                
  zone_id = var.route53_zone_id                                                                                                                                                   
  name    = "_amazonses.${aws_ses_domain_identity.main.id}"                                                                                                                       
  type    = "TXT"                                                                                                                                                                 
  ttl     = "600"                                                                                                                                                                 
  records = concat([aws_ses_domain_identity.main.verification_token], var.extra_ses_records)                                                                                      
} 

So I think the answer is a conditional on the verification record. Would you need other records to be conditionally disabled as well. Like DKIM or MX send mail from?

@blastik
Copy link
Author

blastik commented Dec 10, 2020

I believe not because you already added enable_spf_record & enable_verification

@dynamike
Copy link
Contributor

ah yes, I see what mean now. This is a bug. I'll have a PR up shortly to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants