From 7db3741bc72462e17b4f099c45a39626d6e1ab3c Mon Sep 17 00:00:00 2001 From: Tom Limoncelli Date: Fri, 13 Nov 2020 16:32:40 -0500 Subject: [PATCH] MAINTENANCE: Unknown rtypes should return errors, not a panic (#945) * Eliminate panics related to unknown rtypes --- models/t_parse.go | 8 +-- providers/azuredns/azureDnsProvider.go | 72 +++++++++++++++------- providers/cloudflare/cloudflareProvider.go | 18 +++--- providers/cloudflare/rest.go | 7 ++- providers/cloudns/cloudnsProvider.go | 4 +- providers/cscglobal/cscglobalProvider.go | 2 +- providers/dnsimple/dnsimpleProvider.go | 10 +-- providers/exoscale/exoscaleProvider.go | 4 +- providers/gcloud/gcloudProvider.go | 13 ++-- providers/inwx/inwxProvider.go | 3 +- providers/linode/linodeProvider.go | 7 +-- providers/ovh/ovhProvider.go | 13 ++-- providers/route53/route53Provider.go | 14 +++-- 13 files changed, 105 insertions(+), 70 deletions(-) diff --git a/models/t_parse.go b/models/t_parse.go index 00f496ffe..bf0b4da07 100644 --- a/models/t_parse.go +++ b/models/t_parse.go @@ -10,12 +10,8 @@ import ( // string (all the parameters of an MX, SRV, CAA, etc). Rather than have // each provider rewrite this code many times, here's a helper function to use. // -// At this time, the idiom is to panic rather than continue with potentially -// misunderstood data. We do this panic() at the provider level. -// Therefore the typical calling sequence is: -// if err := rc.PopulateFromString(rtype, value, origin); err != nil { -// panic(fmt.Errorf("unparsable record received from provider: %w", err)) -// } +// If this doesn't work for all rtypes, process the special cases then +// call this for the remainder. func (r *RecordConfig) PopulateFromString(rtype, contents, origin string) error { if r.Type != "" && r.Type != rtype { panic(fmt.Errorf("assertion failed: rtype already set (%s) (%s)", rtype, r.Type)) diff --git a/providers/azuredns/azureDnsProvider.go b/providers/azuredns/azureDnsProvider.go index 21c2380fc..390095bba 100644 --- a/providers/azuredns/azureDnsProvider.go +++ b/providers/azuredns/azureDnsProvider.go @@ -225,9 +225,19 @@ func (a *azurednsProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mod if len(recs) == 0 { var rrset *adns.RecordSet for _, r := range records { - if strings.TrimSuffix(*r.RecordSetProperties.Fqdn, ".") == k.NameFQDN && nativeToRecordType(r.Type) == nativeToRecordType(to.StringPtr(k.Type)) { - rrset = r - break + if strings.TrimSuffix(*r.RecordSetProperties.Fqdn, ".") == k.NameFQDN { + n1, err := nativeToRecordType(r.Type) + if err != nil { + return nil, err + } + n2, err := nativeToRecordType(to.StringPtr(k.Type)) + if err != nil { + return nil, err + } + if n1 == n2 { + rrset = r + break + } } } if rrset != nil { @@ -237,7 +247,11 @@ func (a *azurednsProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mod F: func() error { ctx, cancel := context.WithTimeout(context.Background(), 6000*time.Second) defer cancel() - _, err := a.recordsClient.Delete(ctx, *a.resourceGroup, zoneName, *rrset.Name, nativeToRecordType(rrset.Type), "") + rt, err := nativeToRecordType(rrset.Type) + if err != nil { + return err + } + _, err = a.recordsClient.Delete(ctx, *a.resourceGroup, zoneName, *rrset.Name, rt, "") if err != nil { return err } @@ -248,7 +262,10 @@ func (a *azurednsProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mod return nil, fmt.Errorf("no record set found to delete. Name: '%s'. Type: '%s'", k.NameFQDN, k.Type) } } else { - rrset, recordType := a.recordToNative(k, recs) + rrset, recordType, err := a.recordToNative(k, recs) + if err != nil { + return nil, err + } var recordName string for _, r := range recs { i := int64(r.TTL) @@ -257,8 +274,14 @@ func (a *azurednsProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mod } for _, r := range records { - existingRecordType := nativeToRecordType(r.Type) - changedRecordType := nativeToRecordType(to.StringPtr(k.Type)) + existingRecordType, err := nativeToRecordType(r.Type) + if err != nil { + return nil, err + } + changedRecordType, err := nativeToRecordType(to.StringPtr(k.Type)) + if err != nil { + return nil, err + } if strings.TrimSuffix(*r.RecordSetProperties.Fqdn, ".") == k.NameFQDN && (changedRecordType == adns.CNAME || existingRecordType == adns.CNAME) { if existingRecordType == adns.A || existingRecordType == adns.AAAA || changedRecordType == adns.A || changedRecordType == adns.AAAA { //CNAME cannot coexist with an A or AA corrections = append(corrections, @@ -308,31 +331,32 @@ func (a *azurednsProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mod return corrections, nil } -func nativeToRecordType(recordType *string) adns.RecordType { +func nativeToRecordType(recordType *string) (adns.RecordType, error) { recordTypeStripped := strings.TrimPrefix(*recordType, "Microsoft.Network/dnszones/") switch recordTypeStripped { case "A", "AZURE_ALIAS_A": - return adns.A + return adns.A, nil case "AAAA", "AZURE_ALIAS_AAAA": - return adns.AAAA + return adns.AAAA, nil case "CAA": - return adns.CAA + return adns.CAA, nil case "CNAME", "AZURE_ALIAS_CNAME": - return adns.CNAME + return adns.CNAME, nil case "MX": - return adns.MX + return adns.MX, nil case "NS": - return adns.NS + return adns.NS, nil case "PTR": - return adns.PTR + return adns.PTR, nil case "SRV": - return adns.SRV + return adns.SRV, nil case "TXT": - return adns.TXT + return adns.TXT, nil case "SOA": - return adns.SOA + return adns.SOA, nil default: - panic(fmt.Errorf("rc.String rtype %v unimplemented", *recordType)) + // Unimplemented type. Return adns.A as a decoy, but send an error. + return adns.A, fmt.Errorf("rc.String rtype %v unimplemented", *recordType) } } @@ -463,7 +487,7 @@ func nativeToRecords(set *adns.RecordSet, origin string) []*models.RecordConfig return results } -func (a *azurednsProvider) recordToNative(recordKey models.RecordKey, recordConfig []*models.RecordConfig) (*adns.RecordSet, adns.RecordType) { +func (a *azurednsProvider) recordToNative(recordKey models.RecordKey, recordConfig []*models.RecordConfig) (*adns.RecordSet, adns.RecordType, error) { recordSet := &adns.RecordSet{Type: to.StringPtr(recordKey.Type), RecordSetProperties: &adns.RecordSetProperties{}} for _, rec := range recordConfig { switch recordKey.Type { @@ -516,11 +540,15 @@ func (a *azurednsProvider) recordToNative(recordKey models.RecordKey, recordConf *recordSet.Type = rec.AzureAlias["type"] recordSet.TargetResource = &adns.SubResource{ID: to.StringPtr(rec.Target)} default: - panic(fmt.Errorf("rc.String rtype %v unimplemented", recordKey.Type)) + return nil, adns.A, fmt.Errorf("rc.String rtype %v unimplemented", recordKey.Type) // ands.A is a placeholder } } - return recordSet, nativeToRecordType(to.StringPtr(*recordSet.Type)) + rt, err := nativeToRecordType(to.StringPtr(*recordSet.Type)) + if err != nil { + return nil, adns.A, err // adns.A is a placeholder + } + return recordSet, rt, nil } func (a *azurednsProvider) fetchRecordSets(zoneName string) ([]*adns.RecordSet, error) { diff --git a/providers/cloudflare/cloudflareProvider.go b/providers/cloudflare/cloudflareProvider.go index 9cceedc1c..bbc57a847 100644 --- a/providers/cloudflare/cloudflareProvider.go +++ b/providers/cloudflare/cloudflareProvider.go @@ -546,7 +546,7 @@ type cfRecord struct { Priority json.Number `json:"priority"` } -func (c *cfRecord) nativeToRecord(domain string) *models.RecordConfig { +func (c *cfRecord) nativeToRecord(domain string) (*models.RecordConfig, error) { // normalize cname,mx,ns records with dots to be consistent with our config format. if c.Type == "CNAME" || c.Type == "MX" || c.Type == "NS" || c.Type == "SRV" { if c.Content != "." { @@ -571,28 +571,28 @@ func (c *cfRecord) nativeToRecord(domain string) *models.RecordConfig { if c.Priority == "" { priority = 0 } else { - if p, err := c.Priority.Int64(); err != nil { - panic(fmt.Errorf("error decoding priority from cloudflare record: %w", err)) - } else { - priority = uint16(p) + p, err := c.Priority.Int64() + if err != nil { + return nil, fmt.Errorf("error decoding priority from cloudflare record: %w", err) } + priority = uint16(p) } if err := rc.SetTargetMX(priority, c.Content); err != nil { - panic(fmt.Errorf("unparsable MX record received from cloudflare: %w", err)) + return nil, fmt.Errorf("unparsable MX record received from cloudflare: %w", err) } case "SRV": data := *c.Data if err := rc.SetTargetSRV(data.Priority, data.Weight, data.Port, dnsutil.AddOrigin(data.Target.FQDN(), domain)); err != nil { - panic(fmt.Errorf("unparsable SRV record received from cloudflare: %w", err)) + return nil, fmt.Errorf("unparsable SRV record received from cloudflare: %w", err) } default: // "A", "AAAA", "ANAME", "CAA", "CNAME", "NS", "PTR", "TXT" if err := rc.PopulateFromString(rType, c.Content, domain); err != nil { - panic(fmt.Errorf("unparsable record received from cloudflare: %w", err)) + return nil, fmt.Errorf("unparsable record received from cloudflare: %w", err) } } - return rc + return rc, nil } func getProxyMetadata(r *models.RecordConfig) map[string]string { diff --git a/providers/cloudflare/rest.go b/providers/cloudflare/rest.go index 75e635152..441d00f96 100644 --- a/providers/cloudflare/rest.go +++ b/providers/cloudflare/rest.go @@ -64,8 +64,11 @@ func (c *cloudflareProvider) getRecordsForDomain(id string, domain string) ([]*m return nil, fmt.Errorf("failed fetching record list cloudflare: %s", stringifyErrors(data.Errors)) } for _, rec := range data.Result { - // fmt.Printf("REC: %+v\n", rec) - records = append(records, rec.nativeToRecord(domain)) + rt, err := rec.nativeToRecord(domain) + if err != nil { + return nil, err + } + records = append(records, rt) } ri := data.ResultInfo if len(data.Result) == 0 || ri.Page*ri.PerPage >= ri.TotalCount { diff --git a/providers/cloudns/cloudnsProvider.go b/providers/cloudns/cloudnsProvider.go index 382303aaa..ca198471c 100644 --- a/providers/cloudns/cloudnsProvider.go +++ b/providers/cloudns/cloudnsProvider.go @@ -257,9 +257,7 @@ func toReq(rc *models.RecordConfig) (requestParams, error) { req["algorithm"] = strconv.Itoa(int(rc.SshfpAlgorithm)) req["fptype"] = strconv.Itoa(int(rc.SshfpFingerprint)) default: - msg := fmt.Sprintf("ClouDNS.toReq rtype %v unimplemented", rc.Type) - panic(msg) - // We panic so that we quickly find any switch statements + return nil, fmt.Errorf("ClouDNS.toReq rtype %q unimplemented", rc.Type) } return req, nil diff --git a/providers/cscglobal/cscglobalProvider.go b/providers/cscglobal/cscglobalProvider.go index 9084502b5..38d983819 100644 --- a/providers/cscglobal/cscglobalProvider.go +++ b/providers/cscglobal/cscglobalProvider.go @@ -16,7 +16,7 @@ CSC Global Registrar: Info required in `creds.json`: - api-key Api Key - user-token User Token - - notification_emails (optional) Comma seperated list of email addresses to send notifications to + - notification_emails (optional) Comma separated list of email addresses to send notifications to */ func init() { diff --git a/providers/dnsimple/dnsimpleProvider.go b/providers/dnsimple/dnsimpleProvider.go index d5bb4cc93..5e83a1f70 100644 --- a/providers/dnsimple/dnsimpleProvider.go +++ b/providers/dnsimple/dnsimpleProvider.go @@ -93,15 +93,15 @@ func (c *dnsimpleProvider) GetZoneRecords(domain string) (models.Records, error) case "ALIAS", "URL": rec.Type = r.Type if err := rec.SetTarget(r.Content); err != nil { - panic(fmt.Errorf("unparsable record received from dnsimple: %w", err)) + return nil, fmt.Errorf("unparsable record received from dnsimple: %w", err) } case "DS": if err := rec.SetTargetDSString(r.Content); err != nil { - panic(fmt.Errorf("unparsable record received from dnsimple: %w", err)) + return nil, fmt.Errorf("unparsable record received from dnsimple: %w", err) } case "MX": if err := rec.SetTargetMX(uint16(r.Priority), r.Content); err != nil { - panic(fmt.Errorf("unparsable record received from dnsimple: %w", err)) + return nil, fmt.Errorf("unparsable record received from dnsimple: %w", err) } case "SRV": parts := strings.Fields(r.Content) @@ -109,11 +109,11 @@ func (c *dnsimpleProvider) GetZoneRecords(domain string) (models.Records, error) r.Content += "." } if err := rec.SetTargetSRVPriorityString(uint16(r.Priority), r.Content); err != nil { - panic(fmt.Errorf("unparsable record received from dnsimple: %w", err)) + return nil, fmt.Errorf("unparsable record received from dnsimple: %w", err) } default: if err := rec.PopulateFromString(r.Type, r.Content, domain); err != nil { - panic(fmt.Errorf("unparsable record received from dnsimple: %w", err)) + return nil, fmt.Errorf("unparsable record received from dnsimple: %w", err) } } cleanedRecords = append(cleanedRecords, rec) diff --git a/providers/exoscale/exoscaleProvider.go b/providers/exoscale/exoscaleProvider.go index 6d439772a..49c5f6ac6 100644 --- a/providers/exoscale/exoscaleProvider.go +++ b/providers/exoscale/exoscaleProvider.go @@ -103,11 +103,11 @@ func (c *exoscaleProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mod rec.SetTarget(r.Content) case "MX": if err := rec.SetTargetMX(uint16(r.Prio), r.Content); err != nil { - panic(fmt.Errorf("unparsable record received from exoscale: %w", err)) + return nil, fmt.Errorf("unparsable record received from exoscale: %w", err) } default: if err := rec.PopulateFromString(r.RecordType, r.Content, dc.Name); err != nil { - panic(fmt.Errorf("unparsable record received from exoscale: %w", err)) + return nil, fmt.Errorf("unparsable record received from exoscale: %w", err) } } existingRecords = append(existingRecords, rec) diff --git a/providers/gcloud/gcloudProvider.go b/providers/gcloud/gcloudProvider.go index 682d5807d..535368e3b 100644 --- a/providers/gcloud/gcloudProvider.go +++ b/providers/gcloud/gcloudProvider.go @@ -160,7 +160,12 @@ func (g *gcloudProvider) getZoneSets(domain string) (models.Records, map[key]*gd for _, set := range rrs { oldRRs[keyFor(set)] = set for _, rec := range set.Rrdatas { - existingRecords = append(existingRecords, nativeToRecord(set, rec, domain)) + rt, err := nativeToRecord(set, rec, domain) + if err != nil { + return nil, nil, "", err + } + + existingRecords = append(existingRecords, rt) } } return existingRecords, oldRRs, zoneName, err @@ -235,14 +240,14 @@ func (g *gcloudProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*model }}, nil } -func nativeToRecord(set *gdns.ResourceRecordSet, rec, origin string) *models.RecordConfig { +func nativeToRecord(set *gdns.ResourceRecordSet, rec, origin string) (*models.RecordConfig, error) { r := &models.RecordConfig{} r.SetLabelFromFQDN(set.Name, origin) r.TTL = uint32(set.Ttl) if err := r.PopulateFromString(set.Type, rec, origin); err != nil { - panic(fmt.Errorf("unparsable record received from GCLOUD: %w", err)) + return nil, fmt.Errorf("unparsable record received from GCLOUD: %w", err) } - return r + return r, nil } func (g *gcloudProvider) getRecords(domain string) ([]*gdns.ResourceRecordSet, string, error) { diff --git a/providers/inwx/inwxProvider.go b/providers/inwx/inwxProvider.go index 555e13b31..c1c221e32 100644 --- a/providers/inwx/inwxProvider.go +++ b/providers/inwx/inwxProvider.go @@ -304,9 +304,8 @@ func (api *inwxAPI) GetZoneRecords(domain string) (models.Records, error) { default: err = rc.PopulateFromString(rType, record.Content, domain) } - if err != nil { - panic(fmt.Errorf("INWX: unparsable record received: %w", err)) + return nil, fmt.Errorf("INWX: unparsable record received: %w", err) } records = append(records, rc) diff --git a/providers/linode/linodeProvider.go b/providers/linode/linodeProvider.go index b41b71fd5..093989a79 100644 --- a/providers/linode/linodeProvider.go +++ b/providers/linode/linodeProvider.go @@ -297,10 +297,7 @@ func toReq(dc *models.DomainConfig, rc *models.RecordConfig) (*recordEditRequest case "CNAME": req.Target = fixTarget(req.Target, dc.Name) default: - msg := fmt.Sprintf("linode.toReq rtype %v unimplemented", rc.Type) - panic(msg) - // We panic so that we quickly find any switch statements - // that have not been updated for a new RR type. + return nil, fmt.Errorf("linode.toReq rtype %q unimplemented", rc.Type) } return req, nil @@ -328,3 +325,5 @@ func fixTTL(ttl uint32) uint32 { return allowedTTLValues[0] } + +// that have not been updated for a new RR type. diff --git a/providers/ovh/ovhProvider.go b/providers/ovh/ovhProvider.go index de06e5f17..3604fb5b1 100644 --- a/providers/ovh/ovhProvider.go +++ b/providers/ovh/ovhProvider.go @@ -93,7 +93,10 @@ func (c *ovhProvider) GetZoneRecords(domain string) (models.Records, error) { var actual models.Records for _, r := range records { - rec := nativeToRecord(r, domain) + rec, err := nativeToRecord(r, domain) + if err != nil { + return nil, err + } if rec != nil { actual = append(actual, rec) } @@ -158,9 +161,9 @@ func (c *ovhProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*models.C return corrections, nil } -func nativeToRecord(r *Record, origin string) *models.RecordConfig { +func nativeToRecord(r *Record, origin string) (*models.RecordConfig, error) { if r.FieldType == "SOA" { - return nil + return nil, nil } rec := &models.RecordConfig{ TTL: uint32(r.TTL), @@ -176,7 +179,7 @@ func nativeToRecord(r *Record, origin string) *models.RecordConfig { rec.SetLabel(r.SubDomain, origin) if err := rec.PopulateFromString(rtype, r.Target, origin); err != nil { - panic(fmt.Errorf("unparsable record received from ovh: %w", err)) + return nil, fmt.Errorf("unparsable record received from ovh: %w", err) } // ovh default is 3600 @@ -184,7 +187,7 @@ func nativeToRecord(r *Record, origin string) *models.RecordConfig { rec.TTL = 3600 } - return rec + return rec, nil } func (c *ovhProvider) GetRegistrarCorrections(dc *models.DomainConfig) ([]*models.Correction, error) { diff --git a/providers/route53/route53Provider.go b/providers/route53/route53Provider.go index b666d02f8..a75b735ad 100644 --- a/providers/route53/route53Provider.go +++ b/providers/route53/route53Provider.go @@ -199,7 +199,11 @@ func (r *route53Provider) GetZoneRecords(domain string) (models.Records, error) var existingRecords = []*models.RecordConfig{} for _, set := range records { - existingRecords = append(existingRecords, nativeToRecords(set, domain)...) + rts, err := nativeToRecords(set, domain) + if err != nil { + return nil, err + } + existingRecords = append(existingRecords, rts...) } return existingRecords, nil } @@ -259,7 +263,7 @@ func (r *route53Provider) GetDomainCorrections(dc *models.DomainConfig) ([]*mode // that already exist). var updateOrder []models.RecordKey // Collect the keys - for k, _ := range updates { + for k := range updates { updateOrder = append(updateOrder, k) } // Sort themm @@ -418,7 +422,7 @@ func (r *route53Provider) GetDomainCorrections(dc *models.DomainConfig) ([]*mode } -func nativeToRecords(set *r53.ResourceRecordSet, origin string) []*models.RecordConfig { +func nativeToRecords(set *r53.ResourceRecordSet, origin string) ([]*models.RecordConfig, error) { results := []*models.RecordConfig{} if set.AliasTarget != nil { rc := &models.RecordConfig{ @@ -447,13 +451,13 @@ func nativeToRecords(set *r53.ResourceRecordSet, origin string) []*models.Record rc := &models.RecordConfig{TTL: uint32(*set.TTL)} rc.SetLabelFromFQDN(unescape(set.Name), origin) if err := rc.PopulateFromString(rtype, *rec.Value, origin); err != nil { - panic(fmt.Errorf("unparsable record received from R53: %w", err)) + return nil, fmt.Errorf("unparsable record received from R53: %w", err) } results = append(results, rc) } } } - return results + return results, nil } func getAliasMap(r *models.RecordConfig) map[string]string {