MX Records

[A-Lawrence]

Back again 👋🏼

I thought I'd open a dialogue about MX records before I dive in and just present a PR, as there are probably a few decisions that would need to be made for them.

In route 53, MX records are stored in a single entry, like so:

1 ASPMX.L.GOOGLE.COM,5 ALT1.ASPMX.L.GOOGLE.COM,5 ALT2.ASPMX.L.GOOGLE.COM,10 ALT3.ASPMX.L.GOOGLE.COM,10 ALT4.ASPMX.L.GOOGLE.COM

When running the record importer for Cloudflare, this entry is imported as a single record.

Now, it's possible to easily split these out into their own records inside the getVaporRecords method by modifying the collection building to something akin to (forgive the back-of-a-napkin-nature of this):

return collect($this->vapor->records($vaporZoneId))->map(function ($record) {
            return collect(explode(',', $record['value']))
                ->map(function ($value) use ($record) {
                    $parts = explode(' ', $value);
                    $name = count($parts) == 2 ? $parts[1] : $parts[0];
                    $priority = count($parts) == 2 ? $parts[0] : 0;

                    return [
                        'type' => $record['alias'] ? 'CNAME' : $record['type'],
                        'name' => $name,
                        'value' => $value,
                        'priority' => $priority
                    ];
                });
        })->flatten(1);

Which should return each of those MX records as a single entry in the collection.

The problem then comes when determining if the record already exists/needs to be added. The current logic for determining if it exists is as such:

return $cloudflareRecord->name === $vaporRecord['name'] && $cloudflareRecord->type === $vaporRecord['type'];

An MX record would always be true as the name and type would always match. Instinctively I thought we could match on the priority, but Google provides identical priority MX records, at which point we're back on matching on the value (which makes no sense, as you'd never be able to update them in Vapor and reliably have them sync with Cloudflare).

This made me wonder whether the more appropriate solution was to just reject any MX records from the collection, and add a note/warning to the output that highlights that MX records must be copied manually.

As a side note, I don't believe R53 is presenting information in the same way it serves it when a DNS lookup is made - I think it's a facade. The RFC for mail systems (at least, the only one I could find any reference to: https://www.rfc-editor.org/rfc/rfc974) suggests that:

"Each MX matches a domain name with two pieces of data, a preference value (an unsigned 16-bit integer), and the name of a host."

That implies it's not permissible to have a single entry with a comma-separated list, so I don't believe it'd work in Cloudflare, even though they seem to accept it as a value via the API.

So, some questions to kick us off:

1. Am I over thinking this? Is there something I've missed on CF's side to suggest that they'd treat these comma-separated entries in exactly the same way as R53? I don't have a spare domain I'm able to test it on without risking the loss of someone's emails.

2. Which approach would you prefer? If there's a desire to try to import them, do you have any ideas on how to reliably determine if the record needs adding/updating?

[Orrison]

Hey @A-Lawrence! This is great!

1. I think regardless of what is possible, it would be best to follow the RFC. I just tried to add a MX record in Cloudflare in that format and it would not let me so it looks like it is impossible with Cloudflare anways. So I think if synced it would make sense to always do so as multiple records.
2. In regard to how to best import these.. hmm.. I think it would be great to support it as we want the tool to work for what ever Vapor can throw at it to the best of it's abilities. Though, and correct me if I am wrong, this scenario may be rare as, if you are using this tool you are managing your DNS in Cloudflare. So adding MX records should probably be done directly there. But regardless this is possible, especially for people switching to Cloudflare later, so best to support it.

Due to the nature of the MX records having the same value and some having the same priority it's going to be challenging determining what needs to be updated.
So my thought is to split the MX record given by Route53 as you have described. It would be best to specifically only do this split on MX record entries whos values return at least one match via regex something like this [0-9]+\s[.a-zA-Z0-9]+,[0-9]+\s[.a-zA-Z0-9]+ (https://regex101.com/r/8HKCaT/1)
That way we don't split any other records that have commas for some other reason.

Then during import we can prompt the User with something like "MX records were found how would you like to to handle them?".
The options could be:

1. Ignore
2. Add (Just adds the records if they don't already exist exactly matching name, priority, and value)
3. Sync (Delete all other MX records in the Zone, and then add these ones)

We could then add a flag to skip the prompt and pre-select for CI/CD. Something like --mx-records=[ignore|add|sync].

This is just a kind of rough idea, what do you think?

[A-Lawrence]

1. I think regardless of what is possible, it would be best to follow the RFC. I just tried to add a MX record in Cloudflare in that format and it would not let me so it looks like it is impossible with Cloudflare anways. So I think if synced it would make sense to always do so as multiple records.

Agreed on the RFC front - this implementation should always respect it.

2. In regard to how to best import these.. hmm.. I think it would be great to support it as we want the tool to work for what ever Vapor can throw at it to the best of it's abilities. Though, and correct me if I am wrong, this scenario may be rare as, if you are using this tool you are managing your DNS in Cloudflare. So adding MX records should probably be done directly there. But regardless this is possible, especially for people switching to Cloudflare later, so best to support it.

I guess it depends on how you came to ending up on Cloudflare. For us, we use R53/Vapor as our source of truth and want to just replicate that on Cloudflare (hence the desire for the CI/CD capabilities in my other PR). So whilst I'd be inclined to agree that the majority would probably just enter the MX records directly in Cloudflare, for those that don't, the current implementation throws an error when attempting to sync the records (it gracefully fails, and doesn't terminate execution though, which is good).

Onto the proposed solution!

We could then add a flag to skip the prompt and pre-select for CI/CD. Something like --mx-records=[ignore|add|sync].

I think this could work! In our case, we'd always want to use sync, but I totally see value in add & ignore being options too.

I'm happy to take a look when I've got some free time (probably not for 2 - 3 weeks), but if you (or anyone else reading this) decides to take a stab at it before then, I'm not precious about owning the solution :)

[Orrison]

@A-Lawrence Great! This all sounds good! I may have some time to take a crack at it this week. But, same whoever gets to it first doesn't matter to me!