From 2586e2b611c0054abbc25c15fe64a25d0ee7b51f Mon Sep 17 00:00:00 2001 From: Tom Limoncelli <tlimoncelli@stackoverflow.com> Date: Tue, 28 Feb 2023 01:25:09 -0500 Subject: [PATCH] CORE: Clean up diff2 code in prep for production (#2104) --- integrationTest/integration_test.go | 3 +- pkg/diff/diff2compat.go | 17 +- pkg/diff2/analyze.go | 125 +++-------- pkg/diff2/analyze_test.go | 2 +- pkg/diff2/compareconfig.go | 15 +- pkg/diff2/diff2.go | 15 +- pkg/diff2/groupsort_test.go | 1 + pkg/diff2/handsoff.go | 20 +- pkg/diff2/handsoff_test.go | 1 - pkg/diff2/notes.txt | 198 ------------------ .../akamaiedgedns/akamaiEdgeDnsProvider.go | 1 - providers/cloudflare/cloudflareProvider.go | 15 +- providers/cscglobal/dns.go | 32 +-- .../digitalocean/digitaloceanProvider.go | 17 +- providers/dnsimple/dnsimpleProvider.go | 4 - providers/gandiv5/gandi_v5Provider.go | 2 + providers/gcloud/gcloudProvider.go | 14 +- providers/namecheap/namecheapProvider.go | 6 +- providers/namedotcom/records.go | 9 +- providers/packetframe/packetframeProvider.go | 4 - providers/rwth/dns.go | 4 - 21 files changed, 93 insertions(+), 412 deletions(-) delete mode 100644 pkg/diff2/notes.txt diff --git a/integrationTest/integration_test.go b/integrationTest/integration_test.go index 59b34fdc3..b66dba2c7 100644 --- a/integrationTest/integration_test.go +++ b/integrationTest/integration_test.go @@ -326,7 +326,7 @@ func TestDualProviders(t *testing.T) { t.Fatal(err) } for i, c := range cs { - t.Logf("#%d: %s", i+1, c.Msg) + t.Logf("#%d:\n%s", i+1, c.Msg) if err = c.F(); err != nil { t.Fatal(err) } @@ -913,6 +913,7 @@ func makeTests(t *testing.T) []*TestGroup { not( "AZURE_DNS", // Removed because it is too slow "CLOUDFLAREAPI", // Infinite pagesize but due to slow speed, skipping. + "DIGITALOCEAN", // No paging. Why bother? "CSCGLOBAL", // Doesn't page. Works fine. Due to the slow API we skip. "GANDI_V5", // Their API is so damn slow. We'll add it back as needed. "MSDNS", // No paging done. No need to test. diff --git a/pkg/diff/diff2compat.go b/pkg/diff/diff2compat.go index fff0daa47..613bc8745 100644 --- a/pkg/diff/diff2compat.go +++ b/pkg/diff/diff2compat.go @@ -50,12 +50,7 @@ type differCompat struct { // - The NewCompat() feature `extraValues` is not supported. That // parameter must be set to nil. If you use that feature, consider // one of the pkg/diff2/By*() functions. -func (d *differCompat) IncrementalDiff(existing []*models.RecordConfig) (unchanged, create, toDelete, modify Changeset, err error) { - unchanged = Changeset{} - create = Changeset{} - toDelete = Changeset{} - modify = Changeset{} - +func (d *differCompat) IncrementalDiff(existing []*models.RecordConfig) (unchanged, toCreate, toDelete, toModify Changeset, err error) { instructions, err := diff2.ByRecord(existing, d.dc, nil) if err != nil { return nil, nil, nil, nil, err @@ -70,11 +65,11 @@ func (d *differCompat) IncrementalDiff(existing []*models.RecordConfig) (unchang fmt.Println(inst.MsgsJoined) case diff2.CREATE: cor.Desired = inst.New[0] - create = append(create, cor) + toCreate = append(toCreate, cor) case diff2.CHANGE: cor.Existing = inst.Old[0] cor.Desired = inst.New[0] - modify = append(modify, cor) + toModify = append(toModify, cor) case diff2.DELETE: cor.Existing = inst.Old[0] toDelete = append(toDelete, cor) @@ -89,17 +84,17 @@ func (d *differCompat) IncrementalDiff(existing []*models.RecordConfig) (unchang // ChangedGroups provides the same results as IncrementalDiff but grouped by key. func (d *differCompat) ChangedGroups(existing []*models.RecordConfig) (map[models.RecordKey][]string, error) { changedKeys := map[models.RecordKey][]string{} - _, create, toDelete, modify, err := d.IncrementalDiff(existing) + _, toCreate, toDelete, toModify, err := d.IncrementalDiff(existing) if err != nil { return nil, err } - for _, c := range create { + for _, c := range toCreate { changedKeys[c.Desired.Key()] = append(changedKeys[c.Desired.Key()], c.String()) } for _, d := range toDelete { changedKeys[d.Existing.Key()] = append(changedKeys[d.Existing.Key()], d.String()) } - for _, m := range modify { + for _, m := range toModify { changedKeys[m.Desired.Key()] = append(changedKeys[m.Desired.Key()], m.String()) } return changedKeys, nil diff --git a/pkg/diff2/analyze.go b/pkg/diff2/analyze.go index ea2d20ae6..678a2e285 100644 --- a/pkg/diff2/analyze.go +++ b/pkg/diff2/analyze.go @@ -20,18 +20,14 @@ func analyzeByRecordSet(cc *CompareConfig) ChangeList { dts := rt.desiredTargets msgs := genmsgs(ets, dts) if len(msgs) == 0 { // No differences? - //fmt.Printf("DEBUG: done. Records are the same\n") // The records at this rset are the same. No work to be done. continue } if len(ets) == 0 { // Create a new label. - //fmt.Printf("DEBUG: add\n") instructions = append(instructions, mkAdd(lc.label, rt.rType, msgs, rt.desiredRecs)) } else if len(dts) == 0 { // Delete that label and all its records. - //fmt.Printf("DEBUG: delete\n") instructions = append(instructions, mkDelete(lc.label, rt.rType, msgs, rt.existingRecs)) } else { // Change the records at that label - //fmt.Printf("DEBUG: change\n") instructions = append(instructions, mkChange(lc.label, rt.rType, msgs, rt.existingRecs, rt.desiredRecs)) } } @@ -41,22 +37,21 @@ func analyzeByRecordSet(cc *CompareConfig) ChangeList { func analyzeByLabel(cc *CompareConfig) ChangeList { var instructions ChangeList - //fmt.Printf("DEBUG: START: analyzeByLabel\n") - // Accumulate if there are any changes and collect the info needed to generate instructions. - for i, lc := range cc.ldata { - //fmt.Printf("DEBUG: START LABEL = %q\n", lc.label) + // Accumulate any changes and collect the info needed to generate instructions. + for _, lc := range cc.ldata { + // for each type at that label... label := lc.label var accMsgs []string var accExisting models.Records var accDesired models.Records msgsByKey := map[models.RecordKey][]string{} for _, rt := range lc.tdata { - //fmt.Printf("DEBUG: START RTYPE = %q\n", rt.rType) + // for each type at that label... ets := rt.existingTargets dts := rt.desiredTargets msgs := genmsgs(ets, dts) - msgsByKey[models.RecordKey{NameFQDN: label, Type: rt.rType}] = msgs - //fmt.Printf("DEBUG: appending msgs=%v\n", msgs) + k := models.RecordKey{NameFQDN: label, Type: rt.rType} + msgsByKey[k] = msgs accMsgs = append(accMsgs, msgs...) // Accumulate the messages accExisting = append(accExisting, rt.existingRecs...) // Accumulate records existing at this label. accDesired = append(accDesired, rt.desiredRecs...) // Accumulate records desired at this label. @@ -68,23 +63,14 @@ func analyzeByLabel(cc *CompareConfig) ChangeList { // Based on that info, we can generate the instructions. if len(accMsgs) == 0 { // Nothing changed. - //fmt.Printf("DEBUG: analyzeByLabel: %02d: no change\n", i) } else if len(accDesired) == 0 { // No new records at the label? This must be a delete. - //fmt.Printf("DEBUG: analyzeByLabel: %02d: delete\n", i) instructions = append(instructions, mkDelete(label, "", accMsgs, accExisting)) } else if len(accExisting) == 0 { // No old records at the label? This must be a change. - //fmt.Printf("DEBUG: analyzeByLabel: %02d: create\n", i) - //fmt.Printf("DEBUG: analyzeByLabel mkAdd msgs=%d\n", len(accMsgs)) - instructions = append(instructions, mkAddByLabel(label, "", accMsgs, accDesired)) + c := mkAdd(label, "", accMsgs, accDesired) + c.MsgsByKey = msgsByKey + instructions = append(instructions, c) } else { // If we get here, it must be a change. - _ = i - // fmt.Printf("DEBUG: analyzeByLabel: %02d: change %d{%v} %d{%v} msgs=%v\n", i, - // len(accExisting), accExisting, - // len(accDesired), accDesired, - // accMsgs, - // ) - //fmt.Printf("DEBUG: analyzeByLabel mkchange msgs=%d\n", len(accMsgs)) - instructions = append(instructions, mkChangeByLabel(label, "", accMsgs, accExisting, accDesired, msgsByKey)) + instructions = append(instructions, mkChange(label, "", accMsgs, accExisting, accDesired)) } } @@ -92,38 +78,23 @@ func analyzeByLabel(cc *CompareConfig) ChangeList { } func analyzeByRecord(cc *CompareConfig) ChangeList { - //fmt.Printf("DEBUG: analyzeByRecord: cc=%v\n", cc) var instructions ChangeList // For each label, for each type at that label, see if there are any changes. for _, lc := range cc.ldata { - //fmt.Printf("DEBUG: analyzeByRecord: next lc=%v\n", lc) for _, rt := range lc.tdata { ets := rt.existingTargets dts := rt.desiredTargets cs := diffTargets(ets, dts) - //fmt.Printf("DEBUG: analyzeByRecord: cs=%v\n", cs) instructions = append(instructions, cs...) } } return instructions } -// NB(tlim): there is no analyzeByZone. ByZone calls anayzeByRecords(). +// FYI: there is no analyzeByZone. diff2.ByZone() calls analyzeByRecords(). -func mkAdd(l string, t string, msgs []string, recs models.Records) Change { - //fmt.Printf("DEBUG mkAdd called: (%v, %v, %v, %v)\n", l, t, msgs, recs) - c := Change{Type: CREATE, Msgs: msgs, MsgsJoined: strings.Join(msgs, "\n")} - c.Key.NameFQDN = l - c.Key.Type = t - c.New = recs - return c -} - -// TODO(tlim): Clean these up. Some of them are exact duplicates! - -func mkAddByLabel(l string, t string, msgs []string, newRecs models.Records) Change { - //fmt.Printf("DEBUG mkAddByLabel called: (%v, %v, %v, %v)\n", l, t, msgs, newRecs) +func mkAdd(l string, t string, msgs []string, newRecs models.Records) Change { c := Change{Type: CREATE, Msgs: msgs, MsgsJoined: strings.Join(msgs, "\n")} c.Key.NameFQDN = l c.Key.Type = t @@ -132,7 +103,6 @@ func mkAddByLabel(l string, t string, msgs []string, newRecs models.Records) Cha } func mkChange(l string, t string, msgs []string, oldRecs, newRecs models.Records) Change { - //fmt.Printf("DEBUG mkChange called: (%v, %v, %v, %v, %v)\n", l, t, msgs, oldRecs, newRecs) c := Change{Type: CHANGE, Msgs: msgs, MsgsJoined: strings.Join(msgs, "\n")} c.Key.NameFQDN = l c.Key.Type = t @@ -141,49 +111,21 @@ func mkChange(l string, t string, msgs []string, oldRecs, newRecs models.Records return c } -func mkChangeByLabel(l string, t string, msgs []string, oldRecs, newRecs models.Records, msgsByKey map[models.RecordKey][]string) Change { - //fmt.Printf("DEBUG mkChangeLabel called: (%v, %v, %v, %v, %v, %v)\n", l, t, msgs, oldRecs, newRecs, msgsByKey) - c := Change{Type: CHANGE, Msgs: msgs, MsgsJoined: strings.Join(msgs, "\n")} - c.Key.NameFQDN = l - c.Key.Type = t - c.Old = oldRecs - c.New = newRecs - c.MsgsByKey = msgsByKey - return c -} - func mkDelete(l string, t string, msgs []string, oldRecs models.Records) Change { - //fmt.Printf("DEBUG mkDelete called: (%v, %v, %v, %v)\n", l, t, msgs, oldRecs) - if len(msgs) == 0 { - panic("mkDelete with no msg") - } c := Change{Type: DELETE, Msgs: msgs, MsgsJoined: strings.Join(msgs, "\n")} c.Key.NameFQDN = l c.Key.Type = t c.Old = oldRecs return c } -func mkDeleteByRecord(l string, t string, msgs []string, rec *models.RecordConfig) Change { - //fmt.Printf("DEBUG mkDeleteREc called: (%v, %v, %v, %v)\n", l, t, msgs, rec) - c := Change{Type: DELETE, Msgs: msgs, MsgsJoined: strings.Join(msgs, "\n")} - c.Key.NameFQDN = l - c.Key.Type = t - c.Old = models.Records{rec} - return c -} func removeCommon(existing, desired []targetConfig) ([]targetConfig, []targetConfig) { - // NB(tlim): We could probably make this faster. Some ideas: - // * pre-allocate newexisting/newdesired and assign to indexed elements instead of appending. - // * iterate backwards (len(d) to 0) and delete items that are the same. - // On the other hand, this function typically receives lists of 1-3 elements - // and any optimization is probably fruitless. - - // Sort to make comparisons easier + // Sort by comparableFull. sort.Slice(existing, func(i, j int) bool { return existing[i].comparableFull < existing[j].comparableFull }) sort.Slice(desired, func(i, j int) bool { return desired[i].comparableFull < desired[j].comparableFull }) + // Build maps required by filterBy eKeys := map[string]*targetConfig{} for _, v := range existing { v := v @@ -201,13 +143,12 @@ func removeCommon(existing, desired []targetConfig) ([]targetConfig, []targetCon // findTTLChanges finds the records that ONLY change their TTL. For those, generate a Change. // Remove such items from the list. func findTTLChanges(existing, desired []targetConfig) ([]targetConfig, []targetConfig, ChangeList) { - //fmt.Printf("DEBUG: findTTLChanges(%v,\n%v)\n", existing, desired) if (len(existing) == 0) || (len(desired) == 0) { return existing, desired, nil } - // Sort to make comparisons easier + // Sort by comparableNoTTL sort.Slice(existing, func(i, j int) bool { return existing[i].comparableNoTTL < existing[j].comparableNoTTL }) sort.Slice(desired, func(i, j int) bool { return desired[i].comparableNoTTL < desired[j].comparableNoTTL }) @@ -222,8 +163,9 @@ func findTTLChanges(existing, desired []targetConfig) ([]targetConfig, []targetC dcomp := desired[di].comparableNoTTL if ecomp == dcomp && er.TTL == dr.TTL { - fmt.Printf("DEBUG: ecomp=%q dcomp=%q er.TTL=%v dr.TTL=%v\n", ecomp, dcomp, er.TTL, dr.TTL) - panic("Should not happen. There should be some difference!") + panic(fmt.Sprintf( + "Should not happen. There should be some difference! ecomp=%q dcomp=%q er.TTL=%v dr.TTL=%v\n", + ecomp, dcomp, er.TTL, dr.TTL)) } if ecomp == dcomp && er.TTL != dr.TTL { @@ -247,7 +189,6 @@ func findTTLChanges(existing, desired []targetConfig) ([]targetConfig, []targetC // Any remainder goes to the *Diff result: if ei < len(existing) { - //fmt.Printf("DEBUG: append e len()=%d\n", ei) existDiff = append(existDiff, existing[ei:]...) } if di < len(desired) { @@ -259,14 +200,9 @@ func findTTLChanges(existing, desired []targetConfig) ([]targetConfig, []targetC // Return s but remove any items that can be found in m. func filterBy(s []targetConfig, m map[string]*targetConfig) []targetConfig { - // fmt.Printf("DEBUG: filterBy called with %v\n", s) - // for k := range m { - // fmt.Printf("DEBUG: map %q\n", k) - // } i := 0 // output index for _, x := range s { if _, ok := m[x.comparableFull]; !ok { - //fmt.Printf("DEBUG: comp %q NO\n", x.comparable) // copy and increment index s[i] = x i++ @@ -278,7 +214,6 @@ func filterBy(s []targetConfig, m map[string]*targetConfig) []targetConfig { // s[j] = nil // } s = s[:i] - // fmt.Printf("DEBUG: filterBy returns %v\n", s) return s } @@ -298,22 +233,16 @@ func humanDiff(a, b targetConfig) string { } func diffTargets(existing, desired []targetConfig) ChangeList { - //fmt.Printf("DEBUG: diffTargets called with len(e)=%d len(d)=%d\n", len(existing), len(desired)) // Nothing to do? if len(existing) == 0 && len(desired) == 0 { - //fmt.Printf("DEBUG: diffTargets: nothing to do\n") return nil } var instructions ChangeList - //fmt.Printf("DEBUG: diffTargets BEFORE existing=%+v\n", existing) - //fmt.Printf("DEBUG: diffTargets BEFORE desired=%+v\n", desired) // remove the exact matches. existing, desired = removeCommon(existing, desired) - //fmt.Printf("DEBUG: diffTargets AFTER existing=%+v\n", existing) - //fmt.Printf("DEBUG: diffTargets AFTER desired=%+v\n", desired) // At this point the exact matches are removed. However there may be // records that have the same GetTargetCombined() but different @@ -322,37 +251,32 @@ func diffTargets(existing, desired []targetConfig) ChangeList { existing, desired, newChanges := findTTLChanges(existing, desired) instructions = append(instructions, newChanges...) - // Sort to make comparisons easier + // Sort by comparableFull sort.Slice(existing, func(i, j int) bool { return existing[i].comparableFull < existing[j].comparableFull }) sort.Slice(desired, func(i, j int) bool { return desired[i].comparableFull < desired[j].comparableFull }) // the remaining chunks are changes (regardless of TTL) mi := min(len(existing), len(desired)) - for i := 0; i < mi; i++ { - //fmt.Println(i, "CHANGE") er := existing[i].rec dr := desired[i].rec m := color.YellowString("± MODIFY %s %s %s", dr.NameFQDN, dr.Type, humanDiff(existing[i], desired[i])) - instructions = append(instructions, mkChange(dr.NameFQDN, dr.Type, []string{m}, - models.Records{er}, - models.Records{dr}, - )) + instructions = append(instructions, + mkChange(dr.NameFQDN, dr.Type, []string{m}, models.Records{er}, models.Records{dr}), + ) } // any left-over existing are deletes for i := mi; i < len(existing); i++ { - //fmt.Println(i, "DEL") er := existing[i].rec m := color.RedString("- DELETE %s %s %s", er.NameFQDN, er.Type, existing[i].comparableFull) - instructions = append(instructions, mkDeleteByRecord(er.NameFQDN, er.Type, []string{m}, er)) + instructions = append(instructions, mkDelete(er.NameFQDN, er.Type, []string{m}, models.Records{er})) } // any left-over desired are creates for i := mi; i < len(desired); i++ { - //fmt.Println(i, "CREATE") dr := desired[i].rec m := color.GreenString("+ CREATE %s %s %s", dr.NameFQDN, dr.Type, desired[i].comparableFull) instructions = append(instructions, mkAdd(dr.NameFQDN, dr.Type, []string{m}, models.Records{dr})) @@ -362,8 +286,7 @@ func diffTargets(existing, desired []targetConfig) ChangeList { } func genmsgs(existing, desired []targetConfig) []string { - cl := diffTargets(existing, desired) - return justMsgs(cl) + return justMsgs(diffTargets(existing, desired)) } func justMsgs(cl ChangeList) []string { diff --git a/pkg/diff2/analyze_test.go b/pkg/diff2/analyze_test.go index 190fa4a7b..eae860782 100644 --- a/pkg/diff2/analyze_test.go +++ b/pkg/diff2/analyze_test.go @@ -398,7 +398,7 @@ ChangeList: len=12 compareCL(t, "analyzeByRecord", tt.name, "Rec", cl, tt.wantChangeRec) }) - // NB(tlim): There is no analyzeByZone(). ByZone() uses analyzeByRecord(). + // NB(tlim): There is no analyzeByZone(). diff2.ByZone() uses analyzeByRecord(). } } diff --git a/pkg/diff2/compareconfig.go b/pkg/diff2/compareconfig.go index 22249385b..a14a773f3 100644 --- a/pkg/diff2/compareconfig.go +++ b/pkg/diff2/compareconfig.go @@ -82,7 +82,7 @@ type rTypeConfig struct { } type targetConfig struct { - comparableFull string // A string that can be used to compare two rec's for equality. + comparableFull string // A string that can be used to compare two rec's for exact equality. comparableNoTTL string // A string that can be used to compare two rec's for equality, ignoring the TTL rec *models.RecordConfig // The RecordConfig itself. @@ -110,10 +110,6 @@ func NewCompareConfig(origin string, existing, desired models.Records, compFn Co func (cc *CompareConfig) VerifyCNAMEAssertions() { - // In theory these assertions do not need to be tested as they test - // something that can not happen. In my I've proved this to be - // true. That said, a little paranoia is healthy. - // According to the RFCs if a label has a CNAME, it can not have any other // records at that label... even other CNAMEs. Therefore, we need to be // careful with changes at a label that involve a CNAME. @@ -131,7 +127,7 @@ func (cc *CompareConfig) VerifyCNAMEAssertions() { // there is already an A record at that label. // // To assure that DNS providers don't have to think about this, we order - // the tdata items so that we generate the instructions in the best order. + // the tdata items so that we generate the instructions in the correct order. // In other words: // If there is a CNAME in existing, it should be in front. // If there is a CNAME in desired, it should be at the end. @@ -154,15 +150,12 @@ func (cc *CompareConfig) VerifyCNAMEAssertions() { } if len(td.existingTargets) != 0 { - //fmt.Printf("DEBUG: cname in existing: index=%d\n", j) if j != 0 { panic("should not happen: (CNAME not in first position)") } } if len(td.desiredTargets) != 0 { - //fmt.Printf("DEBUG: cname in desired: index=%d\n", j) - //fmt.Printf("DEBUG: highest: index=%d\n", j) if j != highest(ld.tdata) { panic("should not happen: (CNAME not in last position)") } @@ -273,7 +266,6 @@ func (cc *CompareConfig) addRecords(recs models.Records, storeInExisting bool) { // Now it is safe to add/modify the records. - //fmt.Printf("BEFORE E/D: %v/%v\n", len(td.existingRecs), len(td.desiredRecs)) if storeInExisting { cc.ldata[labelIdx].tdata[rtIdx].existingRecs = append(cc.ldata[labelIdx].tdata[rtIdx].existingRecs, rec) cc.ldata[labelIdx].tdata[rtIdx].existingTargets = append(cc.ldata[labelIdx].tdata[rtIdx].existingTargets, @@ -283,8 +275,5 @@ func (cc *CompareConfig) addRecords(recs models.Records, storeInExisting bool) { cc.ldata[labelIdx].tdata[rtIdx].desiredTargets = append(cc.ldata[labelIdx].tdata[rtIdx].desiredTargets, targetConfig{comparableNoTTL: compNoTTL, comparableFull: compFull, rec: rec}) } - //fmt.Printf("AFTER L: %v\n", len(cc.ldata)) - //fmt.Printf("AFTER E/D: %v/%v\n", len(td.existingRecs), len(td.desiredRecs)) - //fmt.Printf("\n") } } diff --git a/pkg/diff2/diff2.go b/pkg/diff2/diff2.go index c5bc17826..a26ccf31b 100644 --- a/pkg/diff2/diff2.go +++ b/pkg/diff2/diff2.go @@ -20,7 +20,7 @@ const ( CREATE // Create a record/recordset/label where none existed before. CHANGE // Change existing record/recordset/label DELETE // Delete existing record/recordset/label - REPORT // No change, but boy do I have something to say! + REPORT // No change, but I have something to say! ) type ChangeList []Change @@ -83,7 +83,6 @@ General instructions: return corrections, nil } - */ // ByRecordSet takes two lists of records (existing and desired) and @@ -95,7 +94,7 @@ General instructions: // record, if A records are added, changed, or removed, the API takes // www.example.com, A, and a list of all the desired IP addresses. // -// Examples include: +// Examples include: AZURE_DNS, GCORE, NS1, ROUTE53 func ByRecordSet(existing models.Records, dc *models.DomainConfig, compFunc ComparableFunc) (ChangeList, error) { return byHelper(analyzeByRecordSet, existing, dc, compFunc) } @@ -107,7 +106,7 @@ func ByRecordSet(existing models.Records, dc *models.DomainConfig, compFunc Comp // time. That is, updates are done by sending a list of DNS records // to be served at a particular label, or the label itself is deleted. // -// Examples include: +// Examples include: GANDI_V5 func ByLabel(existing models.Records, dc *models.DomainConfig, compFunc ComparableFunc) (ChangeList, error) { return byHelper(analyzeByLabel, existing, dc, compFunc) } @@ -123,7 +122,7 @@ func ByLabel(existing models.Records, dc *models.DomainConfig, compFunc Comparab // A change always has exactly 1 old and 1 new: .Old[0] and .New[0] // A delete always has exactly 1 old: .Old[0] // -// Examples include: INWX +// Examples include: CLOUDFLAREAPI, HEDNS, INWX, MSDNS, OVH, PORKBUN, VULTR func ByRecord(existing models.Records, dc *models.DomainConfig, compFunc ComparableFunc) (ChangeList, error) { return byHelper(analyzeByRecord, existing, dc, compFunc) } @@ -149,7 +148,7 @@ func ByRecord(existing models.Records, dc *models.DomainConfig, compFunc Compara // // (dc.Records are the new records for the zone). // } // -// Example providers include: BIND +// Example providers include: BIND, AUTODNS func ByZone(existing models.Records, dc *models.DomainConfig, compFunc ComparableFunc) ([]string, bool, error) { if len(existing) == 0 { @@ -157,7 +156,7 @@ func ByZone(existing models.Records, dc *models.DomainConfig, compFunc Comparabl return nil, true, nil } - // Only return the messages. + // Only return the messages. The caller has the list of records needed to build the new zone. instructions, err := byHelper(analyzeByRecord, existing, dc, compFunc) return justMsgs(instructions), len(instructions) != 0, err } @@ -199,7 +198,7 @@ func byHelper(fn func(cc *CompareConfig) ChangeList, existing models.Records, dc return instructions, nil } -// Stringify the datastructures for easier debugging +// Stringify the datastructures (for debugging) func (c Change) String() string { var buf bytes.Buffer diff --git a/pkg/diff2/groupsort_test.go b/pkg/diff2/groupsort_test.go index 1017a1184..bd8617e64 100644 --- a/pkg/diff2/groupsort_test.go +++ b/pkg/diff2/groupsort_test.go @@ -25,6 +25,7 @@ func makeRecSet(recs ...*models.RecordConfig) *recset { result.Recs = append(result.Recs, recs...) return &result } + func Test_groupbyRSet(t *testing.T) { wwwa1 := makeRec("www", "A", "1.1.1.1") diff --git a/pkg/diff2/handsoff.go b/pkg/diff2/handsoff.go index ebe1047d3..57ef2b585 100644 --- a/pkg/diff2/handsoff.go +++ b/pkg/diff2/handsoff.go @@ -16,12 +16,11 @@ import ( # How do NO_PURGE, IGNORE_*, ENSURE_ABSENT and friends work? - ## Terminology: * "existing" refers to the records downloaded from the provider via the API. * "desired" refers to the records generated from dnsconfig.js. -* "absences" refers to a list of records tagged with ASSURE_ABSENT. +* "absences" refers to a list of records tagged with ENSURE_ABSENT. ## What are the features? @@ -39,9 +38,9 @@ and 1 way to make exceptions. * IGNORE_TARGET(foo) is the same as UNMANAGED("*", "*", foo) * FYI: You CAN have a label with two A records, one controlled by DNSControl and one controlled by an external system. DNSControl would - need to have an UNMANAGED() statement with a targetglob that matches + need to have an UNMANAGED() statement with a targetglob that matches the external system's target values. -* ASSURE_ABSENT: Override NO_PURGE for specific records. i.e. delete them even +* ENSURE_ABSENT: Override NO_PURGE for specific records. i.e. delete them even though NO_PURGE is enabled. * If any of these records are in desired (matched on label:rtype:target), remove them. This takes priority over @@ -53,8 +52,9 @@ The fundamental premise is "if you don't want it deleted, copy it to the 'desired' list." So, for example, if you want to IGNORE_NAME("www"), then you find any records with the label "www" in "existing" and copy them to "desired". As a result, the diff2 algorithm won't delete them because they are desired! +(Of course "desired" can't have duplicate records. Check before you add.) -This is different than in the old system (pkg/diff) which would generate the +This is different than in the old implementation (pkg/diff) which would generate the diff but but then do a bunch of checking to see if the record was one that shouldn't be deleted. Or, in the case of NO_PURGE, would simply not do the deletions. This was complex because there were many edge cases to deal with. @@ -75,7 +75,7 @@ Here is how we intend to implement these features: NO_PURGE + ENSURE_ABSENT is implemented as: * Take the list of existing records. If any do not appear in desired, add them - to desired UNLESS they appear in absences. + to desired UNLESS they appear in absences. (Yes, that's complex!) * "appear in desired" is done by matching on label:type. * "appear in absences" is done by matching on label:type:target. @@ -154,7 +154,7 @@ func processIgnoreAndNoPurge(domain string, existing, desired, absences models.R absentDB := models.NewRecordDBFromRecords(absences, domain) compileUnmanagedConfigs(unmanagedConfigs) for _, rec := range existing { - if matchAll(unmanagedConfigs, rec) { + if matchAny(unmanagedConfigs, rec) { ignorable = append(ignorable, rec) } else { if noPurge { @@ -176,7 +176,7 @@ func processIgnoreAndNoPurge(domain string, existing, desired, absences models.R func findConflicts(uconfigs []*models.UnmanagedConfig, recs models.Records) models.Records { var conflicts models.Records for _, rec := range recs { - if matchAll(uconfigs, rec) { + if matchAny(uconfigs, rec) { conflicts = append(conflicts, rec) } } @@ -219,8 +219,8 @@ func compileUnmanagedConfigs(configs []*models.UnmanagedConfig) error { return nil } -// matchAll returns true if rec matches any of the uconfigs. -func matchAll(uconfigs []*models.UnmanagedConfig, rec *models.RecordConfig) bool { +// matchAny returns true if rec matches any of the uconfigs. +func matchAny(uconfigs []*models.UnmanagedConfig, rec *models.RecordConfig) bool { for _, uc := range uconfigs { if matchLabel(uc.LabelGlob, rec.GetLabel()) && matchType(uc.RTypeMap, rec.Type) && diff --git a/pkg/diff2/handsoff_test.go b/pkg/diff2/handsoff_test.go index 6d5b0fd19..0ec01eab6 100644 --- a/pkg/diff2/handsoff_test.go +++ b/pkg/diff2/handsoff_test.go @@ -49,7 +49,6 @@ func handsoffHelper(t *testing.T, existingZone, desiredJs string, noPurge bool, if err != nil { panic(err) } - //fmt.Printf("DEBUG: existing=%s\n", showRecs(existing)) dnsconfig, err := js.ExecuteJavascriptString([]byte(desiredJs), false, nil) if err != nil { diff --git a/pkg/diff2/notes.txt b/pkg/diff2/notes.txt deleted file mode 100644 index d0bd536b5..000000000 --- a/pkg/diff2/notes.txt +++ /dev/null @@ -1,198 +0,0 @@ -EXISTING: - laba A 1.2.3.4 [0] - laba MX 10 laba [1] - labc CNAME laba [2] - labe A 10.10.10.15 [3] - labe A 10.10.10.16 [4] - labe A 10.10.10.17 [5] - labe A 10.10.10.18 [6] - labg NS 10.10.10.15 [7] - labg NS 10.10.10.16 [8] - labg NS 10.10.10.17 [9] - labg NS 10.10.10.18 [10] - labh CNAME labd [11] - -DESIRED: - laba A 1.2.3.4 [0'] - laba A 1.2.3.5 [1'] - laba MX 20 labb [2'] - labe A 10.10.10.95 [3'] - labe A 10.10.10.96 [4'] - labe A 10.10.10.97 [5'] - labe A 10.10.10.98 [6'] - labf TXT "foo" [7'] - labg NS 10.10.10.10 [8'] - labg NS 10.10.10.15 [9'] - labg NS 10.10.10.16 [10'] - labg NS 10.10.10.97 [11'] - labh A 1.2.3.4 [12'] - -ByRRSet: - [] laba:A CHANGE NewSet: { [0], [1'] } (ByRecords needs: Old [0] ) - [] laba:MX CHANGE NewSet: { [2'] } (ByLabel needs: Old: [2]) - [] labc:CNAME DELETE Old: { [2 ] } - [] labe:A CHANGE NewSet: { [3'], [4'], [5'], [6'] } - [] labf:TXT CHANGE NewSet: { [7'] } - [] labg:NS CHANGE NewSet: { [7] [8] [8'] [11'] } - [] labh:CNAME DELETE Old { [11] } - [] labh:A CREATE NewSet: { [12'] } - -ByRecord: -CREATE [1'] -CHANGE [1] [2'] -DELETE [2] -CHANGE [3] [3'] -CHANGE [4] [4'] -CHANGE [5] [5'] -CHANGE [6] [6'] -CREATE [7'] -CREATE [8'] -CHANGE [10] [11'] -DELETE [11] -CREATE [12'] - - -ByLabel: (take ByRRSet gather all CHANGES) -laba CHANGE NewSet: { [0'], [1'], [2'] } -labc DELETE Old: { [2] } -labe CHANGE New: { [3'], [4'], [5'], [6'] } -labf CREATE New: { [7'] } -labg CHANGE NewSet: { [7] [8] [8'] [11'] } -labh DELETE Old { [11] } -labh CREATE NewSet: { [12'] } - - - - -By Record: - -rewrite as triples: FQDN+TYPE, TARGET, RC -byRecord: - group-by key=FQDN+TYPE, use targets to make add/change/delete for each record. - -byRSet: - group-by key=FQDN+TYPE, use targets to make add/change/delete for each record. - for each key: - if both have this key: - IF targets are the same, skip. - Else generate CHANGE for KEY: - New = Recs from desired. - Msgs = The msgs from targetdiff(e.Recs, d.Recs) - -byLabel: - group-by key=FQDN, use type+targets to make add/change/delete for each record. - - -rewrite as triples: FQDN {, TYPE, TARGET, RC - -type CompareConfig struct { - existing, desired models.Records - ldata: []LabelConfig -} - -type ByLabelConfig struct { - label string - tdata: []ByRTypeConfig -} - -type ByRTypeConfig struct { - rtype string - existing: []TargetConfig - desired: []TargetConfig - existingRecs: []*models.RecordConfig - desiredRecs: []*models.RecordConfig -} - -type TargetConfig struct { - compareable string - rec *model.RecordConfig -} - -func highest[S ~[]T, T any](s S) int { - return len(s) - 1 -} - -populate CompareConfig. -for rec := range desired { - label = FILL - rtype = FILL - comp = FILL - cc.labelMap[label] = &rc - cc.keyMap[key] = &rc - if not seen label: - append cc.ldata ByLabelConfig{} - labelIdx = last(cc.ldata) - if not seen key: - append cc.ldata[labelIdx].tdata ByRTypeConfig{} - rtIdx = last(cc.ldata[labelIdx].tdata) - cc.ldata[labelIdx].label = label - cc.ldata[labelIdx].tdata[rtIdx].rtype = rtype - cc.ldata[labelIdx].tdata[rtIdx].existing[append].comparable = comp - cc.ldata[labelIdx].tdata[rtIdx].existing[append].rec = &rc -} - -ByRSet: -func traverse(cc CompareConfig) { - for label := range cc.data { - for rtype := range label.data { - Msgs := genmsgs(rtype.existing, rtype.desired) - if no Msgs, continue - if len(rtype.existing) = 0 { - yield create(label, rtype, rtype.desiredRecs, Msgs) - } else if len(rtype.desired) = 0 { - yield delete(label, rtype, rtype.existingRecs, Msgs) - } else { // equal - yield change(label, rtype, rtype.desiredRecs, Msgs) - } - } -} - -byLabel: -func traverse(cc CompareConfig) { - for label := range cc.data { - initialize msgs, desiredRecords - anyExisting = false - for rtype := range label.data { - accumulate Msgs := genmsgs(rtype.existing, rtype.desired) - if Msgs (i.e. there were changes) { - accumulate AllDesired := rtype.desiredRecs - if len(rtype.existing) != 0 { - anyExisting = true - } - } - } - if there are Msgs: - if len(AllDesired) = 0 { - yield delete(label) - } else if countAllExisting == 0 { - yield create(label, AllDesired) - } else { - yield change(label, AllDesired) - } - } -} - -ByRecord: -func traverse(cc CompareConfig) { - for label := range cc.data { - for rtype := range label.data { - create, change, delete := difftargets(rtype.existing, rtype.desired) - yield creates, changes, deletes - } - } -} - -Byzone: -func traverse(cc CompareConfig) { - for label := range cc.data { - for rtype := range label.data { - Msgs := genmsgs(rtype.existing, rtype.desired) - accumulate Msgs - } - } - if len(accumMsgs) == 0 { - return nil, FirstMsg - } else { - return desired, Msgs - } -} \ No newline at end of file diff --git a/providers/akamaiedgedns/akamaiEdgeDnsProvider.go b/providers/akamaiedgedns/akamaiEdgeDnsProvider.go index b1a55829a..eb89182c9 100644 --- a/providers/akamaiedgedns/akamaiEdgeDnsProvider.go +++ b/providers/akamaiedgedns/akamaiEdgeDnsProvider.go @@ -128,7 +128,6 @@ func (a *edgeDNSProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mode keysToUpdate, err = (diff.New(dc)).ChangedGroups(existingRecords) } else { keysToUpdate, err = (diff.NewCompat(dc)).ChangedGroups(existingRecords) - // TODO(tlim): In the future please adopt `pkg/diff2/By*()` } if err != nil { return nil, err diff --git a/providers/cloudflare/cloudflareProvider.go b/providers/cloudflare/cloudflareProvider.go index f9e3f8fca..6ad7276fe 100644 --- a/providers/cloudflare/cloudflareProvider.go +++ b/providers/cloudflare/cloudflareProvider.go @@ -14,6 +14,7 @@ import ( "github.com/StackExchange/dnscontrol/v3/pkg/transform" "github.com/StackExchange/dnscontrol/v3/providers" "github.com/cloudflare/cloudflare-go" + "github.com/fatih/color" "github.com/miekg/dns/dnsutil" ) @@ -415,6 +416,17 @@ func (c *cloudflareProvider) mkCreateCorrection(newrec *models.RecordConfig, dom } func (c *cloudflareProvider) mkChangeCorrection(oldrec, newrec *models.RecordConfig, domainID string, msg string) []*models.Correction { + var idTxt string + switch oldrec.Type { + case "PAGE_RULE": + idTxt = oldrec.Original.(cloudflare.PageRule).ID + case "WORKER_ROUTE": + idTxt = oldrec.Original.(cloudflare.WorkerRoute).ID + default: + idTxt = oldrec.Original.(cloudflare.DNSRecord).ID + } + msg = msg + color.YellowString(" id=%v", idTxt) + switch newrec.Type { case "PAGE_RULE": return []*models.Correction{{ @@ -441,6 +453,7 @@ func (c *cloudflareProvider) mkChangeCorrection(oldrec, newrec *models.RecordCon } func (c *cloudflareProvider) mkDeleteCorrection(recType string, origRec any, domainID string, msg string) []*models.Correction { + var idTxt string switch recType { case "PAGE_RULE": @@ -450,7 +463,7 @@ func (c *cloudflareProvider) mkDeleteCorrection(recType string, origRec any, dom default: idTxt = origRec.(cloudflare.DNSRecord).ID } - msg = msg + fmt.Sprintf(" id=%v", idTxt) + msg = msg + color.RedString(" id=%v", idTxt) correction := &models.Correction{ Msg: msg, diff --git a/providers/cscglobal/dns.go b/providers/cscglobal/dns.go index 87231ebed..b9990da01 100644 --- a/providers/cscglobal/dns.go +++ b/providers/cscglobal/dns.go @@ -118,42 +118,18 @@ func (client *providerClient) GenerateDomainCorrections(dc *models.DomainConfig, //txtutil.SplitSingleLongTxt(dc.Records) // Autosplit long TXT records var corrections []*models.Correction - var creates, dels, modifications diff.Changeset var err error + var differ diff.Differ if !diff2.EnableDiff2 { - differ := diff.New(dc) - _, creates, dels, modifications, err = differ.IncrementalDiff(foundRecords) + differ = diff.New(dc) } else { - differ := diff.NewCompat(dc) - _, creates, dels, modifications, err = differ.IncrementalDiff(foundRecords) + differ = diff.NewCompat(dc) } + _, creates, dels, modifications, err := differ.IncrementalDiff(foundRecords) if err != nil { return nil, err } - // How to generate corrections? - - // (1) Most providers take individual deletes, creates, and - // modifications: - - // // Generate changes. - // corrections := []*models.Correction{} - // for _, del := range dels { - // corrections = append(corrections, client.deleteRec(client.dnsserver, dc.Name, del)) - // } - // for _, cre := range creates { - // corrections = append(corrections, client.createRec(client.dnsserver, dc.Name, cre)...) - // } - // for _, m := range modifications { - // corrections = append(corrections, client.modifyRec(client.dnsserver, dc.Name, m)) - // } - // return corrections, nil - - // (2) Some providers upload the entire zone every time. Look at - // GetDomainCorrections for BIND and NAMECHEAP for inspiration. - - // (3) Others do something entirely different. Like CSCGlobal: - // CSCGlobal has a unique API. A list of edits is sent in one API // call. Edits aren't permitted if an existing edit is being // processed. Therefore, before we do an edit we block until the diff --git a/providers/digitalocean/digitaloceanProvider.go b/providers/digitalocean/digitaloceanProvider.go index 688cc7a5c..5dbc99030 100644 --- a/providers/digitalocean/digitaloceanProvider.go +++ b/providers/digitalocean/digitaloceanProvider.go @@ -147,20 +147,19 @@ func (api *digitaloceanProvider) GetDomainCorrections(dc *models.DomainConfig) ( txtutil.SplitSingleLongTxt(dc.Records) // Autosplit long TXT records var corrections []*models.Correction - var create, delete, modify diff.Changeset + var differ diff.Differ if !diff2.EnableDiff2 { - differ := diff.New(dc) - _, create, delete, modify, err = differ.IncrementalDiff(existingRecords) + differ = diff.New(dc) } else { - differ := diff.NewCompat(dc) - _, create, delete, modify, err = differ.IncrementalDiff(existingRecords) + differ = diff.NewCompat(dc) } + _, toCreate, toDelete, toModify, err := differ.IncrementalDiff(existingRecords) if err != nil { return nil, err } // Deletes first so changing type works etc. - for _, m := range delete { + for _, m := range toDelete { id := m.Existing.Original.(*godo.DomainRecord).ID corr := &models.Correction{ Msg: fmt.Sprintf("%s, DO ID: %d", m.String(), id), @@ -177,7 +176,7 @@ func (api *digitaloceanProvider) GetDomainCorrections(dc *models.DomainConfig) ( } corrections = append(corrections, corr) } - for _, m := range create { + for _, m := range toCreate { req := toReq(dc, m.Desired) corr := &models.Correction{ Msg: m.String(), @@ -194,7 +193,7 @@ func (api *digitaloceanProvider) GetDomainCorrections(dc *models.DomainConfig) ( } corrections = append(corrections, corr) } - for _, m := range modify { + for _, m := range toModify { id := m.Existing.Original.(*godo.DomainRecord).ID req := toReq(dc, m.Desired) corr := &models.Correction{ @@ -281,7 +280,7 @@ func toRc(domain string, r *godo.DomainRecord) *models.RecordConfig { t.SetTarget(target) switch rtype := r.Type; rtype { case "TXT": - t.SetTargetTXTString(target) + t.SetTargetTXTfromRFC1035Quoted(target) default: // nothing additional required } diff --git a/providers/dnsimple/dnsimpleProvider.go b/providers/dnsimple/dnsimpleProvider.go index d8110a099..627d7cde8 100644 --- a/providers/dnsimple/dnsimpleProvider.go +++ b/providers/dnsimple/dnsimpleProvider.go @@ -177,10 +177,6 @@ func (c *dnsimpleProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mod return nil, err } - if err != nil { - return nil, err - } - for _, del := range del { rec := del.Existing.Original.(dnsimpleapi.ZoneRecord) corrections = append(corrections, &models.Correction{ diff --git a/providers/gandiv5/gandi_v5Provider.go b/providers/gandiv5/gandi_v5Provider.go index bf04ec359..457f0b9a8 100644 --- a/providers/gandiv5/gandi_v5Provider.go +++ b/providers/gandiv5/gandi_v5Provider.go @@ -366,6 +366,8 @@ func (client *gandiv5Provider) GenerateDomainCorrections(dc *models.DomainConfig case diff2.CREATE: // We have to create the label one rtype at a time. + // In other words, this is a ByRecordSet API for creation, even though + // this is a ByLabel() API for everything else. natives := recordsToNative(inst.New, dc.Name) for _, n := range natives { label := inst.Key.NameFQDN diff --git a/providers/gcloud/gcloudProvider.go b/providers/gcloud/gcloudProvider.go index 0aa5e21ec..891094d45 100644 --- a/providers/gcloud/gcloudProvider.go +++ b/providers/gcloud/gcloudProvider.go @@ -213,26 +213,22 @@ func (g *gcloudProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*model differ = diff.NewCompat(dc) } _, create, delete, modify, err := differ.IncrementalDiff(existingRecords) - if err != nil { - return nil, err - } - if err != nil { return nil, fmt.Errorf("incdiff error: %w", err) } changedKeys := map[key]bool{} - desc := "" + var msgs []string for _, c := range create { - desc += fmt.Sprintln(c) + msgs = append(msgs, fmt.Sprint(c)) changedKeys[keyForRec(c.Desired)] = true } for _, d := range delete { - desc += fmt.Sprintln(d) + msgs = append(msgs, fmt.Sprint(d)) changedKeys[keyForRec(d.Existing)] = true } for _, m := range modify { - desc += fmt.Sprintln(m) + msgs = append(msgs, fmt.Sprint(m)) changedKeys[keyForRec(m.Existing)] = true } if len(changedKeys) == 0 { @@ -282,7 +278,7 @@ func (g *gcloudProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*model } return []*models.Correction{{ - Msg: desc, + Msg: strings.Join(msgs, "\n"), F: runChange, }}, nil diff --git a/providers/namecheap/namecheapProvider.go b/providers/namecheap/namecheapProvider.go index 46bdc6e76..b52fb1a3a 100644 --- a/providers/namecheap/namecheapProvider.go +++ b/providers/namecheap/namecheapProvider.go @@ -203,9 +203,9 @@ func (n *namecheapProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mo 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 - // // changes though + // because namecheap doesn't have selective create, delete, modify, + // we bundle them all up to send at once. We *do* want to see the + // changes though var desc []string for _, i := range create { diff --git a/providers/namedotcom/records.go b/providers/namedotcom/records.go index 56a3a8878..098b7ae7a 100644 --- a/providers/namedotcom/records.go +++ b/providers/namedotcom/records.go @@ -55,14 +55,13 @@ func (n *namedotcomProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*m models.PostProcessRecords(actual) var corrections []*models.Correction - var create, del, mod diff.Changeset + var differ diff.Differ if !diff2.EnableDiff2 { - differ := diff.New(dc) - _, create, del, mod, err = differ.IncrementalDiff(actual) + differ = diff.New(dc) } else { - differ := diff.NewCompat(dc) - _, create, del, mod, err = differ.IncrementalDiff(actual) + differ = diff.NewCompat(dc) } + _, create, del, mod, err := differ.IncrementalDiff(actual) if err != nil { return nil, err } diff --git a/providers/packetframe/packetframeProvider.go b/providers/packetframe/packetframeProvider.go index 5f6100678..526b259ae 100644 --- a/providers/packetframe/packetframeProvider.go +++ b/providers/packetframe/packetframeProvider.go @@ -137,10 +137,6 @@ func (api *packetframeProvider) GetDomainCorrections(dc *models.DomainConfig) ([ return nil, err } - if err != nil { - return nil, err - } - for _, m := range create { req, err := toReq(zone.ID, dc, m.Desired) if err != nil { diff --git a/providers/rwth/dns.go b/providers/rwth/dns.go index c58159899..85d6aee51 100644 --- a/providers/rwth/dns.go +++ b/providers/rwth/dns.go @@ -65,10 +65,6 @@ func (api *rwthProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*model return nil, err } - if err != nil { - return nil, err - } - for _, d := range create { des := d.Desired corrections = append(corrections, &models.Correction{