mirror/stackexchange-dnscontrol
1
0
Fork 0
mirror of https://github.com/StackExchange/dnscontrol.git synced 2024-05-11 05:55:12 +00:00
Code Issues Releases Activity

BUG: diff2: ttl changes don't always work (#2093)

Browse Source
This commit is contained in:
Tom Limoncelli
2023-02-25 22:40:54 -05:00
committed by GitHub
parent ab872cc13f
commit 05dc26bf2e
8 changed files with 151 additions and 101 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
integrationTest/integration_test.go
View File

@ -750,7 +750,6 @@ func makeTests(t *testing.T) []*TestGroup {
// This is a strange one. It adds a new record to an existing
// label but the pre-existing label has its TTL change.
testgroup("add to label and change orig ttl",
not("NAMEDOTCOM"), // Known bug: https://github.com/StackExchange/dnscontrol/issues/2088
tc("Setup", ttl(a("www", "5.6.7.8"), 400)),
tc("Add at same label, new ttl", ttl(a("www", "5.6.7.8"), 700), ttl(a("www", "1.2.3.4"), 700)),
),

12
models/record.go
View File

@ -307,6 +307,8 @@ func (rc *RecordConfig) GetLabelFQDN() string {
// ToDiffable returns a string that is comparable by a differ.
// extraMaps: a list of maps that should be included in the comparison.
// NB(tlim): This will be deprecated when pkg/diff is replaced by pkg/diff2.
// Use // ToComparableNoTTL() instead.
func (rc *RecordConfig) ToDiffable(extraMaps ...map[string]string) string {
var content string
switch rc.Type {
@ -337,6 +339,16 @@ func (rc *RecordConfig) ToDiffable(extraMaps ...map[string]string) string {
return content
}
// ToComparableNoTTL returns a comparison string. If you need to compare two
// RecordConfigs, you can simply compare the string returned by this function.
// THe comparison includes all fields except TTL and any provider-specific
// metafields. Provider-specific metafields like CF_PROXY are not the same as
// pseudo-records like ANAME or R53_ALIAS
// This replaces ToDiff()
func (rc *RecordConfig) ToComparableNoTTL() string {
return rc.GetTargetCombined()
}
// ToRR converts a RecordConfig to a dns.RR.
func (rc *RecordConfig) ToRR() dns.RR {

112
pkg/diff2/analyze.go
View File

@ -179,58 +179,70 @@ func removeCommon(existing, desired []targetConfig) ([]targetConfig, []targetCon
// 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.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 })
eKeys := map[string]*targetConfig{}
for _, v := range existing {
v := v
eKeys[v.compareable] = &v
eKeys[v.comparableFull] = &v
}
dKeys := map[string]*targetConfig{}
for _, v := range desired {
v := v
dKeys[v.compareable] = &v
dKeys[v.comparableFull] = &v
}
return filterBy(existing, dKeys), filterBy(desired, eKeys)
}
// Find the changes that are exclusively changes in TTL.
func splitTTLOnly(existing, desired []targetConfig) (
existDiff []targetConfig, desireDiff []targetConfig,
existTTL models.Records, desireTTL models.Records,
) {
// 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.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 })
var instructions ChangeList
var existDiff, desiredDiff []targetConfig
ei := 0
di := 0
for (ei < len(existing)) && (di < len(desired)) {
er := existing[ei].rec
dr := desired[di].rec
ecomp := existing[ei].compareable
dcomp := desired[di].compareable
ecomp := existing[ei].comparableNoTTL
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!")
}
//fmt.Printf("DEBUG ecomp=%q dcomp=%q ettl=%d dttl=%d\n", ecomp, dcomp, er.TTL, dr.TTL)
if ecomp == dcomp && er.TTL != dr.TTL {
//fmt.Printf("DEBUG: equal\n")
existTTL = append(existTTL, er)
desireTTL = append(desireTTL, dr)
//m := fmt.Sprintf("CHANGE-TTL %s %s %s ttl=%d->%d", dr.NameFQDN, dr.Type, dr.GetTargetRFC1035Quoted(), er.TTL, dr.TTL)
m := fmt.Sprintf("CHANGE-TTL %s %s ", dr.NameFQDN, dr.Type) + humanDiff(existing[ei], desired[di])
instructions = append(instructions, mkChange(dr.NameFQDN, dr.Type, []string{m},
models.Records{er},
models.Records{dr},
))
ei++
di++
} else if ecomp < dcomp {
//fmt.Printf("DEBUG: less\n")
existDiff = append(existDiff, existing[ei])
ei++
} else if ecomp > dcomp {
//fmt.Printf("DEBUG: greater\n")
desireDiff = append(desireDiff, desired[di])
desiredDiff = append(desiredDiff, desired[di])
di++
} else {
panic("Should not happen. e and d can not be both equal, less and greater")
panic("should not happen. ecomp cant be both == and != dcomp")
}
}
// Any remainder goes to the *Diff result:
@ -240,10 +252,10 @@ func splitTTLOnly(existing, desired []targetConfig) (
}
if di < len(desired) {
//fmt.Printf("DEBUG: append d len()=%d\n", di)
desireDiff = append(desireDiff, desired[di:]...)
desiredDiff = append(desiredDiff, desired[di:]...)
}
return
return existDiff, desiredDiff, instructions
}
// Return s but remove any items that can be found in m.
@ -254,8 +266,8 @@ func filterBy(s []targetConfig, m map[string]*targetConfig) []targetConfig {
// }
i := 0 // output index
for _, x := range s {
if _, ok := m[x.compareable]; !ok {
//fmt.Printf("DEBUG: comp %q NO\n", x.compareable)
if _, ok := m[x.comparableFull]; !ok {
//fmt.Printf("DEBUG: comp %q NO\n", x.comparable)
// copy and increment index
s[i] = x
i++
@ -271,22 +283,19 @@ func filterBy(s []targetConfig, m map[string]*targetConfig) []targetConfig {
return s
}
// humanDiff returns a human-friendly string showing what changed
// between a and b.
func humanDiff(a, b targetConfig) string {
aTTL := a.rec.TTL
bTTL := b.rec.TTL
acombined := a.compareable
bcombined := b.compareable
combinedDiff := acombined != bcombined
ttlDiff := aTTL != bTTL
// TODO(tlim): Records like MX and SRV should have more clever output.
// For example if only the MX priority changes, show that.
if combinedDiff && ttlDiff {
return fmt.Sprintf("(%s ttl=%d) -> (%s ttl=%d)", acombined, aTTL, bcombined, bTTL)
// For example if only the MX priority changes, show just that.
if a.comparableNoTTL != b.comparableNoTTL {
// The recorddata is different:
return fmt.Sprintf("(%s) -> (%s)", a.comparableFull, b.comparableFull)
}
if combinedDiff {
return fmt.Sprintf("(%s) -> (%s)", acombined, bcombined)
}
return fmt.Sprintf("%s (ttl %d->%d)", acombined, aTTL, bTTL)
// Just the TTLs are different:
return fmt.Sprintf("%s ttl=(%d->%d)", a.comparableNoTTL, a.rec.TTL, b.rec.TTL)
}
func diffTargets(existing, desired []targetConfig) ChangeList {
@ -298,15 +307,11 @@ func diffTargets(existing, desired []targetConfig) ChangeList {
return nil
}
// Sort to make comparisons easier
sort.Slice(existing, func(i, j int) bool { return existing[i].compareable < existing[j].compareable })
sort.Slice(desired, func(i, j int) bool { return desired[i].compareable < desired[j].compareable })
var instructions ChangeList
// remove the exact matches.
//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)
@ -315,23 +320,16 @@ func diffTargets(existing, desired []targetConfig) ChangeList {
// records that have the same GetTargetCombined() but different
// TTLs.
// Find TTL changes:
existing, desired, existingTTL, desiredTTL := splitTTLOnly(existing, desired)
for i := range desiredTTL {
er := existingTTL[i]
dr := desiredTTL[i]
existing, desired, newChanges := findTTLChanges(existing, desired)
instructions = append(instructions, newChanges...)
m := fmt.Sprintf("CHANGE %s %s ", dr.NameFQDN, dr.Type) + humanDiff(existing[i], desired[i])
// Sort to make comparisons easier
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 })
instructions = append(instructions, mkChange(dr.NameFQDN, dr.Type, []string{m},
models.Records{er},
models.Records{dr},
))
}
// the common chunk are changes (regardless of TTL)
// the remaining chunks are changes (regardless of TTL)
mi := min(len(existing), len(desired))
//fmt.Printf("DEBUG: min=%d\n", mi)
for i := 0; i < mi; i++ {
//fmt.Println(i, "CHANGE")
er := existing[i].rec
@ -349,7 +347,7 @@ func diffTargets(existing, desired []targetConfig) ChangeList {
for i := mi; i < len(existing); i++ {
//fmt.Println(i, "DEL")
er := existing[i].rec
m := fmt.Sprintf("DELETE %s %s %s", er.NameFQDN, er.Type, existing[i].compareable)
m := fmt.Sprintf("DELETE %s %s %s", er.NameFQDN, er.Type, existing[i].comparableFull)
instructions = append(instructions, mkDeleteRec(er.NameFQDN, er.Type, []string{m}, er))
}
@ -357,7 +355,7 @@ func diffTargets(existing, desired []targetConfig) ChangeList {
for i := mi; i < len(desired); i++ {
//fmt.Println(i, "CREATE")
dr := desired[i].rec
m := fmt.Sprintf("CREATE %s %s %s", dr.NameFQDN, dr.Type, desired[i].compareable)
m := fmt.Sprintf("CREATE %s %s %s", dr.NameFQDN, dr.Type, desired[i].comparableFull)
instructions = append(instructions, mkAdd(dr.NameFQDN, dr.Type, []string{m}, models.Records{dr}))
}

51
pkg/diff2/analyze_test.go
View File

@ -83,8 +83,8 @@ func Test_analyzeByRecordSet(t *testing.T) {
}
origin := "f.com"
existing := models.Records{testDataAA1234, testDataAMX10a, testDataCCa, testDataEA15, e4, e5, e6, e7, e8, e9, e10, e11}
desired := models.Records{testDataAA1234clone, testDataAA12345, testDataAMX20b, d3, d4, d5, d6, d7, d8, d9, d10, d11, d12}
existingSample := models.Records{testDataAA1234, testDataAMX10a, testDataCCa, testDataEA15, e4, e5, e6, e7, e8, e9, e10, e11}
desiredSample := models.Records{testDataAA1234clone, testDataAA12345, testDataAMX20b, d3, d4, d5, d6, d7, d8, d9, d10, d11, d12}
tests := []struct {
name string
@ -227,8 +227,8 @@ ChangeList: len=2
name: "big",
args: args{
origin: origin,
existing: existing,
desired: desired,
existing: existingSample,
desired: desiredSample,
},
wantMsgs: `
CREATE laba.f.com A 1.2.3.5 ttl=300
@ -406,9 +406,11 @@ ChangeList: len=12
func mkTargetConfig(x ...*models.RecordConfig) []targetConfig {
var tc []targetConfig
for _, r := range x {
ct, cf := mkCompareBlobs(r, nil)
tc = append(tc, targetConfig{
compareable: mkCompareBlob(r, nil),
rec: r,
comparableNoTTL: ct,
comparableFull: cf,
rec: r,
})
}
return tc
@ -417,7 +419,7 @@ func mkTargetConfig(x ...*models.RecordConfig) []targetConfig {
func mkTargetConfigMap(x ...*models.RecordConfig) map[string]*targetConfig {
var m = map[string]*targetConfig{}
for _, v := range mkTargetConfig(x...) {
m[v.compareable] = &v
m[v.comparableFull] = &v
}
return m
}
@ -445,8 +447,8 @@ func Test_diffTargets(t *testing.T) {
Key: models.RecordKey{NameFQDN: "laba.f.com", Type: "A"},
New: models.Records{testDataAA5678ttl700, testDataAA1234ttl700},
Msgs: []string{
"CHANGE laba.f.com A (5.6.7.8 ttl=300 ttl=300) -> (1.2.3.4 ttl=700 ttl=700)",
"CREATE laba.f.com A 5.6.7.8 ttl=700",
"CHANGE-TTL laba.f.com A 5.6.7.8 ttl=(300->700)",
"CREATE laba.f.com A 1.2.3.4 ttl=700",
},
},
},
@ -527,9 +529,11 @@ func Test_diffTargets(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
//fmt.Printf("DEBUG: Test %02d\n", i)
got := diffTargets(tt.args.existing, tt.args.desired)
d := diff.Diff(strings.TrimSpace(justMsgString(got)), strings.TrimSpace(justMsgString(tt.want)))
g := strings.TrimSpace(justMsgString(got))
w := strings.TrimSpace(justMsgString(tt.want))
d := diff.Diff(g, w)
if d != "" {
//fmt.Printf("DEBUG: %d %d\n", len(got), len(tt.want))
//fmt.Printf("DEBUG: fail %q %q\n", g, w)
t.Errorf("diffTargets()\n diff=%s", d)
}
})
@ -584,7 +588,7 @@ func Test_removeCommon(t *testing.T) {
func comparables(s []targetConfig) []string {
var r []string
for _, j := range s {
r = append(r, j.compareable)
r = append(r, j.comparableFull)
}
return r
}
@ -646,8 +650,7 @@ func Test_splitTTLOnly(t *testing.T) {
args args
wantExistDiff []targetConfig
wantDesireDiff []targetConfig
wantExistTTL models.Records
wantDesireTTL models.Records
wantChanges string
}{
{
@ -656,10 +659,9 @@ func Test_splitTTLOnly(t *testing.T) {
existing: mkTargetConfig(testDataAA1234),
desired: mkTargetConfig(testDataAA1234ttl700),
},
wantExistDiff: mkTargetConfig(testDataAA1234),
wantDesireDiff: mkTargetConfig(testDataAA1234ttl700),
wantExistTTL: models.Records{testDataAA1234},
wantDesireTTL: models.Records{testDataAA1234ttl700},
wantExistDiff: nil,
wantDesireDiff: nil,
wantChanges: "ChangeList: len=1\n00: Change: verb=CHANGE\n key={laba.f.com A}\n old=[1.2.3.4]\n new=[1.2.3.4]\n msg=[\"CHANGE-TTL laba.f.com A 1.2.3.4 ttl=(300->700)\"]\n",
},
{
@ -670,24 +672,21 @@ func Test_splitTTLOnly(t *testing.T) {
},
wantExistDiff: mkTargetConfig(testDataAA1234),
wantDesireDiff: mkTargetConfig(testDataAA5678),
wantExistTTL: models.Records{},
wantDesireTTL: models.Records{},
wantChanges: "ChangeList: len=0\n",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotExistDiff, gotDesireDiff, gotExistTTL, gotDesireTTL := splitTTLOnly(tt.args.existing, tt.args.desired)
gotExistDiff, gotDesireDiff, gotChanges := findTTLChanges(tt.args.existing, tt.args.desired)
if !reflect.DeepEqual(gotExistDiff, tt.wantExistDiff) {
t.Errorf("splitTTLOnly() gotExistDiff = %v, want %v", gotExistDiff, tt.wantExistDiff)
}
if !reflect.DeepEqual(gotDesireDiff, tt.wantDesireDiff) {
t.Errorf("splitTTLOnly() gotDesireDiff = %v, want %v", gotDesireDiff, tt.wantDesireDiff)
}
if ((len(tt.wantExistTTL) != 0) && len(gotExistTTL) != 0) && !reflect.DeepEqual(gotExistTTL, tt.wantExistTTL) {
t.Errorf("splitTTLOnly() gotExistTTL = %v, want %v (len=%d %d)", gotExistTTL, tt.wantExistTTL, len(gotExistTTL), len(tt.wantExistTTL))
}
if ((len(tt.wantDesireTTL) != 0) && len(gotDesireTTL) != 0) && !reflect.DeepEqual(gotDesireTTL, tt.wantDesireTTL) {
t.Errorf("splitTTLOnly() gotDesireTTL = %v, want %v", gotDesireTTL, tt.wantDesireTTL)
gotChangesString := gotChanges.String()
if gotChangesString != tt.wantChanges {
t.Errorf("splitTTLOnly() gotChanges=\n%q, want=\n%q", gotChangesString, tt.wantChanges)
}
})
}

36
pkg/diff2/compareconfig.go
View File

@ -82,8 +82,10 @@ type rTypeConfig struct {
}
type targetConfig struct {
compareable string // A string that can be used to compare two rec's for equality.
rec *models.RecordConfig // The RecordConfig itself.
comparableFull string // A string that can be used to compare two rec's for 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.
}
func NewCompareConfig(origin string, existing, desired models.Records, compFn ComparableFunc) *CompareConfig {
@ -202,15 +204,23 @@ func (cc *CompareConfig) String() string {
// Generate a string that can be used to compare this record to others
// for equality.
func mkCompareBlob(rc *models.RecordConfig, f func(*models.RecordConfig) string) string {
if f == nil {
return rc.ToDiffable()
func mkCompareBlobs(rc *models.RecordConfig, f func(*models.RecordConfig) string) (string, string) {
// Start with the comparable string
comp := rc.ToComparableNoTTL()
// If the custom function exists, add its output
if f != nil {
addOn := f(rc)
if addOn != "" {
comp += " " + f(rc)
}
}
addOn := f(rc)
if addOn != "" {
return rc.ToDiffable() + " " + f(rc)
}
return rc.ToDiffable()
lenWithoutTTL := len(comp)
compFull := comp + fmt.Sprintf(" ttl=%d", rc.TTL)
return compFull[:lenWithoutTTL], compFull
}
func (cc *CompareConfig) addRecords(recs models.Records, storeInExisting bool) {
@ -229,7 +239,7 @@ func (cc *CompareConfig) addRecords(recs models.Records, storeInExisting bool) {
key := rec.Key()
label := key.NameFQDN
rtype := key.Type
comp := mkCompareBlob(rec, cc.compareableFunc)
compNoTTL, compFull := mkCompareBlobs(rec, cc.compareableFunc)
// Are we seeing this label for the first time?
var labelIdx int
@ -267,11 +277,11 @@ func (cc *CompareConfig) addRecords(recs models.Records, storeInExisting bool) {
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,
targetConfig{compareable: comp, rec: rec})
targetConfig{comparableNoTTL: compNoTTL, comparableFull: compFull, rec: rec})
} else {
cc.ldata[labelIdx].tdata[rtIdx].desiredRecs = append(cc.ldata[labelIdx].tdata[rtIdx].desiredRecs, rec)
cc.ldata[labelIdx].tdata[rtIdx].desiredTargets = append(cc.ldata[labelIdx].tdata[rtIdx].desiredTargets,
targetConfig{compareable: comp, rec: rec})
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))

33
pkg/diff2/compareconfig_test.go
View File

@ -206,3 +206,36 @@ compFn: <nil>
})
}
}
func Test_mkCompareBlobs(t *testing.T) {
type args struct {
rc *models.RecordConfig
f func(*models.RecordConfig) string
}
tests := []struct {
name string
args args
want string
want1 string
}{
{
name: "simple",
args: args{
rc: makeRec("label", "A", "1.1.1.1"),
},
want: "1.1.1.1",
want1: "1.1.1.1 ttl=300",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, got1 := mkCompareBlobs(tt.args.rc, tt.args.f)
if got != tt.want {
t.Errorf("mkCompareBlobs() got = %q, want %q", got, tt.want)
}
if got1 != tt.want1 {
t.Errorf("mkCompareBlobs() got1 = %q, want %q", got1, tt.want1)
}
})
}
}

4
pkg/diff2/diff2.go
View File

@ -140,12 +140,10 @@ func ByRecord(existing models.Records, dc *models.DomainConfig, compFunc Compara
//
// Example usage:
//
// msgs, changes, err := diff2.ByZone(foundRecords, dc, nil)
//
// msgs, changes, err := diff2.ByZone(foundRecords, dc, nil)
// if err != nil {
// return nil, err
// }
//
// if changes {
// // Generate a "correction" that uploads the entire zone.
// // (dc.Records are the new records for the zone).

3
providers/namedotcom/records.go
View File

@ -105,10 +105,11 @@ func checkNSModifications(dc *models.DomainConfig) {
}
func toRecord(r *namecom.Record, origin string) *models.RecordConfig {
heapr := r // NB(tlim): Unsure if this is actually needed.
rc := &models.RecordConfig{
Type: r.Type,
TTL: r.TTL,
Original: r,
Original: heapr,
}
if !strings.HasSuffix(r.Fqdn, ".") {
panic(fmt.Errorf("namedotcom suddenly changed protocol. Bailing. (%v)", r.Fqdn))