Skip to content

Commit 2cb245d

Browse files
j-piecuchj-piecuch
committed
SetRecords: preserve RRSets not set in the input
According to the libdns specification for SetRecords [1]: > SetRecords updates the zone so that the records described in the input > are reflected in the output. It may create or update records > or—depending on the record type—delete records to maintain parity with > the input. No other records are affected. It returns the records which > were set. > > For any (name, type) pair in the input, SetRecords ensures that the > only records in the output zone with that (name, type) pair are those > that were provided in the input. From this description, especially the "No other records are affected" part, I believe that RRSets which are not mentioned in the input should not be modified or deleted by SetRecords. This is also made clear by one of the examples. Note that the TXT record is preserved: > Example 1: > > ;; Original zone > example.com. 3600 IN A 192.0.2.1 > example.com. 3600 IN A 192.0.2.2 > example.com. 3600 IN TXT "hello world" > > ;; Input > example.com. 3600 IN A 192.0.2.3 > > ;; Resultant zone > example.com. 3600 IN A 192.0.2.3 > example.com. 3600 IN TXT "hello world" The current implementation of SetRecords deletes all RRSets from the zone which are not mentioned in the input. This commit changes the behavior to match the spec. [1] https://github.com/libdns/libdns/blob/5ab1d4de259f1eb914085c61784ad9176ea8e803/libdns.go#L108
1 parent e49cf80 commit 2cb245d

2 files changed

Lines changed: 144 additions & 44 deletions

File tree

‎provider.go‎

