From 80f22df705b70af1e6dac57408ed6e45e852b9e0 Mon Sep 17 00:00:00 2001 From: nemunaire Date: Tue, 6 Jul 2021 17:03:29 +0200 Subject: [PATCH] MAINTENANCE: Return error instead of panic when converting RR to RC (#1199) --- models/dnsrr.go | 51 +++++++++++++------------- pkg/prettyzone/prettyzone.go | 7 +++- pkg/prettyzone/prettyzone_test.go | 28 +++++++++++--- providers/axfrddns/axfrddnsProvider.go | 5 ++- providers/bind/bindProvider.go | 5 ++- 5 files changed, 63 insertions(+), 33 deletions(-) diff --git a/models/dnsrr.go b/models/dnsrr.go index 608b172c9..0189e57f5 100644 --- a/models/dnsrr.go +++ b/models/dnsrr.go @@ -4,7 +4,6 @@ package models import ( "fmt" - "log" "strings" "github.com/miekg/dns" @@ -42,17 +41,21 @@ func (rc *RecordConfig) String() string { // Conversions // RRstoRCs converts []dns.RR to []RecordConfigs. -func RRstoRCs(rrs []dns.RR, origin string) Records { +func RRstoRCs(rrs []dns.RR, origin string) (Records, error) { rcs := make(Records, 0, len(rrs)) for _, r := range rrs { - rc := RRtoRC(r, origin) + rc, err := RRtoRC(r, origin) + if err != nil { + return nil, err + } + rcs = append(rcs, &rc) } - return rcs + return rcs, nil } // RRtoRC converts dns.RR to RecordConfig -func RRtoRC(rr dns.RR, origin string) RecordConfig { +func RRtoRC(rr dns.RR, origin string) (RecordConfig, error) { // Convert's dns.RR into our native data type (RecordConfig). // Records are translated directly with no changes. header := rr.Header() @@ -61,43 +64,41 @@ func RRtoRC(rr dns.RR, origin string) RecordConfig { rc.TTL = header.Ttl rc.Original = rr rc.SetLabelFromFQDN(strings.TrimSuffix(header.Name, "."), origin) + var err error switch v := rr.(type) { // #rtype_variations case *dns.A: - panicInvalid(rc.SetTarget(v.A.String())) + err = rc.SetTarget(v.A.String()) case *dns.AAAA: - panicInvalid(rc.SetTarget(v.AAAA.String())) + err = rc.SetTarget(v.AAAA.String()) case *dns.CAA: - panicInvalid(rc.SetTargetCAA(v.Flag, v.Tag, v.Value)) + err = rc.SetTargetCAA(v.Flag, v.Tag, v.Value) case *dns.CNAME: - panicInvalid(rc.SetTarget(v.Target)) + err = rc.SetTarget(v.Target) case *dns.DS: - panicInvalid(rc.SetTargetDS(v.KeyTag, v.Algorithm, v.DigestType, v.Digest)) + err = rc.SetTargetDS(v.KeyTag, v.Algorithm, v.DigestType, v.Digest) case *dns.MX: - panicInvalid(rc.SetTargetMX(v.Preference, v.Mx)) + err = rc.SetTargetMX(v.Preference, v.Mx) case *dns.NS: - panicInvalid(rc.SetTarget(v.Ns)) + err = rc.SetTarget(v.Ns) case *dns.PTR: - panicInvalid(rc.SetTarget(v.Ptr)) + err = rc.SetTarget(v.Ptr) case *dns.NAPTR: - panicInvalid(rc.SetTargetNAPTR(v.Order, v.Preference, v.Flags, v.Service, v.Regexp, v.Replacement)) + err = rc.SetTargetNAPTR(v.Order, v.Preference, v.Flags, v.Service, v.Regexp, v.Replacement) case *dns.SOA: - panicInvalid(rc.SetTargetSOA(v.Ns, v.Mbox, v.Serial, v.Refresh, v.Retry, v.Expire, v.Minttl)) + err = rc.SetTargetSOA(v.Ns, v.Mbox, v.Serial, v.Refresh, v.Retry, v.Expire, v.Minttl) case *dns.SRV: - panicInvalid(rc.SetTargetSRV(v.Priority, v.Weight, v.Port, v.Target)) + err = rc.SetTargetSRV(v.Priority, v.Weight, v.Port, v.Target) case *dns.SSHFP: - panicInvalid(rc.SetTargetSSHFP(v.Algorithm, v.Type, v.FingerPrint)) + err = rc.SetTargetSSHFP(v.Algorithm, v.Type, v.FingerPrint) case *dns.TLSA: - panicInvalid(rc.SetTargetTLSA(v.Usage, v.Selector, v.MatchingType, v.Certificate)) + err = rc.SetTargetTLSA(v.Usage, v.Selector, v.MatchingType, v.Certificate) case *dns.TXT: - panicInvalid(rc.SetTargetTXTs(v.Txt)) + err = rc.SetTargetTXTs(v.Txt) default: - log.Fatalf("rrToRecord: Unimplemented zone record type=%s (%v)\n", rc.Type, rr) + return *rc, fmt.Errorf("rrToRecord: Unimplemented zone record type=%s (%v)\n", rc.Type, rr) } - return *rc -} - -func panicInvalid(err error) { if err != nil { - panic(fmt.Errorf("unparsable record received from BIND: %w", err)) + return *rc, fmt.Errorf("unparsable record received: %w", err) } + return *rc, nil } diff --git a/pkg/prettyzone/prettyzone.go b/pkg/prettyzone/prettyzone.go index ea0245d97..8ce5930e5 100644 --- a/pkg/prettyzone/prettyzone.go +++ b/pkg/prettyzone/prettyzone.go @@ -45,7 +45,12 @@ func MostCommonTTL(records models.Records) uint32 { // WriteZoneFileRR is a helper for when you have []dns.RR instead of models.Records func WriteZoneFileRR(w io.Writer, records []dns.RR, origin string) error { - return WriteZoneFileRC(w, models.RRstoRCs(records, origin), origin, 0, nil) + rcs, err := models.RRstoRCs(records, origin) + if err != nil { + return err + } + + return WriteZoneFileRC(w, rcs, origin, 0, nil) } // WriteZoneFileRC writes a beautifully formatted zone file. diff --git a/pkg/prettyzone/prettyzone_test.go b/pkg/prettyzone/prettyzone_test.go index 5a163d451..4d1f6d408 100644 --- a/pkg/prettyzone/prettyzone_test.go +++ b/pkg/prettyzone/prettyzone_test.go @@ -48,7 +48,10 @@ func TestMostCommonTtl(t *testing.T) { // All records are TTL=100 records = nil records, e = append(records, r1, r1, r1), 100 - x := models.RRstoRCs(records, "bosun.org") + x, err := models.RRstoRCs(records, "bosun.org") + if err != nil { + panic(err) + } g = MostCommonTTL(x) if e != g { t.Fatalf("expected %d; got %d\n", e, g) @@ -57,7 +60,11 @@ func TestMostCommonTtl(t *testing.T) { // Mixture of TTLs with an obvious winner. records = nil records, e = append(records, r1, r2, r2), 200 - g = MostCommonTTL(models.RRstoRCs(records, "bosun.org")) + rcs, err := models.RRstoRCs(records, "bosun.org") + if err != nil { + panic(err) + } + g = MostCommonTTL(rcs) if e != g { t.Fatalf("expected %d; got %d\n", e, g) } @@ -65,7 +72,11 @@ func TestMostCommonTtl(t *testing.T) { // 3-way tie. Largest TTL should be used. records = nil records, e = append(records, r1, r2, r3), 300 - g = MostCommonTTL(models.RRstoRCs(records, "bosun.org")) + rcs, err = models.RRstoRCs(records, "bosun.org") + if err != nil { + panic(err) + } + g = MostCommonTTL(rcs) if e != g { t.Fatalf("expected %d; got %d\n", e, g) } @@ -73,7 +84,11 @@ func TestMostCommonTtl(t *testing.T) { // NS records are ignored. records = nil records, e = append(records, r1, r4, r5), 100 - g = MostCommonTTL(models.RRstoRCs(records, "bosun.org")) + rcs, err = models.RRstoRCs(records, "bosun.org") + if err != nil { + panic(err) + } + g = MostCommonTTL(rcs) if e != g { t.Fatalf("expected %d; got %d\n", e, g) } @@ -289,7 +304,10 @@ func TestWriteZoneFileSynth(t *testing.T) { rsynz := &models.RecordConfig{Type: "R53_ALIAS", TTL: 300} rsynz.SetLabel("zalias", "bosun.org") - recs := models.RRstoRCs([]dns.RR{r1, r2, r3}, "bosun.org") + recs, err := models.RRstoRCs([]dns.RR{r1, r2, r3}, "bosun.org") + if err != nil { + panic(err) + } recs = append(recs, rsynm) recs = append(recs, rsynm) recs = append(recs, rsynz) diff --git a/providers/axfrddns/axfrddnsProvider.go b/providers/axfrddns/axfrddnsProvider.go index d4cbb6521..d1039cce6 100644 --- a/providers/axfrddns/axfrddnsProvider.go +++ b/providers/axfrddns/axfrddnsProvider.go @@ -286,7 +286,10 @@ func (c *axfrddnsProvider) GetZoneRecords(domain string) (models.Records, error) } continue default: - rec := models.RRtoRC(rr, domain) + rec, err := models.RRtoRC(rr, domain) + if err != nil { + return nil, err + } foundRecords = append(foundRecords, &rec) } } diff --git a/providers/bind/bindProvider.go b/providers/bind/bindProvider.go index 08b9f853c..3fff119bf 100644 --- a/providers/bind/bindProvider.go +++ b/providers/bind/bindProvider.go @@ -171,7 +171,10 @@ func (c *bindProvider) GetZoneRecords(domain string) (models.Records, error) { zp := dns.NewZoneParser(strings.NewReader(string(content)), domain, c.zonefile) for rr, ok := zp.Next(); ok; rr, ok = zp.Next() { - rec := models.RRtoRC(rr, domain) + rec, err := models.RRtoRC(rr, domain) + if err != nil { + return nil, err + } // FIXME(tlim): Empty branch? Is the intention to skip SOAs? if rec.Type == "SOA" { }