api+flarectl: allow updating dns record type#400
Conversation
That is, if a caller passes a validly configured `rr DNSRecord` which
includes a non-default (i.e. manually set by them) "Type", don't blindly
reset its `.Type` to the original record's type willy nilly.
The previous behaviour was surprising.
This patch allows one to swiftly change a record's type and value in one
go, i.e. from:
foo.example.com A 127.0.0.1
... to:
foo.example.com CNAME example.com
... with just one operation, without having to first delete a record and
then re-create it with the new type.
i.e. having an A record the likes of:
$ flarectl dns list --zone example.com --type A
ID | TYPE | NAME | CONTENT | PROXIED | TTL
-----------------------------------+-------+----------------------+--------------------------------+---------+------
f00f00f00f00f00f00f00f00f00f00f0 | A | blog.example.com | 127.100.100.100 | true | 1
[...]
... allow both its content _and_ its type to be changed with just one
command:
$ flarectl dns update --zone example.com --id f00f00f00f00f00f00f00f00f00f00f0 --type CNAME --content example.com --proxy
... which afterwards yields:
$ flarectl dns list --zone example.com --type A
ID | TYPE | NAME | CONTENT | PROXIED | TTL
-----------------------------------+-------+----------------------+--------------------------------+---------+------
f00f00f00f00f00f00f00f00f00f00f0 | CNAME | blog.example.com | example.com | true | 1
[...]
... without having to force the user to first delete the record, then
recreate it anew with the proper new type.
|
I have some reservations about the edge cases this will introduce (code itself is fine). The largest being if someone goes from (what I'll dub) a simple record configuration (A/CNAME) to a complex record configuration (SRV/LOC/DNSKEY) thinking this will be a straight swap and having them left with incomplete or broken records. There are explicit rules in the Terraform provider to force recreation of the rule when the type changes to mitigate a bunch of these scenarios. We can take two approaches here:
Thoughts @patryk? |
|
My 2c: 2, then 1. I'm sure I'm not the only one who's had to use another tool in order to reliably change an A record into a CNAME, so having this feature available at all is IMVHO more important than the feature going as far as disallowing the user shooting themselves in the foot. As far as that goes, these couple commits can easily be merged as is, and if you provide me with a link or something as to which switches ("from" type to "to" type) should be disallowed, I'm more than happy to have a go at it and provide a pull request for it once done. I've had to use my own branch in order to reliably go and switch quite a few records in these past few days, and I'm happy I didn't have to use curl for it. I still had that chill and anticipation of "what if I screwed the ID up, and ended up changing a TXT instead of the A record" - but the onus was on me, the user, to get it right. If the tool can stop common mistakes, it's IMVHO a "nice to have". |
|
Originally The API code always performs request validation first, then executes. So I doubt that it would leave the record in broken state. I think we're fine with this change, it's a bit niche use case, but ultimately should be harmless. We'll keep the current behaviour in Terraform, though. |
3 participants
Description
The Cloudflare REST API apparently allows one to change both a record's usual properties, such as the content; but also the record's Type. This wasn't reflected in this repository's API subroutine which implements the
UpdateDNSRecordcall. Consequently, it wasn't implemented in theflarectl dns update ...command, either.It now is.
This allows one to swiftly change a record's value, type and proxy status in one go. Particularly useful (my own use case) when wanting to migrate an
Arecord to aCNAMEwhen implementing a Cloudflare Load Balancer on a domain.Has your change been tested?
Locally, as shown in the commit messages.
Types of changes
What sort of change does your code introduce/modify?
Checklist: