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

create resend.com template #573

Merged
merged 2 commits into from
Jan 17, 2025
Merged

Conversation

danilowoz
Copy link
Contributor

@danilowoz danilowoz commented Jan 16, 2025

Description

Creation of the Domain Connect template for resend.com to enable email signing and specify authorized senders.

Type of change

Please mark options that are relevant.

  • New template
  • Bug fix (non-breaking change which fixes an issue in the template)
  • New feature (non-breaking change which adds functionality to the template)
  • Breaking change (fix or feature that would cause existing template behavior to be not backward compatible)

How Has This Been Tested?

Please mark the following checks done

Example variable values

region: us-east-1
publicKey: RANDOM-VALUE

Copy link

github-actions bot commented Jan 16, 2025

Linter OK:

Linter result for resend.com.mail.json

@kerolasa
Copy link
Collaborator

Commit 9dd057d fixes typo but is titled 'create resend.com template' and the commit ca2a761 has message 'fix' that adds the template. Feels like the messages are mixed up, and to be honest it would be better to just add the template and leave typo fixing out of this review.

Secondly, it would be nice if the template would use SPFM for spf management.

$ dc-template-linter resend.com.mail.json
2025-01-16T16:28:14Z INF code=DCTL1014 dctl_note="use SPFM instead of bare SPF record" groupid= record=1 template=resend.com.mail.json type=TXT

See more info in domain connect spec: https://github.com/Domain-Connect/spec/blob/master/Domain%20Connect%20Spec%20Draft.adoc#6103-spf-record-merging

Finally the "ttl": 0, is not great. I advise to use 10 seconds or something short if you really want to avoid caching.

@danilowoz
Copy link
Contributor Author

danilowoz commented Jan 16, 2025

I totally messed up the commits, and I've dropped the commit fixing the typo to clean up the git history. Regarding the TTL, this must be a typo, and I updated it to use the default value.

Checking the SPFM suggestion...

@kerolasa, ready to review again

@kerolasa
Copy link
Collaborator

The pull looks now good to me. Thanks for fixes @danilowoz

@pawel-kow pawel-kow added this pull request to the merge queue Jan 17, 2025
Merged via the queue into Domain-Connect:master with commit 0a2f241 Jan 17, 2025
2 checks passed
@danilowoz danilowoz deleted the resend branch January 17, 2025 14:13
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 this pull request may close these issues.

3 participants