-
-
Notifications
You must be signed in to change notification settings - Fork 75
Description
/cc @mholt
A few semi-related thoughts:
Rate limitting
Essentially every provider will have a rate limit of some sort. If you hit the rate limit, should the provider catch that itself and automatically retry, or should it just return an error and make that the user's problem?
The provider is probably in a better place to handle this since it can catch a 429 Too Many Requests
and wait specifically until Retry-After
; however, if the API that the provider calls doesn't send a Retry-After
, then the provider will have to do exponential backoff (or something similar), which could block for a very long time and potentially surprise any downstream users.
Atomic transactions
Should we guarantee that SetRecords
is atomic/a complete transaction? That is, if err == nil
, then all of the changes in SetRecords
were applied; and if err != nil
, then calling SetRecords
didn't change the zone at all.
Many providers will need to loop over the records passed to SetRecords
and make multiple individual API calls to set the records. If the provider fails to set one of these records, should we require it to go back and revert all of the changes that it made in earlier loop iterations? And what should it do if it gets an error while reverting a change?
I think that having SetRecords
be atomic is the only sensible solution, since otherwise err != nil
means that the zone is in a completely unknown state. But implementing these semantics would be pretty tricky for some libdns
providers if the upstream APIs that they call don't provide the necessary primitives.
Batched updates
Should we add a function that allows you to collect multiple updates in a single batch? Right now, you can combine multiple SetRecords
and AppendRecords
calls into a single SetRecords
call (possibly preceded by a GetRecords
call), but there's no way to combine this with any DeleteRecords
calls.
Without a method that lets you batch multiple actions into a single atomic transaction, then downstream users would need code like this:
// Assume the zone begins with the following contents:
//
// alpha 3600 IN A 192.0.2.111
// beta 3600 IN A 192.0.2.222
// www 3600 IN CNAME alpha
//
// and you want for the final contents to be:
//
// beta 3600 IN A 192.0.2.111
// www 3600 IN A 192.0.2.111
// www 3600 IN A 192.0.2.222
//
// You want this to either fully succeed or fully fail.
func AtomicSetZone() error {
provider := &Provider{}
ctx := context.Background()
zone := "example.com."
records, err := provider.GetRecords(ctx, zone)
if err != nil {
return err
}
_, err = provider.DeleteRecords(ctx, zone, []Record{
RR{
Name: "www",
Type: "CNAME",
},
})
if err != nil {
for _, record := range records {
rr, _ := record.RR()
if rr.Name == "www" && rr.Type == "CNAME" {
provider.SetRecords(ctx, zone, []Record{rr})
}
}
return err
}
_, err = provider.SetRecords(ctx, zone, []Record{
A{
Name: "beta",
TTL: 3600,
IP: netip.MustParseAddr("192.0.2.111"),
},
})
if err != nil {
for _, record := range records {
rr, _ := record.RR()
if rr.Name == "beta" && rr.Type == "A" {
provider.SetRecords(ctx, zone, []Record{rr})
}
if rr.Name == "www" && rr.Type == "CNAME" {
provider.SetRecords(ctx, zone, []Record{rr})
}
}
return err
}
_, err = provider.AppendRecords(ctx, zone, []Record{
A{
Name: "www",
TTL: 3600,
IP: netip.MustParseAddr("192.0.2.111"),
},
A{
Name: "www",
TTL: 3600,
IP: netip.MustParseAddr("192.0.2.222"),
},
})
if err != nil {
provider.DeleteRecords(ctx, zone, []Record{
RR{
Name: "www",
Type: "A",
},
})
for _, record := range records {
rr, _ := record.RR()
if rr.Name == "beta" && rr.Type == "A" {
provider.SetRecords(ctx, zone, []Record{rr})
}
if rr.Name == "www" && rr.Type == "CNAME" {
provider.SetRecords(ctx, zone, []Record{rr})
}
}
return err
}
return nil
}
But if we had a batching/atomic transaction function, then downstream users would only need something like this:
func AtomicSetZone() error {
provider := &Provider{}
ctx := context.Background()
zone := "example.com."
_, err := provider.ActRecords(ctx, zone,
[]Action{
Delete{[]Record{
RR{
Name: "www",
Type: "CNAME",
},
}},
Set{[]Record{
A{
Name: "beta",
TTL: 3600,
IP: netip.MustParseAddr("192.0.2.111"),
},
}},
Append{[]Record{
A{
Name: "www",
TTL: 3600,
IP: netip.MustParseAddr("192.0.2.111"),
},
A{
Name: "www",
TTL: 3600,
IP: netip.MustParseAddr("192.0.2.222"),
},
}},
})
return err
}
I would guess that most of the upstream APIs that the libdns
providers call offer some way to send multiple updates together in a single API call, so this should be fairly easy to implement for them. And for libdns
providers where the upstream API doesn't offer something like this, we could offer a helper function that will loop through the list of actions, call (Get/Set/Append)Records
as necessary, and if any of them fails, loop through the actions and reverse them.
This would also reduce the likelihood of being rate limited since the libdns
provider will need to make fewer calls to its upstream API, if the API supports multiple actions in one call.