From 779898cc9e502627a33a3b57003ffe258351b1ed Mon Sep 17 00:00:00 2001 From: Shuvam Kumar Date: Sun, 2 Aug 2026 02:28:23 +0530 Subject: [PATCH 01/10] TEST: Add golden-file tests for provider record conversion Implements the harness requested in #4622: record a provider's native API data once, replay it through the provider's conversion functions in "go test", and compare the result with a checked-in golden file. No credentials, no network. Following the decisions in https://github.com/DNSControl/dnscontrol/issues/4622#issuecomment-5153224304: 1. Based on release_candidate_v5. 2. A golden line is LineString() plus the metadata, with the TTL and the space after it omitted when the TTL is zero. The format is built in the harness rather than by calling models.LineString(), so that LineString() keeps its current output. 3. -update rewrites only the golden files. The recorded inputs are never rewritten, so refreshing a golden never reaches for an API token. Gathering inputs from a live account stays a separate step. 5. Runs in "go test". 6. One data file per provider per function tested, named after the provider in lower case. Providers are enrolled one at a time by adding a short adapter that maps the provider's local signature onto a uniform one. A provider with no recorded data is skipped rather than failed, so the providers that are not enrolled yet stay green. Enrolled here: websupport (toRecordConfig, toNative), packetframe (toRc, toReq) and porkbun (toRc, toReq). websupport's goldens are checked against the expectations in its existing hand-written convert_test.go; packetframe and porkbun had no test file of any kind. The recorded inputs are written from each provider's own native struct and the shapes its converter parses, not captured from a live account, so a provider author with credentials should expect to swap them out. Point 4 (autogold) is read as applying to the pkg/js/parse_tests conversion, which is a separate PR, so -update is implemented here directly and adds no dependency. Co-Authored-By: Claude Opus 5 --- documentation/SUMMARY.md | 1 + .../provider-conversion-tests.md | 105 ++++++ pkg/providergolden/providergolden.go | 324 ++++++++++++++++++ pkg/providergolden/providergolden_test.go | 190 ++++++++++ providers/packetframe/convert_golden_test.go | 23 ++ .../testdata/packetframe_torc.golden | 9 + .../testdata/packetframe_torc.json | 83 +++++ .../testdata/packetframe_toreq.golden | 83 +++++ .../testdata/packetframe_toreq.records | 9 + providers/porkbun/convert_golden_test.go | 20 ++ .../porkbun/testdata/porkbun_torc.golden | 13 + providers/porkbun/testdata/porkbun_torc.json | 106 ++++++ .../porkbun/testdata/porkbun_toreq.golden | 96 ++++++ .../porkbun/testdata/porkbun_toreq.records | 15 + providers/websupport/convert_golden_test.go | 20 ++ .../testdata/websupport_tonative.golden | 42 +++ .../testdata/websupport_tonative.records | 6 + .../testdata/websupport_torecordconfig.golden | 6 + .../testdata/websupport_torecordconfig.json | 48 +++ 19 files changed, 1199 insertions(+) create mode 100644 documentation/developer-info/provider-conversion-tests.md create mode 100644 pkg/providergolden/providergolden.go create mode 100644 pkg/providergolden/providergolden_test.go create mode 100644 providers/packetframe/convert_golden_test.go create mode 100644 providers/packetframe/testdata/packetframe_torc.golden create mode 100644 providers/packetframe/testdata/packetframe_torc.json create mode 100644 providers/packetframe/testdata/packetframe_toreq.golden create mode 100644 providers/packetframe/testdata/packetframe_toreq.records create mode 100644 providers/porkbun/convert_golden_test.go create mode 100644 providers/porkbun/testdata/porkbun_torc.golden create mode 100644 providers/porkbun/testdata/porkbun_torc.json create mode 100644 providers/porkbun/testdata/porkbun_toreq.golden create mode 100644 providers/porkbun/testdata/porkbun_toreq.records create mode 100644 providers/websupport/convert_golden_test.go create mode 100644 providers/websupport/testdata/websupport_tonative.golden create mode 100644 providers/websupport/testdata/websupport_tonative.records create mode 100644 providers/websupport/testdata/websupport_torecordconfig.golden create mode 100644 providers/websupport/testdata/websupport_torecordconfig.json diff --git a/documentation/SUMMARY.md b/documentation/SUMMARY.md index ac0930b087..2e4943967d 100644 --- a/documentation/SUMMARY.md +++ b/documentation/SUMMARY.md @@ -222,6 +222,7 @@ * [Writing new DNS providers](advanced-features/writing-providers.md) * [Creating new DNS Resource Types (rtypes)](advanced-features/adding-new-rtypes.md) * [Integration Tests](advanced-features/integration-tests.md) +* [Provider conversion tests](developer-info/provider-conversion-tests.md) * [Test a branch](advanced-features/test-a-branch.md) * [Unit Testing DNS Data](advanced-features/unittests.md) * [Bug Triage Process](advanced-features/bug-triage.md) diff --git a/documentation/developer-info/provider-conversion-tests.md b/documentation/developer-info/provider-conversion-tests.md new file mode 100644 index 0000000000..f850a0c77f --- /dev/null +++ b/documentation/developer-info/provider-conversion-tests.md @@ -0,0 +1,105 @@ +# Provider conversion tests + +Most providers convert between their API's native record format and +`models.RecordConfig`. Those conversion functions are only exercised by the +integration tests, which need credentials and a test zone. When a provider's +author becomes unavailable, nobody can run them. + +`pkg/providergolden` replays recorded data through those functions and compares +the result with a golden file. The recorded data is checked in, so the tests run +in `go test ./...` with no credentials and no network. + +Providers are enrolled one at a time. A provider with no recorded data is +skipped, never failed. + +## Enrolling a provider + +### 1. Record the data + +Collect the native records your provider's API returns for a test zone and store +them as a JSON array in `providers//testdata/`. The file name starts +with the provider's name in lower case and continues with the function under +test: + +```json +[ + { + "content": "192.0.2.1", + "id": 42, + "name": "www.example.com", + "ttl": 3600, + "type": "A" + } +] +``` + +{% hint style="warning" %} +Record from a throwaway zone. Native records carry labels, addresses, and record +and zone IDs verbatim, which for a LAN appliance means your internal DNS ends up +in a public repository. +{% endhint %} + +### 2. Add the test + +The test adapts your conversion function to a uniform signature. That adapter is +the only code you write: + +```go +func TestToRecordConfigGolden(t *testing.T) { + providergolden.CheckToRC(t, "websupport_torecordconfig", "example.com", + func(dc *models.DomainConfig, native nativeRecord) ([]*models.RecordConfig, error) { + rc, err := toRecordConfig(dc, native) + return []*models.RecordConfig{rc}, err + }) +} +``` + +Use `CheckToNative` for the other direction: `toNative`, `toReq`, +`recordToCreateRequest`, or whatever your provider calls it. Its input is a list +of DNS records rather than native records, so it reads a `.records` file: + +```go +func TestToReqGolden(t *testing.T) { + providergolden.CheckToNative(t, "porkbun_toreq", "example.com", toReq) +} +``` + +The `.golden` file produced by `CheckToRC` is a valid `.records` file, so the +usual way to write one is to copy it and add whatever else you want covered. + +### 3. Generate the golden files + +```shell +go test ./providers/websupport/ -update +``` + +Read the generated files before committing them. `-update` records whatever the +code does today, including bugs. + +## The golden format + +One line per record: the label, TTL, class, type and RDATA, with the TTL omitted +when it is zero, followed by the record's metadata when it has any: + +``` +www 300 IN A 192.0.2.1 +@ IN MX 10 mail.example.com. +fwd IN URL https://example.net/landing ; includePath="no" type="temporary" wildcard="no" +``` + +`CheckToNative` writes its golden as JSON, since a native record has no +zonefile representation. + +## Updating + +Run `go test ./providers// -update` after an intentional change and +review the diff. `-update` rewrites only the golden files: the recorded inputs +are never touched, so updating a golden never needs an API key. + +## What these tests do and do not catch + +They pin a provider's conversion functions against their own past behaviour, so +they catch a refactor that changes what a provider sends or understands. They do +not catch a conversion that has been wrong since the day it was written, and +they say nothing about the correction and apply path. That is still worth having +for a provider whose integration tests nobody can run. diff --git a/pkg/providergolden/providergolden.go b/pkg/providergolden/providergolden.go new file mode 100644 index 0000000000..a5289a923f --- /dev/null +++ b/pkg/providergolden/providergolden.go @@ -0,0 +1,324 @@ +// Package providergolden replays recorded provider data through a provider's +// record conversion functions and compares the result with a golden file. +// +// A provider is enrolled by adding one small test per conversion function. The +// test names the recorded data and adapts the provider's function to a uniform +// signature: +// +// func TestToRecordConfig(t *testing.T) { +// providergolden.CheckToRC(t, "websupport_torecordconfig", "example.com", +// func(dc *models.DomainConfig, n nativeRecord) ([]*models.RecordConfig, error) { +// rc, err := toRecordConfig(dc, n) +// return []*models.RecordConfig{rc}, err +// }) +// } +// +// The data lives in the provider's testdata directory and is named after the +// provider and the function under test: +// +// testdata/.json native records, as the provider's API returns them +// testdata/.records DNS records, in the golden line format below +// testdata/.golden the expected output +// +// CheckToRC reads the .json file, CheckToNative reads the .records file, and +// both write the .golden file. A provider with no recorded data is skipped, so +// providers can be enrolled one at a time. +// +// "go test -update" rewrites the golden files from the recorded +// inputs. It never contacts a provider's API and never rewrites an input file, +// so gathering data from a live account stays a separate, explicit step. +// +// A golden line is the record's label, TTL, class, type and RDATA, with the TTL +// omitted when it is zero, followed by the metadata when the record has any: +// +// www 300 IN A 192.0.2.1 +// @ IN MX 10 mail.example.com. +// fwd IN URL https://example.net/landing ; includePath="no" type="temporary" wildcard="no" +package providergolden + +import ( + "encoding/json" + "errors" + "flag" + "fmt" + "io/fs" + "maps" + "os" + "path/filepath" + "slices" + "strconv" + "strings" + "testing" + + "github.com/DNSControl/dnscontrol/v5/models" + "github.com/google/go-cmp/cmp" +) + +var update = flag.Bool("update", false, "rewrite the provider conversion golden files") + +const testdataDir = "testdata" + +// CheckToRC replays the native records recorded in testdata/.json through +// convert and compares the records it returns with testdata/.golden. +func CheckToRC[N any](t *testing.T, name, domain string, convert func(dc *models.DomainConfig, native N) ([]*models.RecordConfig, error)) { + t.Helper() + + data, ok, err := loadInput(testdataDir, name+".json") + if err != nil { + t.Fatal(err) + } + if !ok { + t.Skipf("%s has no recorded data yet", name) + } + + var natives []N + if err := json.Unmarshal(data, &natives); err != nil { + t.Fatalf("%s.json: %v", name, err) + } + + dc := models.MustNewDomainConfig(domain) + var b strings.Builder + for i, native := range natives { + recs, err := convert(dc, native) + if err != nil { + t.Fatalf("%s.json: record %d: %v", name, i, err) + } + for _, rc := range recs { + if rc == nil { + continue + } + b.WriteString(formatRecord(rc)) + b.WriteByte('\n') + } + } + + report(t, testdataDir, name, []byte(b.String())) +} + +// CheckToNative replays the records recorded in testdata/.records through +// convert and compares the native records it returns with testdata/.golden. +func CheckToNative[N any](t *testing.T, name, domain string, convert func(rc *models.RecordConfig) (N, error)) { + t.Helper() + + data, ok, err := loadInput(testdataDir, name+".records") + if err != nil { + t.Fatal(err) + } + if !ok { + t.Skipf("%s has no recorded data yet", name) + } + + recs, err := parseRecords(models.MustNewDomainConfig(domain), string(data)) + if err != nil { + t.Fatalf("%s.records: %v", name, err) + } + + natives := make([]N, 0, len(recs)) + for i, rc := range recs { + native, err := convert(rc) + if err != nil { + t.Fatalf("%s.records: record %d: %v", name, i, err) + } + natives = append(natives, native) + } + + got, err := json.MarshalIndent(natives, "", " ") + if err != nil { + t.Fatal(err) + } + + report(t, testdataDir, name, append(got, '\n')) +} + +// loadInput reads a recorded input file. ok is false when the file does not +// exist, which means the provider has not been enrolled yet. +func loadInput(dir, filename string) (data []byte, ok bool, err error) { + data, err = os.ReadFile(filepath.Join(dir, filename)) + if errors.Is(err, fs.ErrNotExist) { + return nil, false, nil + } + if err != nil { + return nil, false, err + } + return data, true, nil +} + +// report compares got with the golden file, or rewrites the golden file when +// -update was given. +func report(t *testing.T, dir, name string, got []byte) { + t.Helper() + + skip, diff, err := compareGolden(dir, name, got, *update) + switch { + case err != nil: + t.Fatal(err) + case skip != "": + t.Skip(skip) + case diff != "": + t.Errorf("%s.golden does not match the conversion (-want +got):\n%s", name, diff) + } +} + +// compareGolden compares got with /.golden, or writes got to it when +// update is true. It returns a reason to skip when the golden file does not +// exist, and a diff when the contents differ. +func compareGolden(dir, name string, got []byte, update bool) (skip, diff string, err error) { + path := filepath.Join(dir, name+".golden") + + if update { + if err := os.MkdirAll(dir, 0o755); err != nil { + return "", "", err + } + return "", "", os.WriteFile(path, got, 0o644) + } + + want, err := os.ReadFile(path) + if errors.Is(err, fs.ErrNotExist) { + return fmt.Sprintf("%s does not exist: run \"go test . -update\" to record it", path), "", nil + } + if err != nil { + return "", "", err + } + + return "", cmp.Diff(strings.Split(string(want), "\n"), strings.Split(string(got), "\n")), nil +} + +// formatRecord renders rc as one line of a golden file. +func formatRecord(rc *models.RecordConfig) string { + var b strings.Builder + + b.WriteString(rc.Name) + b.WriteByte(' ') + if rc.TTL != 0 { + b.WriteString(strconv.FormatUint(uint64(rc.TTL), 10)) + b.WriteByte(' ') + } + b.WriteString("IN ") + b.WriteString(rc.Type) + b.WriteByte(' ') + b.WriteString(rc.GetRDATA().String()) + + if len(rc.Metadata) != 0 { + b.WriteString(" ;") + for _, k := range slices.Sorted(maps.Keys(rc.Metadata)) { + fmt.Fprintf(&b, " %s=%s", k, strconv.Quote(rc.Metadata[k])) + } + } + + return b.String() +} + +// parseRecords parses the golden line format. Blank lines are ignored. +func parseRecords(dc *models.DomainConfig, text string) ([]*models.RecordConfig, error) { + var recs []*models.RecordConfig + for i, line := range strings.Split(text, "\n") { + if strings.TrimSpace(line) == "" { + continue + } + rc, err := parseRecord(dc, line) + if err != nil { + return nil, fmt.Errorf("line %d: %w", i+1, err) + } + recs = append(recs, rc) + } + return recs, nil +} + +func parseRecord(dc *models.DomainConfig, line string) (*models.RecordConfig, error) { + line, metatext := cutMetadata(line) + + name, rest, ok := strings.Cut(line, " ") + if !ok { + return nil, fmt.Errorf("%q: expected \"label [ttl] IN type rdata\"", line) + } + + var ttl uint64 + if head, tail, ok := strings.Cut(rest, " "); ok { + if n, err := strconv.ParseUint(head, 10, 32); err == nil { + ttl, rest = n, tail + } + } + + class, rest, ok := strings.Cut(rest, " ") + if !ok || class != "IN" { + return nil, fmt.Errorf("%q: expected class \"IN\"", line) + } + + rtype, rdata, ok := strings.Cut(rest, " ") + if !ok { + return nil, fmt.Errorf("%q: expected rdata after the type", line) + } + + rc, err := dc.NewRecordConfigParse(name, uint32(ttl), rtype, rdata) + if err != nil { + return nil, err + } + + metadata, err := parseMetadata(metatext) + if err != nil { + return nil, fmt.Errorf("%q: %w", line, err) + } + if len(metadata) != 0 { + rc.Metadata = metadata + } + + return rc, nil +} + +// cutMetadata splits a line at the first semicolon that is not inside a quoted +// string. +func cutMetadata(line string) (record, metadata string) { + quoted := false + for i := 0; i < len(line); i++ { + switch line[i] { + case '\\': + i++ + case '"': + quoted = !quoted + case ';': + if !quoted { + return strings.TrimRight(line[:i], " "), line[i+1:] + } + } + } + return line, "" +} + +// parseMetadata parses a sequence of key="value" pairs. +func parseMetadata(s string) (map[string]string, error) { + metadata := map[string]string{} + s = strings.TrimLeft(s, " ") + for s != "" { + key, rest, ok := strings.Cut(s, "=") + if !ok { + return nil, fmt.Errorf("metadata %q: expected key=\"value\"", s) + } + value, rest, err := cutQuoted(rest) + if err != nil { + return nil, err + } + metadata[key] = value + s = strings.TrimLeft(rest, " ") + } + return metadata, nil +} + +// cutQuoted removes a Go-quoted string from the front of s. +func cutQuoted(s string) (value, rest string, err error) { + if !strings.HasPrefix(s, `"`) { + return "", "", fmt.Errorf("metadata %q: expected a quoted value", s) + } + for i := 1; i < len(s); i++ { + switch s[i] { + case '\\': + i++ + case '"': + value, err := strconv.Unquote(s[:i+1]) + if err != nil { + return "", "", fmt.Errorf("metadata %q: %w", s, err) + } + return value, s[i+1:], nil + } + } + return "", "", fmt.Errorf("metadata %q: unterminated quoted value", s) +} diff --git a/pkg/providergolden/providergolden_test.go b/pkg/providergolden/providergolden_test.go new file mode 100644 index 0000000000..de40143617 --- /dev/null +++ b/pkg/providergolden/providergolden_test.go @@ -0,0 +1,190 @@ +package providergolden + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/DNSControl/dnscontrol/v5/models" +) + +func TestFormatRecord(t *testing.T) { + dc := models.MustNewDomainConfig("example.com") + + tests := []struct { + name string + rc *models.RecordConfig + metadata map[string]string + want string + }{ + { + name: "A", + rc: dc.MustNewRecordConfig("www", 300, "A", "192.0.2.1"), + want: "www 300 IN A 192.0.2.1", + }, + { + name: "zero TTL is omitted", + rc: dc.MustNewRecordConfig("www", 0, "A", "192.0.2.1"), + want: "www IN A 192.0.2.1", + }, + { + name: "MX at the apex", + rc: dc.MustNewRecordConfig("@", 3600, "MX", 10, "mail.example.com."), + want: "@ 3600 IN MX 10 mail.example.com.", + }, + { + name: "metadata is sorted and quoted", + rc: dc.MustNewRecordConfig("fwd", 300, "A", "192.0.2.1"), + metadata: map[string]string{"wildcard": "no", "includePath": "yes"}, + want: `fwd 300 IN A 192.0.2.1 ; includePath="yes" wildcard="no"`, + }, + { + name: "metadata value with a space and a quote", + rc: dc.MustNewRecordConfig("fwd", 300, "A", "192.0.2.1"), + metadata: map[string]string{"note": `a "b" c`}, + want: `fwd 300 IN A 192.0.2.1 ; note="a \"b\" c"`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.metadata != nil { + tt.rc.Metadata = tt.metadata + } + if got := formatRecord(tt.rc); got != tt.want { + t.Errorf("formatRecord() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestParseRecordsRoundTrip(t *testing.T) { + input := strings.Join([]string{ + "@ 3600 IN A 192.0.2.1", + "www IN A 192.0.2.2", + "@ 3600 IN MX 10 mail.example.com.", + `@ 600 IN TXT "v=spf1 include:_spf.example.net -all"`, + `@ 600 IN CAA 0 issue "letsencrypt.org"`, + "_sip._tcp 300 IN SRV 10 20 5060 sip.example.com.", + `fwd 300 IN A 192.0.2.3 ; includePath="yes" wildcard="no"`, + }, "\n") + "\n" + + recs, err := parseRecords(models.MustNewDomainConfig("example.com"), input) + if err != nil { + t.Fatalf("parseRecords() error: %v", err) + } + + var got strings.Builder + for _, rc := range recs { + got.WriteString(formatRecord(rc)) + got.WriteByte('\n') + } + if got.String() != input { + t.Errorf("round trip produced:\n%s\nwant:\n%s", got.String(), input) + } +} + +func TestParseRecordsMetadata(t *testing.T) { + recs, err := parseRecords(models.MustNewDomainConfig("example.com"), + `fwd 300 IN A 192.0.2.1 ; includePath="yes" note="a; b" wildcard="no"`) + if err != nil { + t.Fatalf("parseRecords() error: %v", err) + } + if len(recs) != 1 { + t.Fatalf("parseRecords() returned %d records, want 1", len(recs)) + } + + want := map[string]string{"includePath": "yes", "note": "a; b", "wildcard": "no"} + for k, v := range want { + if got := recs[0].Metadata[k]; got != v { + t.Errorf("Metadata[%q] = %q, want %q", k, got, v) + } + } +} + +func TestParseRecordsRejectsMalformedInput(t *testing.T) { + tests := []struct { + name string + input string + }{ + {name: "no class", input: "www 300 A 192.0.2.1"}, + {name: "no rdata", input: "www 300 IN A"}, + {name: "unknown type", input: "www 300 IN NOSUCHTYPE 192.0.2.1"}, + {name: "unterminated metadata", input: `www 300 IN A 192.0.2.1 ; wildcard="no`}, + {name: "unquoted metadata", input: "www 300 IN A 192.0.2.1 ; wildcard=no"}, + } + + dc := models.MustNewDomainConfig("example.com") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if _, err := parseRecords(dc, tt.input); err == nil { + t.Errorf("parseRecords(%q) succeeded, want an error", tt.input) + } + }) + } +} + +func TestLoadInputReportsMissingFileAsNotEnrolled(t *testing.T) { + _, ok, err := loadInput(t.TempDir(), "absent.json") + if err != nil { + t.Fatalf("loadInput() error: %v", err) + } + if ok { + t.Error("loadInput() ok = true for a file that does not exist") + } +} + +func TestCompareGoldenSkipsWhenTheGoldenFileIsMissing(t *testing.T) { + skip, diff, err := compareGolden(t.TempDir(), "absent", []byte("anything\n"), false) + if err != nil { + t.Fatalf("compareGolden() error: %v", err) + } + if skip == "" { + t.Error("compareGolden() returned no skip reason for a missing golden file") + } + if diff != "" { + t.Errorf("compareGolden() diff = %q, want empty", diff) + } +} + +func TestCompareGoldenDetectsADifference(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "x.golden"), []byte("www 300 IN A 192.0.2.1\n"), 0o644); err != nil { + t.Fatal(err) + } + + skip, diff, err := compareGolden(dir, "x", []byte("www 300 IN A 192.0.2.99\n"), false) + if err != nil { + t.Fatalf("compareGolden() error: %v", err) + } + if skip != "" { + t.Errorf("compareGolden() skip = %q, want empty", skip) + } + if diff == "" { + t.Error("compareGolden() reported no difference between 192.0.2.1 and 192.0.2.99") + } + + if _, diff, err = compareGolden(dir, "x", []byte("www 300 IN A 192.0.2.1\n"), false); err != nil { + t.Fatalf("compareGolden() error: %v", err) + } else if diff != "" { + t.Errorf("compareGolden() diff = %q for identical content, want empty", diff) + } +} + +func TestCompareGoldenUpdateWritesTheFile(t *testing.T) { + dir := t.TempDir() + want := "www 300 IN A 192.0.2.1\n" + + if _, _, err := compareGolden(dir, "x", []byte(want), true); err != nil { + t.Fatalf("compareGolden() error: %v", err) + } + + got, err := os.ReadFile(filepath.Join(dir, "x.golden")) + if err != nil { + t.Fatal(err) + } + if string(got) != want { + t.Errorf("golden file = %q, want %q", got, want) + } +} diff --git a/providers/packetframe/convert_golden_test.go b/providers/packetframe/convert_golden_test.go new file mode 100644 index 0000000000..a6ed7e6a94 --- /dev/null +++ b/providers/packetframe/convert_golden_test.go @@ -0,0 +1,23 @@ +package packetframe + +import ( + "testing" + + "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" +) + +func TestToRcGolden(t *testing.T) { + providergolden.CheckToRC(t, "packetframe_torc", "example.com", + func(dc *models.DomainConfig, native domainRecord) ([]*models.RecordConfig, error) { + rc, err := toRc(dc, &native) + return []*models.RecordConfig{rc}, err + }) +} + +func TestToReqGolden(t *testing.T) { + providergolden.CheckToNative(t, "packetframe_toreq", "example.com", + func(rc *models.RecordConfig) (*domainRecord, error) { + return toReq("zone-1", rc) + }) +} diff --git a/providers/packetframe/testdata/packetframe_torc.golden b/providers/packetframe/testdata/packetframe_torc.golden new file mode 100644 index 0000000000..0abf6dcc82 --- /dev/null +++ b/providers/packetframe/testdata/packetframe_torc.golden @@ -0,0 +1,9 @@ +@ 3600 IN A 192.0.2.1 +www 300 IN A 192.0.2.2 +www 300 IN AAAA 2001:db8::1 +alias 300 IN CNAME www.example.com. +@ 3600 IN MX 10 mail.example.com. +@ 3600 IN TXT "v=spf1 include:_spf.example.net -all" +sub 86400 IN NS ns1.example.net. +_sip._tcp 300 IN SRV 10 20 5060 sip.example.com. +@ 3600 IN CAA 0 issue "letsencrypt.org" diff --git a/providers/packetframe/testdata/packetframe_torc.json b/providers/packetframe/testdata/packetframe_torc.json new file mode 100644 index 0000000000..a4bedc8d53 --- /dev/null +++ b/providers/packetframe/testdata/packetframe_torc.json @@ -0,0 +1,83 @@ +[ + { + "id": "rec-1", + "label": "example.com.", + "proxy": false, + "ttl": 3600, + "type": "A", + "value": "192.0.2.1", + "zone": "zone-1" + }, + { + "id": "rec-2", + "label": "www.example.com.", + "proxy": false, + "ttl": 300, + "type": "A", + "value": "192.0.2.2", + "zone": "zone-1" + }, + { + "id": "rec-3", + "label": "www.example.com.", + "proxy": false, + "ttl": 300, + "type": "AAAA", + "value": "2001:db8::1", + "zone": "zone-1" + }, + { + "id": "rec-4", + "label": "alias.example.com.", + "proxy": false, + "ttl": 300, + "type": "CNAME", + "value": "www.example.com.", + "zone": "zone-1" + }, + { + "id": "rec-5", + "label": "example.com.", + "proxy": false, + "ttl": 3600, + "type": "MX", + "value": "10 mail.example.com.", + "zone": "zone-1" + }, + { + "id": "rec-6", + "label": "example.com.", + "proxy": false, + "ttl": 3600, + "type": "TXT", + "value": "v=spf1 include:_spf.example.net -all", + "zone": "zone-1" + }, + { + "id": "rec-7", + "label": "sub.example.com.", + "proxy": false, + "ttl": 86400, + "type": "NS", + "value": "ns1.example.net.", + "zone": "zone-1" + }, + { + "id": "rec-8", + "label": "_sip._tcp.example.com.", + "proxy": false, + "ttl": 300, + "type": "SRV", + "value": "10 20 5060 sip.example.com.", + "zone": "zone-1" + }, + { + "id": "rec-9", + "label": "example.com.", + "proxy": false, + "ttl": 3600, + "type": "CAA", + "value": "0 issue \"letsencrypt.org\"", + "zone": "zone-1" + } +] diff --git a/providers/packetframe/testdata/packetframe_toreq.golden b/providers/packetframe/testdata/packetframe_toreq.golden new file mode 100644 index 0000000000..bd1686a3b7 --- /dev/null +++ b/providers/packetframe/testdata/packetframe_toreq.golden @@ -0,0 +1,83 @@ +[ + { + "id": "", + "type": "A", + "label": "@", + "value": "192.0.2.1", + "ttl": 3600, + "proxy": false, + "zone": "zone-1" + }, + { + "id": "", + "type": "A", + "label": "www", + "value": "192.0.2.2", + "ttl": 300, + "proxy": false, + "zone": "zone-1" + }, + { + "id": "", + "type": "AAAA", + "label": "www", + "value": "2001:db8::1", + "ttl": 300, + "proxy": false, + "zone": "zone-1" + }, + { + "id": "", + "type": "CNAME", + "label": "alias", + "value": "www.example.com.", + "ttl": 300, + "proxy": false, + "zone": "zone-1" + }, + { + "id": "", + "type": "MX", + "label": "@", + "value": "10 mail.example.com.", + "ttl": 3600, + "proxy": false, + "zone": "zone-1" + }, + { + "id": "", + "type": "TXT", + "label": "@", + "value": "v=spf1 include:_spf.example.net -all", + "ttl": 3600, + "proxy": false, + "zone": "zone-1" + }, + { + "id": "", + "type": "NS", + "label": "sub", + "value": "ns1.example.net.", + "ttl": 86400, + "proxy": false, + "zone": "zone-1" + }, + { + "id": "", + "type": "SRV", + "label": "_sip._tcp", + "value": "10 20 5060 sip.example.com.", + "ttl": 300, + "proxy": false, + "zone": "zone-1" + }, + { + "id": "", + "type": "CAA", + "label": "@", + "value": "0 issue \"letsencrypt.org\"", + "ttl": 3600, + "proxy": false, + "zone": "zone-1" + } +] diff --git a/providers/packetframe/testdata/packetframe_toreq.records b/providers/packetframe/testdata/packetframe_toreq.records new file mode 100644 index 0000000000..0abf6dcc82 --- /dev/null +++ b/providers/packetframe/testdata/packetframe_toreq.records @@ -0,0 +1,9 @@ +@ 3600 IN A 192.0.2.1 +www 300 IN A 192.0.2.2 +www 300 IN AAAA 2001:db8::1 +alias 300 IN CNAME www.example.com. +@ 3600 IN MX 10 mail.example.com. +@ 3600 IN TXT "v=spf1 include:_spf.example.net -all" +sub 86400 IN NS ns1.example.net. +_sip._tcp 300 IN SRV 10 20 5060 sip.example.com. +@ 3600 IN CAA 0 issue "letsencrypt.org" diff --git a/providers/porkbun/convert_golden_test.go b/providers/porkbun/convert_golden_test.go new file mode 100644 index 0000000000..41eb5adaa6 --- /dev/null +++ b/providers/porkbun/convert_golden_test.go @@ -0,0 +1,20 @@ +package porkbun + +import ( + "testing" + + "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" +) + +func TestToRcGolden(t *testing.T) { + providergolden.CheckToRC(t, "porkbun_torc", "example.com", + func(dc *models.DomainConfig, native domainRecord) ([]*models.RecordConfig, error) { + rc, err := toRc(dc, &native) + return []*models.RecordConfig{rc}, err + }) +} + +func TestToReqGolden(t *testing.T) { + providergolden.CheckToNative(t, "porkbun_toreq", "example.com", toReq) +} diff --git a/providers/porkbun/testdata/porkbun_torc.golden b/providers/porkbun/testdata/porkbun_torc.golden new file mode 100644 index 0000000000..22e2733a92 --- /dev/null +++ b/providers/porkbun/testdata/porkbun_torc.golden @@ -0,0 +1,13 @@ +@ 600 IN A 192.0.2.1 +www 600 IN A 192.0.2.2 +www 600 IN AAAA 2001:db8::1 +alias 600 IN CNAME www.example.com. +@ 3600 IN MX 10 mail.example.com. +@ 600 IN TXT "v=spf1 include:_spf.example.net -all" +sub 86400 IN NS ns1.example.net. +@ 600 IN CAA 0 issue "letsencrypt.org" +_sip._tcp 600 IN SRV 10 20 5060 sip.example.com. +_25._tcp.mail 600 IN TLSA 3 1 1 0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF +@ 600 IN SSHFP 1 2 0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF +@ 600 IN HTTPS 1 . alpn="h3,h2" ipv4hint="192.0.2.1" +_8443._foo 600 IN SVCB 16 svc.example.net. port="8443" alpn="h2" diff --git a/providers/porkbun/testdata/porkbun_torc.json b/providers/porkbun/testdata/porkbun_torc.json new file mode 100644 index 0000000000..90a6a83c78 --- /dev/null +++ b/providers/porkbun/testdata/porkbun_torc.json @@ -0,0 +1,106 @@ +[ + { + "content": "192.0.2.1", + "id": "201", + "name": "example.com", + "prio": "0", + "ttl": "600", + "type": "A" + }, + { + "content": "192.0.2.2", + "id": "202", + "name": "www.example.com", + "prio": "0", + "ttl": "600", + "type": "A" + }, + { + "content": "2001:db8::1", + "id": "203", + "name": "www.example.com", + "prio": "0", + "ttl": "600", + "type": "AAAA" + }, + { + "content": "www.example.com", + "id": "204", + "name": "alias.example.com", + "prio": "0", + "ttl": "600", + "type": "CNAME" + }, + { + "content": "mail.example.com", + "id": "205", + "name": "example.com", + "prio": "10", + "ttl": "3600", + "type": "MX" + }, + { + "content": "v=spf1 include:_spf.example.net -all", + "id": "206", + "name": "example.com", + "prio": "0", + "ttl": "600", + "type": "TXT" + }, + { + "content": "ns1.example.net", + "id": "207", + "name": "sub.example.com", + "prio": "0", + "ttl": "86400", + "type": "NS" + }, + { + "content": "0 issue \"letsencrypt.org\"", + "id": "208", + "name": "example.com", + "prio": "0", + "ttl": "600", + "type": "CAA" + }, + { + "content": "20 5060 sip.example.com.", + "id": "209", + "name": "_sip._tcp.example.com", + "prio": "10", + "ttl": "600", + "type": "SRV" + }, + { + "content": "3 1 1 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + "id": "210", + "name": "_25._tcp.mail.example.com", + "prio": "0", + "ttl": "600", + "type": "TLSA" + }, + { + "content": "1 2 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + "id": "211", + "name": "example.com", + "prio": "0", + "ttl": "600", + "type": "SSHFP" + }, + { + "content": "1 . alpn=h3,h2 ipv4hint=192.0.2.1", + "id": "212", + "name": "example.com", + "prio": "0", + "ttl": "600", + "type": "HTTPS" + }, + { + "content": "16 svc.example.net. port=8443 alpn=h2", + "id": "213", + "name": "_8443._foo.example.com", + "prio": "0", + "ttl": "600", + "type": "SVCB" + } +] diff --git a/providers/porkbun/testdata/porkbun_toreq.golden b/providers/porkbun/testdata/porkbun_toreq.golden new file mode 100644 index 0000000000..c718963dde --- /dev/null +++ b/providers/porkbun/testdata/porkbun_toreq.golden @@ -0,0 +1,96 @@ +[ + { + "content": "192.0.2.1", + "name": "", + "ttl": "600", + "type": "A" + }, + { + "content": "192.0.2.2", + "name": "www", + "ttl": "600", + "type": "A" + }, + { + "content": "2001:db8::1", + "name": "www", + "ttl": "600", + "type": "AAAA" + }, + { + "content": "www.example.com.", + "name": "alias", + "ttl": "600", + "type": "CNAME" + }, + { + "content": "mail.example.com.", + "name": "", + "prio": "10", + "ttl": "3600", + "type": "MX" + }, + { + "content": "v=spf1 include:_spf.example.net -all", + "name": "", + "ttl": "600", + "type": "TXT" + }, + { + "content": "ns1.example.net.", + "name": "sub", + "ttl": "86400", + "type": "NS" + }, + { + "content": "0 issue \"letsencrypt.org\"", + "name": "", + "ttl": "600", + "type": "CAA" + }, + { + "content": "20 5060 sip.example.com.", + "name": "_sip._tcp", + "prio": "10", + "ttl": "600", + "type": "SRV" + }, + { + "content": "3 1 1 0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF", + "name": "_25._tcp.mail", + "ttl": "600", + "type": "TLSA" + }, + { + "content": "1 2 0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF", + "name": "", + "ttl": "600", + "type": "SSHFP" + }, + { + "content": "1 . alpn=h3,h2 ipv4hint=192.0.2.1", + "name": "", + "ttl": "600", + "type": "HTTPS" + }, + { + "content": "16 svc.example.net. port=8443 alpn=h2", + "name": "_8443._foo", + "ttl": "600", + "type": "SVCB" + }, + { + "includePath": "no", + "location": "https://example.net/landing", + "subdomain": "fwd", + "type": "temporary", + "wildcard": "no" + }, + { + "includePath": "yes", + "location": "https://example.net/moved", + "subdomain": "perm", + "type": "permanent", + "wildcard": "yes" + } +] diff --git a/providers/porkbun/testdata/porkbun_toreq.records b/providers/porkbun/testdata/porkbun_toreq.records new file mode 100644 index 0000000000..eb26fe6e78 --- /dev/null +++ b/providers/porkbun/testdata/porkbun_toreq.records @@ -0,0 +1,15 @@ +@ 600 IN A 192.0.2.1 +www 600 IN A 192.0.2.2 +www 600 IN AAAA 2001:db8::1 +alias 600 IN CNAME www.example.com. +@ 3600 IN MX 10 mail.example.com. +@ 600 IN TXT "v=spf1 include:_spf.example.net -all" +sub 86400 IN NS ns1.example.net. +@ 600 IN CAA 0 issue "letsencrypt.org" +_sip._tcp 600 IN SRV 10 20 5060 sip.example.com. +_25._tcp.mail 600 IN TLSA 3 1 1 0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF +@ 600 IN SSHFP 1 2 0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF +@ 600 IN HTTPS 1 . alpn="h3,h2" ipv4hint="192.0.2.1" +_8443._foo 600 IN SVCB 16 svc.example.net. port="8443" alpn="h2" +fwd IN URL https://example.net/landing ; includePath="no" type="temporary" wildcard="no" +perm IN URL301 https://example.net/moved ; includePath="yes" type="permanent" wildcard="yes" diff --git a/providers/websupport/convert_golden_test.go b/providers/websupport/convert_golden_test.go new file mode 100644 index 0000000000..ce143c366d --- /dev/null +++ b/providers/websupport/convert_golden_test.go @@ -0,0 +1,20 @@ +package websupport + +import ( + "testing" + + "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" +) + +func TestToRecordConfigGolden(t *testing.T) { + providergolden.CheckToRC(t, "websupport_torecordconfig", testDomain, + func(dc *models.DomainConfig, native nativeRecord) ([]*models.RecordConfig, error) { + rc, err := toRecordConfig(dc, native) + return []*models.RecordConfig{rc}, err + }) +} + +func TestToNativeGolden(t *testing.T) { + providergolden.CheckToNative(t, "websupport_tonative", testDomain, toNative) +} diff --git a/providers/websupport/testdata/websupport_tonative.golden b/providers/websupport/testdata/websupport_tonative.golden new file mode 100644 index 0000000000..02a69b3128 --- /dev/null +++ b/providers/websupport/testdata/websupport_tonative.golden @@ -0,0 +1,42 @@ +[ + { + "type": "A", + "name": "@", + "content": "1.2.3.4", + "ttl": 3600 + }, + { + "type": "AAAA", + "name": "ipv6", + "content": "2a00:4b40:aaaa:2001::6", + "ttl": 3600 + }, + { + "type": "CNAME", + "name": "www", + "content": "ghs.example.net", + "ttl": 3600 + }, + { + "type": "MX", + "name": "@", + "content": "mail.example.com", + "ttl": 3600, + "priority": 10 + }, + { + "type": "SRV", + "name": "_sip._tcp", + "content": "sip.example.com", + "ttl": 3600, + "priority": 10, + "port": 5060, + "weight": 20 + }, + { + "type": "TXT", + "name": "@", + "content": "hello world", + "ttl": 3600 + } +] diff --git a/providers/websupport/testdata/websupport_tonative.records b/providers/websupport/testdata/websupport_tonative.records new file mode 100644 index 0000000000..f24f721739 --- /dev/null +++ b/providers/websupport/testdata/websupport_tonative.records @@ -0,0 +1,6 @@ +@ 3600 IN A 1.2.3.4 +ipv6 3600 IN AAAA 2a00:4b40:aaaa:2001::6 +www 3600 IN CNAME ghs.example.net. +@ 3600 IN MX 10 mail.example.com. +_sip._tcp 3600 IN SRV 10 20 5060 sip.example.com. +@ 3600 IN TXT "hello world" diff --git a/providers/websupport/testdata/websupport_torecordconfig.golden b/providers/websupport/testdata/websupport_torecordconfig.golden new file mode 100644 index 0000000000..f24f721739 --- /dev/null +++ b/providers/websupport/testdata/websupport_torecordconfig.golden @@ -0,0 +1,6 @@ +@ 3600 IN A 1.2.3.4 +ipv6 3600 IN AAAA 2a00:4b40:aaaa:2001::6 +www 3600 IN CNAME ghs.example.net. +@ 3600 IN MX 10 mail.example.com. +_sip._tcp 3600 IN SRV 10 20 5060 sip.example.com. +@ 3600 IN TXT "hello world" diff --git a/providers/websupport/testdata/websupport_torecordconfig.json b/providers/websupport/testdata/websupport_torecordconfig.json new file mode 100644 index 0000000000..17f5d4054d --- /dev/null +++ b/providers/websupport/testdata/websupport_torecordconfig.json @@ -0,0 +1,48 @@ +[ + { + "content": "1.2.3.4", + "id": 42, + "name": "example.com", + "ttl": 3600, + "type": "A" + }, + { + "content": "2a00:4b40:aaaa:2001::6", + "id": 43, + "name": "ipv6.example.com", + "ttl": 3600, + "type": "AAAA" + }, + { + "content": "ghs.example.net", + "id": 44, + "name": "www.example.com", + "ttl": 3600, + "type": "CNAME" + }, + { + "content": "mail.example.com", + "id": 45, + "name": "example.com", + "priority": 10, + "ttl": 3600, + "type": "MX" + }, + { + "content": "sip.example.com", + "id": 46, + "name": "_sip._tcp.example.com", + "port": 5060, + "priority": 10, + "ttl": 3600, + "type": "SRV", + "weight": 20 + }, + { + "content": "hello world", + "id": 47, + "name": "example.com", + "ttl": 3600, + "type": "TXT" + } +] From 1cbcd5a4b5749721adc95b013e57e627f5e819e2 Mon Sep 17 00:00:00 2001 From: Shuvam Kumar Date: Sun, 2 Aug 2026 05:28:22 +0530 Subject: [PATCH 02/10] TEST: providergolden: always include the TTL in the golden line format formatRecord() dropped the TTL and its trailing space when the TTL was zero, so a zero-TTL record rendered as "fwd IN URL ..." instead of "fwd 0 IN URL ...". parseRecords() then had to guess whether the second field of a line was a TTL or the class in order to read that back. The TTL is now written unconditionally. parseRecord() splits a line into its five fixed fields in one step and checks the count once, instead of cutting the line field by field and testing each cut. The conditional that guessed whether the second field was a TTL is gone, and the whole function goes from 40 lines to 32. Two side effects of that rewrite, both in error reporting. A TTL that does not parse is now named as such: "www abc IN A 192.0.2.1" reported `expected class "IN"` and now reports `strconv.ParseUint: parsing "abc": invalid syntax`. And parseRecord() no longer shadows its "line" parameter with the metadata-stripped record, so its error messages quote the whole line rather than truncating it at the semicolon. All six .golden files were regenerated with "go test -update" and are byte-identical: none of the recorded records has a zero TTL. The only recorded input that encoded the old format is porkbun's .records, where the URL and URL301 records now carry an explicit 0. Requested by @TomOnTime in https://github.com/DNSControl/dnscontrol/pull/4653#issuecomment-5153944898 Co-Authored-By: Claude Opus 5 --- .../provider-conversion-tests.md | 8 ++-- pkg/providergolden/providergolden.go | 41 +++++++------------ pkg/providergolden/providergolden_test.go | 11 +++-- .../porkbun/testdata/porkbun_toreq.records | 4 +- 4 files changed, 28 insertions(+), 36 deletions(-) diff --git a/documentation/developer-info/provider-conversion-tests.md b/documentation/developer-info/provider-conversion-tests.md index f850a0c77f..505ae3c3a7 100644 --- a/documentation/developer-info/provider-conversion-tests.md +++ b/documentation/developer-info/provider-conversion-tests.md @@ -78,13 +78,13 @@ code does today, including bugs. ## The golden format -One line per record: the label, TTL, class, type and RDATA, with the TTL omitted -when it is zero, followed by the record's metadata when it has any: +One line per record: the label, TTL, class, type and RDATA, followed by the +record's metadata when it has any: ``` www 300 IN A 192.0.2.1 -@ IN MX 10 mail.example.com. -fwd IN URL https://example.net/landing ; includePath="no" type="temporary" wildcard="no" +@ 3600 IN MX 10 mail.example.com. +fwd 0 IN URL https://example.net/landing ; includePath="no" type="temporary" wildcard="no" ``` `CheckToNative` writes its golden as JSON, since a native record has no diff --git a/pkg/providergolden/providergolden.go b/pkg/providergolden/providergolden.go index a5289a923f..c1ab229918 100644 --- a/pkg/providergolden/providergolden.go +++ b/pkg/providergolden/providergolden.go @@ -28,12 +28,12 @@ // inputs. It never contacts a provider's API and never rewrites an input file, // so gathering data from a live account stays a separate, explicit step. // -// A golden line is the record's label, TTL, class, type and RDATA, with the TTL -// omitted when it is zero, followed by the metadata when the record has any: +// A golden line is the record's label, TTL, class, type and RDATA, followed by +// the metadata when the record has any: // // www 300 IN A 192.0.2.1 -// @ IN MX 10 mail.example.com. -// fwd IN URL https://example.net/landing ; includePath="no" type="temporary" wildcard="no" +// @ 3600 IN MX 10 mail.example.com. +// fwd 0 IN URL https://example.net/landing ; includePath="no" type="temporary" wildcard="no" package providergolden import ( @@ -189,11 +189,8 @@ func formatRecord(rc *models.RecordConfig) string { b.WriteString(rc.Name) b.WriteByte(' ') - if rc.TTL != 0 { - b.WriteString(strconv.FormatUint(uint64(rc.TTL), 10)) - b.WriteByte(' ') - } - b.WriteString("IN ") + b.WriteString(strconv.FormatUint(uint64(rc.TTL), 10)) + b.WriteString(" IN ") b.WriteString(rc.Type) b.WriteByte(' ') b.WriteString(rc.GetRDATA().String()) @@ -225,30 +222,22 @@ func parseRecords(dc *models.DomainConfig, text string) ([]*models.RecordConfig, } func parseRecord(dc *models.DomainConfig, line string) (*models.RecordConfig, error) { - line, metatext := cutMetadata(line) + record, metatext := cutMetadata(line) - name, rest, ok := strings.Cut(line, " ") - if !ok { - return nil, fmt.Errorf("%q: expected \"label [ttl] IN type rdata\"", line) + fields := strings.SplitN(record, " ", 5) + if len(fields) != 5 { + return nil, fmt.Errorf("%q: expected \"label ttl IN type rdata\"", line) } + name, ttltext, class, rtype, rdata := fields[0], fields[1], fields[2], fields[3], fields[4] - var ttl uint64 - if head, tail, ok := strings.Cut(rest, " "); ok { - if n, err := strconv.ParseUint(head, 10, 32); err == nil { - ttl, rest = n, tail - } + ttl, err := strconv.ParseUint(ttltext, 10, 32) + if err != nil { + return nil, fmt.Errorf("%q: %w", line, err) } - - class, rest, ok := strings.Cut(rest, " ") - if !ok || class != "IN" { + if class != "IN" { return nil, fmt.Errorf("%q: expected class \"IN\"", line) } - rtype, rdata, ok := strings.Cut(rest, " ") - if !ok { - return nil, fmt.Errorf("%q: expected rdata after the type", line) - } - rc, err := dc.NewRecordConfigParse(name, uint32(ttl), rtype, rdata) if err != nil { return nil, err diff --git a/pkg/providergolden/providergolden_test.go b/pkg/providergolden/providergolden_test.go index de40143617..29223f3d13 100644 --- a/pkg/providergolden/providergolden_test.go +++ b/pkg/providergolden/providergolden_test.go @@ -24,9 +24,9 @@ func TestFormatRecord(t *testing.T) { want: "www 300 IN A 192.0.2.1", }, { - name: "zero TTL is omitted", + name: "zero TTL is included", rc: dc.MustNewRecordConfig("www", 0, "A", "192.0.2.1"), - want: "www IN A 192.0.2.1", + want: "www 0 IN A 192.0.2.1", }, { name: "MX at the apex", @@ -62,7 +62,7 @@ func TestFormatRecord(t *testing.T) { func TestParseRecordsRoundTrip(t *testing.T) { input := strings.Join([]string{ "@ 3600 IN A 192.0.2.1", - "www IN A 192.0.2.2", + "www 0 IN A 192.0.2.2", "@ 3600 IN MX 10 mail.example.com.", `@ 600 IN TXT "v=spf1 include:_spf.example.net -all"`, `@ 600 IN CAA 0 issue "letsencrypt.org"`, @@ -108,7 +108,10 @@ func TestParseRecordsRejectsMalformedInput(t *testing.T) { name string input string }{ - {name: "no class", input: "www 300 A 192.0.2.1"}, + {name: "no ttl", input: "www IN A 192.0.2.1"}, + {name: "non-numeric ttl", input: "www abc IN A 192.0.2.1"}, + {name: "wrong class", input: "www 300 CH A 192.0.2.1"}, + {name: "too few fields", input: "www 300 A 192.0.2.1"}, {name: "no rdata", input: "www 300 IN A"}, {name: "unknown type", input: "www 300 IN NOSUCHTYPE 192.0.2.1"}, {name: "unterminated metadata", input: `www 300 IN A 192.0.2.1 ; wildcard="no`}, diff --git a/providers/porkbun/testdata/porkbun_toreq.records b/providers/porkbun/testdata/porkbun_toreq.records index eb26fe6e78..d2da6b2109 100644 --- a/providers/porkbun/testdata/porkbun_toreq.records +++ b/providers/porkbun/testdata/porkbun_toreq.records @@ -11,5 +11,5 @@ _25._tcp.mail 600 IN TLSA 3 1 1 0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF @ 600 IN SSHFP 1 2 0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF @ 600 IN HTTPS 1 . alpn="h3,h2" ipv4hint="192.0.2.1" _8443._foo 600 IN SVCB 16 svc.example.net. port="8443" alpn="h2" -fwd IN URL https://example.net/landing ; includePath="no" type="temporary" wildcard="no" -perm IN URL301 https://example.net/moved ; includePath="yes" type="permanent" wildcard="yes" +fwd 0 IN URL https://example.net/landing ; includePath="no" type="temporary" wildcard="no" +perm 0 IN URL301 https://example.net/moved ; includePath="yes" type="permanent" wildcard="yes" From 22cc55df08c627eb43a4043e756415399f5bbd42 Mon Sep 17 00:00:00 2001 From: Shuvam Kumar Date: Sun, 2 Aug 2026 06:25:22 +0530 Subject: [PATCH 03/10] REFACTOR: Extract the inlined record conversion in 6 providers The golden-file harness needs a function it can call with a native record and a DomainConfig. These providers had no such function: the conversion ran inline in GetZoneRecords, or inline in the method that makes the API call, so there was nothing to replay recorded data through. Extracted, one function per provider, no behaviour change: akamaiedgedns nativeToRecords(dc, akarecset) getRecords hetznerv2 nativeToRecords(dc, rrSet, zoneTTL) GetZoneRecords inwx toRecordConfig(dc, record) GetZoneRecords netlify toRecordConfig(dc, r) GetZoneRecords oracle toRecordConfig(dc, record) GetZoneRecords rwth toRecordConfig(dc, apiRecord) getAllRecords No signature takes an origin or a domain: every one of these functions used dc.Name only for the API call, which stays in the caller. hetznerv2 is the only one with a third parameter, the zone's default TTL, which the RRSet does not carry and cannot be derived from dc. Records that are dropped rather than converted (SOA everywhere, NETLIFY and NETLIFYv6 in netlify, the locked records in rwth) are now dropped by returning a nil RecordConfig, and the callers skip nils. That follows what dynu, exoscale and ovh already do, and it keeps the decision inside the function, so replaying a recorded zone that contains an SOA produces the same records the provider produces today. The counts in the survey posted to #4622 were measured with a looser rule ("no free function returning *models.RecordConfig") and were also one too high: vercel was listed as inlined but already had vercelRecordToRC. Of the 15 providers that rule flags today, 8 do have a separable converter, it just returns models.Records or hangs off a type. 7 genuinely had none; axfrddns is the seventh and is left alone because #4358 rewrites that function onto dnsv2. Behaviour preservation was checked per provider with a throwaway A/B test that ran the pre-extraction loop, copied verbatim from 1c629cfc, and the extracted function over the same inputs in the same binary, comparing label, FQDN, TTL, type, RDATA, metadata and Original. All six agree byte for byte across every branch, including the error paths. Deleting rc.Original in oracle, or the NETLIFY skip in netlify, turns the corresponding comparison red. Co-Authored-By: Claude Opus 5 --- .../akamaiedgedns/akamaiEdgeDnsService.go | 81 ++++++++------ providers/hetznerv2/hetznerv2Provider.go | 45 +++++--- providers/inwx/inwxProvider.go | 101 ++++++++++-------- providers/netlify/netlifyProvider.go | 76 +++++++------ providers/oracle/oracleProvider.go | 50 ++++++--- providers/rwth/api.go | 13 +-- providers/rwth/convert.go | 22 ++++ 7 files changed, 236 insertions(+), 152 deletions(-) diff --git a/providers/akamaiedgedns/akamaiEdgeDnsService.go b/providers/akamaiedgedns/akamaiEdgeDnsService.go index 99199bb50c..eb1d8edcdf 100644 --- a/providers/akamaiedgedns/akamaiEdgeDnsService.go +++ b/providers/akamaiedgedns/akamaiEdgeDnsService.go @@ -320,46 +320,57 @@ func (a *edgeDNSProvider) getRecords(ctx context.Context, dc *models.DomainConfi // For each AkamaiEdgeDNS recordset... for _, akarecset := range akaRecordsets { - akaname := akarecset.Name - akatype := akarecset.Type - akattl := akarecset.TTL - label := dc.LabelFromFQDNNoDot(akaname) - - // Don't report the existence of an SOA record (because DnsControl will try to delete the SOA record). - if akatype == "SOA" { - continue + recs, err := nativeToRecords(dc, akarecset) + if err != nil { + return nil, err } + recordConfigs = append(recordConfigs, recs...) + } + + return recordConfigs, nil +} + +// nativeToRecords converts an AkamaiEdgeDNS recordset into 1 or more +// RecordConfig structs. It returns nothing for an SOA recordset, whose existence +// is not reported (because DnsControl will try to delete the SOA record). +func nativeToRecords(dc *models.DomainConfig, akarecset dns.RecordSet) ([]*models.RecordConfig, error) { + akaname := akarecset.Name + akatype := akarecset.Type + akattl := akarecset.TTL + label := dc.LabelFromFQDNNoDot(akaname) + + if akatype == "SOA" { + return nil, nil + } - // AKAMAITLC has 2 rdata entries that form 1 logical record: [answerType, target] - if akatype == "AKAMAITLC" { - combined := strings.Join(akarecset.Rdata, " ") - parts := strings.Fields(combined) - if len(parts) != 2 { - return nil, fmt.Errorf("AKAMAITLC rdata must contain 2 fields, got: %v", akarecset.Rdata) - } - rc, err := dc.NewRecordConfig(label, uint32(akattl), privatetypes.TypeAKAMAITLC, parts[0], parts[1]) - if err != nil { - return nil, err - } - rc.Metadata = map[string]string{"akamai_raw_rdata": combined} - recordConfigs = append(recordConfigs, rc) - continue + // AKAMAITLC has 2 rdata entries that form 1 logical record: [answerType, target] + if akatype == "AKAMAITLC" { + combined := strings.Join(akarecset.Rdata, " ") + parts := strings.Fields(combined) + if len(parts) != 2 { + return nil, fmt.Errorf("AKAMAITLC rdata must contain 2 fields, got: %v", akarecset.Rdata) + } + rc, err := dc.NewRecordConfig(label, uint32(akattl), privatetypes.TypeAKAMAITLC, parts[0], parts[1]) + if err != nil { + return nil, err } + rc.Metadata = map[string]string{"akamai_raw_rdata": combined} + return []*models.RecordConfig{rc}, nil + } - // ... convert the recordset into 1 or more RecordConfig structs - for _, r := range akarecset.Rdata { - data := r - if akatype == "LOC" { - data = fixLocAltitude(r) - } - rc, err := dc.NewRecordConfigParse(label, uint32(akattl), akatype, data) - if err != nil { - return nil, err - } - - rc.Metadata = map[string]string{"akamai_raw_rdata": r} - recordConfigs = append(recordConfigs, rc) + var recordConfigs []*models.RecordConfig + for _, r := range akarecset.Rdata { + data := r + if akatype == "LOC" { + data = fixLocAltitude(r) } + rc, err := dc.NewRecordConfigParse(label, uint32(akattl), akatype, data) + if err != nil { + return nil, err + } + + rc.Metadata = map[string]string{"akamai_raw_rdata": r} + recordConfigs = append(recordConfigs, rc) } return recordConfigs, nil diff --git a/providers/hetznerv2/hetznerv2Provider.go b/providers/hetznerv2/hetznerv2Provider.go index a38df71a5c..5ae158fd57 100644 --- a/providers/hetznerv2/hetznerv2Provider.go +++ b/providers/hetznerv2/hetznerv2Provider.go @@ -233,27 +233,38 @@ func (h *hetznerv2Provider) GetZoneRecords(dc *models.DomainConfig) (models.Reco } existingRecords := make(models.Records, 0, len(records)) for _, rrSet := range records { - if rrSet.Type == hcloud.ZoneRRSetTypeSOA { - // SOA records are not available for editing, hide them. - continue - } - var ttl uint32 - if rrSet.TTL != nil { - ttl = uint32(*rrSet.TTL) - } else { - ttl = uint32(z.TTL) + recs, err := nativeToRecords(dc, rrSet, uint32(z.TTL)) + if err != nil { + return nil, err } + existingRecords = append(existingRecords, recs...) + } + return existingRecords, nil +} - for _, r := range rrSet.Records { - rc, err := dc.NewRecordConfigParse(rrSet.Name, ttl, string(rrSet.Type), r.Value) - if err != nil { - return nil, err - } - rc.Original = rrSet - existingRecords = append(existingRecords, rc) +// nativeToRecords converts a Hetzner RRSet to RecordConfigs, one per value. +// zoneTTL is the TTL of the zone the RRSet belongs to, used when the RRSet does +// not carry one of its own. It returns nothing for SOA RRSets, which are hidden. +func nativeToRecords(dc *models.DomainConfig, rrSet *hcloud.ZoneRRSet, zoneTTL uint32) (models.Records, error) { + if rrSet.Type == hcloud.ZoneRRSetTypeSOA { + // SOA records are not available for editing, hide them. + return nil, nil + } + ttl := zoneTTL + if rrSet.TTL != nil { + ttl = uint32(*rrSet.TTL) + } + + recs := make(models.Records, 0, len(rrSet.Records)) + for _, r := range rrSet.Records { + rc, err := dc.NewRecordConfigParse(rrSet.Name, ttl, string(rrSet.Type), r.Value) + if err != nil { + return nil, err } + rc.Original = rrSet + recs = append(recs, rc) } - return existingRecords, nil + return recs, nil } // ListZones lists the zones on this account. diff --git a/providers/inwx/inwxProvider.go b/providers/inwx/inwxProvider.go index 6440c8f553..77376c2fe7 100644 --- a/providers/inwx/inwxProvider.go +++ b/providers/inwx/inwxProvider.go @@ -420,58 +420,73 @@ func (api *inwxAPI) GetZoneRecords(dc *models.DomainConfig) (models.Records, err records := models.Records{} for _, record := range info.Records { - if record.Type == "SOA" { + rc, err := toRecordConfig(dc, record) + if err != nil { + return nil, err + } + if rc == nil { continue } - /* - INWX is a little bit special for CNAME,NS,MX and SRV records: - The API will not accept any target with a final dot but will - instead always add this final dot internally. - Records with empty targets (i.e. records with target ".") - are allowed. - */ - rtypeAddDot := map[string]bool{ - "ALIAS": true, - "CNAME": true, - "MX": true, - "NS": true, - "SRV": true, - "PTR": true, - } - if rtypeAddDot[record.Type] { - if record.Type == "MX" && record.Content == "." { - // null records don't need to be modified - } else if record.Type == "SRV" && strings.HasSuffix(record.Content, ".") { - // null targets don't need to be modified - } else { - record.Content = record.Content + "." - } - } + records = append(records, rc) + } - label := dc.ToShort(record.Name) - ttl := uint32(record.TTL) + return records, nil +} - var rc *models.RecordConfig - switch rType := record.Type; rType { - case "MX": - rc, err = dc.NewRecordConfig(label, ttl, rType, record.Priority, record.Content) - case "SRV": - rc, err = dc.NewRecordConfig(label, ttl, rType, record.Priority, record.Content, - nrc.Flags{SrvWeirdSplit: true}) - default: - rc, err = dc.NewRecordConfigParse(label, ttl, rType, record.Content, - nrc.Flags{TxtDontParse: true}) - } - if err != nil { - return nil, fmt.Errorf("INWX: unparsable record received: %w", err) +// toRecordConfig converts an INWX record to a RecordConfig. It returns nil for +// SOA records, which are not managed. +func toRecordConfig(dc *models.DomainConfig, record goinwx.NameserverRecord) (*models.RecordConfig, error) { + if record.Type == "SOA" { + return nil, nil + } + + /* + INWX is a little bit special for CNAME,NS,MX and SRV records: + The API will not accept any target with a final dot but will + instead always add this final dot internally. + Records with empty targets (i.e. records with target ".") + are allowed. + */ + rtypeAddDot := map[string]bool{ + "ALIAS": true, + "CNAME": true, + "MX": true, + "NS": true, + "SRV": true, + "PTR": true, + } + if rtypeAddDot[record.Type] { + if record.Type == "MX" && record.Content == "." { + // null records don't need to be modified + } else if record.Type == "SRV" && strings.HasSuffix(record.Content, ".") { + // null targets don't need to be modified + } else { + record.Content = record.Content + "." } - rc.Original = record + } - records = append(records, rc) + label := dc.ToShort(record.Name) + ttl := uint32(record.TTL) + + var rc *models.RecordConfig + var err error + switch rType := record.Type; rType { + case "MX": + rc, err = dc.NewRecordConfig(label, ttl, rType, record.Priority, record.Content) + case "SRV": + rc, err = dc.NewRecordConfig(label, ttl, rType, record.Priority, record.Content, + nrc.Flags{SrvWeirdSplit: true}) + default: + rc, err = dc.NewRecordConfigParse(label, ttl, rType, record.Content, + nrc.Flags{TxtDontParse: true}) } + if err != nil { + return nil, fmt.Errorf("INWX: unparsable record received: %w", err) + } + rc.Original = record - return records, nil + return rc, nil } // ListZones returns the zones configured in INWX. diff --git a/providers/netlify/netlifyProvider.go b/providers/netlify/netlifyProvider.go index 2361c97078..486661ea51 100644 --- a/providers/netlify/netlifyProvider.go +++ b/providers/netlify/netlifyProvider.go @@ -105,45 +105,61 @@ func (n *netlifyProvider) GetZoneRecords(dc *models.DomainConfig) (models.Record cleanRecords := make(models.Records, 0) for _, r := range records { - if r.Type == "SOA" { + rec, err := toRecordConfig(dc, r) + if err != nil { + return nil, err + } + if rec == nil { continue } - label := dc.LabelFromFQDNNoDot(r.Hostname) // Netlify returns the FQDN. - ttl := uint32(r.TTL) + cleanRecords = append(cleanRecords, rec) + } - if r.Type == "CNAME" || r.Type == "MX" || r.Type == "NS" { - r.Value = dnsutil.Canonical(r.Value) - } + return cleanRecords, nil +} - var rec *models.RecordConfig - switch rtype := r.Type; rtype { - case "NETLIFY", "NETLIFYv6": // transparently ignore - continue - case "MX": - rec, err = dc.NewRecordConfig(label, ttl, dnsv2.TypeMX, r.Priority, r.Value) - case "SRV": - parts := strings.Fields(r.Value) - if len(parts) == 3 { - r.Value += "." - } - rec, err = dc.NewRecordConfig(label, ttl, dnsv2.TypeSRV, r.Priority, r.Weight, r.Port, r.Value) - case "TXT": - rec, err = dc.NewRecordConfig(label, ttl, dnsv2.TypeTXT, r.Value) - case "CAA": - rec, err = dc.NewRecordConfig(label, ttl, dnsv2.TypeCAA, r.Flag, r.Tag, r.Value) - default: - rec, err = dc.NewRecordConfigParse(label, ttl, r.Type, r.Value) - } +// toRecordConfig converts a Netlify record to a RecordConfig. It returns nil for +// SOA records and for the NETLIFY and NETLIFYv6 pseudo-types, which are ignored. +func toRecordConfig(dc *models.DomainConfig, r *dnsRecord) (*models.RecordConfig, error) { + if r.Type == "SOA" { + return nil, nil + } - if err != nil { - return nil, fmt.Errorf("unparsable record received from Netlify: %w", err) + label := dc.LabelFromFQDNNoDot(r.Hostname) // Netlify returns the FQDN. + ttl := uint32(r.TTL) + + if r.Type == "CNAME" || r.Type == "MX" || r.Type == "NS" { + r.Value = dnsutil.Canonical(r.Value) + } + + var rec *models.RecordConfig + var err error + switch rtype := r.Type; rtype { + case "NETLIFY", "NETLIFYv6": // transparently ignore + return nil, nil + case "MX": + rec, err = dc.NewRecordConfig(label, ttl, dnsv2.TypeMX, r.Priority, r.Value) + case "SRV": + parts := strings.Fields(r.Value) + if len(parts) == 3 { + r.Value += "." } - rec.Original = r - cleanRecords = append(cleanRecords, rec) + rec, err = dc.NewRecordConfig(label, ttl, dnsv2.TypeSRV, r.Priority, r.Weight, r.Port, r.Value) + case "TXT": + rec, err = dc.NewRecordConfig(label, ttl, dnsv2.TypeTXT, r.Value) + case "CAA": + rec, err = dc.NewRecordConfig(label, ttl, dnsv2.TypeCAA, r.Flag, r.Tag, r.Value) + default: + rec, err = dc.NewRecordConfigParse(label, ttl, r.Type, r.Value) } - return cleanRecords, nil + if err != nil { + return nil, fmt.Errorf("unparsable record received from Netlify: %w", err) + } + rec.Original = r + + return rec, nil } // ListZones returns all DNS zones managed by this provider. diff --git a/providers/oracle/oracleProvider.go b/providers/oracle/oracleProvider.go index 1fab1cfe82..993c27bb7e 100644 --- a/providers/oracle/oracleProvider.go +++ b/providers/oracle/oracleProvider.go @@ -208,26 +208,13 @@ func (o *oracleProvider) GetZoneRecords(dc *models.DomainConfig) (models.Records } for _, record := range getResp.Items { - // Hide SOAs - if *record.Rtype == "SOA" { - continue - } - - label := dc.LabelFromFQDNNoDot(*record.Domain) - ttl := uint32(*record.Ttl) - var rc *models.RecordConfig - - switch *record.Rtype { - case "ALIAS": - rc, err = dc.NewRecordConfig(label, ttl, *record.Rtype, *record.Rdata) - default: - rc, err = dc.NewRecordConfigParse(label, ttl, *record.Rtype, *record.Rdata) - } - + rc, err := toRecordConfig(dc, record) if err != nil { return nil, err } - rc.Original = record + if rc == nil { + continue + } records = append(records, rc) } @@ -242,6 +229,35 @@ func (o *oracleProvider) GetZoneRecords(dc *models.DomainConfig) (models.Records return records, nil } +// toRecordConfig converts an Oracle record to a RecordConfig. It returns nil for +// SOA records, which are hidden. +func toRecordConfig(dc *models.DomainConfig, record dns.Record) (*models.RecordConfig, error) { + // Hide SOAs + if *record.Rtype == "SOA" { + return nil, nil + } + + label := dc.LabelFromFQDNNoDot(*record.Domain) + ttl := uint32(*record.Ttl) + + var rc *models.RecordConfig + var err error + + switch *record.Rtype { + case "ALIAS": + rc, err = dc.NewRecordConfig(label, ttl, *record.Rtype, *record.Rdata) + default: + rc, err = dc.NewRecordConfigParse(label, ttl, *record.Rtype, *record.Rdata) + } + + if err != nil { + return nil, err + } + rc.Original = record + + return rc, nil +} + // GetZoneRecordsCorrections returns a list of corrections that will turn existing records into dc.Records. func (o *oracleProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, existingRecords models.Records) ([]*models.Correction, int, error) { var err error diff --git a/providers/rwth/api.go b/providers/rwth/api.go index bdc919a6a4..9ec5fb24c1 100644 --- a/providers/rwth/api.go +++ b/providers/rwth/api.go @@ -15,7 +15,6 @@ import ( "time" "github.com/DNSControl/dnscontrol/v5/models" - "github.com/DNSControl/dnscontrol/v5/pkg/dnsrr" "github.com/DNSControl/dnscontrol/v5/pkg/printer" ) @@ -140,19 +139,13 @@ func (api *rwthProvider) getAllRecords(dc *models.DomainConfig) (models.Records, return nil, fmt.Errorf("failed fetching zone records for %q: %w", dc.Name, err) } for _, apiRecord := range response { - if checkIsLockedSystemAPIRecord(apiRecord) != nil { - continue - } - dnsRec, err := NewRR(apiRecord.Content) // Parse content as DNS record + recConfig, err := toRecordConfig(dc, apiRecord) if err != nil { return nil, err } - - recConfig, err := dnsrr.RRv2toRC(dc, dnsRec) // and make it a RC - if err != nil { - return nil, err + if recConfig == nil { + continue } - recConfig.Original = apiRecord // but keep our ApiRecord as the original records = append(records, recConfig) } diff --git a/providers/rwth/convert.go b/providers/rwth/convert.go index cf6cf958ab..761719a3b2 100644 --- a/providers/rwth/convert.go +++ b/providers/rwth/convert.go @@ -9,6 +9,7 @@ import ( dnsv2 "codeberg.org/miekg/dns" dnsutilv2 "codeberg.org/miekg/dns/dnsutil" "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/dnsrr" "github.com/DNSControl/dnscontrol/v5/pkg/prettyzone" ) @@ -45,6 +46,27 @@ func (api *rwthProvider) printRecConfig(rr models.RecordConfig) string { prefix, prettyzone.FormatLine([]int{10, 5, 2, 5, 0}, []string{rr.NameFQDN, ttl, "IN", typeStr, target}), comment) } +// toRecordConfig converts an RWTH record to a RecordConfig. It returns nil for +// the records that RWTH locks, which cannot be managed. +func toRecordConfig(dc *models.DomainConfig, apiRecord RecordReply) (*models.RecordConfig, error) { + if checkIsLockedSystemAPIRecord(apiRecord) != nil { + return nil, nil + } + + dnsRec, err := NewRR(apiRecord.Content) // Parse content as DNS record + if err != nil { + return nil, err + } + + recConfig, err := dnsrr.RRv2toRC(dc, dnsRec) // and make it a RC + if err != nil { + return nil, err + } + recConfig.Original = apiRecord // but keep our ApiRecord as the original + + return recConfig, nil +} + // NewRR returns custom dns.NewRR with RWTH default TTL. func NewRR(s string) (dnsv2.RR, error) { if len(s) > 0 && s[len(s)-1] != '\n' { // We need a closing newline From 1331bbccf2d8cb416adffd92c03bcbc5cac50ca9 Mon Sep 17 00:00:00 2001 From: Shuvam Kumar Date: Sun, 2 Aug 2026 07:52:43 +0530 Subject: [PATCH 04/10] TEST: providergolden: record conversion inputs from an integration run The golden harness needs recorded data, and that data has to come from a real provider. "Without executing a shell command" is read here as: our code must not exec the Go toolchain and scrape its output. The command stays the one already documented; the recording happens inside the process it starts. providergolden.Recorder collects both kinds of harness input: - the records a provider is asked to store, written as .records, which is what CheckToNative replays; - the native record each returned record came from, read from RecordConfig.Original and written as .json, which is what CheckToRC replays. providergolden.Record wraps a models.DNSProvider and observes them at GetZoneRecordsCorrections. That is the one point where both are in the form the provider's own conversion functions see them: dc.Records has been downcased, canonicalized and punycoded by zonerecs, and existing still carries each native record in Original. All four integration tests are wrapped; the three that call zonerecs.CorrectZoneRecords contribute data. TestNameserverDots only calls GetNameservers, so it contributes none. No provider code changes. integrationTest gains "-record ". Without it nothing is collected, no file is written and the provider is not wrapped, so a normal run is unchanged. With it: go test -run TestDNSProviders -timeout 1h -failfast -v ./integrationTest \ -args -verbose -profile CLOUDFLAREAPI -record providers/cloudflare/testdata Duplicates are discarded and the output is sorted, so a run that revisits the same records over hundreds of test cases produces a small file that is the same on every run. Recording BIND this way yields 339 unique records over 23 types; all 339 lines parse back through the harness's own parser and re-render byte for byte, and two runs produce identical files. .json is written only for providers that fill in Original. Of the 21 providers named on the PR, 17 assign it at a9374d89; bind, axfrddns, mythicbeasts and transip do not, so those record only the .records half. Original is read where the wrapper sees it, which is not always what the API sent. providers/netlify canonicalizes a CNAME, MX or NS value in place before assigning the record to Original, so a native recorded from it carries a trailing dot the API did not send and a golden replayed from it never exercises that canonicalization. No wrapper can see the earlier value: the mutation happens inside the provider's own converter. The documentation says so and says to check recorded natives against the API's responses. Co-Authored-By: Claude Opus 5 --- .../provider-conversion-tests.md | 42 +++- integrationTest/helpers_test.go | 23 ++ pkg/providergolden/providergolden.go | 4 + pkg/providergolden/record.go | 127 ++++++++++ pkg/providergolden/record_test.go | 233 ++++++++++++++++++ 5 files changed, 425 insertions(+), 4 deletions(-) create mode 100644 pkg/providergolden/record.go create mode 100644 pkg/providergolden/record_test.go diff --git a/documentation/developer-info/provider-conversion-tests.md b/documentation/developer-info/provider-conversion-tests.md index 505ae3c3a7..71b04892b1 100644 --- a/documentation/developer-info/provider-conversion-tests.md +++ b/documentation/developer-info/provider-conversion-tests.md @@ -16,10 +16,44 @@ skipped, never failed. ### 1. Record the data -Collect the native records your provider's API returns for a test zone and store -them as a JSON array in `providers//testdata/`. The file name starts -with the provider's name in lower case and continues with the function under -test: +The integration tests already drive every conversion a provider has, so the +easiest way to collect the data is to record an integration run. Add `-record` +and a directory to the command you normally use: + +```shell +go test -run TestDNSProviders -timeout 1h -failfast -v ./integrationTest \ + -args -verbose -profile CLOUDFLAREAPI -record providers/cloudflare/testdata +``` + +That writes two files named after the profile: + +- `cloudflareapi.records` — every record the tests asked the provider to store, + which is what a `CheckToNative` function is given. +- `cloudflareapi.json` — the native record each returned record came from, read + from `RecordConfig.Original`. That is what a `CheckToRC` function is given. It + is written only for providers that fill `Original` in. + +Rename them to match the test you are about to add, and read them before +committing: they contain whatever your zone contained during the run. + +{% hint style="warning" %} +`Original` is recorded as it stands once the provider has built the zone, which +is not always what the API sent. `providers/netlify` canonicalizes a CNAME, MX +or NS value in place before storing the record in `Original`, so a recorded +native carries a trailing dot the API did not send, and a golden replayed from +it never exercises the canonicalization. Check the recorded natives against the +API's own responses when a provider's converter writes to the record it was +given. +{% endhint %} + +Without `-record` nothing is collected, no file is written and the provider is +not wrapped, so a normal run is unchanged. Under `-record` the provider is +wrapped in a `models.DNSProvider`, which is all the integration tests ask of it +today. A test that type-asserts a provider to an optional interface such as +`ZoneCreator` would need the wrapper to forward that interface too. + +Data can also be collected by hand. The `.json` file is a JSON array of the +native records your provider's API returns: ```json [ diff --git a/integrationTest/helpers_test.go b/integrationTest/helpers_test.go index 7d2b6d44cc..6af6951845 100644 --- a/integrationTest/helpers_test.go +++ b/integrationTest/helpers_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/DNSControl/dnscontrol/v5/pkg/credsfile" + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" "github.com/DNSControl/dnscontrol/v5/pkg/providers" "github.com/DNSControl/dnscontrol/v5/providers/cloudflare" ) @@ -17,12 +18,17 @@ import ( var ( providerFlag = flag.String("provider", "", "Provider to run (if empty, deduced from -profile)") profileFlag = flag.String("profile", "", "Entry in profiles.json to use (if empty, copied from -provider)") + recordFlag = flag.String("record", "", "Directory to write the record conversion inputs seen during the run to") enableCFWorkers = flag.Bool("cfworkers", true, "enable CF worker tests (default false)") enableCFRedirectMode = flag.Bool("cfredirect", true, "enable CF SingleRedirect tests (default false)") enableCFFlatten = flag.Bool("cfflatten", false, "enable CF CNAME flattening tests (requires paid plan, default false)") enableCFTags = flag.Bool("cftags", false, "enable CF tag tests (requires paid plan, default false)") ) +// recorder accumulates the conversion inputs of every provider call made by +// this run, and is written out when -record names a directory. +var recorder = providergolden.NewRecorder() + func init() { testing.Init() @@ -124,5 +130,22 @@ func getProvider(t *testing.T) (providers.DNSServiceProvider, string, map[string } } + if *recordFlag != "" { + t.Cleanup(func() { writeRecording(t) }) + return providergolden.Record(provider, recorder), cfg["domain"], cfg + } + return provider, cfg["domain"], cfg } + +// writeRecording writes everything recorded so far to the -record directory, +// named after the profile under test. +func writeRecording(t *testing.T) { + written, err := recorder.WriteTo(*recordFlag, strings.ToLower(*profileFlag)) + for _, path := range written { + t.Logf("Recorded %s", path) + } + if err != nil { + t.Error(err) + } +} diff --git a/pkg/providergolden/providergolden.go b/pkg/providergolden/providergolden.go index c1ab229918..c537af611d 100644 --- a/pkg/providergolden/providergolden.go +++ b/pkg/providergolden/providergolden.go @@ -28,6 +28,10 @@ // inputs. It never contacts a provider's API and never rewrites an input file, // so gathering data from a live account stays a separate, explicit step. // +// That step is Recorder, which collects both kinds of input from a provider as +// it is used. The integration tests wrap their provider in one when they are +// given "-record ". +// // A golden line is the record's label, TTL, class, type and RDATA, followed by // the metadata when the record has any: // diff --git a/pkg/providergolden/record.go b/pkg/providergolden/record.go new file mode 100644 index 0000000000..65e5f807da --- /dev/null +++ b/pkg/providergolden/record.go @@ -0,0 +1,127 @@ +package providergolden + +import ( + "encoding/json" + "errors" + "fmt" + "maps" + "os" + "path/filepath" + "slices" + "strings" + "sync" + + "github.com/DNSControl/dnscontrol/v5/models" +) + +// Recorder collects the inputs of a provider's record conversion functions as +// the provider is used, and writes them in the formats CheckToRC and +// CheckToNative read. Duplicates are discarded, so a long run that revisits the +// same records produces a small file. +type Recorder struct { + mu sync.Mutex + records map[string]bool + natives map[string]bool + errs []error +} + +// NewRecorder returns a Recorder that has seen nothing. +func NewRecorder() *Recorder { + return &Recorder{ + records: map[string]bool{}, + natives: map[string]bool{}, + } +} + +// Observe adds one round of conversion inputs. desired holds the records the +// provider is about to convert into its own format; existing holds the records +// it has just converted out of its own format, each carrying the native record +// it came from in RecordConfig.Original. +func (r *Recorder) Observe(desired, existing models.Records) { + r.mu.Lock() + defer r.mu.Unlock() + + for _, rc := range desired { + r.records[formatRecord(rc)] = true + } + + for _, rc := range existing { + if rc.Original == nil { + continue + } + native, err := json.Marshal(rc.Original) + if err != nil { + r.errs = append(r.errs, fmt.Errorf("%s %s: %w", rc.NameFQDN, rc.Type, err)) + continue + } + r.natives[string(native)] = true + } +} + +// WriteTo writes what has been observed to dir as .records and +// .json, sorted so that two runs of the same tests produce the same file. +// A file is not written when nothing of that kind was observed: a provider that +// does not fill in RecordConfig.Original produces no .json. WriteTo +// returns the paths it wrote. +func (r *Recorder) WriteTo(dir, name string) ([]string, error) { + r.mu.Lock() + defer r.mu.Unlock() + + var written []string + + if len(r.records) != 0 { + text := strings.Join(slices.Sorted(maps.Keys(r.records)), "\n") + "\n" + path, err := writeFile(dir, name+".records", []byte(text)) + if err != nil { + return written, err + } + written = append(written, path) + } + + if len(r.natives) != 0 { + natives := make([]json.RawMessage, 0, len(r.natives)) + for _, native := range slices.Sorted(maps.Keys(r.natives)) { + natives = append(natives, json.RawMessage(native)) + } + text, err := json.MarshalIndent(natives, "", " ") + if err != nil { + return written, err + } + path, err := writeFile(dir, name+".json", append(text, '\n')) + if err != nil { + return written, err + } + written = append(written, path) + } + + return written, errors.Join(r.errs...) +} + +// writeFile writes data to / and returns the path, creating dir +// when it does not exist. +func writeFile(dir, filename string, data []byte) (string, error) { + if err := os.MkdirAll(dir, 0o755); err != nil { + return "", err + } + path := filepath.Join(dir, filename) + return path, os.WriteFile(path, data, 0o644) +} + +// Record wraps p so that rec observes every record conversion p is asked to +// perform. The wrapper is how the integration tests gather data: they drive a +// real provider in the same process, and every zone they build passes through +// GetZoneRecordsCorrections on its way to the provider's own conversion +// functions. +func Record(p models.DNSProvider, rec *Recorder) models.DNSProvider { + return &recordingProvider{DNSProvider: p, rec: rec} +} + +type recordingProvider struct { + models.DNSProvider + rec *Recorder +} + +func (p *recordingProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, existing models.Records) ([]*models.Correction, int, error) { + p.rec.Observe(dc.Records, existing) + return p.DNSProvider.GetZoneRecordsCorrections(dc, existing) +} diff --git a/pkg/providergolden/record_test.go b/pkg/providergolden/record_test.go new file mode 100644 index 0000000000..a366ada1fb --- /dev/null +++ b/pkg/providergolden/record_test.go @@ -0,0 +1,233 @@ +package providergolden + +import ( + "encoding/json" + "errors" + "os" + "path/filepath" + "testing" + + "github.com/DNSControl/dnscontrol/v5/models" +) + +type fakeNative struct { + Name string `json:"name"` + Type string `json:"type"` +} + +type fakeProvider struct { + corrections []*models.Correction + count int + err error + calls int +} + +func (p *fakeProvider) GetNameservers(string) ([]*models.Nameserver, error) { return nil, nil } + +func (p *fakeProvider) GetZoneRecords(*models.DomainConfig) (models.Records, error) { + return nil, nil +} + +func (p *fakeProvider) GetZoneRecordsCorrections(*models.DomainConfig, models.Records) ([]*models.Correction, int, error) { + p.calls++ + return p.corrections, p.count, p.err +} + +func TestRecorderWritesTheRecordsItObserved(t *testing.T) { + dc := models.MustNewDomainConfig("example.com") + desired := models.Records{ + dc.MustNewRecordConfig("www", 300, "A", "192.0.2.1"), + dc.MustNewRecordConfig("@", 3600, "MX", 10, "mail.example.com."), + dc.MustNewRecordConfig("mail", 300, "A", "192.0.2.2"), + dc.MustNewRecordConfig("@", 600, "TXT", "hello world"), + } + + rec := NewRecorder() + rec.Observe(desired, nil) + + dir := t.TempDir() + written, err := rec.WriteTo(dir, "example") + if err != nil { + t.Fatalf("WriteTo() error: %v", err) + } + if want := []string{filepath.Join(dir, "example.records")}; len(written) != 1 || written[0] != want[0] { + t.Fatalf("WriteTo() wrote %v, want %v", written, want) + } + + got, err := os.ReadFile(written[0]) + if err != nil { + t.Fatal(err) + } + want := "@ 3600 IN MX 10 mail.example.com.\n" + + "@ 600 IN TXT \"hello world\"\n" + + "mail 300 IN A 192.0.2.2\n" + + "www 300 IN A 192.0.2.1\n" + if string(got) != want { + t.Errorf("example.records =\n%s\nwant:\n%s", got, want) + } + + recs, err := parseRecords(dc, string(got)) + if err != nil { + t.Fatalf("parseRecords() rejected the recorded file: %v", err) + } + if len(recs) != len(desired) { + t.Errorf("parseRecords() returned %d records, want %d", len(recs), len(desired)) + } +} + +func TestRecorderWritesTheNativeRecordsBehindTheConvertedOnes(t *testing.T) { + dc := models.MustNewDomainConfig("example.com") + + withOriginal := dc.MustNewRecordConfig("www", 300, "A", "192.0.2.1") + withOriginal.Original = fakeNative{Name: "www", Type: "A"} + withoutOriginal := dc.MustNewRecordConfig("mail", 300, "A", "192.0.2.2") + + rec := NewRecorder() + rec.Observe(nil, models.Records{withOriginal, withoutOriginal}) + + dir := t.TempDir() + written, err := rec.WriteTo(dir, "example") + if err != nil { + t.Fatalf("WriteTo() error: %v", err) + } + if want := []string{filepath.Join(dir, "example.json")}; len(written) != 1 || written[0] != want[0] { + t.Fatalf("WriteTo() wrote %v, want %v", written, want) + } + + data, err := os.ReadFile(written[0]) + if err != nil { + t.Fatal(err) + } + var natives []fakeNative + if err := json.Unmarshal(data, &natives); err != nil { + t.Fatalf("example.json: %v", err) + } + if len(natives) != 1 { + t.Fatalf("example.json holds %d natives, want 1", len(natives)) + } + if want := (fakeNative{Name: "www", Type: "A"}); natives[0] != want { + t.Errorf("example.json holds %+v, want %+v", natives[0], want) + } +} + +func TestRecorderDiscardsDuplicates(t *testing.T) { + dc := models.MustNewDomainConfig("example.com") + rc := dc.MustNewRecordConfig("www", 300, "A", "192.0.2.1") + rc.Original = fakeNative{Name: "www", Type: "A"} + + rec := NewRecorder() + for range 3 { + rec.Observe(models.Records{rc}, models.Records{rc}) + } + + dir := t.TempDir() + if _, err := rec.WriteTo(dir, "example"); err != nil { + t.Fatalf("WriteTo() error: %v", err) + } + + records, err := os.ReadFile(filepath.Join(dir, "example.records")) + if err != nil { + t.Fatal(err) + } + if want := "www 300 IN A 192.0.2.1\n"; string(records) != want { + t.Errorf("example.records = %q, want %q", records, want) + } + + data, err := os.ReadFile(filepath.Join(dir, "example.json")) + if err != nil { + t.Fatal(err) + } + var natives []fakeNative + if err := json.Unmarshal(data, &natives); err != nil { + t.Fatal(err) + } + if len(natives) != 1 { + t.Errorf("example.json holds %d natives, want 1", len(natives)) + } +} + +func TestRecorderReportsANativeItCannotMarshal(t *testing.T) { + dc := models.MustNewDomainConfig("example.com") + rc := dc.MustNewRecordConfig("www", 300, "A", "192.0.2.1") + rc.Original = make(chan int) + + rec := NewRecorder() + rec.Observe(nil, models.Records{rc}) + + written, err := rec.WriteTo(t.TempDir(), "example") + if err == nil { + t.Error("WriteTo() returned no error for a native that cannot be marshalled") + } + if len(written) != 0 { + t.Errorf("WriteTo() wrote %v, want nothing", written) + } +} + +func TestRecorderWritesNothingWhenItObservedNothing(t *testing.T) { + dir := t.TempDir() + + written, err := NewRecorder().WriteTo(dir, "example") + if err != nil { + t.Fatalf("WriteTo() error: %v", err) + } + if len(written) != 0 { + t.Errorf("WriteTo() wrote %v, want nothing", written) + } + + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + if len(entries) != 0 { + t.Errorf("WriteTo() created %d files, want 0", len(entries)) + } +} + +func TestRecordObservesTheConversionsAndReturnsWhatTheProviderReturned(t *testing.T) { + dc := models.MustNewDomainConfig("example.com") + dc.Records = models.Records{dc.MustNewRecordConfig("www", 300, "A", "192.0.2.1")} + + existing := dc.MustNewRecordConfig("mail", 300, "A", "192.0.2.2") + existing.Original = fakeNative{Name: "mail", Type: "A"} + + wantErr := errors.New("provider failed") + fake := &fakeProvider{ + corrections: []*models.Correction{{Msg: "a correction"}}, + count: 7, + err: wantErr, + } + + rec := NewRecorder() + corrections, count, err := Record(fake, rec).GetZoneRecordsCorrections(dc, models.Records{existing}) + + if fake.calls != 1 { + t.Errorf("provider was called %d times, want 1", fake.calls) + } + if len(corrections) != 1 || corrections[0].Msg != "a correction" { + t.Errorf("corrections = %v, want the provider's own", corrections) + } + if count != 7 { + t.Errorf("count = %d, want 7", count) + } + if !errors.Is(err, wantErr) { + t.Errorf("err = %v, want %v", err, wantErr) + } + + dir := t.TempDir() + written, err := rec.WriteTo(dir, "example") + if err != nil { + t.Fatalf("WriteTo() error: %v", err) + } + want := []string{filepath.Join(dir, "example.records"), filepath.Join(dir, "example.json")} + if len(written) != len(want) || written[0] != want[0] || written[1] != want[1] { + t.Fatalf("WriteTo() wrote %v, want %v", written, want) + } + + records, err := os.ReadFile(written[0]) + if err != nil { + t.Fatal(err) + } + if want := "www 300 IN A 192.0.2.1\n"; string(records) != want { + t.Errorf("example.records = %q, want %q", records, want) + } +} From 63624fa6813d378dcc3988ac455d11d9eec83998 Mon Sep 17 00:00:00 2001 From: Shuvam Kumar Date: Sun, 2 Aug 2026 10:03:07 +0530 Subject: [PATCH 05/10] TEST: providergolden: enrol 16 credentialed providers in the golden harness Each file adapts one provider's record conversion functions to CheckToRC / CheckToNative and names the recorded data after the provider and the function under test. All 22 tests skip today (" has no recorded data yet"), so dropping a -record capture into providers//testdata/ and running "go test ./providers// -update" produces the goldens with no further code. The adapters are per-provider because the signatures are: cloudflare's nativeToRecord is a method; ns1's convert, route53's and azure's nativeToRecords and gandiv5's nativeToRecords already return several records; gcloud's nativeToRecord takes one rdata string out of a set, so the adapter loops over set.Rrdatas the way getZoneSets does; digitalocean's and netlify's toReq return no error; cnr's createRecordString is a method that also needs the domain; vercel's toVercelCreateRequest needs the domain; luadns's recordsToNative takes a slice but maps each record independently, so a one-element slice exercises it faithfully. Providers of the 21 that are not enrolled here, and why: BIND ParseZoneContents takes a whole zone file, not a record MYTHICBEASTS zoneFileToRecords takes an io.Reader AXFRDDNS no separable converter (#4658 rewrites it) POWERDNS toRecordConfig needs the RRset's name/TTL/type, but RecordConfig.Original holds only zones.Record{Content,Disabled}; buildRecordList is the only encoder and it takes a diff2.Change NETNOD same on both counts: Original is netnodPrimaryDNS.Record{Content,Disabled} CNR and TRANSIP get the toNative half only: CNR sets Original to deleteRecordString(rc), a string rather than the map[string]string toRC reads, and TRANSIP never sets Original at all, so -record cannot produce a .json for either. The set-level encoders are left out because CheckToNative converts one record at a time: gandiv5's recordsToNative, azuredns's and azureprivatedns's recordToNativeDiff2, gcloud's mkRRSs and ns1's buildRecord. Each merges a whole recordset into one native, so calling it with a one-element slice would not exercise what it is for. testDomain is "example.com" in every file; it has to match the zone the data was recorded against. Co-Authored-By: Claude Opus 5 --- providers/azuredns/convert_golden_test.go | 18 ++++++++++++ .../azureprivatedns/convert_golden_test.go | 18 ++++++++++++ providers/cloudflare/convert_golden_test.go | 20 +++++++++++++ providers/cloudns/convert_golden_test.go | 22 ++++++++++++++ providers/cnr/convert_golden_test.go | 17 +++++++++++ providers/digitalocean/convert_golden_test.go | 26 +++++++++++++++++ providers/gandiv5/convert_golden_test.go | 13 +++++++++ providers/gcloud/convert_golden_test.go | 27 +++++++++++++++++ providers/hedns/convert_golden_test.go | 18 ++++++++++++ providers/luadns/convert_golden_test.go | 27 +++++++++++++++++ providers/namedotcom/convert_golden_test.go | 19 ++++++++++++ providers/netlify/convert_golden_test.go | 25 ++++++++++++++++ providers/ns1/convert_golden_test.go | 18 ++++++++++++ providers/route53/convert_golden_test.go | 19 ++++++++++++ providers/transip/convert_golden_test.go | 13 +++++++++ providers/vercel/convert_golden_test.go | 29 +++++++++++++++++++ 16 files changed, 329 insertions(+) create mode 100644 providers/azuredns/convert_golden_test.go create mode 100644 providers/azureprivatedns/convert_golden_test.go create mode 100644 providers/cloudflare/convert_golden_test.go create mode 100644 providers/cloudns/convert_golden_test.go create mode 100644 providers/cnr/convert_golden_test.go create mode 100644 providers/digitalocean/convert_golden_test.go create mode 100644 providers/gandiv5/convert_golden_test.go create mode 100644 providers/gcloud/convert_golden_test.go create mode 100644 providers/hedns/convert_golden_test.go create mode 100644 providers/luadns/convert_golden_test.go create mode 100644 providers/namedotcom/convert_golden_test.go create mode 100644 providers/netlify/convert_golden_test.go create mode 100644 providers/ns1/convert_golden_test.go create mode 100644 providers/route53/convert_golden_test.go create mode 100644 providers/transip/convert_golden_test.go create mode 100644 providers/vercel/convert_golden_test.go diff --git a/providers/azuredns/convert_golden_test.go b/providers/azuredns/convert_golden_test.go new file mode 100644 index 0000000000..ea77685dc8 --- /dev/null +++ b/providers/azuredns/convert_golden_test.go @@ -0,0 +1,18 @@ +package azuredns + +import ( + "testing" + + adns "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/dns/armdns" + "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" +) + +const testDomain = "example.com" + +func TestNativeToRecordsGolden(t *testing.T) { + providergolden.CheckToRC(t, "azuredns_nativetorecords", testDomain, + func(dc *models.DomainConfig, native adns.RecordSet) ([]*models.RecordConfig, error) { + return nativeToRecords(&native, dc), nil + }) +} diff --git a/providers/azureprivatedns/convert_golden_test.go b/providers/azureprivatedns/convert_golden_test.go new file mode 100644 index 0000000000..26dd530e40 --- /dev/null +++ b/providers/azureprivatedns/convert_golden_test.go @@ -0,0 +1,18 @@ +package azureprivatedns + +import ( + "testing" + + adns "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/privatedns/armprivatedns" + "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" +) + +const testDomain = "example.com" + +func TestNativeToRecordsGolden(t *testing.T) { + providergolden.CheckToRC(t, "azureprivatedns_nativetorecords", testDomain, + func(dc *models.DomainConfig, native adns.RecordSet) ([]*models.RecordConfig, error) { + return nativeToRecords(&native, dc), nil + }) +} diff --git a/providers/cloudflare/convert_golden_test.go b/providers/cloudflare/convert_golden_test.go new file mode 100644 index 0000000000..64c9606211 --- /dev/null +++ b/providers/cloudflare/convert_golden_test.go @@ -0,0 +1,20 @@ +package cloudflare + +import ( + "testing" + + "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" + "github.com/cloudflare/cloudflare-go" +) + +const testDomain = "example.com" + +func TestNativeToRecordGolden(t *testing.T) { + c := &cloudflareProvider{} + providergolden.CheckToRC(t, "cloudflare_nativetorecord", testDomain, + func(dc *models.DomainConfig, native cloudflare.DNSRecord) ([]*models.RecordConfig, error) { + rc, err := c.nativeToRecord(dc, native) + return []*models.RecordConfig{rc}, err + }) +} diff --git a/providers/cloudns/convert_golden_test.go b/providers/cloudns/convert_golden_test.go new file mode 100644 index 0000000000..f0e2e556d8 --- /dev/null +++ b/providers/cloudns/convert_golden_test.go @@ -0,0 +1,22 @@ +package cloudns + +import ( + "testing" + + "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" +) + +const testDomain = "example.com" + +func TestToRcGolden(t *testing.T) { + providergolden.CheckToRC(t, "cloudns_torc", testDomain, + func(dc *models.DomainConfig, native domainRecord) ([]*models.RecordConfig, error) { + rc, err := toRc(dc, &native) + return []*models.RecordConfig{rc}, err + }) +} + +func TestToReqGolden(t *testing.T) { + providergolden.CheckToNative(t, "cloudns_toreq", testDomain, toReq) +} diff --git a/providers/cnr/convert_golden_test.go b/providers/cnr/convert_golden_test.go new file mode 100644 index 0000000000..73a080b4a2 --- /dev/null +++ b/providers/cnr/convert_golden_test.go @@ -0,0 +1,17 @@ +package cnr + +import ( + "testing" + + "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" +) + +const testDomain = "example.com" + +func TestCreateRecordStringGolden(t *testing.T) { + providergolden.CheckToNative(t, "cnr_createrecordstring", testDomain, + func(rc *models.RecordConfig) (string, error) { + return (&Client{}).createRecordString(rc, testDomain) + }) +} diff --git a/providers/digitalocean/convert_golden_test.go b/providers/digitalocean/convert_golden_test.go new file mode 100644 index 0000000000..dfb1acc385 --- /dev/null +++ b/providers/digitalocean/convert_golden_test.go @@ -0,0 +1,26 @@ +package digitalocean + +import ( + "testing" + + "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" + "github.com/digitalocean/godo" +) + +const testDomain = "example.com" + +func TestToRcGolden(t *testing.T) { + providergolden.CheckToRC(t, "digitalocean_torc", testDomain, + func(dc *models.DomainConfig, native godo.DomainRecord) ([]*models.RecordConfig, error) { + rc, err := toRc(dc, &native) + return []*models.RecordConfig{rc}, err + }) +} + +func TestToReqGolden(t *testing.T) { + providergolden.CheckToNative(t, "digitalocean_toreq", testDomain, + func(rc *models.RecordConfig) (*godo.DomainRecordEditRequest, error) { + return toReq(rc), nil + }) +} diff --git a/providers/gandiv5/convert_golden_test.go b/providers/gandiv5/convert_golden_test.go new file mode 100644 index 0000000000..882b87d1a9 --- /dev/null +++ b/providers/gandiv5/convert_golden_test.go @@ -0,0 +1,13 @@ +package gandiv5 + +import ( + "testing" + + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" +) + +const testDomain = "example.com" + +func TestNativeToRecordsGolden(t *testing.T) { + providergolden.CheckToRC(t, "gandiv5_nativetorecords", testDomain, nativeToRecords) +} diff --git a/providers/gcloud/convert_golden_test.go b/providers/gcloud/convert_golden_test.go new file mode 100644 index 0000000000..85c7fb1d8e --- /dev/null +++ b/providers/gcloud/convert_golden_test.go @@ -0,0 +1,27 @@ +package gcloud + +import ( + "testing" + + "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" + gdns "google.golang.org/api/dns/v1" +) + +const testDomain = "example.com" + +func TestNativeToRecordGolden(t *testing.T) { + providergolden.CheckToRC(t, "gcloud_nativetorecord", testDomain, + func(dc *models.DomainConfig, native gdns.ResourceRecordSet) ([]*models.RecordConfig, error) { + // GCLOUD returns every value of a label/rtype pair in one set. + rcs := make([]*models.RecordConfig, 0, len(native.Rrdatas)) + for _, rdata := range native.Rrdatas { + rc, err := nativeToRecord(&native, rdata, dc) + if err != nil { + return nil, err + } + rcs = append(rcs, rc) + } + return rcs, nil + }) +} diff --git a/providers/hedns/convert_golden_test.go b/providers/hedns/convert_golden_test.go new file mode 100644 index 0000000000..9dbdf10f1c --- /dev/null +++ b/providers/hedns/convert_golden_test.go @@ -0,0 +1,18 @@ +package hedns + +import ( + "testing" + + "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" +) + +const testDomain = "example.com" + +func TestRecordToRCGolden(t *testing.T) { + providergolden.CheckToRC(t, "hedns_recordtorc", testDomain, + func(dc *models.DomainConfig, native Record) ([]*models.RecordConfig, error) { + rc, err := recordToRC(dc, native) + return []*models.RecordConfig{rc}, err + }) +} diff --git a/providers/luadns/convert_golden_test.go b/providers/luadns/convert_golden_test.go new file mode 100644 index 0000000000..ee11a3cf9c --- /dev/null +++ b/providers/luadns/convert_golden_test.go @@ -0,0 +1,27 @@ +package luadns + +import ( + "testing" + + api "github.com/luadns/luadns-go" + + "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" +) + +const testDomain = "example.com" + +func TestNativeToRecordGolden(t *testing.T) { + providergolden.CheckToRC(t, "luadns_nativetorecord", testDomain, + func(dc *models.DomainConfig, native api.Record) ([]*models.RecordConfig, error) { + rc, err := nativeToRecord(dc, &native) + return []*models.RecordConfig{rc}, err + }) +} + +func TestRecordsToNativeGolden(t *testing.T) { + providergolden.CheckToNative(t, "luadns_recordstonative", testDomain, + func(rc *models.RecordConfig) (*api.RR, error) { + return recordsToNative([]*models.RecordConfig{rc})[0], nil + }) +} diff --git a/providers/namedotcom/convert_golden_test.go b/providers/namedotcom/convert_golden_test.go new file mode 100644 index 0000000000..998dedcd14 --- /dev/null +++ b/providers/namedotcom/convert_golden_test.go @@ -0,0 +1,19 @@ +package namedotcom + +import ( + "testing" + + "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" + "github.com/namedotcom/go/namecom" +) + +const testDomain = "example.com" + +func TestToRecordGolden(t *testing.T) { + providergolden.CheckToRC(t, "namedotcom_torecord", testDomain, + func(dc *models.DomainConfig, native namecom.Record) ([]*models.RecordConfig, error) { + rc, err := toRecord(&native, dc) + return []*models.RecordConfig{rc}, err + }) +} diff --git a/providers/netlify/convert_golden_test.go b/providers/netlify/convert_golden_test.go new file mode 100644 index 0000000000..523c4afd60 --- /dev/null +++ b/providers/netlify/convert_golden_test.go @@ -0,0 +1,25 @@ +package netlify + +import ( + "testing" + + "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" +) + +const testDomain = "example.com" + +func TestToRecordConfigGolden(t *testing.T) { + providergolden.CheckToRC(t, "netlify_torecordconfig", testDomain, + func(dc *models.DomainConfig, native dnsRecord) ([]*models.RecordConfig, error) { + rc, err := toRecordConfig(dc, &native) + return []*models.RecordConfig{rc}, err + }) +} + +func TestToReqGolden(t *testing.T) { + providergolden.CheckToNative(t, "netlify_toreq", testDomain, + func(rc *models.RecordConfig) (*dnsRecordCreate, error) { + return toReq(rc), nil + }) +} diff --git a/providers/ns1/convert_golden_test.go b/providers/ns1/convert_golden_test.go new file mode 100644 index 0000000000..452b4e3360 --- /dev/null +++ b/providers/ns1/convert_golden_test.go @@ -0,0 +1,18 @@ +package ns1 + +import ( + "testing" + + "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" + "gopkg.in/ns1/ns1-go.v2/rest/model/dns" +) + +const testDomain = "example.com" + +func TestConvertGolden(t *testing.T) { + providergolden.CheckToRC(t, "ns1_convert", testDomain, + func(dc *models.DomainConfig, native dns.ZoneRecord) ([]*models.RecordConfig, error) { + return convert(&native, dc) + }) +} diff --git a/providers/route53/convert_golden_test.go b/providers/route53/convert_golden_test.go new file mode 100644 index 0000000000..c21668b756 --- /dev/null +++ b/providers/route53/convert_golden_test.go @@ -0,0 +1,19 @@ +package route53 + +import ( + "testing" + + r53Types "github.com/aws/aws-sdk-go-v2/service/route53/types" + + "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" +) + +const testDomain = "example.com" + +func TestNativeToRecordsGolden(t *testing.T) { + providergolden.CheckToRC(t, "route53_nativetorecords", testDomain, + func(dc *models.DomainConfig, native r53Types.ResourceRecordSet) ([]*models.RecordConfig, error) { + return nativeToRecords(dc, native, dc.Name) + }) +} diff --git a/providers/transip/convert_golden_test.go b/providers/transip/convert_golden_test.go new file mode 100644 index 0000000000..c699ca364c --- /dev/null +++ b/providers/transip/convert_golden_test.go @@ -0,0 +1,13 @@ +package transip + +import ( + "testing" + + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" +) + +const testDomain = "example.com" + +func TestRecordToNativeGolden(t *testing.T) { + providergolden.CheckToNative(t, "transip_recordtonative", testDomain, recordToNative) +} diff --git a/providers/vercel/convert_golden_test.go b/providers/vercel/convert_golden_test.go new file mode 100644 index 0000000000..198e88874d --- /dev/null +++ b/providers/vercel/convert_golden_test.go @@ -0,0 +1,29 @@ +package vercel + +import ( + "testing" + + "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" +) + +const testDomain = "example.com" + +func TestVercelRecordToRCGolden(t *testing.T) { + providergolden.CheckToRC(t, "vercel_vercelrecordtorc", testDomain, + func(dc *models.DomainConfig, native DNSRecord) ([]*models.RecordConfig, error) { + rc, err := vercelRecordToRC(dc, native) + return []*models.RecordConfig{rc}, err + }) +} + +func TestToVercelCreateRequestGolden(t *testing.T) { + providergolden.CheckToNative(t, "vercel_tovercelcreaterequest", testDomain, + func(rc *models.RecordConfig) (createDNSRecordRequest, error) { + return toVercelCreateRequest(testDomain, rc) + }) +} + +func TestToVercelUpdateRequestGolden(t *testing.T) { + providergolden.CheckToNative(t, "vercel_tovercelupdaterequest", testDomain, toVercelUpdateRequest) +} From abbea40b27370125f10404e6fd37ca39edab3b4e Mon Sep 17 00:00:00 2001 From: Shuvam Kumar Date: Sun, 2 Aug 2026 10:03:15 +0530 Subject: [PATCH 06/10] DOCS: providergolden: warn that the test's domain must match the recorded zone Nothing enforces that the domain passed to CheckToRC / CheckToNative is the zone the fixture was recorded against, and getting it wrong does not fail. Replaying a fixture recorded from realzone.net through a test that says example.com leaves every label fully qualified in the golden: www.realzone.net 300 IN A 192.0.2.1 LabelFromFQDNNoDot prints "ERROR: ... called WRONG" but returns the name lowercased rather than shortened, so the test passes, -update writes that golden and it becomes the baseline. Executed against providers/netlify. Also drops netlify as the named example from the Original warning: #4671 removed its in-place canonicalization of CNAME, MX and NS values, so the example no longer holds. The hazard it illustrates is general, so only the example goes. Co-Authored-By: Claude Opus 5 --- .../provider-conversion-tests.md | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/documentation/developer-info/provider-conversion-tests.md b/documentation/developer-info/provider-conversion-tests.md index 71b04892b1..0725a647bd 100644 --- a/documentation/developer-info/provider-conversion-tests.md +++ b/documentation/developer-info/provider-conversion-tests.md @@ -36,14 +36,31 @@ That writes two files named after the profile: Rename them to match the test you are about to add, and read them before committing: they contain whatever your zone contained during the run. +{% hint style="warning" %} +The domain the test passes to `CheckToRC` and `CheckToNative` has to be the zone +the data was recorded against, and nothing enforces it. Native records carry +labels as the API returned them, so a fixture recorded against `realzone.net` +replayed by a test that says `example.com` produces a golden whose labels are +still fully qualified: + +``` +www.realzone.net 300 IN A 192.0.2.1 +realzone.net 3600 IN MX 10 mail.example.org. +``` + +`LabelFromFQDNNoDot` and its siblings print `ERROR: ... called WRONG` when this +happens, but they return the name lowercased rather than shortened and the test +passes, so `-update` writes that golden and it becomes the baseline. Set the +domain before recording the golden, not after. +{% endhint %} + {% hint style="warning" %} `Original` is recorded as it stands once the provider has built the zone, which -is not always what the API sent. `providers/netlify` canonicalizes a CNAME, MX -or NS value in place before storing the record in `Original`, so a recorded -native carries a trailing dot the API did not send, and a golden replayed from -it never exercises the canonicalization. Check the recorded natives against the -API's own responses when a provider's converter writes to the record it was -given. +is not always what the API sent. A converter that canonicalizes a value in place +before storing the record in `Original` records a native that already carries +the canonicalization, and a golden replayed from it never exercises the line +that applies it. Check the recorded natives against the API's own responses when +a provider's converter writes to the record it was given. {% endhint %} Without `-record` nothing is collected, no file is written and the provider is From 7c2c06accc046d4fff5ca6924f967f57cb53eac8 Mon Sep 17 00:00:00 2001 From: Shuvam Kumar Date: Tue, 4 Aug 2026 01:58:14 +0530 Subject: [PATCH 07/10] TEST: Default the -record directory and take testDomain from the environment Addresses three of the four points raised in review. The documented -record path was relative to integrationTest/ rather than to the repo root, so the command in provider-conversion-tests.md wrote to integrationTest/providers//testdata. Executed with -profile BIND: the documented shape landed bind.records under integrationTest/. -record is now a bool, and the destination defaults to the testdata directory of the package the provider under test is implemented in, derived from the provider's own type: -profile BIND writes providers/bind/testdata/bind.records. -recorddir overrides it and implies -record. A relative -recorddir is still relative to integrationTest/ and needs the ../ prefix, which the documentation now states and shows. Passing a directory to -record, the old spelling, now fails naming the new one instead of being silently dropped by flag parsing. testDomain comes from providergolden.Domain(), which reads $_DOMAIN and falls back to "example.com" - the same variable integrationTest takes its test zone from. Applied to the 16 providers whose data is not recorded yet. packetframe, porkbun and websupport keep the literal, because their committed goldens are hand-written against example.com and would no longer match for anyone who has those variables set. One consequence of that, executed against a throwaway netlify fixture: a golden recorded while _DOMAIN names a real zone only matches while that variable still names it, so it does not match in a checkout that leaves it unset. Co-Authored-By: Claude Opus 5 --- .../provider-conversion-tests.md | 43 +++++++++++++++---- integrationTest/helpers_test.go | 33 ++++++++++---- pkg/providergolden/providergolden.go | 17 +++++++- pkg/providergolden/providergolden_test.go | 14 ++++++ pkg/providergolden/record.go | 40 +++++++++++++++++ pkg/providergolden/record_test.go | 14 ++++++ providers/azuredns/convert_golden_test.go | 2 +- .../azureprivatedns/convert_golden_test.go | 2 +- providers/cloudflare/convert_golden_test.go | 2 +- providers/cloudns/convert_golden_test.go | 2 +- providers/cnr/convert_golden_test.go | 2 +- providers/digitalocean/convert_golden_test.go | 2 +- providers/gandiv5/convert_golden_test.go | 2 +- providers/gcloud/convert_golden_test.go | 2 +- providers/hedns/convert_golden_test.go | 2 +- providers/luadns/convert_golden_test.go | 2 +- providers/namedotcom/convert_golden_test.go | 2 +- providers/netlify/convert_golden_test.go | 2 +- providers/ns1/convert_golden_test.go | 2 +- providers/route53/convert_golden_test.go | 2 +- providers/transip/convert_golden_test.go | 2 +- providers/vercel/convert_golden_test.go | 2 +- 22 files changed, 158 insertions(+), 35 deletions(-) diff --git a/documentation/developer-info/provider-conversion-tests.md b/documentation/developer-info/provider-conversion-tests.md index 0725a647bd..e84f4675ed 100644 --- a/documentation/developer-info/provider-conversion-tests.md +++ b/documentation/developer-info/provider-conversion-tests.md @@ -18,14 +18,25 @@ skipped, never failed. The integration tests already drive every conversion a provider has, so the easiest way to collect the data is to record an integration run. Add `-record` -and a directory to the command you normally use: +to the command you normally use: ```shell go test -run TestDNSProviders -timeout 1h -failfast -v ./integrationTest \ - -args -verbose -profile CLOUDFLAREAPI -record providers/cloudflare/testdata + -args -verbose -profile CLOUDFLAREAPI -record ``` -That writes two files named after the profile: +The recording goes to `providers//testdata`, the testdata directory of +the package the provider under test is implemented in, so `CLOUDFLAREAPI` writes +to `providers/cloudflare/testdata`. Use `-recorddir` to write somewhere else. +`go test` runs a test binary in its own package directory, so a relative +`-recorddir` is relative to `integrationTest/` and needs a `../` prefix: + +```shell +go test -run TestDNSProviders -timeout 1h -failfast -v ./integrationTest \ + -args -verbose -profile CLOUDFLAREAPI -recorddir ../providers/cloudflare/testdata +``` + +Either way two files are written, named after the profile: - `cloudflareapi.records` — every record the tests asked the provider to store, which is what a `CheckToNative` function is given. @@ -38,10 +49,13 @@ committing: they contain whatever your zone contained during the run. {% hint style="warning" %} The domain the test passes to `CheckToRC` and `CheckToNative` has to be the zone -the data was recorded against, and nothing enforces it. Native records carry -labels as the API returned them, so a fixture recorded against `realzone.net` -replayed by a test that says `example.com` produces a golden whose labels are -still fully qualified: +the data was recorded against, and nothing enforces it. `providergolden.Domain` +takes it from the same `_DOMAIN` variable the integration tests use, so +a recording and the test that replays it agree as long as that variable holds the +zone the data came from. Where they disagree, native records carry labels as the +API returned them, so a fixture recorded against `realzone.net` replayed by a +test that says `example.com` produces a golden whose labels are still fully +qualified: ``` www.realzone.net 300 IN A 192.0.2.1 @@ -52,6 +66,11 @@ realzone.net 3600 IN MX 10 mail.example.org. happens, but they return the name lowercased rather than shortened and the test passes, so `-update` writes that golden and it becomes the baseline. Set the domain before recording the golden, not after. + +A golden recorded against a zone other than `example.com` only matches while +`_DOMAIN` still names that zone, so it does not match in a checkout +that does not set it. Record from `example.com` when the golden is to be +committed. {% endhint %} {% hint style="warning" %} @@ -96,8 +115,10 @@ The test adapts your conversion function to a uniform signature. That adapter is the only code you write: ```go +var testDomain = providergolden.Domain("WEBSUPPORT") + func TestToRecordConfigGolden(t *testing.T) { - providergolden.CheckToRC(t, "websupport_torecordconfig", "example.com", + providergolden.CheckToRC(t, "websupport_torecordconfig", testDomain, func(dc *models.DomainConfig, native nativeRecord) ([]*models.RecordConfig, error) { rc, err := toRecordConfig(dc, native) return []*models.RecordConfig{rc}, err @@ -105,13 +126,17 @@ func TestToRecordConfigGolden(t *testing.T) { } ``` +`providergolden.Domain` returns `$WEBSUPPORT_DOMAIN`, or `example.com` when that +is unset, so the same test replays a recording of your own zone and the +committed fixtures. + Use `CheckToNative` for the other direction: `toNative`, `toReq`, `recordToCreateRequest`, or whatever your provider calls it. Its input is a list of DNS records rather than native records, so it reads a `.records` file: ```go func TestToReqGolden(t *testing.T) { - providergolden.CheckToNative(t, "porkbun_toreq", "example.com", toReq) + providergolden.CheckToNative(t, "porkbun_toreq", testDomain, toReq) } ``` diff --git a/integrationTest/helpers_test.go b/integrationTest/helpers_test.go index 6af6951845..93f84877a9 100644 --- a/integrationTest/helpers_test.go +++ b/integrationTest/helpers_test.go @@ -18,7 +18,8 @@ import ( var ( providerFlag = flag.String("provider", "", "Provider to run (if empty, deduced from -profile)") profileFlag = flag.String("profile", "", "Entry in profiles.json to use (if empty, copied from -provider)") - recordFlag = flag.String("record", "", "Directory to write the record conversion inputs seen during the run to") + recordFlag = flag.Bool("record", false, "Write the record conversion inputs seen during the run to the provider's testdata directory") + recordDirFlag = flag.String("recorddir", "", "Directory to record into, and implies -record (default: the provider's testdata directory)") enableCFWorkers = flag.Bool("cfworkers", true, "enable CF worker tests (default false)") enableCFRedirectMode = flag.Bool("cfredirect", true, "enable CF SingleRedirect tests (default false)") enableCFFlatten = flag.Bool("cfflatten", false, "enable CF CNAME flattening tests (requires paid plan, default false)") @@ -26,7 +27,7 @@ var ( ) // recorder accumulates the conversion inputs of every provider call made by -// this run, and is written out when -record names a directory. +// this run, and is written out when -record is given. var recorder = providergolden.NewRecorder() func init() { @@ -130,18 +131,34 @@ func getProvider(t *testing.T) (providers.DNSServiceProvider, string, map[string } } - if *recordFlag != "" { - t.Cleanup(func() { writeRecording(t) }) + if *recordFlag || *recordDirFlag != "" { + if flag.NArg() != 0 { + t.Fatalf("unexpected argument %q; the recording directory is set with -recorddir", flag.Arg(0)) + } + dir, err := recordingDir(provider) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { writeRecording(t, dir) }) return providergolden.Record(provider, recorder), cfg["domain"], cfg } return provider, cfg["domain"], cfg } -// writeRecording writes everything recorded so far to the -record directory, -// named after the profile under test. -func writeRecording(t *testing.T) { - written, err := recorder.WriteTo(*recordFlag, strings.ToLower(*profileFlag)) +// recordingDir is where a recording of p is written: -recorddir when it is +// given, and otherwise p's own testdata directory. +func recordingDir(p providers.DNSServiceProvider) (string, error) { + if *recordDirFlag != "" { + return *recordDirFlag, nil + } + return providergolden.TestdataDir(p) +} + +// writeRecording writes everything recorded so far to dir, named after the +// profile under test. +func writeRecording(t *testing.T, dir string) { + written, err := recorder.WriteTo(dir, strings.ToLower(*profileFlag)) for _, path := range written { t.Logf("Recorded %s", path) } diff --git a/pkg/providergolden/providergolden.go b/pkg/providergolden/providergolden.go index c537af611d..9824cb8853 100644 --- a/pkg/providergolden/providergolden.go +++ b/pkg/providergolden/providergolden.go @@ -5,8 +5,10 @@ // test names the recorded data and adapts the provider's function to a uniform // signature: // +// var testDomain = providergolden.Domain("WEBSUPPORT") +// // func TestToRecordConfig(t *testing.T) { -// providergolden.CheckToRC(t, "websupport_torecordconfig", "example.com", +// providergolden.CheckToRC(t, "websupport_torecordconfig", testDomain, // func(dc *models.DomainConfig, n nativeRecord) ([]*models.RecordConfig, error) { // rc, err := toRecordConfig(dc, n) // return []*models.RecordConfig{rc}, err @@ -30,7 +32,7 @@ // // That step is Recorder, which collects both kinds of input from a provider as // it is used. The integration tests wrap their provider in one when they are -// given "-record ". +// given "-record", and write what it collected to TestdataDir. // // A golden line is the record's label, TTL, class, type and RDATA, followed by // the metadata when the record has any: @@ -62,6 +64,17 @@ var update = flag.Bool("update", false, "rewrite the provider conversion golden const testdataDir = "testdata" +// Domain returns the zone the data for provider was recorded against: the value +// of the provider's _DOMAIN environment variable, or "example.com" +// when that is unset. It is the variable the integration tests take their test +// zone from, so a recording and the test that replays it agree by default. +func Domain(provider string) string { + if domain := os.Getenv(provider + "_DOMAIN"); domain != "" { + return domain + } + return "example.com" +} + // CheckToRC replays the native records recorded in testdata/.json through // convert and compares the records it returns with testdata/.golden. func CheckToRC[N any](t *testing.T, name, domain string, convert func(dc *models.DomainConfig, native N) ([]*models.RecordConfig, error)) { diff --git a/pkg/providergolden/providergolden_test.go b/pkg/providergolden/providergolden_test.go index 29223f3d13..ee97dc71d8 100644 --- a/pkg/providergolden/providergolden_test.go +++ b/pkg/providergolden/providergolden_test.go @@ -9,6 +9,20 @@ import ( "github.com/DNSControl/dnscontrol/v5/models" ) +func TestDomainReadsTheProvidersDomainVariable(t *testing.T) { + t.Setenv("VERCEL_DOMAIN", "recorded.example.net") + if got := Domain("VERCEL"); got != "recorded.example.net" { + t.Errorf("Domain() = %q, want %q", got, "recorded.example.net") + } +} + +func TestDomainFallsBackToExampleCom(t *testing.T) { + t.Setenv("VERCEL_DOMAIN", "") + if got := Domain("VERCEL"); got != "example.com" { + t.Errorf("Domain() = %q, want %q", got, "example.com") + } +} + func TestFormatRecord(t *testing.T) { dc := models.MustNewDomainConfig("example.com") diff --git a/pkg/providergolden/record.go b/pkg/providergolden/record.go index 65e5f807da..7af94ce489 100644 --- a/pkg/providergolden/record.go +++ b/pkg/providergolden/record.go @@ -6,7 +6,9 @@ import ( "fmt" "maps" "os" + "path" "path/filepath" + "reflect" "slices" "strings" "sync" @@ -97,6 +99,44 @@ func (r *Recorder) WriteTo(dir, name string) ([]string, error) { return written, errors.Join(r.errs...) } +// TestdataDir returns the testdata directory that belongs to p: +// providers//testdata under the module root, where is the +// package p is implemented in. +func TestdataDir(p models.DNSProvider) (string, error) { + t := reflect.TypeOf(p) + for t != nil && t.Kind() == reflect.Pointer { + t = t.Elem() + } + if t == nil || t.PkgPath() == "" { + return "", fmt.Errorf("cannot derive a testdata directory from provider type %T", p) + } + + root, err := moduleRoot() + if err != nil { + return "", err + } + return filepath.Join(root, "providers", path.Base(t.PkgPath()), testdataDir), nil +} + +// moduleRoot returns the directory of the nearest go.mod at or above the +// working directory. +func moduleRoot() (string, error) { + dir, err := os.Getwd() + if err != nil { + return "", err + } + for { + if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil { + return dir, nil + } + parent := filepath.Dir(dir) + if parent == dir { + return "", errors.New("no go.mod above the working directory") + } + dir = parent + } +} + // writeFile writes data to / and returns the path, creating dir // when it does not exist. func writeFile(dir, filename string, data []byte) (string, error) { diff --git a/pkg/providergolden/record_test.go b/pkg/providergolden/record_test.go index a366ada1fb..d1453137d7 100644 --- a/pkg/providergolden/record_test.go +++ b/pkg/providergolden/record_test.go @@ -5,6 +5,7 @@ import ( "errors" "os" "path/filepath" + "strings" "testing" "github.com/DNSControl/dnscontrol/v5/models" @@ -231,3 +232,16 @@ func TestRecordObservesTheConversionsAndReturnsWhatTheProviderReturned(t *testin t.Errorf("example.records = %q, want %q", records, want) } } + +func TestTestdataDirIsTheProvidersOwnTestdataDirectory(t *testing.T) { + dir, err := TestdataDir(&fakeProvider{}) + if err != nil { + t.Fatalf("TestdataDir() error: %v", err) + } + if !filepath.IsAbs(dir) { + t.Errorf("TestdataDir() = %q, want an absolute path", dir) + } + if want := filepath.Join("providers", "providergolden", testdataDir); !strings.HasSuffix(dir, want) { + t.Errorf("TestdataDir() = %q, want a path ending in %q", dir, want) + } +} diff --git a/providers/azuredns/convert_golden_test.go b/providers/azuredns/convert_golden_test.go index ea77685dc8..70a60d1256 100644 --- a/providers/azuredns/convert_golden_test.go +++ b/providers/azuredns/convert_golden_test.go @@ -8,7 +8,7 @@ import ( "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" ) -const testDomain = "example.com" +var testDomain = providergolden.Domain("AZURE_DNS") func TestNativeToRecordsGolden(t *testing.T) { providergolden.CheckToRC(t, "azuredns_nativetorecords", testDomain, diff --git a/providers/azureprivatedns/convert_golden_test.go b/providers/azureprivatedns/convert_golden_test.go index 26dd530e40..052dcc1a35 100644 --- a/providers/azureprivatedns/convert_golden_test.go +++ b/providers/azureprivatedns/convert_golden_test.go @@ -8,7 +8,7 @@ import ( "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" ) -const testDomain = "example.com" +var testDomain = providergolden.Domain("AZURE_PRIVATE_DNS") func TestNativeToRecordsGolden(t *testing.T) { providergolden.CheckToRC(t, "azureprivatedns_nativetorecords", testDomain, diff --git a/providers/cloudflare/convert_golden_test.go b/providers/cloudflare/convert_golden_test.go index 64c9606211..cc9fa9ff5f 100644 --- a/providers/cloudflare/convert_golden_test.go +++ b/providers/cloudflare/convert_golden_test.go @@ -8,7 +8,7 @@ import ( "github.com/cloudflare/cloudflare-go" ) -const testDomain = "example.com" +var testDomain = providergolden.Domain("CLOUDFLAREAPI") func TestNativeToRecordGolden(t *testing.T) { c := &cloudflareProvider{} diff --git a/providers/cloudns/convert_golden_test.go b/providers/cloudns/convert_golden_test.go index f0e2e556d8..d7e2ede0d4 100644 --- a/providers/cloudns/convert_golden_test.go +++ b/providers/cloudns/convert_golden_test.go @@ -7,7 +7,7 @@ import ( "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" ) -const testDomain = "example.com" +var testDomain = providergolden.Domain("CLOUDNS") func TestToRcGolden(t *testing.T) { providergolden.CheckToRC(t, "cloudns_torc", testDomain, diff --git a/providers/cnr/convert_golden_test.go b/providers/cnr/convert_golden_test.go index 73a080b4a2..43fdd9f146 100644 --- a/providers/cnr/convert_golden_test.go +++ b/providers/cnr/convert_golden_test.go @@ -7,7 +7,7 @@ import ( "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" ) -const testDomain = "example.com" +var testDomain = providergolden.Domain("CNR") func TestCreateRecordStringGolden(t *testing.T) { providergolden.CheckToNative(t, "cnr_createrecordstring", testDomain, diff --git a/providers/digitalocean/convert_golden_test.go b/providers/digitalocean/convert_golden_test.go index dfb1acc385..101f73c87c 100644 --- a/providers/digitalocean/convert_golden_test.go +++ b/providers/digitalocean/convert_golden_test.go @@ -8,7 +8,7 @@ import ( "github.com/digitalocean/godo" ) -const testDomain = "example.com" +var testDomain = providergolden.Domain("DIGITALOCEAN") func TestToRcGolden(t *testing.T) { providergolden.CheckToRC(t, "digitalocean_torc", testDomain, diff --git a/providers/gandiv5/convert_golden_test.go b/providers/gandiv5/convert_golden_test.go index 882b87d1a9..0e73a5ec7e 100644 --- a/providers/gandiv5/convert_golden_test.go +++ b/providers/gandiv5/convert_golden_test.go @@ -6,7 +6,7 @@ import ( "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" ) -const testDomain = "example.com" +var testDomain = providergolden.Domain("GANDI_V5") func TestNativeToRecordsGolden(t *testing.T) { providergolden.CheckToRC(t, "gandiv5_nativetorecords", testDomain, nativeToRecords) diff --git a/providers/gcloud/convert_golden_test.go b/providers/gcloud/convert_golden_test.go index 85c7fb1d8e..d731792af8 100644 --- a/providers/gcloud/convert_golden_test.go +++ b/providers/gcloud/convert_golden_test.go @@ -8,7 +8,7 @@ import ( gdns "google.golang.org/api/dns/v1" ) -const testDomain = "example.com" +var testDomain = providergolden.Domain("GCLOUD") func TestNativeToRecordGolden(t *testing.T) { providergolden.CheckToRC(t, "gcloud_nativetorecord", testDomain, diff --git a/providers/hedns/convert_golden_test.go b/providers/hedns/convert_golden_test.go index 9dbdf10f1c..2d6f787dc8 100644 --- a/providers/hedns/convert_golden_test.go +++ b/providers/hedns/convert_golden_test.go @@ -7,7 +7,7 @@ import ( "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" ) -const testDomain = "example.com" +var testDomain = providergolden.Domain("HEDNS") func TestRecordToRCGolden(t *testing.T) { providergolden.CheckToRC(t, "hedns_recordtorc", testDomain, diff --git a/providers/luadns/convert_golden_test.go b/providers/luadns/convert_golden_test.go index ee11a3cf9c..62cc40d246 100644 --- a/providers/luadns/convert_golden_test.go +++ b/providers/luadns/convert_golden_test.go @@ -9,7 +9,7 @@ import ( "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" ) -const testDomain = "example.com" +var testDomain = providergolden.Domain("LUADNS") func TestNativeToRecordGolden(t *testing.T) { providergolden.CheckToRC(t, "luadns_nativetorecord", testDomain, diff --git a/providers/namedotcom/convert_golden_test.go b/providers/namedotcom/convert_golden_test.go index 998dedcd14..1e73883e95 100644 --- a/providers/namedotcom/convert_golden_test.go +++ b/providers/namedotcom/convert_golden_test.go @@ -8,7 +8,7 @@ import ( "github.com/namedotcom/go/namecom" ) -const testDomain = "example.com" +var testDomain = providergolden.Domain("NAMEDOTCOM") func TestToRecordGolden(t *testing.T) { providergolden.CheckToRC(t, "namedotcom_torecord", testDomain, diff --git a/providers/netlify/convert_golden_test.go b/providers/netlify/convert_golden_test.go index 523c4afd60..9aa45a4351 100644 --- a/providers/netlify/convert_golden_test.go +++ b/providers/netlify/convert_golden_test.go @@ -7,7 +7,7 @@ import ( "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" ) -const testDomain = "example.com" +var testDomain = providergolden.Domain("NETLIFY") func TestToRecordConfigGolden(t *testing.T) { providergolden.CheckToRC(t, "netlify_torecordconfig", testDomain, diff --git a/providers/ns1/convert_golden_test.go b/providers/ns1/convert_golden_test.go index 452b4e3360..564991722e 100644 --- a/providers/ns1/convert_golden_test.go +++ b/providers/ns1/convert_golden_test.go @@ -8,7 +8,7 @@ import ( "gopkg.in/ns1/ns1-go.v2/rest/model/dns" ) -const testDomain = "example.com" +var testDomain = providergolden.Domain("NS1") func TestConvertGolden(t *testing.T) { providergolden.CheckToRC(t, "ns1_convert", testDomain, diff --git a/providers/route53/convert_golden_test.go b/providers/route53/convert_golden_test.go index c21668b756..40fbf09431 100644 --- a/providers/route53/convert_golden_test.go +++ b/providers/route53/convert_golden_test.go @@ -9,7 +9,7 @@ import ( "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" ) -const testDomain = "example.com" +var testDomain = providergolden.Domain("ROUTE53") func TestNativeToRecordsGolden(t *testing.T) { providergolden.CheckToRC(t, "route53_nativetorecords", testDomain, diff --git a/providers/transip/convert_golden_test.go b/providers/transip/convert_golden_test.go index c699ca364c..e4748b7c87 100644 --- a/providers/transip/convert_golden_test.go +++ b/providers/transip/convert_golden_test.go @@ -6,7 +6,7 @@ import ( "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" ) -const testDomain = "example.com" +var testDomain = providergolden.Domain("TRANSIP") func TestRecordToNativeGolden(t *testing.T) { providergolden.CheckToNative(t, "transip_recordtonative", testDomain, recordToNative) diff --git a/providers/vercel/convert_golden_test.go b/providers/vercel/convert_golden_test.go index 198e88874d..20bf6ddad0 100644 --- a/providers/vercel/convert_golden_test.go +++ b/providers/vercel/convert_golden_test.go @@ -7,7 +7,7 @@ import ( "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" ) -const testDomain = "example.com" +var testDomain = providergolden.Domain("VERCEL") func TestVercelRecordToRCGolden(t *testing.T) { providergolden.CheckToRC(t, "vercel_vercelrecordtorc", testDomain, From a3e46c007ab94c480ab669f170ab9002fd0b7900 Mon Sep 17 00:00:00 2001 From: Shuvam Kumar Date: Tue, 4 Aug 2026 02:31:01 +0530 Subject: [PATCH 08/10] TEST: Reject stray arguments and resolve -recorddir from the module root The previous commit claimed that passing a directory to -record "now fails naming the new one instead of being silently dropped by flag parsing". That was wrong, and this corrects it. -record is a bool, so the directory is a positional argument, and Go's flag package stops parsing there: -args -verbose -record ../providers/bind/testdata -profile BIND lost -profile, took getProvider's "No -provider or -profile specified" path and exited 0 having recorded nothing. The guard meant to catch it sat below that early return, so it never ran. It is now the first thing getProvider does, and no longer conditional on -record: integrationTest takes no positional arguments, so a stray one is always a mistake. The same command now reports unexpected argument "../providers/bind/testdata"; the recording directory is set with -recorddir A relative -recorddir was interpreted by the test binary's own working directory, so -recorddir providers/bind/testdata wrote to integrationTest/providers/bind/testdata - the same surprise under a new flag name, which the previous commit documented rather than fixed. It is now resolved against the module root, the root TestdataDir already derives, so the ../ prefix is no longer needed and the note about it is gone. The package doc and the developer documentation offered providergolden.Domain("WEBSUPPORT") as the example to copy. websupport is one of the three providers deliberately left on the literal, and its convert_test.go already declares const testDomain, so a provider author following the example there gets a duplicate declaration. The example is netlify now. Executed: the misordered command above (fails, naming the argument), -profile BIND -recorddir providers/bind/testdata (writes to /providers/bind/testdata), bare -record (writes to the provider's own testdata directory), and BIND_DOMAIN still selects the zone under test. go build, go test -count=1 ./... (76 ok, 0 FAIL), golangci-lint, staticcheck, go vet and the six CI go-checks commands are clean. Co-Authored-By: Claude Opus 5 --- .../provider-conversion-tests.md | 21 ++++++------ integrationTest/helpers_test.go | 9 ++--- pkg/providergolden/providergolden.go | 8 ++--- pkg/providergolden/record.go | 13 +++++++ pkg/providergolden/record_test.go | 34 +++++++++++++++++++ 5 files changed, 66 insertions(+), 19 deletions(-) diff --git a/documentation/developer-info/provider-conversion-tests.md b/documentation/developer-info/provider-conversion-tests.md index e84f4675ed..ec9d8f30ed 100644 --- a/documentation/developer-info/provider-conversion-tests.md +++ b/documentation/developer-info/provider-conversion-tests.md @@ -27,13 +27,12 @@ go test -run TestDNSProviders -timeout 1h -failfast -v ./integrationTest \ The recording goes to `providers//testdata`, the testdata directory of the package the provider under test is implemented in, so `CLOUDFLAREAPI` writes -to `providers/cloudflare/testdata`. Use `-recorddir` to write somewhere else. -`go test` runs a test binary in its own package directory, so a relative -`-recorddir` is relative to `integrationTest/` and needs a `../` prefix: +to `providers/cloudflare/testdata`. Use `-recorddir` to write somewhere else. A +relative `-recorddir` is relative to the top of the repository: ```shell go test -run TestDNSProviders -timeout 1h -failfast -v ./integrationTest \ - -args -verbose -profile CLOUDFLAREAPI -recorddir ../providers/cloudflare/testdata + -args -verbose -profile CLOUDFLAREAPI -recorddir providers/cloudflare/testdata ``` Either way two files are written, named after the profile: @@ -115,18 +114,18 @@ The test adapts your conversion function to a uniform signature. That adapter is the only code you write: ```go -var testDomain = providergolden.Domain("WEBSUPPORT") +var testDomain = providergolden.Domain("NETLIFY") func TestToRecordConfigGolden(t *testing.T) { - providergolden.CheckToRC(t, "websupport_torecordconfig", testDomain, - func(dc *models.DomainConfig, native nativeRecord) ([]*models.RecordConfig, error) { - rc, err := toRecordConfig(dc, native) + providergolden.CheckToRC(t, "netlify_torecordconfig", testDomain, + func(dc *models.DomainConfig, native dnsRecord) ([]*models.RecordConfig, error) { + rc, err := toRecordConfig(dc, &native) return []*models.RecordConfig{rc}, err }) } ``` -`providergolden.Domain` returns `$WEBSUPPORT_DOMAIN`, or `example.com` when that +`providergolden.Domain` returns `$NETLIFY_DOMAIN`, or `example.com` when that is unset, so the same test replays a recording of your own zone and the committed fixtures. @@ -136,7 +135,7 @@ of DNS records rather than native records, so it reads a `.records` file: ```go func TestToReqGolden(t *testing.T) { - providergolden.CheckToNative(t, "porkbun_toreq", testDomain, toReq) + providergolden.CheckToNative(t, "netlify_toreq", testDomain, toReq) } ``` @@ -146,7 +145,7 @@ usual way to write one is to copy it and add whatever else you want covered. ### 3. Generate the golden files ```shell -go test ./providers/websupport/ -update +go test ./providers/netlify/ -update ``` Read the generated files before committing them. `-update` records whatever the diff --git a/integrationTest/helpers_test.go b/integrationTest/helpers_test.go index 93f84877a9..4dc7bd9e5f 100644 --- a/integrationTest/helpers_test.go +++ b/integrationTest/helpers_test.go @@ -45,6 +45,10 @@ func panicOnErr(err error) { } func getProvider(t *testing.T) (providers.DNSServiceProvider, string, map[string]string) { + if flag.NArg() != 0 { + t.Fatalf("unexpected argument %q; the recording directory is set with -recorddir", flag.Arg(0)) + } + if *providerFlag == "" && *profileFlag == "" { t.Log("No -provider or -profile specified") return nil, "", nil @@ -132,9 +136,6 @@ func getProvider(t *testing.T) (providers.DNSServiceProvider, string, map[string } if *recordFlag || *recordDirFlag != "" { - if flag.NArg() != 0 { - t.Fatalf("unexpected argument %q; the recording directory is set with -recorddir", flag.Arg(0)) - } dir, err := recordingDir(provider) if err != nil { t.Fatal(err) @@ -150,7 +151,7 @@ func getProvider(t *testing.T) (providers.DNSServiceProvider, string, map[string // given, and otherwise p's own testdata directory. func recordingDir(p providers.DNSServiceProvider) (string, error) { if *recordDirFlag != "" { - return *recordDirFlag, nil + return providergolden.ResolveDir(*recordDirFlag) } return providergolden.TestdataDir(p) } diff --git a/pkg/providergolden/providergolden.go b/pkg/providergolden/providergolden.go index 9824cb8853..f3431bd9a0 100644 --- a/pkg/providergolden/providergolden.go +++ b/pkg/providergolden/providergolden.go @@ -5,12 +5,12 @@ // test names the recorded data and adapts the provider's function to a uniform // signature: // -// var testDomain = providergolden.Domain("WEBSUPPORT") +// var testDomain = providergolden.Domain("NETLIFY") // // func TestToRecordConfig(t *testing.T) { -// providergolden.CheckToRC(t, "websupport_torecordconfig", testDomain, -// func(dc *models.DomainConfig, n nativeRecord) ([]*models.RecordConfig, error) { -// rc, err := toRecordConfig(dc, n) +// providergolden.CheckToRC(t, "netlify_torecordconfig", testDomain, +// func(dc *models.DomainConfig, n dnsRecord) ([]*models.RecordConfig, error) { +// rc, err := toRecordConfig(dc, &n) // return []*models.RecordConfig{rc}, err // }) // } diff --git a/pkg/providergolden/record.go b/pkg/providergolden/record.go index 7af94ce489..9e366abe43 100644 --- a/pkg/providergolden/record.go +++ b/pkg/providergolden/record.go @@ -118,6 +118,19 @@ func TestdataDir(p models.DNSProvider) (string, error) { return filepath.Join(root, "providers", path.Base(t.PkgPath()), testdataDir), nil } +// ResolveDir returns dir as an absolute path, resolving a relative dir against +// the module root rather than the directory the test binary happens to run in. +func ResolveDir(dir string) (string, error) { + if filepath.IsAbs(dir) { + return dir, nil + } + root, err := moduleRoot() + if err != nil { + return "", err + } + return filepath.Join(root, dir), nil +} + // moduleRoot returns the directory of the nearest go.mod at or above the // working directory. func moduleRoot() (string, error) { diff --git a/pkg/providergolden/record_test.go b/pkg/providergolden/record_test.go index d1453137d7..cfce7dfaeb 100644 --- a/pkg/providergolden/record_test.go +++ b/pkg/providergolden/record_test.go @@ -245,3 +245,37 @@ func TestTestdataDirIsTheProvidersOwnTestdataDirectory(t *testing.T) { t.Errorf("TestdataDir() = %q, want a path ending in %q", dir, want) } } + +func TestResolveDirIsRelativeToTheModuleRoot(t *testing.T) { + rel := filepath.Join("providers", "bind", testdataDir) + + dir, err := ResolveDir(rel) + if err != nil { + t.Fatalf("ResolveDir(%q) error: %v", rel, err) + } + + wd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + if dir == filepath.Join(wd, rel) { + t.Errorf("ResolveDir(%q) = %q, want a path under the module root, not the working directory", rel, dir) + } + + root := strings.TrimSuffix(dir, string(filepath.Separator)+rel) + if _, err := os.Stat(filepath.Join(root, "go.mod")); err != nil { + t.Errorf("ResolveDir(%q) = %q, want a path under the directory holding go.mod: %v", rel, dir, err) + } +} + +func TestResolveDirKeepsAnAbsolutePath(t *testing.T) { + want := t.TempDir() + + dir, err := ResolveDir(want) + if err != nil { + t.Fatalf("ResolveDir(%q) error: %v", want, err) + } + if dir != want { + t.Errorf("ResolveDir(%q) = %q, want %q", want, dir, want) + } +} From ae3eede10ce25183354081a206fe58f0442aa7d1 Mon Sep 17 00:00:00 2001 From: shuvamk Date: Tue, 4 Aug 2026 02:52:23 +0530 Subject: [PATCH 09/10] TEST: Fix the CheckToNative doc example and cover recordingDir The `CheckToNative` example in provider-conversion-tests.md did not compile. It passed netlify's `toReq` directly, but that function is `func(rc *models.RecordConfig) *dnsRecordCreate` and `CheckToNative[N]` takes `func(rc *models.RecordConfig) (N, error)`: vet: in call to providergolden.CheckToNative, type func(rc *models.RecordConfig) *dnsRecordCreate of toReq does not match func(rc *models.RecordConfig) (N, error) (cannot infer N) The section is headed "That adapter is the only code you write", and providers/netlify/convert_golden_test.go wraps `toReq` in a closure for exactly this reason, so the page contradicted the file it names. The snippet is now byte-identical to that file, and both Go blocks of the section compile as written. `recordingDir` gained a test. The previous commit changed it to resolve `-recorddir` through `providergolden.ResolveDir`, but only `ResolveDir` itself was covered: reverting the call site left `go test ./...` green. With the call site reverted the new test reports recordingDir() = "providers/bind/testdata", want an absolute path Also drops the editorialising clause from the `ResolveDir` comment. Co-Authored-By: Claude Opus 5 --- .../provider-conversion-tests.md | 5 ++- integrationTest/recording_test.go | 31 +++++++++++++++++++ pkg/providergolden/record.go | 2 +- 3 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 integrationTest/recording_test.go diff --git a/documentation/developer-info/provider-conversion-tests.md b/documentation/developer-info/provider-conversion-tests.md index ec9d8f30ed..16cd10e6f8 100644 --- a/documentation/developer-info/provider-conversion-tests.md +++ b/documentation/developer-info/provider-conversion-tests.md @@ -135,7 +135,10 @@ of DNS records rather than native records, so it reads a `.records` file: ```go func TestToReqGolden(t *testing.T) { - providergolden.CheckToNative(t, "netlify_toreq", testDomain, toReq) + providergolden.CheckToNative(t, "netlify_toreq", testDomain, + func(rc *models.RecordConfig) (*dnsRecordCreate, error) { + return toReq(rc), nil + }) } ``` diff --git a/integrationTest/recording_test.go b/integrationTest/recording_test.go new file mode 100644 index 0000000000..9c8135b206 --- /dev/null +++ b/integrationTest/recording_test.go @@ -0,0 +1,31 @@ +package main + +// Test where a recording is written. + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestRecordingDirResolvesRecordDirFromTheModuleRoot(t *testing.T) { + rel := filepath.Join("providers", "bind", "testdata") + + old := *recordDirFlag + *recordDirFlag = rel + defer func() { *recordDirFlag = old }() + + dir, err := recordingDir(nil) + if err != nil { + t.Fatalf("recordingDir() error: %v", err) + } + if !filepath.IsAbs(dir) { + t.Fatalf("recordingDir() = %q, want an absolute path", dir) + } + + root := strings.TrimSuffix(dir, string(filepath.Separator)+rel) + if _, err := os.Stat(filepath.Join(root, "go.mod")); err != nil { + t.Errorf("recordingDir() = %q, want a path under the directory holding go.mod: %v", dir, err) + } +} diff --git a/pkg/providergolden/record.go b/pkg/providergolden/record.go index 9e366abe43..c88faeef19 100644 --- a/pkg/providergolden/record.go +++ b/pkg/providergolden/record.go @@ -119,7 +119,7 @@ func TestdataDir(p models.DNSProvider) (string, error) { } // ResolveDir returns dir as an absolute path, resolving a relative dir against -// the module root rather than the directory the test binary happens to run in. +// the module root. func ResolveDir(dir string) (string, error) { if filepath.IsAbs(dir) { return dir, nil From f450dee4fc0ef3736b50c822c5b5515267d5acf4 Mon Sep 17 00:00:00 2001 From: Shuvam Kumar Date: Tue, 4 Aug 2026 10:24:37 +0530 Subject: [PATCH 10/10] TEST: Name a recording so the golden tests find it `-record` wrote `.records` and `.json`, but CheckToRC and CheckToNative read `.json` and `.records` for the name the test passes, and every enrolled provider named that after the function it wraps. Nothing lined up, so a fresh recording was invisible: a CLOUDNS run wrote providers/cloudns/testdata/cloudns.records and cloudns.json, and providers/cloudns still reported --- SKIP: TestToRcGolden: cloudns_torc has no recorded data yet --- SKIP: TestToReqGolden: cloudns_toreq has no recorded data yet The recorder cannot name a file after a conversion function: it observes a provider, not a function, and it produces exactly one `.records` and one `.json` per run. vercel is where that bites, with two CheckToNative functions and one recording. So the two names are separated instead of merged. A recorded input belongs to the provider and is named after it; the golden belongs to the test and keeps the name the test passes. vercel's two functions now replay the same `vercel.records` into two goldens, which is what covering both of them means. The recording is named after the package the provider is implemented in, the same reflection that already picks the testdata directory, rather than after `-profile`. A profile name is chosen by whoever wrote profiles.json, and for four providers the type does not match the directory either: CLOUDFLAREAPI lives in providers/cloudflare, GANDI_V5 in providers/gandiv5, and both Azure types in providers/azuredns and providers/azureprivatedns. The tests take the same name from the directory they run in, so the two ends agree by construction and a recording needs no renaming. The six committed inputs are renamed; their goldens and contents are unchanged. All 19 enrolled providers now resolve to `testdata/.{json,records}`. Verified end to end without credentials by recording a BIND run and replaying it through a throwaway CheckToNative: before, the test skipped with "no recorded data yet" while the 341-record recording sat in the directory it reads; after, it read all 341. Co-Authored-By: Claude Opus 5 --- .../provider-conversion-tests.md | 23 ++++-- integrationTest/helpers_test.go | 12 ++-- integrationTest/recording_test.go | 18 +++++ pkg/providergolden/providergolden.go | 71 ++++++++++++++----- pkg/providergolden/providergolden_test.go | 51 +++++++++++++ pkg/providergolden/record.go | 31 +++++--- pkg/providergolden/record_test.go | 42 +++++++++++ ...packetframe_torc.json => packetframe.json} | 0 ...rame_toreq.records => packetframe.records} | 0 .../{porkbun_torc.json => porkbun.json} | 0 ...{porkbun_toreq.records => porkbun.records} | 0 ...rt_torecordconfig.json => websupport.json} | 0 ...rt_tonative.records => websupport.records} | 0 13 files changed, 211 insertions(+), 37 deletions(-) rename providers/packetframe/testdata/{packetframe_torc.json => packetframe.json} (100%) rename providers/packetframe/testdata/{packetframe_toreq.records => packetframe.records} (100%) rename providers/porkbun/testdata/{porkbun_torc.json => porkbun.json} (100%) rename providers/porkbun/testdata/{porkbun_toreq.records => porkbun.records} (100%) rename providers/websupport/testdata/{websupport_torecordconfig.json => websupport.json} (100%) rename providers/websupport/testdata/{websupport_tonative.records => websupport.records} (100%) diff --git a/documentation/developer-info/provider-conversion-tests.md b/documentation/developer-info/provider-conversion-tests.md index 16cd10e6f8..98468fe2ee 100644 --- a/documentation/developer-info/provider-conversion-tests.md +++ b/documentation/developer-info/provider-conversion-tests.md @@ -35,16 +35,18 @@ go test -run TestDNSProviders -timeout 1h -failfast -v ./integrationTest \ -args -verbose -profile CLOUDFLAREAPI -recorddir providers/cloudflare/testdata ``` -Either way two files are written, named after the profile: +Either way two files are written, named after the provider's package: -- `cloudflareapi.records` — every record the tests asked the provider to store, +- `cloudflare.records` — every record the tests asked the provider to store, which is what a `CheckToNative` function is given. -- `cloudflareapi.json` — the native record each returned record came from, read +- `cloudflare.json` — the native record each returned record came from, read from `RecordConfig.Original`. That is what a `CheckToRC` function is given. It is written only for providers that fill `Original` in. -Rename them to match the test you are about to add, and read them before -committing: they contain whatever your zone contained during the run. +Those are the names the tests read, so there is nothing to rename. One recording +feeds every test the provider has: a provider with two `CheckToNative` functions +replays the same `.records` file through both. Read the files before committing +them, though: they contain whatever your zone contained during the run. {% hint style="warning" %} The domain the test passes to `CheckToRC` and `CheckToNative` has to be the zone @@ -87,8 +89,10 @@ wrapped in a `models.DNSProvider`, which is all the integration tests ask of it today. A test that type-asserts a provider to an optional interface such as `ZoneCreator` would need the wrapper to forward that interface too. -Data can also be collected by hand. The `.json` file is a JSON array of the -native records your provider's API returns: +Data can also be collected by hand. It goes in `providers//testdata` +under the package's own name, `providers/netlify/testdata/netlify.json` and +`providers/netlify/testdata/netlify.records`. The `.json` file is a JSON array +of the native records your provider's API returns: ```json [ @@ -125,6 +129,11 @@ func TestToRecordConfigGolden(t *testing.T) { } ``` +The name is the golden file's, `netlify_torecordconfig.golden`, so name it after +the function under test. The recorded input is `netlify.json`, found from the +directory the test runs in rather than from that name, so several tests can +replay one recording. + `providergolden.Domain` returns `$NETLIFY_DOMAIN`, or `example.com` when that is unset, so the same test replays a recording of your own zone and the committed fixtures. diff --git a/integrationTest/helpers_test.go b/integrationTest/helpers_test.go index 4dc7bd9e5f..9d109d05a4 100644 --- a/integrationTest/helpers_test.go +++ b/integrationTest/helpers_test.go @@ -140,7 +140,11 @@ func getProvider(t *testing.T) (providers.DNSServiceProvider, string, map[string if err != nil { t.Fatal(err) } - t.Cleanup(func() { writeRecording(t, dir) }) + name, err := providergolden.ProviderName(provider) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { writeRecording(t, dir, name) }) return providergolden.Record(provider, recorder), cfg["domain"], cfg } @@ -157,9 +161,9 @@ func recordingDir(p providers.DNSServiceProvider) (string, error) { } // writeRecording writes everything recorded so far to dir, named after the -// profile under test. -func writeRecording(t *testing.T, dir string) { - written, err := recorder.WriteTo(dir, strings.ToLower(*profileFlag)) +// provider under test. +func writeRecording(t *testing.T, dir, name string) { + written, err := recorder.WriteTo(dir, name) for _, path := range written { t.Logf("Recorded %s", path) } diff --git a/integrationTest/recording_test.go b/integrationTest/recording_test.go index 9c8135b206..78e6187403 100644 --- a/integrationTest/recording_test.go +++ b/integrationTest/recording_test.go @@ -7,8 +7,26 @@ import ( "path/filepath" "strings" "testing" + + "github.com/DNSControl/dnscontrol/v5/pkg/providergolden" + "github.com/DNSControl/dnscontrol/v5/pkg/providers" ) +func TestARecordingIsNamedAfterTheProvidersPackageNotItsType(t *testing.T) { + provider, err := providers.CreateDNSProvider("GANDI_V5", map[string]string{"token": "not used"}, nil) + if err != nil { + t.Fatal(err) + } + + name, err := providergolden.ProviderName(provider) + if err != nil { + t.Fatalf("ProviderName() error: %v", err) + } + if name != "gandiv5" { + t.Errorf("ProviderName() = %q, want %q", name, "gandiv5") + } +} + func TestRecordingDirResolvesRecordDirFromTheModuleRoot(t *testing.T) { rel := filepath.Join("providers", "bind", "testdata") diff --git a/pkg/providergolden/providergolden.go b/pkg/providergolden/providergolden.go index f3431bd9a0..8ca7112b87 100644 --- a/pkg/providergolden/providergolden.go +++ b/pkg/providergolden/providergolden.go @@ -15,12 +15,17 @@ // }) // } // -// The data lives in the provider's testdata directory and is named after the -// provider and the function under test: +// The data lives in the provider's testdata directory. A recording covers the +// whole provider, so the input files are named after it, and the golden file is +// named after the test that produced it: // -// testdata/.json native records, as the provider's API returns them -// testdata/.records DNS records, in the golden line format below -// testdata/.golden the expected output +// testdata/.json native records, as the provider's API returns them +// testdata/.records DNS records, in the golden line format below +// testdata/.golden the expected output +// +// is the directory the test is running in, which is the same name +// the integration tests record under, so a recording needs no renaming and one +// recording feeds every test the provider has. // // CheckToRC reads the .json file, CheckToNative reads the .records file, and // both write the .golden file. A provider with no recorded data is skipped, so @@ -62,7 +67,14 @@ import ( var update = flag.Bool("update", false, "rewrite the provider conversion golden files") -const testdataDir = "testdata" +const ( + testdataDir = "testdata" + + // Extensions of the two kinds of recorded input, written by Recorder and + // read by CheckToRC and CheckToNative. + nativesExt = ".json" + recordsExt = ".records" +) // Domain returns the zone the data for provider was recorded against: the value // of the provider's _DOMAIN environment variable, or "example.com" @@ -75,22 +87,28 @@ func Domain(provider string) string { return "example.com" } -// CheckToRC replays the native records recorded in testdata/.json through -// convert and compares the records it returns with testdata/.golden. +// CheckToRC replays the native records recorded in testdata/.json +// through convert and compares the records it returns with +// testdata/.golden. func CheckToRC[N any](t *testing.T, name, domain string, convert func(dc *models.DomainConfig, native N) ([]*models.RecordConfig, error)) { t.Helper() - data, ok, err := loadInput(testdataDir, name+".json") + input, err := inputFile(nativesExt) + if err != nil { + t.Fatal(err) + } + + data, ok, err := loadInput(testdataDir, input) if err != nil { t.Fatal(err) } if !ok { - t.Skipf("%s has no recorded data yet", name) + t.Skipf("%s has no recorded data yet", filepath.Join(testdataDir, input)) } var natives []N if err := json.Unmarshal(data, &natives); err != nil { - t.Fatalf("%s.json: %v", name, err) + t.Fatalf("%s: %v", input, err) } dc := models.MustNewDomainConfig(domain) @@ -98,7 +116,7 @@ func CheckToRC[N any](t *testing.T, name, domain string, convert func(dc *models for i, native := range natives { recs, err := convert(dc, native) if err != nil { - t.Fatalf("%s.json: record %d: %v", name, i, err) + t.Fatalf("%s: record %d: %v", input, i, err) } for _, rc := range recs { if rc == nil { @@ -112,29 +130,35 @@ func CheckToRC[N any](t *testing.T, name, domain string, convert func(dc *models report(t, testdataDir, name, []byte(b.String())) } -// CheckToNative replays the records recorded in testdata/.records through -// convert and compares the native records it returns with testdata/.golden. +// CheckToNative replays the records recorded in testdata/.records +// through convert and compares the native records it returns with +// testdata/.golden. func CheckToNative[N any](t *testing.T, name, domain string, convert func(rc *models.RecordConfig) (N, error)) { t.Helper() - data, ok, err := loadInput(testdataDir, name+".records") + input, err := inputFile(recordsExt) + if err != nil { + t.Fatal(err) + } + + data, ok, err := loadInput(testdataDir, input) if err != nil { t.Fatal(err) } if !ok { - t.Skipf("%s has no recorded data yet", name) + t.Skipf("%s has no recorded data yet", filepath.Join(testdataDir, input)) } recs, err := parseRecords(models.MustNewDomainConfig(domain), string(data)) if err != nil { - t.Fatalf("%s.records: %v", name, err) + t.Fatalf("%s: %v", input, err) } natives := make([]N, 0, len(recs)) for i, rc := range recs { native, err := convert(rc) if err != nil { - t.Fatalf("%s.records: record %d: %v", name, i, err) + t.Fatalf("%s: record %d: %v", input, i, err) } natives = append(natives, native) } @@ -147,6 +171,17 @@ func CheckToNative[N any](t *testing.T, name, domain string, convert func(rc *mo report(t, testdataDir, name, append(got, '\n')) } +// inputFile returns the recorded input file with extension ext that belongs to +// the provider under test: the directory the test is running in is the +// provider's package directory, and a recording is named after it. +func inputFile(ext string) (string, error) { + wd, err := os.Getwd() + if err != nil { + return "", err + } + return filepath.Base(wd) + ext, nil +} + // loadInput reads a recorded input file. ok is false when the file does not // exist, which means the provider has not been enrolled yet. func loadInput(dir, filename string) (data []byte, ok bool, err error) { diff --git a/pkg/providergolden/providergolden_test.go b/pkg/providergolden/providergolden_test.go index ee97dc71d8..798c1d4ac1 100644 --- a/pkg/providergolden/providergolden_test.go +++ b/pkg/providergolden/providergolden_test.go @@ -142,6 +142,57 @@ func TestParseRecordsRejectsMalformedInput(t *testing.T) { } } +func TestInputFileIsNamedAfterTheProvidersDirectory(t *testing.T) { + dir := filepath.Join(t.TempDir(), "cloudns") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + t.Chdir(dir) + + for ext, want := range map[string]string{nativesExt: "cloudns.json", recordsExt: "cloudns.records"} { + got, err := inputFile(ext) + if err != nil { + t.Fatalf("inputFile(%q) error: %v", ext, err) + } + if got != want { + t.Errorf("inputFile(%q) = %q, want %q", ext, got, want) + } + } +} + +func TestRecordedInputsAreNamedAfterTheirProvider(t *testing.T) { + root, err := moduleRoot() + if err != nil { + t.Fatal(err) + } + + dirs, err := filepath.Glob(filepath.Join(root, "providers", "*", testdataDir)) + if err != nil { + t.Fatal(err) + } + if len(dirs) == 0 { + t.Fatal("no provider has a testdata directory") + } + + for _, dir := range dirs { + provider := filepath.Base(filepath.Dir(dir)) + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + for _, entry := range entries { + ext := filepath.Ext(entry.Name()) + if ext != nativesExt && ext != recordsExt { + continue + } + if want := provider + ext; entry.Name() != want { + t.Errorf("providers/%s/%s holds %s, want %s: a recorded input is named after its provider, not after a test", + provider, testdataDir, entry.Name(), want) + } + } + } +} + func TestLoadInputReportsMissingFileAsNotEnrolled(t *testing.T) { _, ok, err := loadInput(t.TempDir(), "absent.json") if err != nil { diff --git a/pkg/providergolden/record.go b/pkg/providergolden/record.go index c88faeef19..cacf277745 100644 --- a/pkg/providergolden/record.go +++ b/pkg/providergolden/record.go @@ -62,6 +62,9 @@ func (r *Recorder) Observe(desired, existing models.Records) { // WriteTo writes what has been observed to dir as .records and // .json, sorted so that two runs of the same tests produce the same file. +// name is the provider the recording is of, as ProviderName returns it, which +// is the name CheckToRC and CheckToNative look for. +// // A file is not written when nothing of that kind was observed: a provider that // does not fill in RecordConfig.Original produces no .json. WriteTo // returns the paths it wrote. @@ -73,7 +76,7 @@ func (r *Recorder) WriteTo(dir, name string) ([]string, error) { if len(r.records) != 0 { text := strings.Join(slices.Sorted(maps.Keys(r.records)), "\n") + "\n" - path, err := writeFile(dir, name+".records", []byte(text)) + path, err := writeFile(dir, name+recordsExt, []byte(text)) if err != nil { return written, err } @@ -89,7 +92,7 @@ func (r *Recorder) WriteTo(dir, name string) ([]string, error) { if err != nil { return written, err } - path, err := writeFile(dir, name+".json", append(text, '\n')) + path, err := writeFile(dir, name+nativesExt, append(text, '\n')) if err != nil { return written, err } @@ -99,23 +102,35 @@ func (r *Recorder) WriteTo(dir, name string) ([]string, error) { return written, errors.Join(r.errs...) } -// TestdataDir returns the testdata directory that belongs to p: -// providers//testdata under the module root, where is the -// package p is implemented in. -func TestdataDir(p models.DNSProvider) (string, error) { +// ProviderName returns the name a recording of p is written under: the package +// p is implemented in. CheckToRC and CheckToNative take the same name from the +// directory they run in, so a recording lands where the provider's own tests +// look for it. +func ProviderName(p models.DNSProvider) (string, error) { t := reflect.TypeOf(p) for t != nil && t.Kind() == reflect.Pointer { t = t.Elem() } if t == nil || t.PkgPath() == "" { - return "", fmt.Errorf("cannot derive a testdata directory from provider type %T", p) + return "", fmt.Errorf("cannot derive a provider name from provider type %T", p) + } + return path.Base(t.PkgPath()), nil +} + +// TestdataDir returns the testdata directory that belongs to p: +// providers//testdata under the module root, where is the +// package p is implemented in. +func TestdataDir(p models.DNSProvider) (string, error) { + name, err := ProviderName(p) + if err != nil { + return "", err } root, err := moduleRoot() if err != nil { return "", err } - return filepath.Join(root, "providers", path.Base(t.PkgPath()), testdataDir), nil + return filepath.Join(root, "providers", name, testdataDir), nil } // ResolveDir returns dir as an absolute path, resolving a relative dir against diff --git a/pkg/providergolden/record_test.go b/pkg/providergolden/record_test.go index cfce7dfaeb..e8705eec8d 100644 --- a/pkg/providergolden/record_test.go +++ b/pkg/providergolden/record_test.go @@ -233,6 +233,48 @@ func TestRecordObservesTheConversionsAndReturnsWhatTheProviderReturned(t *testin } } +func TestProviderNameIsThePackageTheProviderIsIn(t *testing.T) { + name, err := ProviderName(&fakeProvider{}) + if err != nil { + t.Fatalf("ProviderName() error: %v", err) + } + if name != "providergolden" { + t.Errorf("ProviderName() = %q, want %q", name, "providergolden") + } +} + +func TestARecordingIsNamedTheWayTheChecksReadIt(t *testing.T) { + name, err := ProviderName(&fakeProvider{}) + if err != nil { + t.Fatalf("ProviderName() error: %v", err) + } + + dc := models.MustNewDomainConfig("example.com") + rc := dc.MustNewRecordConfig("www", 300, "A", "192.0.2.1") + rc.Original = fakeNative{Name: "www", Type: "A"} + + rec := NewRecorder() + rec.Observe(models.Records{rc}, models.Records{rc}) + + packageDir := filepath.Join(t.TempDir(), name) + if _, err := rec.WriteTo(filepath.Join(packageDir, testdataDir), name); err != nil { + t.Fatalf("WriteTo() error: %v", err) + } + + t.Chdir(packageDir) + for _, ext := range []string{nativesExt, recordsExt} { + input, err := inputFile(ext) + if err != nil { + t.Fatalf("inputFile(%q) error: %v", ext, err) + } + if _, ok, err := loadInput(testdataDir, input); err != nil { + t.Fatalf("loadInput(%q) error: %v", input, err) + } else if !ok { + t.Errorf("the recording is not readable as %s", filepath.Join(testdataDir, input)) + } + } +} + func TestTestdataDirIsTheProvidersOwnTestdataDirectory(t *testing.T) { dir, err := TestdataDir(&fakeProvider{}) if err != nil { diff --git a/providers/packetframe/testdata/packetframe_torc.json b/providers/packetframe/testdata/packetframe.json similarity index 100% rename from providers/packetframe/testdata/packetframe_torc.json rename to providers/packetframe/testdata/packetframe.json diff --git a/providers/packetframe/testdata/packetframe_toreq.records b/providers/packetframe/testdata/packetframe.records similarity index 100% rename from providers/packetframe/testdata/packetframe_toreq.records rename to providers/packetframe/testdata/packetframe.records diff --git a/providers/porkbun/testdata/porkbun_torc.json b/providers/porkbun/testdata/porkbun.json similarity index 100% rename from providers/porkbun/testdata/porkbun_torc.json rename to providers/porkbun/testdata/porkbun.json diff --git a/providers/porkbun/testdata/porkbun_toreq.records b/providers/porkbun/testdata/porkbun.records similarity index 100% rename from providers/porkbun/testdata/porkbun_toreq.records rename to providers/porkbun/testdata/porkbun.records diff --git a/providers/websupport/testdata/websupport_torecordconfig.json b/providers/websupport/testdata/websupport.json similarity index 100% rename from providers/websupport/testdata/websupport_torecordconfig.json rename to providers/websupport/testdata/websupport.json diff --git a/providers/websupport/testdata/websupport_tonative.records b/providers/websupport/testdata/websupport.records similarity index 100% rename from providers/websupport/testdata/websupport_tonative.records rename to providers/websupport/testdata/websupport.records