REFACTOR: diff2 should use .ComparableV3 nothing else

.github/workflows/pr_integration_tests.yml

@@ -295,6 +295,7 @@ jobs:
           echo "DO_WORKERS=$DO_WORKERS" >> $GITHUB_ENV
 
       - name: Run integration tests for ${{ matrix.provider }} provider
+        shell: bash -eo pipefail {0}
         run: |-
           #
           echo "END: Running tests 0 to $END"

integrationTest/helpers_integration_test.go

@@ -13,6 +13,7 @@ import (
 	"testing"
 	"time"
 
+	dnsv2 "codeberg.org/miekg/dns"
 	"github.com/DNSControl/dnscontrol/v4/models"
 	"github.com/DNSControl/dnscontrol/v4/pkg/domaintags"
 	"github.com/DNSControl/dnscontrol/v4/pkg/nameservers"
@@ -520,6 +521,24 @@ func https(name string, priority uint16, target string, params string) *models.R
 	r := makeRec(name, target, "HTTPS")
 	r.SvcPriority = priority
 	r.SvcParams = params
+
+	// Hack to set .RDATA without importing miekg/dns in pkg/rtypecontrol/fixlegacy.go
+	rty := dnsv2.TypeSVCB
+	cp := params
+	if strings.Contains(cp, "ech=IGNORE") {
+		cp = strings.ReplaceAll(cp, "ech=IGNORE", "")
+	}
+	rrv2, err := dnsv2.NewData(rty, fmt.Sprintf("%d %s %s", priority, target, cp))
+	if err != nil {
+		panic(fmt.Sprintf("could not parse SVCB record: %s (%d %s %s)", err, priority, target, cp))
+	}
+	r.RDATA = rrv2
+	old := r.RDATA.String()
+	r.ComparableV3 = r.RDATA.String()
+	if r.ComparableV3 != old {
+		panic("DEBUG CV3")
+	}
+
 	return r
 }
 
@@ -652,6 +671,16 @@ func svcb(name string, priority uint16, target string, params string) *models.Re
 	r := makeRec(name, target, "SVCB")
 	r.SvcPriority = priority
 	r.SvcParams = params
+
+	// Hack to set .RDATA without importing miekg/dns in pkg/rtypecontrol/fixlegacy.go
+	rty := dnsv2.TypeSVCB
+	rrv2, err := dnsv2.NewData(rty, fmt.Sprintf("%d %s %s", priority, target, params))
+	if err != nil {
+		panic(fmt.Sprintf("could not parse SVCB record: %s", err))
+	}
+	r.RDATA = rrv2
+	r.ComparableV3 = r.RDATA.String()
+
 	return r
 }
 

models/record.go

