CLOUDFLARE: BUGFIX: Proxy=full not handled in diff2 (#2525)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

integrationTest/integration_test.go

@@ -35,6 +35,18 @@ func init() {
 	flag.Parse()
 }
 
+// Helper constants/funcs for the CLOUDFLARE proxy testing:
+
+func CF_PROXY_OFF() *TestCase   { return tc("proxyoff", cfProxyA("prxy", "174.136.107.111", "off")) }
+func CF_PROXY_ON() *TestCase    { return tc("proxyon", cfProxyA("prxy", "174.136.107.111", "on")) }
+func CF_PROXY_FULL1() *TestCase { return tc("proxyf1", cfProxyA("prxy", "174.136.107.111", "full")) }
+func CF_PROXY_FULL2() *TestCase { return tc("proxyf2", cfProxyA("prxy", "174.136.107.222", "full")) }
+func CF_CPROXY_OFF() *TestCase  { return tc("cproxyoff", cfProxyCNAME("cproxy", "example.com.", "off")) }
+func CF_CPROXY_ON() *TestCase   { return tc("cproxyon", cfProxyCNAME("cproxy", "example.com.", "on")) }
+func CF_CPROXY_FULL() *TestCase { return tc("cproxyf", cfProxyCNAME("cproxy", "example.com.", "full")) }
+
+// ---
+
 func getProvider(t *testing.T) (providers.DNSServiceProvider, string, map[int]bool, map[string]string) {
 	if *providerToRun == "" {
 		t.Log("No provider specified with -provider")
@@ -1673,7 +1685,7 @@ func makeTests(t *testing.T) []*TestGroup {
 			tc("change", cfRedir("cnn.**current-domain-no-trailing**/*", "https://change.cnn.com/$1")),
 			tc("changelabel", cfRedir("cable.**current-domain-no-trailing**/*", "https://change.cnn.com/$1")),
 
-			// Removed these for speed.  They were testing if order matters,
+			// Removed these for speed.  They tested if order matters,
 			// which it doesn't seem to.  Re-add if needed.
 			//clear(),
 			//tc("multipleA",
@@ -1702,7 +1714,7 @@ func makeTests(t *testing.T) []*TestGroup {
 			//	cfRedir("nytimes.**current-domain-no-trailing**/*", "https://www.nytimes.com/$1"),
 			//),
 
-			// Repeat the above using CF_TEMP_REDIR instead
+			// Repeat the above tests using CF_TEMP_REDIR instead
 			clear(),
 			tc("tempredir", cfRedirTemp("cnn.**current-domain-no-trailing**/*", "https://www.cnn.com/$1")),
 			tc("tempchange", cfRedirTemp("cnn.**current-domain-no-trailing**/*", "https://change.cnn.com/$1")),
@@ -1726,7 +1738,6 @@ func makeTests(t *testing.T) []*TestGroup {
 				cfRedirTemp("cablenews.**current-domain-no-trailing**/*", "https://change.cnn.com/$1"),
 			),
 			// TODO(tlim): Fix this test case:
-			//clear(),
 			//tc("tempmultiple3",
 			//	cfRedirTemp("msnbc.**current-domain-no-trailing**/*", "https://msnbc.cnn.com/$1"),
 			//	cfRedirTemp("cnn.**current-domain-no-trailing**/*", "https://www.cnn.com/$1"),
@@ -1734,18 +1745,76 @@ func makeTests(t *testing.T) []*TestGroup {
 			//),
 		),
 
-		testgroup("CF_PROXY",
+		testgroup("CF_PROXY A create",
 			only("CLOUDFLAREAPI"),
-			tc("proxyon", cfProxyA("proxyme", "1.2.3.4", "on")),
+			CF_PROXY_OFF(), clear(),
-			tc("proxychangetarget", cfProxyA("proxyme", "1.2.3.5", "on")),
+			CF_PROXY_ON(), clear(),
-			tc("proxychangeonoff", cfProxyA("proxyme", "1.2.3.5", "off")),
+			CF_PROXY_FULL1(), clear(),
-			tc("proxychangeoffon", cfProxyA("proxyme", "1.2.3.5", "on")),
+			CF_PROXY_FULL2(), clear(),
-			clear(),
+		),
-			tc("proxycname", cfProxyCNAME("anewproxy", "example.com.", "on")),
+
-			tc("proxycnamechange", cfProxyCNAME("anewproxy", "example.com.", "off")),
+		// These next testgroups attempt every possible transition between off, on, full1 and full2.
-			tc("proxycnameoffon", cfProxyCNAME("anewproxy", "example.com.", "on")),
+		// "full1" simulates "full" without the IP being translated.
-			tc("proxycnameonoff", cfProxyCNAME("anewproxy", "example.com.", "off")),
+		// "full2" simulates "full" WITH the IP translated.
-			clear(),
+
+		testgroup("CF_PROXY A off to X",
+			only("CLOUDFLAREAPI"),
+			//CF_PROXY_OFF(), CF_PROXY_OFF(), clear(), // redundant
+			CF_PROXY_OFF(), CF_PROXY_ON(), clear(),
+			CF_PROXY_OFF(), CF_PROXY_FULL1(), clear(),
+			CF_PROXY_OFF(), CF_PROXY_FULL2(), clear(),
+		),
+
+		testgroup("CF_PROXY A on to X",
+			only("CLOUDFLAREAPI"),
+			CF_PROXY_ON(), CF_PROXY_OFF(), clear(),
+			//CF_PROXY_ON(), CF_PROXY_ON(), clear(), // redundant
+			//CF_PROXY_ON(), CF_PROXY_FULL1().ExpectNoChanges(), clear(), // Removed for speed
+			CF_PROXY_ON(), CF_PROXY_FULL2(), clear(),
+		),
+
+		testgroup("CF_PROXY A full1 to X",
+			only("CLOUDFLAREAPI"),
+			CF_PROXY_FULL1(), CF_PROXY_OFF(), clear(),
+			//CF_PROXY_FULL1(), CF_PROXY_ON().ExpectNoChanges(), clear(), // Removed for speed
+			//CF_PROXY_FULL1(), CF_PROXY_FULL1(), clear(), // redundant
+			CF_PROXY_FULL1(), CF_PROXY_FULL2(), clear(),
+		),
+
+		testgroup("CF_PROXY A full2 to X",
+			only("CLOUDFLAREAPI"),
+			CF_PROXY_FULL2(), CF_PROXY_OFF(), clear(),
+			CF_PROXY_FULL2(), CF_PROXY_ON(), clear(),
+			CF_PROXY_FULL2(), CF_PROXY_FULL1(), clear(),
+			//CF_PROXY_FULL2(), CF_PROXY_FULL2(), clear(), // redundant
+		),
+
+		testgroup("CF_PROXY CNAME create",
+			only("CLOUDFLAREAPI"),
+			CF_CPROXY_OFF(), clear(),
+			CF_CPROXY_ON(), clear(),
+			CF_CPROXY_FULL(), clear(),
+		),
+
+		testgroup("CF_PROXY CNAME off to X",
+			only("CLOUDFLAREAPI"),
+			//CF_CPROXY_OFF(), CF_CPROXY_OFF(), clear(),  // redundant
+			CF_CPROXY_OFF(), CF_CPROXY_ON(), clear(),
+			CF_CPROXY_OFF(), CF_CPROXY_FULL(), clear(),
+		),
+
+		testgroup("CF_PROXY CNAME on to X",
+			only("CLOUDFLAREAPI"),
+			CF_CPROXY_ON(), CF_CPROXY_OFF(), clear(),
+			//CF_CPROXY_ON(), CF_CPROXY_ON(), clear(), // redundant
+			//CF_CPROXY_ON(), CF_CPROXY_FULL().ExpectNoChanges(), clear(), // Removed for speed
+		),
+
+		testgroup("CF_PROXY CNAME full to X",
+			only("CLOUDFLAREAPI"),
+			CF_CPROXY_FULL(), CF_CPROXY_OFF(), clear(),
+			//CF_CPROXY_FULL(), CF_CPROXY_ON().ExpectNoChanges(), clear(), // Removed for speed
+			//CF_CPROXY_FULL(), CF_CPROXY_FULL(), clear(), // redundant
 		),
 
 		testgroup("CF_WORKER_ROUTE",

pkg/diff2/analyze.go

@@ -236,6 +236,8 @@ func humanDiff(a, b targetConfig) string {
 
 func diffTargets(existing, desired []targetConfig) ChangeList {
 
+	//fmt.Printf("DEBUG: diffTargets(\nexisting=%v\ndesired=%v\nDEBUG.\n", existing, desired)
+
 	// Nothing to do?
 	if len(existing) == 0 && len(desired) == 0 {
 		return nil

providers/cloudflare/cloudflareProvider.go

@@ -3,12 +3,13 @@ package cloudflare
 import (
 	"encoding/json"
 	"fmt"
-	"golang.org/x/net/idna"
 	"net"
 	"os"
 	"strconv"
 	"strings"
 
+	"golang.org/x/net/idna"
+
 	"github.com/StackExchange/dnscontrol/v4/models"
 	"github.com/StackExchange/dnscontrol/v4/pkg/diff"
 	"github.com/StackExchange/dnscontrol/v4/pkg/diff2"
@@ -389,7 +390,7 @@ func genComparable(rec *models.RecordConfig) string {
 	if rec.Type == "A" || rec.Type == "AAAA" || rec.Type == "CNAME" {
 		proxy := rec.Metadata[metaProxy]
 		if proxy != "" {
-			if proxy == "on" {
+			if proxy == "on" || proxy == "full" {
 				proxy = "true"
 			}
 			if proxy == "off" {
@@ -449,6 +450,7 @@ func (c *cloudflareProvider) mkChangeCorrection(oldrec, newrec *models.RecordCon
 	default:
 		e := oldrec.Original.(cloudflare.DNSRecord)
 		proxy := e.Proxiable && newrec.Metadata[metaProxy] != "off"
+		//fmt.Fprintf(os.Stderr, "DEBUG: proxy := %v && %v != off is... %v\n", e.Proxiable, newrec.Metadata[metaProxy], proxy)
 		return []*models.Correction{{
 			Msg: msg,
 			F:   func() error { return c.modifyRecord(domainID, e.ID, proxy, newrec) },
@@ -636,6 +638,7 @@ func (c *cloudflareProvider) preprocessConfig(dc *models.DomainConfig) error {
 
 	// look for ip conversions and transform records
 	for _, rec := range dc.Records {
+		// Only transform A records
 		if rec.Type != "A" {
 			continue
 		}

providers/cloudflare/rest.go

@@ -3,10 +3,11 @@ package cloudflare
 import (
 	"context"
 	"fmt"
-	"golang.org/x/net/idna"
 	"strconv"
 	"strings"
 
+	"golang.org/x/net/idna"
+
 	"github.com/StackExchange/dnscontrol/v4/models"
 	"github.com/cloudflare/cloudflare-go"
 )
@@ -196,7 +197,7 @@ func (c *cloudflareProvider) createRecDiff2(rec *models.RecordConfig, domainID s
 	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" {
+	if rec.Metadata[metaProxy] == "on" || rec.Metadata[metaProxy] == "full" {
 		msg = msg + fmt.Sprintf("\nACTIVATE PROXY for new record %s %s %d %s", rec.GetLabel(), rec.Type, rec.TTL, rec.GetTargetField())
 	}
 	arr := []*models.Correction{{
@@ -232,7 +233,7 @@ func (c *cloudflareProvider) createRecDiff2(rec *models.RecordConfig, domainID s
 			// Records are created with the proxy off. If proxy should be
 			// enabled, we do a second API call.
 			resultID := resp.ID
-			if rec.Metadata[metaProxy] == "on" {
+			if rec.Metadata[metaProxy] == "on" || rec.Metadata[metaProxy] == "full" {
 				return c.modifyRecord(domainID, resultID, true, rec)
 			}
 			return nil