From e1ce6ff34f9b1504bd06b1c881e15f4711535bc6 Mon Sep 17 00:00:00 2001 From: Tom Limoncelli Date: Wed, 1 Feb 2023 16:18:01 -0500 Subject: [PATCH] CLOUDFLARE: Adopt diff2 (#2040) --- integrationTest/integration_test.go | 5 +- pkg/diff2/analyze.go | 35 ++-- pkg/diff2/analyze_test.go | 124 +++++++------- pkg/diff2/compareconfig.go | 10 +- providers/cloudflare/cloudflareProvider.go | 184 +++++++++++++++++++-- providers/cloudflare/rest.go | 68 ++++++++ 6 files changed, 329 insertions(+), 97 deletions(-) diff --git a/integrationTest/integration_test.go b/integrationTest/integration_test.go index dfccd6e9c..119074b0d 100644 --- a/integrationTest/integration_test.go +++ b/integrationTest/integration_test.go @@ -1300,10 +1300,13 @@ func makeTests(t *testing.T) []*TestGroup { only("CLOUDFLAREAPI"), tc("proxyon", cfProxyA("proxyme", "1.2.3.4", "on")), tc("proxychangetarget", cfProxyA("proxyme", "1.2.3.5", "on")), - tc("proxychangeproxy", cfProxyA("proxyme", "1.2.3.5", "off")), + tc("proxychangeonoff", cfProxyA("proxyme", "1.2.3.5", "off")), + tc("proxychangeoffon", cfProxyA("proxyme", "1.2.3.5", "on")), clear(), tc("proxycname", cfProxyCNAME("anewproxy", "example.com.", "on")), tc("proxycnamechange", cfProxyCNAME("anewproxy", "example.com.", "off")), + tc("proxycnameoffon", cfProxyCNAME("anewproxy", "example.com.", "on")), + tc("proxycnameonoff", cfProxyCNAME("anewproxy", "example.com.", "off")), clear(), ), diff --git a/pkg/diff2/analyze.go b/pkg/diff2/analyze.go index 721f6bacd..6c142e7f3 100644 --- a/pkg/diff2/analyze.go +++ b/pkg/diff2/analyze.go @@ -204,10 +204,11 @@ func splitTTLOnly(existing, desired []targetConfig) ( for (ei < len(existing)) && (di < len(desired)) { er := existing[ei].rec dr := desired[di].rec - ecomp := er.GetTargetCombined() - dcomp := dr.GetTargetCombined() + ecomp := existing[ei].compareable + dcomp := desired[di].compareable 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!") } @@ -270,20 +271,22 @@ func filterBy(s []targetConfig, m map[string]*targetConfig) []targetConfig { return s } -func humanDiff(a, b *models.RecordConfig) string { - acombined := a.GetTargetCombined() - bcombined := b.GetTargetCombined() +func humanDiff(a, b targetConfig) string { + aTTL := a.rec.TTL + bTTL := b.rec.TTL + acombined := a.compareable + bcombined := b.compareable combinedDiff := acombined != bcombined - ttlDiff := a.TTL != b.TTL - // TODO(tlim): It would be nice if we included special cases for MX - // records and others. + 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, a.TTL, bcombined, b.TTL) + return fmt.Sprintf("(%s ttl=%d) -> (%s ttl=%d)", acombined, aTTL, bcombined, bTTL) } if combinedDiff { return fmt.Sprintf("(%s) -> (%s)", acombined, bcombined) } - return fmt.Sprintf("%s (ttl %d->%d)", acombined, a.TTL, b.TTL) + return fmt.Sprintf("%s (ttl %d->%d)", acombined, aTTL, bTTL) } func diffTargets(existing, desired []targetConfig) ChangeList { @@ -302,7 +305,11 @@ func diffTargets(existing, desired []targetConfig) ChangeList { 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) 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 @@ -314,7 +321,7 @@ func diffTargets(existing, desired []targetConfig) ChangeList { er := existingTTL[i] dr := desiredTTL[i] - m := fmt.Sprintf("CHANGE %s %s ", dr.NameFQDN, dr.Type) + humanDiff(er, dr) + m := fmt.Sprintf("CHANGE %s %s ", dr.NameFQDN, dr.Type) + humanDiff(existing[i], desired[i]) instructions = append(instructions, mkChange(dr.NameFQDN, dr.Type, []string{m}, models.Records{er}, @@ -330,7 +337,7 @@ func diffTargets(existing, desired []targetConfig) ChangeList { er := existing[i].rec dr := desired[i].rec - m := fmt.Sprintf("CHANGE %s %s ", dr.NameFQDN, dr.Type) + humanDiff(existing[i].rec, desired[i].rec) + m := fmt.Sprintf("CHANGE %s %s ", dr.NameFQDN, dr.Type) + humanDiff(existing[i], desired[i]) instructions = append(instructions, mkChange(dr.NameFQDN, dr.Type, []string{m}, models.Records{er}, @@ -342,7 +349,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, er.GetTargetCombined()) + m := fmt.Sprintf("DELETE %s %s %s", er.NameFQDN, er.Type, existing[i].compareable) instructions = append(instructions, mkDeleteRec(er.NameFQDN, er.Type, []string{m}, er)) } @@ -350,7 +357,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, dr.GetTargetCombined()) + m := fmt.Sprintf("CREATE %s %s %s", dr.NameFQDN, dr.Type, desired[i].compareable) instructions = append(instructions, mkAdd(dr.NameFQDN, dr.Type, []string{m}, models.Records{dr})) } diff --git a/pkg/diff2/analyze_test.go b/pkg/diff2/analyze_test.go index b0992377f..92dba0bdf 100644 --- a/pkg/diff2/analyze_test.go +++ b/pkg/diff2/analyze_test.go @@ -116,14 +116,14 @@ func Test_analyzeByRecordSet(t *testing.T) { existing: models.Records{testDataAA1234, testDataAMX10a}, desired: models.Records{testDataAA1234clone, testDataAMX20b}, }, - wantMsgs: "CHANGE laba.f.com MX (10 laba) -> (20 labb)", + wantMsgs: "CHANGE laba.f.com MX (10 laba ttl=300) -> (20 labb ttl=300)", wantChangeRSet: ` ChangeList: len=1 00: Change: verb=CHANGE key={laba.f.com MX} old=[10 laba] new=[20 labb] - msg=["CHANGE laba.f.com MX (10 laba) -> (20 labb)"] + msg=["CHANGE laba.f.com MX (10 laba ttl=300) -> (20 labb ttl=300)"] `, wantChangeLabel: ` ChangeList: len=1 @@ -131,7 +131,7 @@ ChangeList: len=1 key={laba.f.com } old=[1.2.3.4 10 laba] new=[1.2.3.4 20 labb] - msg=["CHANGE laba.f.com MX (10 laba) -> (20 labb)"] + msg=["CHANGE laba.f.com MX (10 laba ttl=300) -> (20 labb ttl=300)"] `, wantChangeRec: ` ChangeList: len=1 @@ -139,7 +139,7 @@ ChangeList: len=1 key={laba.f.com MX} old=[10 laba] new=[20 labb] - msg=["CHANGE laba.f.com MX (10 laba) -> (20 labb)"] + msg=["CHANGE laba.f.com MX (10 laba ttl=300) -> (20 labb ttl=300)"] `, }, @@ -150,14 +150,14 @@ ChangeList: len=1 existing: models.Records{testDataAA1234, testDataApexMX1aaa}, desired: models.Records{testDataAA1234clone, testDataApexMX22bbb}, }, - wantMsgs: "CHANGE f.com MX (1 aaa) -> (22 bbb)", + wantMsgs: "CHANGE f.com MX (1 aaa ttl=300) -> (22 bbb ttl=300)", wantChangeRSet: ` ChangeList: len=1 00: Change: verb=CHANGE key={f.com MX} old=[1 aaa] new=[22 bbb] - msg=["CHANGE f.com MX (1 aaa) -> (22 bbb)"] + msg=["CHANGE f.com MX (1 aaa ttl=300) -> (22 bbb ttl=300)"] `, wantChangeLabel: ` ChangeList: len=1 @@ -165,7 +165,7 @@ ChangeList: len=1 key={f.com } old=[1 aaa] new=[22 bbb] - msg=["CHANGE f.com MX (1 aaa) -> (22 bbb)"] + msg=["CHANGE f.com MX (1 aaa ttl=300) -> (22 bbb ttl=300)"] `, wantChangeRec: ` ChangeList: len=1 @@ -173,7 +173,7 @@ ChangeList: len=1 key={f.com MX} old=[1 aaa] new=[22 bbb] - msg=["CHANGE f.com MX (1 aaa) -> (22 bbb)"] + msg=["CHANGE f.com MX (1 aaa ttl=300) -> (22 bbb ttl=300)"] `, }, @@ -185,8 +185,8 @@ ChangeList: len=1 desired: models.Records{testDataAA1234clone, testDataAA12345, testDataAMX20b}, }, wantMsgs: ` -CREATE laba.f.com A 1.2.3.5 -CHANGE laba.f.com MX (10 laba) -> (20 labb) +CREATE laba.f.com A 1.2.3.5 ttl=300 +CHANGE laba.f.com MX (10 laba ttl=300) -> (20 labb ttl=300) `, wantChangeRSet: ` ChangeList: len=2 @@ -194,12 +194,12 @@ ChangeList: len=2 key={laba.f.com A} old=[1.2.3.4] new=[1.2.3.4 1.2.3.5] - msg=["CREATE laba.f.com A 1.2.3.5"] + msg=["CREATE laba.f.com A 1.2.3.5 ttl=300"] 01: Change: verb=CHANGE key={laba.f.com MX} old=[10 laba] new=[20 labb] - msg=["CHANGE laba.f.com MX (10 laba) -> (20 labb)"] + msg=["CHANGE laba.f.com MX (10 laba ttl=300) -> (20 labb ttl=300)"] `, wantChangeLabel: ` ChangeList: len=1 @@ -207,19 +207,19 @@ ChangeList: len=1 key={laba.f.com } old=[1.2.3.4 10 laba] new=[1.2.3.4 1.2.3.5 20 labb] - msg=["CREATE laba.f.com A 1.2.3.5" "CHANGE laba.f.com MX (10 laba) -> (20 labb)"] + msg=["CREATE laba.f.com A 1.2.3.5 ttl=300" "CHANGE laba.f.com MX (10 laba ttl=300) -> (20 labb ttl=300)"] `, wantChangeRec: ` ChangeList: len=2 00: Change: verb=CREATE key={laba.f.com A} new=[1.2.3.5] - msg=["CREATE laba.f.com A 1.2.3.5"] + msg=["CREATE laba.f.com A 1.2.3.5 ttl=300"] 01: Change: verb=CHANGE key={laba.f.com MX} old=[10 laba] new=[20 labb] - msg=["CHANGE laba.f.com MX (10 laba) -> (20 labb)"] + msg=["CHANGE laba.f.com MX (10 laba ttl=300) -> (20 labb ttl=300)"] `, }, @@ -231,18 +231,18 @@ ChangeList: len=2 desired: desired, }, wantMsgs: ` -CREATE laba.f.com A 1.2.3.5 -CHANGE laba.f.com MX (10 laba) -> (20 labb) -DELETE labc.f.com CNAME laba -CHANGE labe.f.com A (10.10.10.15) -> (10.10.10.95) -CHANGE labe.f.com A (10.10.10.16) -> (10.10.10.96) -CHANGE labe.f.com A (10.10.10.17) -> (10.10.10.97) -CHANGE labe.f.com A (10.10.10.18) -> (10.10.10.98) -CREATE labf.f.com TXT "foo" -CHANGE labg.f.com NS (10.10.10.17) -> (10.10.10.10) -CHANGE labg.f.com NS (10.10.10.18) -> (10.10.10.97) -DELETE labh.f.com CNAME labd -CREATE labh.f.com A 1.2.3.4 +CREATE laba.f.com A 1.2.3.5 ttl=300 +CHANGE laba.f.com MX (10 laba ttl=300) -> (20 labb ttl=300) +DELETE labc.f.com CNAME laba ttl=300 +CHANGE labe.f.com A (10.10.10.15 ttl=300) -> (10.10.10.95 ttl=300) +CHANGE labe.f.com A (10.10.10.16 ttl=300) -> (10.10.10.96 ttl=300) +CHANGE labe.f.com A (10.10.10.17 ttl=300) -> (10.10.10.97 ttl=300) +CHANGE labe.f.com A (10.10.10.18 ttl=300) -> (10.10.10.98 ttl=300) +CREATE labf.f.com TXT "foo" ttl=300 +CHANGE labg.f.com NS (10.10.10.17 ttl=300) -> (10.10.10.10 ttl=300) +CHANGE labg.f.com NS (10.10.10.18 ttl=300) -> (10.10.10.97 ttl=300) +DELETE labh.f.com CNAME labd ttl=300 +CREATE labh.f.com A 1.2.3.4 ttl=300 `, wantChangeRSet: ` ChangeList: len=8 @@ -250,38 +250,38 @@ ChangeList: len=8 key={laba.f.com A} old=[1.2.3.4] new=[1.2.3.4 1.2.3.5] - msg=["CREATE laba.f.com A 1.2.3.5"] + msg=["CREATE laba.f.com A 1.2.3.5 ttl=300"] 01: Change: verb=CHANGE key={laba.f.com MX} old=[10 laba] new=[20 labb] - msg=["CHANGE laba.f.com MX (10 laba) -> (20 labb)"] + msg=["CHANGE laba.f.com MX (10 laba ttl=300) -> (20 labb ttl=300)"] 02: Change: verb=DELETE key={labc.f.com CNAME} old=[laba] - msg=["DELETE labc.f.com CNAME laba"] + msg=["DELETE labc.f.com CNAME laba ttl=300"] 03: Change: verb=CHANGE key={labe.f.com A} old=[10.10.10.15 10.10.10.16 10.10.10.17 10.10.10.18] new=[10.10.10.95 10.10.10.96 10.10.10.97 10.10.10.98] - msg=["CHANGE labe.f.com A (10.10.10.15) -> (10.10.10.95)" "CHANGE labe.f.com A (10.10.10.16) -> (10.10.10.96)" "CHANGE labe.f.com A (10.10.10.17) -> (10.10.10.97)" "CHANGE labe.f.com A (10.10.10.18) -> (10.10.10.98)"] + msg=["CHANGE labe.f.com A (10.10.10.15 ttl=300) -> (10.10.10.95 ttl=300)" "CHANGE labe.f.com A (10.10.10.16 ttl=300) -> (10.10.10.96 ttl=300)" "CHANGE labe.f.com A (10.10.10.17 ttl=300) -> (10.10.10.97 ttl=300)" "CHANGE labe.f.com A (10.10.10.18 ttl=300) -> (10.10.10.98 ttl=300)"] 04: Change: verb=CREATE key={labf.f.com TXT} new=["foo"] - msg=["CREATE labf.f.com TXT \"foo\""] + msg=["CREATE labf.f.com TXT \"foo\" ttl=300"] 05: Change: verb=CHANGE key={labg.f.com NS} old=[10.10.10.15 10.10.10.16 10.10.10.17 10.10.10.18] new=[10.10.10.10 10.10.10.15 10.10.10.16 10.10.10.97] - msg=["CHANGE labg.f.com NS (10.10.10.17) -> (10.10.10.10)" "CHANGE labg.f.com NS (10.10.10.18) -> (10.10.10.97)"] + msg=["CHANGE labg.f.com NS (10.10.10.17 ttl=300) -> (10.10.10.10 ttl=300)" "CHANGE labg.f.com NS (10.10.10.18 ttl=300) -> (10.10.10.97 ttl=300)"] 06: Change: verb=DELETE key={labh.f.com CNAME} old=[labd] - msg=["DELETE labh.f.com CNAME labd"] + msg=["DELETE labh.f.com CNAME labd ttl=300"] 07: Change: verb=CREATE key={labh.f.com A} new=[1.2.3.4] - msg=["CREATE labh.f.com A 1.2.3.4"] + msg=["CREATE labh.f.com A 1.2.3.4 ttl=300"] `, wantChangeLabel: ` ChangeList: len=6 @@ -289,88 +289,88 @@ ChangeList: len=6 key={laba.f.com } old=[1.2.3.4 10 laba] new=[1.2.3.4 1.2.3.5 20 labb] - msg=["CREATE laba.f.com A 1.2.3.5" "CHANGE laba.f.com MX (10 laba) -> (20 labb)"] + msg=["CREATE laba.f.com A 1.2.3.5 ttl=300" "CHANGE laba.f.com MX (10 laba ttl=300) -> (20 labb ttl=300)"] 01: Change: verb=DELETE key={labc.f.com } old=[laba] - msg=["DELETE labc.f.com CNAME laba"] + msg=["DELETE labc.f.com CNAME laba ttl=300"] 02: Change: verb=CHANGE key={labe.f.com } old=[10.10.10.15 10.10.10.16 10.10.10.17 10.10.10.18] new=[10.10.10.95 10.10.10.96 10.10.10.97 10.10.10.98] - msg=["CHANGE labe.f.com A (10.10.10.15) -> (10.10.10.95)" "CHANGE labe.f.com A (10.10.10.16) -> (10.10.10.96)" "CHANGE labe.f.com A (10.10.10.17) -> (10.10.10.97)" "CHANGE labe.f.com A (10.10.10.18) -> (10.10.10.98)"] + msg=["CHANGE labe.f.com A (10.10.10.15 ttl=300) -> (10.10.10.95 ttl=300)" "CHANGE labe.f.com A (10.10.10.16 ttl=300) -> (10.10.10.96 ttl=300)" "CHANGE labe.f.com A (10.10.10.17 ttl=300) -> (10.10.10.97 ttl=300)" "CHANGE labe.f.com A (10.10.10.18 ttl=300) -> (10.10.10.98 ttl=300)"] 03: Change: verb=CREATE key={labf.f.com } new=["foo"] - msg=["CREATE labf.f.com TXT \"foo\""] + msg=["CREATE labf.f.com TXT \"foo\" ttl=300"] 04: Change: verb=CHANGE key={labg.f.com } old=[10.10.10.15 10.10.10.16 10.10.10.17 10.10.10.18] new=[10.10.10.10 10.10.10.15 10.10.10.16 10.10.10.97] - msg=["CHANGE labg.f.com NS (10.10.10.17) -> (10.10.10.10)" "CHANGE labg.f.com NS (10.10.10.18) -> (10.10.10.97)"] + msg=["CHANGE labg.f.com NS (10.10.10.17 ttl=300) -> (10.10.10.10 ttl=300)" "CHANGE labg.f.com NS (10.10.10.18 ttl=300) -> (10.10.10.97 ttl=300)"] 05: Change: verb=CHANGE key={labh.f.com } old=[labd] new=[1.2.3.4] - msg=["DELETE labh.f.com CNAME labd" "CREATE labh.f.com A 1.2.3.4"] + msg=["DELETE labh.f.com CNAME labd ttl=300" "CREATE labh.f.com A 1.2.3.4 ttl=300"] `, wantChangeRec: ` ChangeList: len=12 00: Change: verb=CREATE key={laba.f.com A} new=[1.2.3.5] - msg=["CREATE laba.f.com A 1.2.3.5"] + msg=["CREATE laba.f.com A 1.2.3.5 ttl=300"] 01: Change: verb=CHANGE key={laba.f.com MX} old=[10 laba] new=[20 labb] - msg=["CHANGE laba.f.com MX (10 laba) -> (20 labb)"] + msg=["CHANGE laba.f.com MX (10 laba ttl=300) -> (20 labb ttl=300)"] 02: Change: verb=DELETE key={labc.f.com CNAME} old=[laba] - msg=["DELETE labc.f.com CNAME laba"] + msg=["DELETE labc.f.com CNAME laba ttl=300"] 03: Change: verb=CHANGE key={labe.f.com A} old=[10.10.10.15] new=[10.10.10.95] - msg=["CHANGE labe.f.com A (10.10.10.15) -> (10.10.10.95)"] + msg=["CHANGE labe.f.com A (10.10.10.15 ttl=300) -> (10.10.10.95 ttl=300)"] 04: Change: verb=CHANGE key={labe.f.com A} old=[10.10.10.16] new=[10.10.10.96] - msg=["CHANGE labe.f.com A (10.10.10.16) -> (10.10.10.96)"] + msg=["CHANGE labe.f.com A (10.10.10.16 ttl=300) -> (10.10.10.96 ttl=300)"] 05: Change: verb=CHANGE key={labe.f.com A} old=[10.10.10.17] new=[10.10.10.97] - msg=["CHANGE labe.f.com A (10.10.10.17) -> (10.10.10.97)"] + msg=["CHANGE labe.f.com A (10.10.10.17 ttl=300) -> (10.10.10.97 ttl=300)"] 06: Change: verb=CHANGE key={labe.f.com A} old=[10.10.10.18] new=[10.10.10.98] - msg=["CHANGE labe.f.com A (10.10.10.18) -> (10.10.10.98)"] + msg=["CHANGE labe.f.com A (10.10.10.18 ttl=300) -> (10.10.10.98 ttl=300)"] 07: Change: verb=CREATE key={labf.f.com TXT} new=["foo"] - msg=["CREATE labf.f.com TXT \"foo\""] + msg=["CREATE labf.f.com TXT \"foo\" ttl=300"] 08: Change: verb=CHANGE key={labg.f.com NS} old=[10.10.10.17] new=[10.10.10.10] - msg=["CHANGE labg.f.com NS (10.10.10.17) -> (10.10.10.10)"] + msg=["CHANGE labg.f.com NS (10.10.10.17 ttl=300) -> (10.10.10.10 ttl=300)"] 09: Change: verb=CHANGE key={labg.f.com NS} old=[10.10.10.18] new=[10.10.10.97] - msg=["CHANGE labg.f.com NS (10.10.10.18) -> (10.10.10.97)"] + msg=["CHANGE labg.f.com NS (10.10.10.18 ttl=300) -> (10.10.10.97 ttl=300)"] 10: Change: verb=DELETE key={labh.f.com CNAME} old=[labd] - msg=["DELETE labh.f.com CNAME labd"] + msg=["DELETE labh.f.com CNAME labd ttl=300"] 11: Change: verb=CREATE key={labh.f.com A} new=[1.2.3.4] - msg=["CREATE labh.f.com A 1.2.3.4"] + msg=["CREATE labh.f.com A 1.2.3.4 ttl=300"] `, }, } @@ -407,7 +407,7 @@ func mkTargetConfig(x ...*models.RecordConfig) []targetConfig { var tc []targetConfig for _, r := range x { tc = append(tc, targetConfig{ - compareable: comparable(r, nil), + compareable: mkCompareBlob(r, nil), rec: r, }) } @@ -445,8 +445,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->700)", - "CREATE laba.f.com A 1.2.3.4", + "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", }, }, }, @@ -471,7 +471,7 @@ func Test_diffTargets(t *testing.T) { Change{Type: CREATE, Key: models.RecordKey{NameFQDN: "laba.f.com", Type: "MX"}, New: models.Records{testDataAMX10a}, - Msgs: []string{"CREATE laba.f.com MX 10 laba"}, + Msgs: []string{"CREATE laba.f.com MX 10 laba ttl=300"}, }, }, }, @@ -486,7 +486,7 @@ func Test_diffTargets(t *testing.T) { Change{Type: DELETE, Key: models.RecordKey{NameFQDN: "laba.f.com", Type: "MX"}, Old: models.Records{testDataAMX10a}, - Msgs: []string{"DELETE laba.f.com MX 10 laba"}, + Msgs: []string{"DELETE laba.f.com MX 10 laba ttl=300"}, }, }, }, @@ -502,7 +502,7 @@ func Test_diffTargets(t *testing.T) { Key: models.RecordKey{NameFQDN: "laba.f.com", Type: "MX"}, Old: models.Records{testDataAMX10a}, New: models.Records{testDataAMX20b}, - Msgs: []string{"CHANGE laba.f.com MX (10 laba) -> (20 labb)"}, + Msgs: []string{"CHANGE laba.f.com MX (10 laba ttl=300) -> (20 labb ttl=300)"}, }, }, }, @@ -518,7 +518,7 @@ func Test_diffTargets(t *testing.T) { Key: models.RecordKey{NameFQDN: "laba.f.com", Type: "A"}, Old: models.Records{testDataAA1234, testDataAA5678}, New: models.Records{testDataAA1234}, - Msgs: []string{"DELETE laba.f.com A 5.6.7.8"}, + Msgs: []string{"DELETE laba.f.com A 5.6.7.8 ttl=300"}, }, }, }, @@ -656,8 +656,8 @@ func Test_splitTTLOnly(t *testing.T) { existing: mkTargetConfig(testDataAA1234), desired: mkTargetConfig(testDataAA1234ttl700), }, - wantExistDiff: mkTargetConfig(), - wantDesireDiff: mkTargetConfig(), + wantExistDiff: mkTargetConfig(testDataAA1234), + wantDesireDiff: mkTargetConfig(testDataAA1234ttl700), wantExistTTL: models.Records{testDataAA1234}, wantDesireTTL: models.Records{testDataAA1234ttl700}, }, diff --git a/pkg/diff2/compareconfig.go b/pkg/diff2/compareconfig.go index 3835d973a..fc381d6e9 100644 --- a/pkg/diff2/compareconfig.go +++ b/pkg/diff2/compareconfig.go @@ -191,11 +191,15 @@ func (cc *CompareConfig) String() string { // Generate a string that can be used to compare this record to others // for equality. -func comparable(rc *models.RecordConfig, f func(*models.RecordConfig) string) string { +func mkCompareBlob(rc *models.RecordConfig, f func(*models.RecordConfig) string) string { if f == nil { return rc.ToDiffable() } - return rc.ToDiffable() + " " + f(rc) + addOn := f(rc) + if addOn != "" { + return rc.ToDiffable() + " " + f(rc) + } + return rc.ToDiffable() } func (cc *CompareConfig) addRecords(recs models.Records, storeInExisting bool) { @@ -213,7 +217,7 @@ func (cc *CompareConfig) addRecords(recs models.Records, storeInExisting bool) { label := rec.NameFQDN rtype := rec.Type - comp := comparable(rec, cc.compareableFunc) + comp := mkCompareBlob(rec, cc.compareableFunc) // Are we seeing this label for the first time? var labelIdx int diff --git a/providers/cloudflare/cloudflareProvider.go b/providers/cloudflare/cloudflareProvider.go index 400599321..3f3458a00 100644 --- a/providers/cloudflare/cloudflareProvider.go +++ b/providers/cloudflare/cloudflareProvider.go @@ -154,11 +154,11 @@ func (c *cloudflareProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*m return nil, err } - id, err := c.getDomainID(dc.Name) + domainID, err := c.getDomainID(dc.Name) if err != nil { return nil, err } - records, err := c.getRecordsForDomain(id, dc.Name) + records, err := c.getRecordsForDomain(domainID, dc.Name) if err != nil { return nil, err } @@ -176,7 +176,7 @@ func (c *cloudflareProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*m } if c.manageRedirects { - prs, err := c.getPageRules(id, dc.Name) + prs, err := c.getPageRules(domainID, dc.Name) //printer.Printf("GET PAGE RULES:\n") //for i, p := range prs { // printer.Printf("%03d: %q\n", i, p.GetTargetField()) @@ -188,7 +188,7 @@ func (c *cloudflareProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*m } if c.manageWorkers { - wrs, err := c.getWorkerRoutes(id, dc.Name) + wrs, err := c.getWorkerRoutes(domainID, dc.Name) if err != nil { return nil, err } @@ -224,7 +224,7 @@ func (c *cloudflareProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*m // one string in the first element of .TxtStrings. var corrections []*models.Correction - if !diff2.EnableDiff2 || true { // Remove "|| true" when diff2 version arrives + if !diff2.EnableDiff2 { differ := diff.New(dc, getProxyMetadata) _, create, del, mod, err := differ.IncrementalDiff(records) @@ -239,15 +239,15 @@ func (c *cloudflareProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*m if ex.Type == "PAGE_RULE" { corrections = append(corrections, &models.Correction{ Msg: d.String(), - F: func() error { return c.deletePageRule(ex.Original.(cloudflare.PageRule).ID, id) }, + F: func() error { return c.deletePageRule(ex.Original.(cloudflare.PageRule).ID, domainID) }, }) } else if ex.Type == "WORKER_ROUTE" { corrections = append(corrections, &models.Correction{ Msg: d.String(), - F: func() error { return c.deleteWorkerRoute(ex.Original.(cloudflare.WorkerRoute).ID, id) }, + F: func() error { return c.deleteWorkerRoute(ex.Original.(cloudflare.WorkerRoute).ID, domainID) }, }) } else { - corr := c.deleteRec(ex.Original.(cloudflare.DNSRecord), id) + corr := c.deleteRec(ex.Original.(cloudflare.DNSRecord), domainID) // DS records must always have a corresponding NS record. // Therefore, we remove DS records before any NS records. if d.Existing.Type == "DS" { @@ -262,15 +262,15 @@ func (c *cloudflareProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*m if des.Type == "PAGE_RULE" { corrections = append(corrections, &models.Correction{ Msg: d.String(), - F: func() error { return c.createPageRule(id, des.GetTargetField()) }, + F: func() error { return c.createPageRule(domainID, des.GetTargetField()) }, }) } else if des.Type == "WORKER_ROUTE" { corrections = append(corrections, &models.Correction{ Msg: d.String(), - F: func() error { return c.createWorkerRoute(id, des.GetTargetField()) }, + F: func() error { return c.createWorkerRoute(domainID, des.GetTargetField()) }, }) } else { - corr := c.createRec(des, id) + corr := c.createRec(des, domainID) // DS records must always have a corresponding NS record. // Therefore, we create NS records before any DS records. if d.Desired.Type == "NS" { @@ -287,13 +287,15 @@ func (c *cloudflareProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*m if rec.Type == "PAGE_RULE" { corrections = append(corrections, &models.Correction{ Msg: d.String(), - F: func() error { return c.updatePageRule(ex.Original.(cloudflare.PageRule).ID, id, rec.GetTargetField()) }, + F: func() error { + return c.updatePageRule(ex.Original.(cloudflare.PageRule).ID, domainID, rec.GetTargetField()) + }, }) } else if rec.Type == "WORKER_ROUTE" { corrections = append(corrections, &models.Correction{ Msg: d.String(), F: func() error { - return c.updateWorkerRoute(ex.Original.(cloudflare.WorkerRoute).ID, id, rec.GetTargetField()) + return c.updateWorkerRoute(ex.Original.(cloudflare.WorkerRoute).ID, domainID, rec.GetTargetField()) }, }) } else { @@ -301,13 +303,13 @@ func (c *cloudflareProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*m proxy := e.Proxiable && rec.Metadata[metaProxy] != "off" corrections = append(corrections, &models.Correction{ Msg: d.String(), - F: func() error { return c.modifyRecord(id, e.ID, proxy, rec) }, + F: func() error { return c.modifyRecord(domainID, e.ID, proxy, rec) }, }) } } // Add universalSSL change to corrections when needed - if changed, newState, err := c.checkUniversalSSL(dc, id); err == nil && changed { + if changed, newState, err := c.checkUniversalSSL(dc, domainID); err == nil && changed { var newStateString string if newState { newStateString = "enabled" @@ -316,17 +318,154 @@ func (c *cloudflareProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*m } corrections = append(corrections, &models.Correction{ Msg: fmt.Sprintf("Universal SSL will be %s for this domain.", newStateString), - F: func() error { return c.changeUniversalSSL(id, newState) }, + F: func() error { return c.changeUniversalSSL(domainID, newState) }, }) } return corrections, nil } - // Insert Future diff2 version here. + // Cloudflare is a "ByRecord" API. + instructions, err := diff2.ByRecord(records, dc, genComparable) + if err != nil { + return nil, err + } + + for _, inst := range instructions { + + addToFront := false + var corrs []*models.Correction + + domainID := domainID + msg := inst.Msgs[0] + + switch inst.Type { + case diff2.CREATE: + createRec := inst.New[0] + corrs = c.mkCreateCorrection(createRec, domainID, msg) + // DS records must always have a corresponding NS record. + // Therefore, we create NS records before any DS records. + addToFront = createRec.Type == "NS" + case diff2.CHANGE: + newrec := inst.New[0] + oldrec := inst.Old[0] + corrs = c.mkChangeCorrection(oldrec, newrec, domainID, msg) + case diff2.DELETE: + deleteRec := inst.Old[0] + deleteRecType := deleteRec.Type + deleteRecOrig := deleteRec.Original + corrs = c.mkDeleteCorrection(deleteRecType, deleteRecOrig, domainID, msg) + // DS records must always have a corresponding NS record. + // Therefore, we remove DS records before any NS records. + addToFront = deleteRecType == "DS" + } + + if addToFront { + corrections = append(corrs, corrections...) + } else { + corrections = append(corrections, corrs...) + } + } + + // Add universalSSL change when needed + if changed, newState, err := c.checkUniversalSSL(dc, domainID); err == nil && changed { + var newStateString string + if newState { + newStateString = "enabled" + } else { + newStateString = "disabled" + } + corrections = append(corrections, &models.Correction{ + Msg: fmt.Sprintf("Universal SSL will be %s for this domain.", newStateString), + F: func() error { return c.changeUniversalSSL(domainID, newState) }, + }) + } return corrections, nil +} +func genComparable(rec *models.RecordConfig) string { + //fmt.Printf("DEBUG: genComparable called %v:%v meta=%+v\n", rec.Type, rec.GetLabel(), rec.Metadata) + if rec.Type == "A" || rec.Type == "AAAA" || rec.Type == "CNAME" { + proxy := rec.Metadata[metaProxy] + if proxy != "" { + //return "proxy=" + rec.Metadata[metaProxy] + return "proxy=" + proxy + //return fmt.Sprintf("proxy:%v=%s", rec.Type, proxy) + } + } + return "" +} + +func (c *cloudflareProvider) mkCreateCorrection(newrec *models.RecordConfig, domainID, msg string) []*models.Correction { + switch newrec.Type { + case "PAGE_RULE": + return []*models.Correction{{ + Msg: msg, + F: func() error { return c.createPageRule(domainID, newrec.GetTargetField()) }, + }} + case "WORKER_ROUTE": + return []*models.Correction{{ + Msg: msg, + F: func() error { return c.createWorkerRoute(domainID, newrec.GetTargetField()) }, + }} + default: + return c.createRecDiff2(newrec, domainID, msg) + } +} + +func (c *cloudflareProvider) mkChangeCorrection(oldrec, newrec *models.RecordConfig, domainID string, msg string) []*models.Correction { + switch newrec.Type { + case "PAGE_RULE": + return []*models.Correction{{ + Msg: msg, + F: func() error { + return c.updatePageRule(oldrec.Original.(cloudflare.PageRule).ID, domainID, newrec.GetTargetField()) + }, + }} + case "WORKER_ROUTE": + return []*models.Correction{{ + Msg: msg, + F: func() error { + return c.updateWorkerRoute(oldrec.Original.(cloudflare.WorkerRoute).ID, domainID, newrec.GetTargetField()) + }, + }} + default: + e := oldrec.Original.(cloudflare.DNSRecord) + proxy := e.Proxiable && newrec.Metadata[metaProxy] != "off" + return []*models.Correction{{ + Msg: msg, + F: func() error { return c.modifyRecord(domainID, e.ID, proxy, newrec) }, + }} + } +} + +func (c *cloudflareProvider) mkDeleteCorrection(recType string, origRec any, domainID string, msg string) []*models.Correction { + var idTxt string + switch recType { + case "PAGE_RULE": + idTxt = origRec.(cloudflare.PageRule).ID + case "WORKER_ROUTE": + idTxt = origRec.(cloudflare.WorkerRoute).ID + default: + idTxt = origRec.(cloudflare.DNSRecord).ID + } + msg = msg + fmt.Sprintf(" id=%v", idTxt) + + correction := &models.Correction{ + Msg: msg, + F: func() error { + switch recType { + case "PAGE_RULE": + return c.deletePageRule(origRec.(cloudflare.PageRule).ID, domainID) + case "WORKER_ROUTE": + return c.deleteWorkerRoute(origRec.(cloudflare.WorkerRoute).ID, domainID) + default: + return c.deleteDNSRecord(origRec.(cloudflare.DNSRecord), domainID) + } + }, + } + return []*models.Correction{correction} } func checkNSModifications(dc *models.DomainConfig) { @@ -669,6 +808,7 @@ func (c *cloudflareProvider) nativeToRecord(domain string, cr cloudflare.DNSReco rc := &models.RecordConfig{ TTL: uint32(cr.TTL), Original: cr, + Metadata: map[string]string{}, } rc.SetLabelFromFQDN(cr.Name, domain) @@ -677,6 +817,16 @@ func (c *cloudflareProvider) nativeToRecord(domain string, cr cloudflare.DNSReco cr.Type = "TXT" } + if cr.Type == "A" || cr.Type == "AAAA" || cr.Type == "CNAME" { + if cr.Proxied != nil { + if *(cr.Proxied) { + rc.Metadata[metaProxy] = "on" + } else { + rc.Metadata[metaProxy] = "off" + } + } + } + switch rType := cr.Type; rType { // #rtype_variations case "MX": if err := rc.SetTargetMX(*cr.Priority, cr.Content); err != nil { diff --git a/providers/cloudflare/rest.go b/providers/cloudflare/rest.go index 8c0bfdf3a..08f642fb0 100644 --- a/providers/cloudflare/rest.go +++ b/providers/cloudflare/rest.go @@ -44,6 +44,10 @@ func (c *cloudflareProvider) getRecordsForDomain(id string, domain string) ([]*m return records, nil } +func (c *cloudflareProvider) deleteDNSRecord(rec cloudflare.DNSRecord, domainID string) error { + return c.cfClient.DeleteDNSRecord(context.Background(), domainID, rec.ID) +} + // create a correction to delete a record func (c *cloudflareProvider) deleteRec(rec cloudflare.DNSRecord, domainID string) *models.Correction { return &models.Correction{ @@ -168,6 +172,70 @@ func (c *cloudflareProvider) createRec(rec *models.RecordConfig, domainID string return arr } +func (c *cloudflareProvider) createRecDiff2(rec *models.RecordConfig, domainID string, msg string) []*models.Correction { + + content := rec.GetTargetField() + if rec.Metadata[metaOriginalIP] != "" { + content = rec.Metadata[metaOriginalIP] + } + prio := "" + if rec.Type == "MX" { + prio = fmt.Sprintf(" %d ", rec.MxPreference) + } + if rec.Type == "TXT" { + content = rec.GetTargetTXTJoined() + } + if rec.Type == "DS" { + content = fmt.Sprintf("%d %d %d %s", rec.DsKeyTag, rec.DsAlgorithm, rec.DsDigestType, rec.DsDigest) + } + if msg == "" { + msg = fmt.Sprintf("CREATE record: %s %s %d%s %s", rec.GetLabel(), rec.Type, rec.TTL, prio, content) + } + if rec.Metadata[metaProxy] == "on" { + msg = msg + fmt.Sprintf("\nACTIVATE PROXY for new record %s %s %d %s", rec.GetLabel(), rec.Type, rec.TTL, rec.GetTargetField()) + } + arr := []*models.Correction{{ + Msg: msg, + F: func() error { + cf := cloudflare.DNSRecord{ + Name: rec.GetLabel(), + Type: rec.Type, + TTL: int(rec.TTL), + Content: content, + Priority: &rec.MxPreference, + } + if rec.Type == "SRV" { + cf.Data = cfSrvData(rec) + cf.Name = rec.GetLabelFQDN() + } else if rec.Type == "CAA" { + cf.Data = cfCaaData(rec) + cf.Name = rec.GetLabelFQDN() + cf.Content = "" + } else if rec.Type == "TLSA" { + cf.Data = cfTlsaData(rec) + cf.Name = rec.GetLabelFQDN() + } else if rec.Type == "SSHFP" { + cf.Data = cfSshfpData(rec) + cf.Name = rec.GetLabelFQDN() + } else if rec.Type == "DS" { + cf.Data = cfDSData(rec) + } + resp, err := c.cfClient.CreateDNSRecord(context.Background(), domainID, cf) + if err != nil { + return err + } + // Records are created with the proxy off. If proxy should be + // enabled, we do a second API call. + resultID := resp.Result.ID + if rec.Metadata[metaProxy] == "on" { + return c.modifyRecord(domainID, resultID, true, rec) + } + return nil + }, + }} + return arr +} + func (c *cloudflareProvider) modifyRecord(domainID, recID string, proxied bool, rec *models.RecordConfig) error { if domainID == "" || recID == "" { return fmt.Errorf("cannot modify record if domain or record id are empty")