@@ -6,6 +6,7 @@ import (
 	"log"
 	"strings"
 
+	dnsv2 "codeberg.org/miekg/dns"
 	"github.com/DNSControl/dnscontrol/v4/pkg/txtutil"
 	"github.com/jinzhu/copier"
 	dnsv1 "github.com/miekg/dns"
@@ -16,9 +17,25 @@ import (
 // RecordConfig stores a DNS record whether it was created from data downloaded from
 // a provider's API ("actual") or from user input in dndsconfig.js ("desired").
 type RecordConfig struct {
+
 	// Type is the DNS record type (rtype), all caps, "A", "MX", etc.
 	Type string `json:"type"`
 
+	// TypeNum is the assigned number of the record's type. 1 for A, 5 for CNAME, etc. See dnsv2.TypeToString and dnsv2.StringToType.
+	// NB(tlim): Not currently used. Placeholder for future feature.
+	TypeNum uint16 `json:"typenum,omitempty"`
+
+	// RDATA is (the fields of the record).
+	// NB(tlim): Not currently used. Placeholder for future feature.
+	RDATA dnsv2.RDATA `json:"rdata,omitempty"`
+
+	// ComparableV3 is an opaque string that can be used to compare two
+	// RecordConfigs for equality. Typically this is the Zonefile line
+	// minus the label and TTL.
+	// The V3 distingues itself from .Comparable, which it will eventually replace.
+	// NB(tlim): Not currently used. Placeholder for future feature.
+	ComparableV3 string `json:"comparablev3,omitempty"`
+
 	// TTL is the DNS record's TTL in seconds. 0 means provider default.
 	TTL uint32 `json:"ttl,omitempty"`
 
@@ -353,6 +370,7 @@ func (rc *RecordConfig) ToComparableNoTTL() string {
 		return fmt.Sprintf("rtype=%s rdata=%s", rc.UnknownTypeName, rc.target)
 	case "HTTPS", "SVCB":
 		return rc.targetCombinedSVCBRaw()
+
 	}
 	return rc.GetTargetCombined()
 }

models/t_svcb.go

@@ -4,6 +4,9 @@ import (
 	"fmt"
 	"strings"
 
+	dnsv2 "codeberg.org/miekg/dns"
+	dnsrdatav2 "codeberg.org/miekg/dns/rdata"
+	svcbv2 "codeberg.org/miekg/dns/svcb"
 	dnsv1 "github.com/miekg/dns"
 )
 
@@ -31,6 +34,14 @@ func (rc *RecordConfig) SetTargetSVCB(priority uint16, target string, params []d
 	if rc.Type != "SVCB" && rc.Type != "HTTPS" {
 		panic("assertion failed: SetTargetSVCB called when .Type is not SVCB or HTTPS")
 	}
+
+	// Hack to set .RDATA without importing miekg/dns in pkg/rtypecontrol/fixlegacy.go
+	valuev2, err := convertSVCBv1v2(params)
+	if err != nil {
+		return fmt.Errorf("failed to convert SVCB parameters from v1 to v2: %w", err)
+	}
+	rc.RDATA = dnsrdatav2.SVCB{Priority: rc.SvcPriority, Target: target, Value: valuev2}
+
 	return nil
 }
 
@@ -43,6 +54,23 @@ func (rc *RecordConfig) SetTargetSVCBString(origin, contents string) error {
 	if err != nil {
 		return fmt.Errorf("could not parse SVCB record: %w", err)
 	}
+
+	// Hack to set .RDATA without importing miekg/dns in pkg/rtypecontrol/fixlegacy.go
+	var rty uint16
+	switch record.(type) {
+	case *dnsv1.HTTPS:
+		rty = dnsv1.TypeHTTPS
+	case *dnsv1.SVCB:
+		rty = dnsv1.TypeSVCB
+	default:
+		return fmt.Errorf("unexpected record type after parsing SVCB record: %T", record)
+	}
+	rrv2, err := dnsv2.NewData(rty, contents, origin)
+	if err != nil {
+		return fmt.Errorf("could not parse SVCB record: %w", err)
+	}
+	rc.RDATA = rrv2
+
 	switch r := record.(type) {
 	case *dnsv1.HTTPS:
 		return rc.SetTargetSVCB(r.Priority, r.Target, r.Value)
@@ -51,3 +79,29 @@ func (rc *RecordConfig) SetTargetSVCBString(origin, contents string) error {
 	}
 	return nil
 }
+
+func convertSVCBv1v2(params []dnsv1.SVCBKeyValue) ([]svcbv2.Pair, error) {
+	var value []svcbv2.Pair
+	for _, kv := range params {
+		k := kv.Key().String()
+		keyCode := svcbv2.StringToKey(k)
+		v := kv.String()
+
+		pairFn := svcbv2.KeyToPair(keyCode)
+		if pairFn == nil {
+			return nil, fmt.Errorf("failed to lookup svc key: %s", k)
+		}
+		pair := pairFn()
+		if svcbv2.PairToKey(pair) != keyCode {
+			return nil, fmt.Errorf("key constant is not in sync: %v", keyCode)
+		}
+		err := svcbv2.Parse(pair, v, "")
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse svc pair: %s", k)
+		}
+
+		value = append(value, pair)
+	}
+
+	return value, nil
+}

pkg/diff2/analyze_test.go

@@ -7,6 +7,8 @@ import (
 	"strings"
 	"testing"
 
+	dnsv2 "codeberg.org/miekg/dns"
+	dnsutilv2 "codeberg.org/miekg/dns/dnsutil"
 	"github.com/DNSControl/dnscontrol/v4/models"
 	"github.com/fatih/color"
 	"github.com/kylelemons/godebug/diff"
@@ -63,6 +65,21 @@ func makeRec(label, rtype, content string) *models.RecordConfig {
 	if err := r.PopulateFromString(rtype, content, origin); err != nil {
 		panic(err)
 	}
+
+	// Hack to set .RDATA without importing miekg/dns in pkg/rtypecontrol/fixlegacy.go
+	tn, err := dnsutilv2.StringToType(rtype)
+	if err != nil {
+		panic(fmt.Sprintf("BUG: HackFixRecord: %s IN %s %v", r.Name, r.Type, r))
+	}
+	r.TypeNum = tn
+	rrv2, err := dnsv2.NewData(tn, content, origin+".")
+	if err != nil {
+		panic(fmt.Sprintf("could not parse: %s IN %s %s: %s", r.Name, rtype, content, err))
+	}
+	r.RDATA = rrv2
+	r.ComparableV3 = r.RDATA.String()
+	// End of hack
+
 	return &r
 }
 

pkg/diff2/compareconfig.go

@@ -170,8 +170,12 @@ func (cc *CompareConfig) verifyCNAMEAssertions() {
 // Generate a string that can be used to compare this record to others
 // for equality.
 func mkCompareBlobs(rc *models.RecordConfig, f func(*models.RecordConfig) string) (string, string) {
-	// Start with the comparable string
-	comp := rc.ToComparableNoTTL()
+	// // Start with the comparable string
+	// comp := rc.ToComparableNoTTL()
+	comp := rc.ComparableV3
+	if comp == "" {
+		panic(fmt.Sprintf("mkCompareBlobs: record %s IN %s %s has empty ComparableV3", rc.NameFQDN, rc.Type, rc))
+	}
 
 	// If the custom function exists, add its output
 	if f != nil {

pkg/js/parse_tests/027-ds.json

@@ -33,6 +33,12 @@
           "name": "@",
           "name_raw": "@",
           "name_unicode": "@",
+          "rdata": {
+            "Algorithm": 1,
+            "Digest": "FFFF",
+            "DigestType": 1,
+            "KeyTag": 1
+          },
           "target": "",
           "ttl": 300,
           "type": "DS",
@@ -61,6 +67,12 @@
           "name": "@",
           "name_raw": "@",
           "name_unicode": "@",
+          "rdata": {
+            "Algorithm": 13,
+            "Digest": "AABBCCDDEEFF",
+            "DigestType": 2,
+            "KeyTag": 1000
+          },
           "target": "",
           "ttl": 300,
           "type": "DS",

pkg/js/parse_tests/059-rawttls.json

@@ -12,6 +12,7 @@
       "records": [
         {
           "comparable": "user.example.com. mytxt.example.com.",
+          "comparablev3": "user.example.com. mytxt.example.com.",
           "fields": {
             "Hdr": {
               "Class": 1,
@@ -27,13 +28,19 @@
           "name": "@",
           "name_raw": "@",
           "name_unicode": "@",
+          "rdata": {
+            "Mbox": "user.example.com.",
+            "Txt": "mytxt.example.com."
+          },
           "target": "",
           "ttl": 300,
           "type": "RP",
+          "typenum": 17,
           "zonfefilepartial": "user.example.com. mytxt.example.com."
         },
         {
           "comparable": "user2.example.com. mytxt.example.com.",
+          "comparablev3": "user2.example.com. mytxt.example.com.",
           "fields": {
             "Hdr": {
               "Class": 1,
@@ -49,13 +56,19 @@
           "name": "@",
           "name_raw": "@",
           "name_unicode": "@",
+          "rdata": {
+            "Mbox": "user2.example.com.",
+            "Txt": "mytxt.example.com."
+          },
           "target": "",
           "ttl": 300,
           "type": "RP",
+          "typenum": 17,
           "zonfefilepartial": "user2.example.com. mytxt.example.com."
         },
         {
           "comparable": "user.example.com. mytxt.example.com.",
+          "comparablev3": "user.example.com. mytxt.example.com.",
           "fields": {
             "Hdr": {
               "Class": 1,
@@ -71,13 +84,19 @@
           "name": "aaa300",
           "name_raw": "aaa300",
           "name_unicode": "aaa300",
+          "rdata": {
+            "Mbox": "user.example.com.",
+            "Txt": "mytxt.example.com."
+          },
           "target": "",
           "ttl": 300,
           "type": "RP",
+          "typenum": 17,
           "zonfefilepartial": "user.example.com. mytxt.example.com."
         },
         {
           "comparable": "user.example.com. mytxt.example.com.",
+          "comparablev3": "user.example.com. mytxt.example.com.",
           "fields": {
             "Hdr": {
               "Class": 1,
@@ -93,13 +112,19 @@
           "name": "bbb1",
           "name_raw": "bbb1",
           "name_unicode": "bbb1",
+          "rdata": {
+            "Mbox": "user.example.com.",
+            "Txt": "mytxt.example.com."
+          },
           "target": "",
           "ttl": 1111,
           "type": "RP",
+          "typenum": 17,
           "zonfefilepartial": "user.example.com. mytxt.example.com."
         },
         {
           "comparable": "user.example.com. mytxt.example.com.",
+          "comparablev3": "user.example.com. mytxt.example.com.",
           "fields": {
             "Hdr": {
               "Class": 1,
@@ -115,13 +140,19 @@
           "name": "ccc2",
           "name_raw": "ccc2",
           "name_unicode": "ccc2",
+          "rdata": {
+            "Mbox": "user.example.com.",
+            "Txt": "mytxt.example.com."
+          },
           "target": "",
           "ttl": 2222,
           "type": "RP",
+          "typenum": 17,
           "zonfefilepartial": "user.example.com. mytxt.example.com."
         },
         {
           "comparable": "user.example.com. mytxt.example.com.",
+          "comparablev3": "user.example.com. mytxt.example.com.",
           "fields": {
             "Hdr": {
               "Class": 1,
@@ -137,13 +168,19 @@
           "name": "ddd1",
           "name_raw": "ddd1",
           "name_unicode": "ddd1",
+          "rdata": {
+            "Mbox": "user.example.com.",
+            "Txt": "mytxt.example.com."
+          },
           "target": "",
           "ttl": 1111,
           "type": "RP",
+          "typenum": 17,
           "zonfefilepartial": "user.example.com. mytxt.example.com."
         },
         {
           "comparable": "user.example.com. mytxt.example.com.",
+          "comparablev3": "user.example.com. mytxt.example.com.",
           "fields": {
             "Hdr": {
               "Class": 1,
@@ -159,9 +196,14 @@
           "name": "eee3",
           "name_raw": "eee3",
           "name_unicode": "eee3",
+          "rdata": {
+            "Mbox": "user.example.com.",
+            "Txt": "mytxt.example.com."
+          },
           "target": "",
           "ttl": 3333,
           "type": "RP",
+          "typenum": 17,
           "zonfefilepartial": "user.example.com. mytxt.example.com."
         },
         {