Cleanup: Bubble errors up from diff instead of panic (#799) (#817)

pkg/diff/diff.go

@@ -23,10 +23,10 @@ type Changeset []Correlation
 // Differ is an interface for computing the difference between two zones.
 type Differ interface {
 	// IncrementalDiff performs a diff on a record-by-record basis, and returns a sets for which records need to be created, deleted, or modified.
-	IncrementalDiff(existing []*models.RecordConfig) (unchanged, create, toDelete, modify Changeset)
+	IncrementalDiff(existing []*models.RecordConfig) (unchanged, create, toDelete, modify Changeset, err error)
 	// ChangedGroups performs a diff more appropriate for providers with a "RecordSet" model, where all records with the same name and type are grouped.
 	// Individual record changes are often not useful in such scenarios. Instead we return a map of record keys to a list of change descriptions within that group.
-	ChangedGroups(existing []*models.RecordConfig) map[models.RecordKey][]string
+	ChangedGroups(existing []*models.RecordConfig) (map[models.RecordKey][]string, error)
 }
 
 // New is a constructor for a Differ.
@@ -85,7 +85,7 @@ func (d *differ) content(r *models.RecordConfig) string {
 	return content
 }
 
-func (d *differ) IncrementalDiff(existing []*models.RecordConfig) (unchanged, create, toDelete, modify Changeset) {
+func (d *differ) IncrementalDiff(existing []*models.RecordConfig) (unchanged, create, toDelete, modify Changeset, err error) {
 	unchanged = Changeset{}
 	create = Changeset{}
 	toDelete = Changeset{}
@@ -108,9 +108,9 @@ func (d *differ) IncrementalDiff(existing []*models.RecordConfig) (unchanged, cr
 	}
 	for _, dr := range desired {
 		if d.matchIgnoredName(dr.GetLabel()) {
-			panic(fmt.Sprintf("Trying to update/add IGNORE_NAMEd record: %s %s", dr.GetLabel(), dr.Type))
+			return nil, nil, nil, nil, fmt.Errorf("Trying to update/add IGNORE_NAMEd record: %s %s", dr.GetLabel(), dr.Type)
 		} else if d.matchIgnoredTarget(dr.GetTargetField(), dr.Type) {
-			panic(fmt.Sprintf("Trying to update/add IGNORE_TARGETd record: %s %s", dr.GetLabel(), dr.Type))
+			return nil, nil, nil, nil, fmt.Errorf("Trying to update/add IGNORE_TARGETd record: %s %s", dr.GetLabel(), dr.Type)
 		} else {
 			k := dr.Key()
 			desiredByNameAndType[k] = append(desiredByNameAndType[k], dr)
@@ -165,14 +165,14 @@ func (d *differ) IncrementalDiff(existing []*models.RecordConfig) (unchanged, cr
 		for _, ex := range existingRecords {
 			normalized := d.content(ex)
 			if existingLookup[normalized] != nil {
-				panic(fmt.Sprintf("DUPLICATE E_RECORD FOUND: %s %s", key, normalized))
+				return nil, nil, nil, nil, fmt.Errorf("DUPLICATE E_RECORD FOUND: %s %s", key, normalized)
 			}
 			existingLookup[normalized] = ex
 		}
 		for _, de := range desiredRecords {
 			normalized := d.content(de)
 			if desiredLookup[normalized] != nil {
-				panic(fmt.Sprintf("DUPLICATE D_RECORD FOUND: %s %s", key, normalized))
+				return nil, nil, nil, nil, fmt.Errorf("DUPLICATE D_RECORD FOUND: %s %s", key, normalized)
 			}
 			desiredLookup[normalized] = de
 		}
@@ -260,9 +260,12 @@ func CorrectionLess(c []*models.Correction, i, j int) bool {
 	return c[i].Msg < c[j].Msg
 }
 
-func (d *differ) ChangedGroups(existing []*models.RecordConfig) map[models.RecordKey][]string {
+func (d *differ) ChangedGroups(existing []*models.RecordConfig) (map[models.RecordKey][]string, error) {
 	changedKeys := map[models.RecordKey][]string{}
-	_, create, delete, modify := d.IncrementalDiff(existing)
+	_, create, delete, modify, err := d.IncrementalDiff(existing)
+	if err != nil {
+		return nil, err
+	}
 	for _, c := range create {
 		changedKeys[c.Desired.Key()] = append(changedKeys[c.Desired.Key()], c.String())
 	}
@@ -272,7 +275,7 @@ func (d *differ) ChangedGroups(existing []*models.RecordConfig) map[models.Recor
 	for _, m := range modify {
 		changedKeys[m.Desired.Key()] = append(changedKeys[m.Desired.Key()], m.String())
 	}
-	return changedKeys
+	return changedKeys, nil
 }
 
 // DebugKeyMapMap debug prints the results from ChangedGroups.

pkg/diff/diff_test.go

@@ -170,7 +170,10 @@ func checkLengthsFull(t *testing.T, existing, desired []*models.RecordConfig, un
 		IgnoredTargets: ignoredTargets,
 	}
 	d := New(dc, valFuncs...)
-	un, cre, del, mod = d.IncrementalDiff(existing)
+	un, cre, del, mod, err := d.IncrementalDiff(existing)
+	if err != nil {
+		panic(err)
+	}
 	if len(un) != unCount {
 		t.Errorf("Got %d unchanged records, but expected %d", len(un), unCount)
 	}

providers/activedir/domains.go

@@ -70,7 +70,10 @@ func (c *adProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Co
 	models.PostProcessRecords(foundRecords)
 
 	differ := diff.New(dc)
-	_, creates, dels, modifications := differ.IncrementalDiff(foundRecords)
+	_, creates, dels, modifications, err := differ.IncrementalDiff(foundRecords)
+	if err != nil {
+		return nil, err
+	}
 	// NOTE(tlim): This provider does not delete records.  If
 	// you need to delete a record, either delete it manually
 	// or see providers/activedir/doc.md for implementation tips.

providers/axfrddns/axfrddnsProvider.go

@@ -289,7 +289,10 @@ func (c *AxfrDdns) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Corr
 	models.PostProcessRecords(foundRecords)
 
 	differ := diff.New(dc)
-	_, create, del, mod := differ.IncrementalDiff(foundRecords)
+	_, create, del, mod, err := differ.IncrementalDiff(foundRecords)
+	if err != nil {
+		return nil, err
+	}
 
 	buf := &bytes.Buffer{}
 	// Print a list of changes. Generate an actual change that is the zone

providers/azuredns/azureDnsProvider.go

@@ -204,7 +204,10 @@ func (a *azureDNSProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mod
 	}
 
 	differ := diff.New(dc)
-	namesToUpdate := differ.ChangedGroups(existingRecords)
+	namesToUpdate, err := differ.ChangedGroups(existingRecords)
+	if err != nil {
+		return nil, err
+	}
 
 	if len(namesToUpdate) == 0 {
 		return nil, nil

providers/bind/bindProvider.go

@@ -212,7 +212,10 @@ func (c *Bind) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Correcti
 	models.PostProcessRecords(foundRecords)
 
 	differ := diff.New(dc)
-	_, create, del, mod := differ.IncrementalDiff(foundRecords)
+	_, create, del, mod, err := differ.IncrementalDiff(foundRecords)
+	if err != nil {
+		return nil, err
+	}
 
 	buf := &bytes.Buffer{}
 	// Print a list of changes. Generate an actual change that is the zone

providers/cloudflare/cloudflareProvider.go

@@ -190,7 +190,11 @@ func (c *CloudflareAPI) GetDomainCorrections(dc *models.DomainConfig) ([]*models
 	models.PostProcessRecords(records)
 
 	differ := diff.New(dc, getProxyMetadata)
-	_, create, del, mod := differ.IncrementalDiff(records)
+	_, create, del, mod, err := differ.IncrementalDiff(records)
+	if err != nil {
+		return nil, err
+	}
+
 	corrections := []*models.Correction{}
 
 	for _, d := range del {

providers/cloudns/cloudnsProvider.go

@@ -97,7 +97,10 @@ func (c *api) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Correctio
 	}
 
 	differ := diff.New(dc)
-	_, create, del, modify := differ.IncrementalDiff(existingRecords)
+	_, create, del, modify, err := differ.IncrementalDiff(existingRecords)
+	if err != nil {
+		return nil, err
+	}
 
 	var corrections []*models.Correction
 

providers/desec/desecProvider.go

@@ -153,7 +153,10 @@ func (c *api) GenerateDomainCorrections(dc *models.DomainConfig, existing models
 
 	// diff existing vs. current.
 	differ := diff.New(dc)
-	keysToUpdate := differ.ChangedGroups(existing)
+	keysToUpdate, err := differ.ChangedGroups(existing)
+	if err != nil {
+		return nil, err
+	}
 	if len(keysToUpdate) == 0 {
 		return nil, nil
 	}

providers/digitalocean/digitaloceanProvider.go

@@ -129,7 +129,10 @@ func (api *DoAPI) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Corre
 	models.PostProcessRecords(existingRecords)
 
 	differ := diff.New(dc)
-	_, create, delete, modify := differ.IncrementalDiff(existingRecords)
+	_, create, delete, modify, err := differ.IncrementalDiff(existingRecords)
+	if err != nil {
+		return nil, err
+	}
 
 	var corrections = []*models.Correction{}
 

providers/dnsimple/dnsimpleProvider.go

@@ -147,7 +147,10 @@ func (c *DnsimpleAPI) GetDomainCorrections(dc *models.DomainConfig) ([]*models.C
 	models.PostProcessRecords(actual)
 
 	differ := diff.New(dc)
-	_, create, del, modify := differ.IncrementalDiff(actual)
+	_, create, del, modify, err := differ.IncrementalDiff(actual)
+	if err != nil {
+		return nil, err
+	}
 
 	for _, del := range del {
 		rec := del.Existing.Original.(dnsimpleapi.ZoneRecord)

providers/exoscale/exoscaleProvider.go

@@ -118,7 +118,10 @@ func (c *exoscaleProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mod
 	models.PostProcessRecords(existingRecords)
 
 	differ := diff.New(dc)
-	_, create, delete, modify := differ.IncrementalDiff(existingRecords)
+	_, create, delete, modify, err := differ.IncrementalDiff(existingRecords)
+	if err != nil {
+		return nil, err
+	}
 
 	var corrections = []*models.Correction{}
 

providers/gandi_v5/gandi_v5Provider.go

@@ -196,7 +196,10 @@ func (client *api) GenerateDomainCorrections(dc *models.DomainConfig, existing m
 
 	// diff existing vs. current.
 	differ := diff.New(dc)
-	keysToUpdate := differ.ChangedGroups(existing)
+	keysToUpdate, err := differ.ChangedGroups(existing)
+	if err != nil {
+		return nil, err
+	}
 	if client.debug {
 		diff.DebugKeyMapMap("GenDC diff", keysToUpdate)
 	}

providers/gcloud/gcloudProvider.go

@@ -179,7 +179,11 @@ func (g *gcloud) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Correc
 
 	// first collect keys that have changed
 	differ := diff.New(dc)
-	_, create, delete, modify := differ.IncrementalDiff(existingRecords)
+	_, create, delete, modify, err := differ.IncrementalDiff(existingRecords)
+	if err != nil {
+		return nil, err
+	}
+
 	changedKeys := map[key]bool{}
 	desc := ""
 	for _, c := range create {

providers/hexonet/records.go

@@ -67,7 +67,11 @@ func (n *HXClient) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Corr
 	models.PostProcessRecords(actual)
 
 	differ := diff.New(dc)
-	_, create, del, mod := differ.IncrementalDiff(actual)
+	_, create, del, mod, err := differ.IncrementalDiff(actual)
+	if err != nil {
+		return nil, err
+	}
+
 	corrections := []*models.Correction{}
 
 	buf := &bytes.Buffer{}

providers/inwx/inwxProvider.go

@@ -217,7 +217,11 @@ func (api *InwxApi) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Cor
 	models.PostProcessRecords(foundRecords)
 
 	differ := diff.New(dc)
-	_, create, del, mod := differ.IncrementalDiff(foundRecords)
+	_, create, del, mod, err := differ.IncrementalDiff(foundRecords)
+    if err != nil {
+        return nil, err
+    }
+
 	corrections := []*models.Correction{}
 
 	for _, d := range create {

providers/linode/linodeProvider.go

@@ -163,7 +163,10 @@ func (api *LinodeAPI) GetDomainCorrections(dc *models.DomainConfig) ([]*models.C
 	}
 
 	differ := diff.New(dc)
-	_, create, del, modify := differ.IncrementalDiff(existingRecords)
+	_, create, del, modify, err := differ.IncrementalDiff(existingRecords)
+	if err != nil {
+		return nil, err
+	}
 
 	var corrections []*models.Correction
 

providers/namecheap/namecheapProvider.go

@@ -178,7 +178,10 @@ func (n *Namecheap) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Cor
 	models.PostProcessRecords(actual)
 
 	differ := diff.New(dc)
-	_, create, delete, modify := differ.IncrementalDiff(actual)
+	_, create, delete, modify, err := differ.IncrementalDiff(actual)
+	if err != nil {
+		return nil, err
+	}
 
 	// // because namecheap doesn't have selective create, delete, modify,
 	// // we bundle them all up to send at once.  We *do* want to see the

providers/namedotcom/records.go

@@ -55,7 +55,11 @@ func (n *NameCom) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Corre
 	models.PostProcessRecords(actual)
 
 	differ := diff.New(dc)
-	_, create, del, mod := differ.IncrementalDiff(actual)
+	_, create, del, mod, err := differ.IncrementalDiff(actual)
+	if err != nil {
+		return nil, err
+	}
+
 	corrections := []*models.Correction{}
 
 	for _, d := range del {

providers/netcup/netcupProvider.go

@@ -95,7 +95,10 @@ func (api *api) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Correct
 	// Normalize
 	models.PostProcessRecords(existingRecords)
 	differ := diff.New(dc)
-	_, create, del, modify := differ.IncrementalDiff(existingRecords)
+	_, create, del, modify, err := differ.IncrementalDiff(existingRecords)
+	if err != nil {
+		return nil, err
+	}
 
 	var corrections []*models.Correction
 

providers/ns1/ns1provider.go

@@ -77,7 +77,10 @@ func (n *nsone) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Correct
 	models.PostProcessRecords(found)
 
 	differ := diff.New(dc)
-	changedGroups := differ.ChangedGroups(found)
+	changedGroups, err := differ.ChangedGroups(found)
+	if err != nil {
+		return nil, err
+	}
 	corrections := []*models.Correction{}
 	// each name/type is given to the api as a unit.
 	for k, descs := range changedGroups {

providers/octodns/octodnsProvider.go

@@ -125,7 +125,10 @@ func (c *Provider) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Corr
 	models.PostProcessRecords(foundRecords)
 
 	differ := diff.New(dc)
-	_, create, del, mod := differ.IncrementalDiff(foundRecords)
+	_, create, del, mod, err := differ.IncrementalDiff(foundRecords)
+	if err != nil {
+		return nil, err
+	}
 
 	buf := &bytes.Buffer{}
 	// Print a list of changes. Generate an actual change that is the zone

providers/ovh/ovhProvider.go

@@ -114,7 +114,10 @@ func (c *ovhProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*models.C
 	models.PostProcessRecords(actual)
 
 	differ := diff.New(dc)
-	_, create, delete, modify := differ.IncrementalDiff(actual)
+	_, create, delete, modify, err := differ.IncrementalDiff(actual)
+	if err != nil {
+		return nil, err
+	}
 
 	corrections := []*models.Correction{}
 

providers/powerdns/powerdnsProvider.go

@@ -152,7 +152,10 @@ func (api *PowerDNS) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Co
 	models.PostProcessRecords(curRecords)
 
 	// create record diff by group
-	keysToUpdate := (diff.New(dc)).ChangedGroups(curRecords)
+	keysToUpdate, err := (diff.New(dc)).ChangedGroups(curRecords)
+	if err != nil {
+		return nil, err
+	}
 	desiredRecords := dc.Records.GroupedByKey()
 
 	// create corrections by group

providers/route53/route53Provider.go

@@ -230,7 +230,10 @@ func (r *route53Provider) GetDomainCorrections(dc *models.DomainConfig) ([]*mode
 
 	// diff
 	differ := diff.New(dc, getAliasMap)
-	namesToUpdate := differ.ChangedGroups(existingRecords)
+	namesToUpdate, err := differ.ChangedGroups(existingRecords)
+	if err != nil {
+		return nil, err
+	}
 
 	if len(namesToUpdate) == 0 {
 		return nil, nil

providers/softlayer/softlayerProvider.go

@@ -76,7 +76,10 @@ func (s *SoftLayer) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Cor
 		return nil, err
 	}
 
-	_, create, delete, modify := diff.New(dc).IncrementalDiff(actual)
+	_, create, delete, modify, err := diff.New(dc).IncrementalDiff(actual)
+	if err != nil {
+		return nil, err
+	}
 
 	for _, del := range delete {
 		existing := del.Existing.Original.(datatypes.Dns_Domain_ResourceRecord)

providers/vultr/vultrProvider.go

@@ -98,7 +98,10 @@ func (api *Provider) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Co
 	models.PostProcessRecords(curRecords)
 
 	differ := diff.New(dc)
-	_, create, delete, modify := differ.IncrementalDiff(curRecords)
+	_, create, delete, modify, err := differ.IncrementalDiff(curRecords)
+	if err != nil {
+		return nil, err
+	}
 
 	var corrections []*models.Correction