From d765ced927bb5332ecf7cbd97a68362bad60c32c Mon Sep 17 00:00:00 2001 From: Tom Limoncelli Date: Fri, 30 Dec 2022 21:33:14 -0500 Subject: [PATCH] CHORE: Make Test_filterBy more readable (#1869) --- pkg/diff2/analyze.go | 17 +++-- pkg/diff2/analyze_test.go | 140 +++++++++++++++--------------------- pkg/diff2/groupsort_test.go | 7 +- pkg/diff2/unmanaged.go | 10 +-- 4 files changed, 81 insertions(+), 93 deletions(-) diff --git a/pkg/diff2/analyze.go b/pkg/diff2/analyze.go index 8c4677d57..6df621e71 100644 --- a/pkg/diff2/analyze.go +++ b/pkg/diff2/analyze.go @@ -73,7 +73,7 @@ func analyzeByLabel(cc *CompareConfig) ChangeList { instructions = append(instructions, mkDelete(label, "", accExisting, accMsgs)) } 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)) + //fmt.Printf("DEBUG: analyzeByLabel mkAdd msgs=%d\n", len(accMsgs)) instructions = append(instructions, mkAddByLabel(label, "", accMsgs, accDesired)) } else { // If we get here, it must be a change. _ = i @@ -82,7 +82,7 @@ func analyzeByLabel(cc *CompareConfig) ChangeList { // len(accDesired), accDesired, // accMsgs, // ) - fmt.Printf("DEBUG: analyzeByLabel mkchange msgs=%d\n", len(accMsgs)) + //fmt.Printf("DEBUG: analyzeByLabel mkchange msgs=%d\n", len(accMsgs)) instructions = append(instructions, mkChangeLabel(label, "", accMsgs, accExisting, accDesired, msgsByKey)) } } @@ -121,8 +121,8 @@ func mkAdd(l string, t string, msgs []string, recs models.Records) Change { // 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: len(o)=%d len(m)=%d\n", len(newRecs), len(msgs)) - fmt.Printf("DEBUG: mkAddByLabel: msgs = %v\n", msgs) + //fmt.Printf("DEBUG: mkAddByLabel: len(o)=%d len(m)=%d\n", len(newRecs), len(msgs)) + //fmt.Printf("DEBUG: mkAddByLabel: msgs = %v\n", msgs) c := Change{Type: CREATE, Msgs: msgs} c.Key.NameFQDN = l c.Key.Type = t @@ -185,13 +185,21 @@ func removeCommon(existing, desired []targetConfig) ([]targetConfig, []targetCon return filterBy(existing, dKeys), filterBy(desired, eKeys) } +// 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.compareable]; !ok { + //fmt.Printf("DEBUG: comp %q NO\n", x.compareable) // copy and increment index s[i] = x i++ + } else { + //fmt.Printf("DEBUG: comp %q YES\n", x.compareable) } } // // Prevent memory leak by erasing truncated values @@ -200,6 +208,7 @@ 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 } diff --git a/pkg/diff2/analyze_test.go b/pkg/diff2/analyze_test.go index 7a959a808..cba206146 100644 --- a/pkg/diff2/analyze_test.go +++ b/pkg/diff2/analyze_test.go @@ -9,20 +9,22 @@ import ( "github.com/kylelemons/godebug/diff" ) -var testDataAA1234 = makeRec("laba", "A", "1.2.3.4") // [0] -var testDataAA5678 = makeRec("laba", "A", "5.6.7.8") // [0] -var testDataAMX10a = makeRec("laba", "MX", "10 laba") // [1] -var testDataCCa = makeRec("labc", "CNAME", "laba") // [2] -var testDataEA15 = makeRec("labe", "A", "10.10.10.15") // [3] -var e4 = makeRec("labe", "A", "10.10.10.16") // [4] -var e5 = makeRec("labe", "A", "10.10.10.17") // [5] -var e6 = makeRec("labe", "A", "10.10.10.18") // [6] -var e7 = makeRec("labg", "NS", "10.10.10.15") // [7] -var e8 = makeRec("labg", "NS", "10.10.10.16") // [8] -var e9 = makeRec("labg", "NS", "10.10.10.17") // [9] -var e10 = makeRec("labg", "NS", "10.10.10.18") // [10] -var e11mx = makeRec("labh", "MX", "22 ttt") // [11] -var e11 = makeRec("labh", "CNAME", "labd") // [11] +var testDataAA1234 = makeRec("laba", "A", "1.2.3.4") // [0] +var testDataAA5678 = makeRec("laba", "A", "5.6.7.8") // +var testDataAA1234ttl700 = makeRecTTL("laba", "A", "1.2.3.4", 700) // +var testDataAA5678ttl700 = makeRecTTL("laba", "A", "5.6.7.8", 700) // +var testDataAMX10a = makeRec("laba", "MX", "10 laba") // [1] +var testDataCCa = makeRec("labc", "CNAME", "laba") // [2] +var testDataEA15 = makeRec("labe", "A", "10.10.10.15") // [3] +var e4 = makeRec("labe", "A", "10.10.10.16") // [4] +var e5 = makeRec("labe", "A", "10.10.10.17") // [5] +var e6 = makeRec("labe", "A", "10.10.10.18") // [6] +var e7 = makeRec("labg", "NS", "10.10.10.15") // [7] +var e8 = makeRec("labg", "NS", "10.10.10.16") // [8] +var e9 = makeRec("labg", "NS", "10.10.10.17") // [9] +var e10 = makeRec("labg", "NS", "10.10.10.18") // [10] +var e11mx = makeRec("labh", "MX", "22 ttt") // [11] +var e11 = makeRec("labh", "CNAME", "labd") // [11] var testDataApexMX1aaa = makeRec("", "MX", "1 aaa") var testDataAA1234clone = makeRec("laba", "A", "1.2.3.4") // [0'] @@ -40,7 +42,7 @@ var d11 = makeRec("labg", "NS", "10.10.10.97") // [11'] var d12 = makeRec("labh", "A", "1.2.3.4") // [12'] var testDataApexMX22bbb = makeRec("", "MX", "22 bbb") -var d0tc = targetConfig{compareable: "1.2.3.4 ttl=0", rec: testDataAA1234clone} +var d0tc = mkTargetConfig(testDataAA1234clone) func makeChange(v Verb, l, t string, old, new models.Records, msgs []string) Change { c := Change{ @@ -402,6 +404,26 @@ ChangeList: len=12 } } + +func mkTargetConfig(x ...*models.RecordConfig) []targetConfig { + var tc []targetConfig + for _, r := range x { + tc = append(tc, targetConfig{ + compareable: comparable(r, nil), + rec: r, + }) + } + return tc +} + +func mkTargetConfigMap(x ...*models.RecordConfig) map[string]*targetConfig { + var m = map[string]*targetConfig{} + for _, v := range mkTargetConfig(x...) { + m[v.compareable] = &v + } + return m +} + func Test_diffTargets(t *testing.T) { type args struct { existing []targetConfig @@ -416,12 +438,8 @@ func Test_diffTargets(t *testing.T) { { name: "single", args: args{ - existing: []targetConfig{ - {compareable: "1.2.3.4", rec: testDataAA1234}, - }, - desired: []targetConfig{ - {compareable: "1.2.3.4", rec: testDataAA1234}, - }, + existing: mkTargetConfig(testDataAA1234), + desired: mkTargetConfig(testDataAA1234), }, //want: , }, @@ -429,13 +447,8 @@ func Test_diffTargets(t *testing.T) { { name: "add1", args: args{ - existing: []targetConfig{ - {compareable: "1.2.3.4", rec: testDataAA1234}, - }, - desired: []targetConfig{ - {compareable: "1.2.3.4", rec: testDataAA1234}, - {compareable: "10 laba", rec: testDataAMX10a}, - }, + existing: mkTargetConfig(testDataAA1234), + desired: mkTargetConfig(testDataAA1234, testDataAMX10a), }, want: ChangeList{ Change{Type: CREATE, @@ -449,13 +462,8 @@ func Test_diffTargets(t *testing.T) { { name: "del1", args: args{ - existing: []targetConfig{ - {compareable: "1.2.3.4", rec: testDataAA1234}, - {compareable: "10 laba", rec: testDataAMX10a}, - }, - desired: []targetConfig{ - {compareable: "1.2.3.4", rec: testDataAA1234}, - }, + existing: mkTargetConfig(testDataAA1234, testDataAMX10a), + desired: mkTargetConfig(testDataAA1234), }, want: ChangeList{ Change{Type: DELETE, @@ -469,14 +477,8 @@ func Test_diffTargets(t *testing.T) { { name: "change2nd", args: args{ - existing: []targetConfig{ - {compareable: "1.2.3.4", rec: testDataAA1234}, - {compareable: "10 laba", rec: testDataAMX10a}, - }, - desired: []targetConfig{ - {compareable: "1.2.3.4", rec: testDataAA1234}, - {compareable: "20 laba", rec: testDataAMX20b}, - }, + existing: mkTargetConfig(testDataAA1234, testDataAMX10a), + desired: mkTargetConfig(testDataAA1234, testDataAMX20b), }, want: ChangeList{ Change{Type: CHANGE, @@ -491,13 +493,8 @@ func Test_diffTargets(t *testing.T) { { name: "del2nd", args: args{ - existing: []targetConfig{ - {compareable: "1.2.3.4", rec: testDataAA1234}, - {compareable: "5.6.7.8", rec: testDataAA5678}, - }, - desired: []targetConfig{ - {compareable: "1.2.3.4", rec: testDataAA1234}, - }, + existing: mkTargetConfig(testDataAA1234, testDataAA5678), + desired: mkTargetConfig(testDataAA1234), }, want: ChangeList{ Change{Type: CHANGE, @@ -536,8 +533,8 @@ func Test_removeCommon(t *testing.T) { { name: "same", args: args{ - existing: []targetConfig{d0tc}, - desired: []targetConfig{d0tc}, + existing: d0tc, + desired: d0tc, }, want: []targetConfig{}, want1: []targetConfig{}, @@ -578,14 +575,8 @@ func Test_filterBy(t *testing.T) { { name: "removeall", args: args{ - s: []targetConfig{ - {compareable: "1.2.3.4", rec: testDataAA1234}, - {compareable: "10 laba", rec: testDataAMX10a}, - }, - m: map[string]*targetConfig{ - "1.2.3.4": {compareable: "1.2.3.4", rec: testDataAA1234}, - "10 laba": {compareable: "10 laba", rec: testDataAMX10a}, - }, + s: mkTargetConfig(testDataAA1234, testDataAMX10a), + m: mkTargetConfigMap(testDataAA1234, testDataAMX10a), }, want: []targetConfig{}, }, @@ -593,36 +584,19 @@ func Test_filterBy(t *testing.T) { { name: "keepall", args: args{ - s: []targetConfig{ - {compareable: "1.2.3.4", rec: testDataAA1234}, - {compareable: "10 laba", rec: testDataAMX10a}, - }, - m: map[string]*targetConfig{ - "nothing": {compareable: "1.2.3.4", rec: testDataAA1234}, - "matches": {compareable: "10 laba", rec: testDataAMX10a}, - }, - }, - want: []targetConfig{ - {compareable: "1.2.3.4", rec: testDataAA1234}, - {compareable: "10 laba", rec: testDataAMX10a}, + s: mkTargetConfig(testDataAA1234, testDataAMX10a), + m: mkTargetConfigMap(), }, + want: mkTargetConfig(testDataAA1234, testDataAMX10a), }, { name: "keepsome", args: args{ - s: []targetConfig{ - {compareable: "1.2.3.4", rec: testDataAA1234}, - {compareable: "10 laba", rec: testDataAMX10a}, - }, - m: map[string]*targetConfig{ - "nothing": {compareable: "1.2.3.4", rec: testDataAA1234}, - "10 laba": {compareable: "10 laba", rec: testDataAMX10a}, - }, - }, - want: []targetConfig{ - {compareable: "1.2.3.4", rec: testDataAA1234}, + s: mkTargetConfig(testDataAA1234, testDataAMX10a), + m: mkTargetConfigMap(testDataAMX10a), }, + want: mkTargetConfig(testDataAA1234), }, } for _, tt := range tests { diff --git a/pkg/diff2/groupsort_test.go b/pkg/diff2/groupsort_test.go index 07f806649..1017a1184 100644 --- a/pkg/diff2/groupsort_test.go +++ b/pkg/diff2/groupsort_test.go @@ -9,11 +9,16 @@ import ( func makeRec(label, rtype, content string) *models.RecordConfig { origin := "f.com" - r := models.RecordConfig{} + r := models.RecordConfig{TTL: 300} r.SetLabel(label, origin) r.PopulateFromString(rtype, content, origin) return &r } +func makeRecTTL(label, rtype, content string, ttl uint32) *models.RecordConfig { + r := makeRec(label, rtype, content) + r.TTL = ttl + return r +} func makeRecSet(recs ...*models.RecordConfig) *recset { result := recset{} result.Key = recs[0].Key() diff --git a/pkg/diff2/unmanaged.go b/pkg/diff2/unmanaged.go index 9401fafd6..5171752b2 100644 --- a/pkg/diff2/unmanaged.go +++ b/pkg/diff2/unmanaged.go @@ -97,7 +97,7 @@ func compileTypeGlob(g string) map[string]bool { } func match(rc *models.RecordConfig, glabel, gtarget glob.Glob, hasRType map[string]bool) bool { - printer.Printf("DEBUG: match(%v, %v, %v, %v)\n", rc.NameFQDN, glabel, gtarget, hasRType) + //printer.Printf("DEBUG: match(%v, %v, %v, %v)\n", rc.NameFQDN, glabel, gtarget, hasRType) // _ = glabel.Match(rc.NameFQDN) // _ = matchType(rc.Type, hasRType) @@ -105,22 +105,22 @@ func match(rc *models.RecordConfig, glabel, gtarget glob.Glob, hasRType map[stri // _ = gtarget.Match(x) if !glabel.Match(rc.NameFQDN) { - printer.Printf("DEBUG: REJECTED LABEL: %s:%v\n", rc.NameFQDN, glabel) + //printer.Printf("DEBUG: REJECTED LABEL: %s:%v\n", rc.NameFQDN, glabel) return false } else if !matchType(rc.Type, hasRType) { - printer.Printf("DEBUG: REJECTED TYPE: %s:%v\n", rc.Type, hasRType) + //printer.Printf("DEBUG: REJECTED TYPE: %s:%v\n", rc.Type, hasRType) return false } else if gtarget == nil { return true } else if !gtarget.Match(rc.GetTargetField()) { - printer.Printf("DEBUG: REJECTED TARGET: %v:%v\n", rc.GetTargetField(), gtarget) + //printer.Printf("DEBUG: REJECTED TARGET: %v:%v\n", rc.GetTargetField(), gtarget) return false } return true } func matchType(s string, hasRType map[string]bool) bool { - printer.Printf("DEBUG: matchType map=%v\n", hasRType) + //printer.Printf("DEBUG: matchType map=%v\n", hasRType) if len(hasRType) == 0 { return true }