Lines changed: 11 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -199,72 +199,45 @@ func (p *Provider) AppendRecords(ctx context.Context, zone string, records []lib
199199

200200
// SetRecords sets the records in the zone, either by updating existing records or creating new ones.
201201
// It returns the updated records.
202-
//
203-
// Caveat: This method will fail if there are more than 500 RRsets in the zone. See package
204-
// documentation for more detail.
205202
func (p *Provider) SetRecords(ctx context.Context, zone string, records []libdns.Record) ([]libdns.Record, error) {
206203
err := acquireWriteToken(ctx)
207204
if err != nil {
208205
return nil, fmt.Errorf("waiting for inflight requests to finish: %v", err)
209206
}
210207
defer releaseWriteToken()
211208

212-
// Build the desired state
213-
rrsets := make(map[rrKey]*rrSet)
209+
// Group records by rrKey (name, type)
210+
rrsetMap := make(map[rrKey]*rrSet)
214211
for _, r := range records {
215212
rr := r.RR()
216213
key := rrKey{rrSetSubname(rr), rr.Type}
217-
rrset := rrsets[key]
214+
rrset := rrsetMap[key]
218215
if rrset == nil {
219216
rrset = &rrSet{
220217
Subname: key.Subname,
221218
Type: key.Type,
222219
Records: nil,
223220
TTL: rrSetTTL(rr),
224221
}
225-
rrsets[key] = rrset
222+
rrsetMap[key] = rrset
226223
}
227224
rrset.Records = append(rrset.Records, rrSetRecord(rr))
228225
}
229226

230-
// Fetch existing rrSets and compare to desired state
231-
existing, err := p.listRRSets(ctx, zone)
232-
if err != nil {
233-
return nil, fmt.Errorf("listing RRSets: %v", err)
234-
}
235-
for _, g := range existing {
236-
key := rrKey{g.Subname, g.Type}
237-
w := rrsets[key]
238-
switch {
239-
case w == nil:
240-
// rrset exists, but not in the input, delete it by adding it to rrsets and set
241-
// records to an empty slice to represent the deletion.
242-
// See https://desec.readthedocs.io/en/latest/dns/rrsets.html#deleting-an-rrset
243-
w0 := g
244-
w0.Records = []string{}
245-
rrsets[key] = &w0
246-
case g.equal(w):
247-
// rrset exists and is equal to the one we want; skip it in the update.
248-
delete(rrsets, key)
249-
}
250-
}
251-
252-
// Generate updates to arrive at desired state.
253-
update := make([]rrSet, 0, len(rrsets))
254-
var ret []libdns.Record
255-
for _, rrset := range rrsets {
256-
update = append(update, *rrset)
257-
258-
// Add all records being set here. This ignores records that are being deleted, because
259-
// those are represented as an rrset without any records.
227+
// Build list of RRSets to pass to the API and list of libdns records
228+
// to return from the function
229+
rrsetList := make([]rrSet, 0, len(rrsetMap))
230+
ret := make([]libdns.Record, 0, len(records))
231+
for _, rrset := range rrsetMap {
232+
rrsetList = append(rrsetList, *rrset)
260233
records0, err := libdnsRecords(*rrset)
261234
if err != nil {
262235
return nil, fmt.Errorf("parsing RRSet: %v", err)
263236
}
264237
ret = append(ret, records0...)
265238
}
266239

267-
if err := p.putRRSets(ctx, zone, update); err != nil {
240+
if err := p.putRRSets(ctx, zone, rrsetList); err != nil {
268241
return nil, fmt.Errorf("writing RRSets: %v", err)
269242
}
270243
return ret, nil

‎provider_test.go‎

Lines changed: 133 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -293,10 +293,12 @@ func TestSetRecords(t *testing.T) {
293293
ctx := setup(t, `[
294294
{"subname": "www", "type": "A", "ttl": 3600, "records": ["127.0.1.1", "127.0.1.2"]},
295295
{"subname": "", "type": "TXT", "ttl": 3600, "records": ["\"will be overridden\""]},
296+
{"subname": "sub", "type": "TXT", "ttl": 3600, "records": ["\"will stay the same\""]},
296297
{"subname": "www", "type": "HTTPS", "ttl": 3600, "records": ["1 . alpn=\"h2\""]},
297298
{"subname": "_sip._tcp.sub", "type": "SRV", "ttl": 3600, "records": ["1 100 5061 sip.example.com."]},
298299
{"subname": "_ftp._tcp", "type": "URI", "ttl": 3600, "records": ["1 2 \"ftp://example.com/arst\""]},
299-
{"subname": "", "type": "MX", "ttl": 3600, "records": ["0 mx0.example.com.", "10 mx1.example.com."]}
300+
{"subname": "", "type": "MX", "ttl": 3600, "records": ["0 mx0.example.com.", "10 mx1.example.com."]},
301+
{"subname": "www", "type": "NS", "ttl": 3600, "records": ["ns0.example.com.", "ns1.example.com."]}
300302
]`)
301303

302304
p := &desec.Provider{
@@ -369,13 +371,14 @@ func TestSetRecords(t *testing.T) {
369371
},
370372
}
371373

372-
created, err := p.SetRecords(ctx, *domain+".", records)
374+
recordsSet, err := p.SetRecords(ctx, *domain+".", records)
373375
if err != nil {
374376
t.Fatal(err)
375377
}
376378

377-
// The records that already existed are not returned by SetRecords.
378-
wantCreated := []libdns.Record{
379+
// All set records, including the ones that already existed, should be returned
380+
// by SetRecords.
381+
wantSet := []libdns.Record{
379382
libdns.Address{
380383
Name: "@",
381384
IP: netip.MustParseAddr("127.0.0.3"),
@@ -391,6 +394,16 @@ func TestSetRecords(t *testing.T) {
391394
IP: netip.MustParseAddr("127.0.0.1"),
392395
TTL: 3600 * time.Second,
393396
},
397+
libdns.ServiceBinding{
398+
Scheme: "https",
399+
Name: "www",
400+
Target: ".",
401+
Params: libdns.SvcParams{
402+
"alpn": []string{"h2"},
403+
},
404+
Priority: 1,
405+
TTL: 3600 * time.Second,
406+
},
394407
libdns.Address{
395408
Name: "www",
396409
IP: netip.MustParseAddr("127.0.0.2"),
@@ -401,16 +414,130 @@ func TestSetRecords(t *testing.T) {
401414
IP: netip.MustParseAddr("127.0.0.5"),
402415
TTL: 3600 * time.Second,
403416
},
417+
libdns.MX{
418+
Name: "@",
419+
Target: "mx0.example.com.",
420+
TTL: 3600 * time.Second,
421+
Preference: 0,
422+
},
423+
libdns.MX{
424+
Name: "@",
425+
Target: "mx1.example.com.",
426+
TTL: 3600 * time.Second,
427+
Preference: 10,
428+
},
429+
libdns.SRV{
430+
Service: "sip",
431+
Transport: "tcp",
432+
Name: "sub",
433+
Target: "sip.example.com.",
434+
TTL: 3600 * time.Second,
435+
Priority: 1,
436+
Weight: 100,
437+
Port: 5061,
438+
},
439+
libdns.RR{
440+
Type: "URI",
441+
Name: "_ftp._tcp",
442+
Data: `1 2 "ftp://example.com/arst"`,
443+
TTL: 3600 * time.Second,
444+
},
404445
}
405-
if diff := cmp.Diff(wantCreated, created, cmpRecord); diff != "" {
446+
447+
if diff := cmp.Diff(wantSet, recordsSet, cmpRecord); diff != "" {
406448
t.Fatalf("p.SetRecords() unexpected diff [-want +got]: %s", diff)
407449
}
408450

451+
wantCurrent := []libdns.Record{
452+
// Records for (name, type) pairs which were not present in the SetRecords input
453+
// should be unaffected.
454+
libdns.TXT{
455+
Name: "sub",
456+
Text: `will stay the same`,
457+
TTL: time.Second * 3600,
458+
},
459+
libdns.NS{
460+
Name: "www",
461+
Target: "ns0.example.com.",
462+
TTL: time.Second * 3600,
463+
},
464+
libdns.NS{
465+
Name: "www",
466+
Target: "ns1.example.com.",
467+
TTL: time.Second * 3600,
468+
},
469+
// Records for (name, type) pairs which were present in the SetRecords input
470+
// should match the output of SetRecords.
471+
libdns.Address{
472+
Name: "@",
473+
IP: netip.MustParseAddr("127.0.0.3"),
474+
TTL: time.Second * 3601,
475+
},
476+
libdns.TXT{
477+
Name: "@",
478+
Text: `hello dns!`,
479+
TTL: time.Second * 3600,
480+
},
481+
libdns.Address{
482+
Name: "www",
483+
IP: netip.MustParseAddr("127.0.0.1"),
484+
TTL: 3600 * time.Second,
485+
},
486+
libdns.ServiceBinding{
487+
Scheme: "https",
488+
Name: "www",
489+
Target: ".",
490+
Params: libdns.SvcParams{
491+
"alpn": []string{"h2"},
492+
},
493+
Priority: 1,
494+
TTL: 3600 * time.Second,
495+
},
496+
libdns.Address{
497+
Name: "www",
498+
IP: netip.MustParseAddr("127.0.0.2"),
499+
TTL: 3600 * time.Second,
500+
},
501+
libdns.Address{
502+
Name: "subsub.sub",
503+
IP: netip.MustParseAddr("127.0.0.5"),
504+
TTL: 3600 * time.Second,
505+
},
506+
libdns.MX{
507+
Name: "@",
508+
Target: "mx0.example.com.",
509+
TTL: 3600 * time.Second,
510+
Preference: 0,
511+
},
512+
libdns.MX{
513+
Name: "@",
514+
Target: "mx1.example.com.",
515+
TTL: 3600 * time.Second,
516+
Preference: 10,
517+
},
518+
libdns.SRV{
519+
Service: "sip",
520+
Transport: "tcp",
521+
Name: "sub",
522+
Target: "sip.example.com.",
523+
TTL: 3600 * time.Second,
524+
Priority: 1,
525+
Weight: 100,
526+
Port: 5061,
527+
},
528+
libdns.RR{
529+
Type: "URI",
530+
Name: "_ftp._tcp",
531+
Data: `1 2 "ftp://example.com/arst"`,
532+
TTL: 3600 * time.Second,
533+
},
534+
}
535+
409536
got, err := p.GetRecords(ctx, *domain+".")
410537
if err != nil {
411538
t.Fatal(err)
412539
}
413-
if diff := cmp.Diff(records, got, cmpRecord); diff != "" {
540+
if diff := cmp.Diff(wantCurrent, got, cmpRecord); diff != "" {
414541
t.Fatalf("p.GetRecords() unexpected diff [-want +got]: %s", diff)
415542
}
416543
}

0 commit comments

Comments
 (0)