Skip to content

sendgrid integration#79

Merged
davidcheung merged 3 commits intomainfrom
sendgrid
Sep 4, 2020
Merged

sendgrid integration#79
davidcheung merged 3 commits intomainfrom
sendgrid

Conversation

@davidcheung
Copy link
Contributor

@davidcheung davidcheung commented Sep 2, 2020

Order of things that happens

  • while zero apply it will run scripts/init(in the project_dir) with parameters [sendgridApiKey] along with other env
  • running scripts/init.sh results in 2 files being create [sendgrid.auto.tfvars.json / sendgrid-secrets.auto.tfvars.json] in the environment's root (stage/prod)
  • sendgrid-secrets.auto.tfvars.json contains the sendgrid-api-key; and gets .gitignored so it will not be commited
  • sendgrid.auto.tfvars.json will contain the sendgrid_domain_id(unique identifier from sendgrid) / CNAMEs we need for verification, and send_enabled:true (to override default sendgrid is off)
  • when apply runs make apply-env it will already have the variables in the root, then it will create route53, then call API to complete the verification

assumes commitdev/terraform-aws-zero#1 to be released as 0.0.2

@davidcheung davidcheung requested a review from bmonkman September 2, 2020 00:47
@davidcheung davidcheung force-pushed the sendgrid branch 2 times, most recently from 10cf22b to 42f2cab Compare September 2, 2020 23:00
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should do something similar to what I did in post-apply, and source a script that is specific to sendgrid, then we can optionally include it. We may want to make this support optional via a parameter.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All this stuff is quite vulnerable to them making changes to their API, but it seems like it's the only option.. :feelsbadman:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah hopefully they won't make breaking changes on their /v3 api, then we would have the option to manually upgrade it 🤞

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be templated rather than a var?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm relying on the script to generate tfvars.json that will look something like

//terraform/environments/stage/sendgrid.auto.tfvars.json
{
  "sendgrid_enabled": true,
  "sendgrid_domain_id": 9104854,
  "sendgrid_cname": [
    [
      "em7596.email.get0.dev",
      "u18291290.wl041.sendgrid.net"
    ],
    [
      "s1._domainkey.email.get0.dev",
      "s1.domainkey.u18291290.wl041.sendgrid.net"
    ],
    [
      "s2._domainkey.email.get0.dev",
      "s2.domainkey.u18291290.wl041.sendgrid.net"
    ]
  ]
}

// terraform/environments/stage/sendgrid-secrets.auto.tfvars.json
{"sendgrid_apikey": "SG.blahblah-secret"}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, I must have missed that. Hm, not sure how I feel about this.. I'll think a bit more about it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, actually this would make the tf totally unusable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with the variables and default values in the stage/prod, they will just go to the default (enabled:false) and empty values

@bmonkman
Copy link
Contributor

bmonkman commented Sep 3, 2020

Could use something in the readme about how to create a sendgrid account and an api key and have everything in the right state to start this process.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be "sendgrid_cnames"

@davidcheung davidcheung force-pushed the sendgrid branch 4 times, most recently from bcfa649 to de15e2c Compare September 3, 2020 16:45
@davidcheung davidcheung force-pushed the sendgrid branch 2 times, most recently from 2a90088 to af652d7 Compare September 4, 2020 01:04
count = var.sendgrid_enabled ? 1 : 0

zone_name = var.domain_name
sendgrid_api_key_secret_name = var.sendgrid_api_key_secret_name
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

almost all the logic has been moved to commitdev/terraform-aws-zero@bd194a0 with the external-datasource 🎉


module "sendgrid_api_key" {
source = "../../modules/secret"
name = "${local.project}-sendgrid-<% index .Params `randomSeed` %>"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assumes the same api key will be shared for both staging / production, which I think is probably fine for now.

- "kibana"
- field: sendgridApiKey
label: API key to setup email integration
info: Signup at https://signup.sendgrid.com or create an API key from https://app.sendgrid.com/settings/api_keys. Sendgrid is an email delivery service enabling transactional email sending and more.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about a note here that they can leave this blank to not set up sendgrid?

@davidcheung davidcheung merged commit 83d2633 into main Sep 4, 2020
@davidcheung davidcheung deleted the sendgrid branch September 4, 2020 21:40
bmonkman pushed a commit that referenced this pull request Oct 10, 2020
* sendgrid integration

* reimpl with external datasource

* docs: message for optional sendgird api key
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.

2 participants