Add `dnsimple_email_forward` resource

[issyl0]

• First of all, thanks for building this provider! 😍
• While I was setting a bunch of DNSimple records up with Terraform, I thought it would be nice to be able to configure DNSimple email forwarding for domains in the same way.
• This implements CRUD and importing of DNSimple Email Forwarding rules.
• The following example Terraform config documents the new resource:

resource "dnsimple_email_forward" "test" {
  domain    = "issyl0.dev"
  from      = "tf"
  to        = "me@issyl0.co.uk"
}

---

This is draft for two reasons (hopefully that's OK):

• ~The tests don't pass, and the plan post-apply doesn't come out with "no changes". This is because the DNSimple API seems to accept only the first part of a from address as input, but returns the whole thing as the response. I don't know if that's intentional. I could probably work around it with some splitting on @? If I try "tf@issyl0.dev" as from, the created email forward ends up as "tf@issyl0.dev@issyl0.dev". I'd appreciate maintainers' opinions on approaches here!~
• ~Importing this resource doesn't work yet (I get the same error as in #27). This would be very useful for me and I suspect others, but I can't figure it out right now.~

[issyl0]

I managed to implement terraform import for this new resource - I must have made a typo somewhere when I first tried!

For some more thorough information on what goes wrong with make testacc, see below. I've not yet had time to dig into why this happens from the API's end (say in a plain Go/Ruby script) to compare the two.

=== RUN   TestAccDnsimpleEmailForward_import
    TestAccDnsimpleEmailForward_import: testing.go:569: Step 0 error: After applying this step, the plan was not empty:

        DIFF:

        UPDATE: dnsimple_email_forward.wildcard
          domain: "issyl0.dev" => "issyl0.dev"
          from:   "(.*)@issyl0.dev" => "(.*)"
          id:     "40281" => "40281"
          to:     "contacts@example.org" => "contacts@example.org"

        STATE:

        dnsimple_email_forward.wildcard:
          ID = 40281
          provider = provider.dnsimple
          domain = issyl0.dev
          from = (.*)@issyl0.dev
          to = contacts@example.org
--- FAIL: TestAccDnsimpleEmailForward_import (3.11s)

[...]

=== RUN   TestAccCheckDNSimpleEmailForwardConfig_Basic
    TestAccCheckDNSimpleEmailForwardConfig_Basic: testing.go:569: Step 0 error: After applying this step, the plan was not empty:

        DIFF:

        UPDATE: dnsimple_email_forward.hello
          domain: "issyl0.dev" => "issyl0.dev"
          from:   "hello@issyl0.dev" => "hello"
          id:     "40282" => "40282"
          to:     "hi@example.org" => "hi@example.org"

        STATE:

        dnsimple_email_forward.hello:
          ID = 40282
          provider = provider.dnsimple
          domain = issyl0.dev
          from = hello@issyl0.dev
          to = hi@example.org
--- FAIL: TestAccCheckDNSimpleEmailForwardConfig_Basic (2.95s)

[issyl0]

This fully works now and the tests pass 🎉 I split the API response From address on @ for reading it back, and the post-apply and post-import plan now comes back clean. Marking as not draft. 👍

[weppos]

Thanks for the patch @issyl0 ! I cannot review it right away, but I'm setting myself a reminder to follow up shortly.

I also want to take the opportunity to look into the from part. Admittedly, it was not a great design decision to use from/to (also with different formats) and it caused us a few headaches internally and in the API. We've been evaluating changes to the naming and format, and I may take the opportunity to adjust this PR to use the new format right away, to avoid starting already with some legacy naming.

I'll get back to you ASAP.

[issyl0]

@weppos Thanks so much. Take all the time you need! On the From stuff, yeah, it was an interesting problem! I wonder if it would be better if DNSimple at API level stripped @domain from the from input (if it can't support the full email address)? The email address spec (excuse the Wikipedia link) doesn't allow @ in the foo section of foo@domain, so that should be safe?

[skorfmann]

Awesome - Looking forward to use this one 👍

[issyl0]

@weppos The most gentle of pings on this - I hope everything's going well for you! 🙏🏻

[weppos]

I apologize for the delay @issyl0 . I just want to post a brief update.

I reviewed the PR. I could have merged it right away, but as mentioned I decided to take it as an opportunity to commit to some long overdue work on the attribute renames on our side.

I did some progress today, I was able to move forward our local codebase with some changes. They are not shipped, so I cannot adjust this PR yet. I will try to time-box the changes, should I not be able to have them ready in the next few days, I will find a way to merge and ship this PR as it is.

[weppos]

@issyl0 is it common practice in Terraform to pass the whole email or just the name part when referring to the "from" address?

DNSimple API currently requires "from" to be only the name, but they return "from" as a full address. This inconsistency will be fixed soon. I wonder which version would be preferable in the Terraform world, if passing around the whole email or just the name.

Do you know what other services do?

[issyl0]

Thanks for taking another look at this, @weppos!

I don't know what the common practice is, or what other services do. However, I would probably infer from the dnsimple_record Terraform conventions that only specifying the first part of the email as part of dnsimple_email_forward is the right thing to do, given we set the domain we're operating on as domain in the stanza already (see the example), so people would expect the first part to be returned only. Obviously, the to address has to be complete as it's on a different domain.

This approach would also be consistent with the DNSimple web UI for email forwards, where there's a textbox only for the first part of the "from" email address.

[weppos]

It makes sense. Thanks for the feedback. I made some small adjustments to better reflect future API changes:

• to is renamed to destination_email
• from is renamed to alias_email. Unlike today, alias_email will be consistently accepting/returning a full email in the API payload. To preserve compatibility, we'll expose also a alias_name which is the name-only counterpart. For this reason, I've actually switched the code to get/set alias_name, whereas both alias_name and alias_email will be exported.

Sadly GH doesn't allow me to push to your branch, so it ended up creating a new one. You can find the changes here https://github.com/terraform-providers/terraform-provider-dnsimple/pull/30

I also just realized we are currently defaulting the client to production, which will require to pay to test this lib. I will look into merging https://github.com/terraform-providers/terraform-provider-dnsimple/pull/12 shortlyafter.

If you have any last-minute feedback, let me know. Otherwise, I'll go ahead and merge the (updated) PR.

[issyl0]

Sadly GH doesn't allow me to push to your branch, so it ended up creating a new one.

Strange, the box is definitely checked to say that maintainers can push to my fork's branch:

[weppos]

Sadly GH doesn't allow me to push to your branch, so it ended up creating a new one.

Strange, the box is definitely checked to say that maintainers can push to my fork's branch:

I guess it only works from the UI.

[issyl0]

Thanks, I'll close out this one and follow #30 for updates. Thanks again for all your help here, @weppos!