From b0ebc34c58eb23d31ee6ec8fc31b7fcf65886f04 Mon Sep 17 00:00:00 2001 From: Alice39s Date: Tue, 15 Sep 2026 23:37:15 +0900 Subject: [PATCH 1/4] feat: let providers declare a record identity for duplicate checks --- documentation/provider/tencentdns.md | 14 +++ pkg/normalize/validate.go | 56 +++++++++-- pkg/normalize/validate_identity_test.go | 107 +++++++++++++++++++++ pkg/normalize/validate_test.go | 10 +- pkg/providers/providers.go | 19 ++++ providers/tencentdns/tencentdnsProvider.go | 5 +- 6 files changed, 198 insertions(+), 13 deletions(-) create mode 100644 pkg/normalize/validate_identity_test.go diff --git a/documentation/provider/tencentdns.md b/documentation/provider/tencentdns.md index 0f36991e65..f509e4e45a 100644 --- a/documentation/provider/tencentdns.md +++ b/documentation/provider/tencentdns.md @@ -100,6 +100,20 @@ D("example.com", REG_TENCENT, DnsProvider(DSP_TENCENT), Available line names, IDs, and weighted-routing features depend on the domain's DNSPod plan and site. Use the DNSPod `DescribeRecordLineList` API to obtain the valid line values for the domain. Using `tencentdns_line_id` avoids ambiguity and is recommended when managing records across different Tencent Cloud sites. +### Per-line answers for one name + +Each line stores its own record, so one name and type may appear once per line. Records that share a target are stored separately, which is how a name returns a different edge per region: + +{% code title="dnsconfig.js" %} +```javascript +D("example.com", REG_TENCENT, DnsProvider(DSP_TENCENT), + CNAME("www", "edge-hkg.example.net.", {tencentdns_line_id: "5=2"}), + CNAME("www", "edge-hkg.example.net.", {tencentdns_line_id: "5=5"}), + CNAME("www", "edge-lax.example.net.", {tencentdns_line_id: "5=4"}) +); +``` +{% endcode %} + ### Why use `ALIAS` for DNSPod DNSPod does not natively support the `ALIAS` record type. diff --git a/pkg/normalize/validate.go b/pkg/normalize/validate.go index 9ba0079e8b..6e5781f4c4 100644 --- a/pkg/normalize/validate.go +++ b/pkg/normalize/validate.go @@ -550,8 +550,9 @@ func ValidateAndNormalizeConfig(config *models.DNSConfig) (errs []error) { } for _, d := range config.Domains { + identityFn := domainRecordIdentity(d) // Check that CNAMES don't have to co-exist with any other records - errs = append(errs, checkCNAMEs(d)...) + errs = append(errs, checkCNAMEs(d, identityFn)...) // Check that only one SOA record exist for a zone errs = append(errs, checkMultipleSOAs(d)...) // Check that if any advanced record types are used in a domain, every provider for that domain supports them @@ -560,7 +561,7 @@ func ValidateAndNormalizeConfig(config *models.DNSConfig) (errs []error) { errs = append(errs, err) } // Check for duplicates - errs = append(errs, checkDuplicates(d.Records)...) + errs = append(errs, checkDuplicates(d.Records, identityFn)...) // Check for different TTLs under the same label errs = append(errs, checkRecordSetHasMultipleTTLs(d.Records)...) // Check for inconsistent R53 weighted routing metadata within a group @@ -632,14 +633,57 @@ func checkAutoDNSSEC(dc *models.DomainConfig) (errs []error) { return } -func checkCNAMEs(dc *models.DomainConfig) (errs []error) { +// recordIdentityString returns the string used to detect duplicate records. +// It is the label, the rType, the RDATA and any provider-declared identity +// text (for example DNSPod's record line). +func recordIdentityString(r *models.RecordConfig, extra func(*models.RecordConfig) string) string { + id := fmt.Sprintf("%s %s %s", r.GetLabelFQDN(), r.Type, r.ComparableV3) + if x := providerIdentity(r, extra); x != "" { + id += " " + x + } + return id +} + +// domainRecordIdentity returns the identity function declared by one of the +// domain's DNS providers, or nil if none declares one. +func domainRecordIdentity(d *models.DomainConfig) func(*models.RecordConfig) string { + for _, provider := range d.DNSProviderInstances { + if provider.ProviderType == "-" { + continue + } + if f := providers.GetRecordIdentity(provider.ProviderType); f != nil { + return f(d.Records) + } + } + return nil +} + +// providerIdentity returns the provider-declared identity text for a record, +// or "" when no provider declares one. +func providerIdentity(r *models.RecordConfig, extra func(*models.RecordConfig) string) string { + if extra == nil { + return "" + } + return extra(r) +} + +func checkCNAMEs(dc *models.DomainConfig, extra func(*models.RecordConfig) string) (errs []error) { cnames := map[string]bool{} proxiedCnames := map[string]bool{} + seenIdentity := map[string]bool{} for _, r := range dc.Records { if r.Type == "CNAME" { - if cnames[r.GetLabel()] { + // Without a provider-declared identity this is exactly the old + // rule: one CNAME per label. With one, two CNAMEs may share a + // label as long as the provider treats them as separate objects. + id := r.GetLabel() + if x := providerIdentity(r, extra); x != "" { + id += "|" + x + } + if seenIdentity[id] { errs = append(errs, fmt.Errorf("%s: cannot have multiple CNAMEs with same name: %s", r.FilePos, r.GetLabelFQDN())) } + seenIdentity[id] = true cnames[r.GetLabel()] = true if p, ok := r.Metadata["cloudflare_proxy"]; ok && (p == "on" || p == "full") { proxiedCnames[r.GetLabel()] = true @@ -677,10 +721,10 @@ func checkMultipleSOAs(dc *models.DomainConfig) (errs []error) { return } -func checkDuplicates(records models.Records) (errs []error) { +func checkDuplicates(records models.Records, extra func(*models.RecordConfig) string) (errs []error) { seen := make(map[string]*models.RecordConfig) for _, r := range records { - diffable := fmt.Sprintf("%s %s %s", r.GetLabelFQDN(), r.Type, r.ComparableV3) + diffable := recordIdentityString(r, extra) if seen[diffable] != nil { errs = append(errs, fmt.Errorf("exact duplicate record found: %s", diffable)) diff --git a/pkg/normalize/validate_identity_test.go b/pkg/normalize/validate_identity_test.go new file mode 100644 index 0000000000..07d003568a --- /dev/null +++ b/pkg/normalize/validate_identity_test.go @@ -0,0 +1,107 @@ +package normalize + +import ( + "strings" + "testing" + + dnsv2 "codeberg.org/miekg/dns" + + "github.com/DNSControl/dnscontrol/v5/models" + _ "github.com/DNSControl/dnscontrol/v5/providers/tencentdns" +) + +const lineZone = "smart-cluster.hats-saas.top" + +// Per-line answers for one name: four lines point at the same target. +var lineRoutes = []struct { + line string + target string +}{ + {"0", "edge-hkg.sgs.smart-cluster.hats-saas.top."}, + {"7=0", "edge-hkg.sgs.smart-cluster.hats-saas.top."}, + {"5=2", "edge-hkg.sgs.smart-cluster.hats-saas.top."}, + {"5=5", "edge-hkg.sgs.smart-cluster.hats-saas.top."}, + {"5=4", "edge-lax.sgs.smart-cluster.hats-saas.top."}, + {"5=6", "edge-lax.sgs.smart-cluster.hats-saas.top."}, + {"5=3", "edge-lon.sgs.smart-cluster.hats-saas.top."}, + {"5=0", "edge-lon.sgs.smart-cluster.hats-saas.top."}, +} + +func lineDomain(providerType string) *models.DomainConfig { + dc := models.MustNewDomainConfig(lineZone) + dc.RegistrarName = "NONE" + dc.DNSProviderNames = map[string]int{"lines": 1} + dc.DNSProviderInstances = []*models.DNSProviderInstance{{ + Name: "lines", + ProviderType: providerType, + }} + return dc +} + +func addLineRecords(dc *models.DomainConfig, rType uint16, target string) { + for _, route := range lineRoutes { + r := dc.MustNewRecordConfig("*.sgs", 60, rType, target) + r.Metadata["tencentdns_line_id"] = route.line + dc.AddRecordConfig(r) + } +} + +func validateDomain(t *testing.T, dc *models.DomainConfig) []error { + t.Helper() + return ValidateAndNormalizeConfig(&models.DNSConfig{ + Domains: []*models.DomainConfig{dc}, + }) +} + +func TestPerLineCNAMEsShareOneName(t *testing.T) { + dc := lineDomain("TENCENTDNS") + for _, route := range lineRoutes { + r := dc.MustNewRecordConfig("*.sgs", 60, dnsv2.TypeCNAME, route.target) + r.Metadata["tencentdns_line_id"] = route.line + dc.AddRecordConfig(r) + } + + if errs := validateDomain(t, dc); len(errs) != 0 { + t.Fatalf("expected no validation errors, got %v", errs) + } +} + +func TestPerLineAddressesMayShareOneTarget(t *testing.T) { + dc := lineDomain("TENCENTDNS") + addLineRecords(dc, dnsv2.TypeA, "109.105.193.70") + + if errs := validateDomain(t, dc); len(errs) != 0 { + t.Fatalf("expected no validation errors, got %v", errs) + } +} + +func TestPerLineRecordsStillFailWithoutProviderIdentity(t *testing.T) { + dc := lineDomain("ROUTE53") + addLineRecords(dc, dnsv2.TypeA, "109.105.193.70") + + errs := validateDomain(t, dc) + if len(errs) == 0 { + t.Fatal("expected duplicate errors for a provider that declares no record identity") + } + if !strings.Contains(errs[0].Error(), "exact duplicate record found") { + t.Fatalf("unexpected first error: %v", errs[0]) + } +} + +func TestTwoCNAMEsOnOneLineRemainAnError(t *testing.T) { + dc := lineDomain("TENCENTDNS") + first := dc.MustNewRecordConfig("*.sgs", 60, dnsv2.TypeCNAME, lineRoutes[0].target) + first.Metadata["tencentdns_line_id"] = lineRoutes[0].line + dc.AddRecordConfig(first) + second := dc.MustNewRecordConfig("*.sgs", 60, dnsv2.TypeCNAME, "other.example.net.") + second.Metadata["tencentdns_line_id"] = lineRoutes[0].line + dc.AddRecordConfig(second) + + errs := validateDomain(t, dc) + if len(errs) == 0 { + t.Fatal("expected an error for two CNAMEs on the same line") + } + if !strings.Contains(errs[0].Error(), "cannot have multiple CNAMEs with same name") { + t.Fatalf("unexpected first error: %v", errs[0]) + } +} diff --git a/pkg/normalize/validate_test.go b/pkg/normalize/validate_test.go index d539ed6568..8c5fe1ae88 100644 --- a/pkg/normalize/validate_test.go +++ b/pkg/normalize/validate_test.go @@ -322,7 +322,7 @@ func TestCNAMECloudflareProxied(t *testing.T) { dc.AddRecordConfig(recCNAME) recMX := dc.MustNewRecordConfig("mail", 0, dnsv2.TypeMX, 10, "smtp.example.com.") dc.AddRecordConfig(recMX) - errs := checkCNAMEs(dc) + errs := checkCNAMEs(dc, nil) if len(errs) != 0 { t.Errorf("Expected no errors for proxied CNAME + MX, got: %v", errs) } @@ -334,7 +334,7 @@ func TestCNAMECloudflareProxied(t *testing.T) { recMX2 := dc2.MustNewRecordConfig("mail", 0, dnsv2.TypeMX, 10, "smtp.example.com.") dc2.AddRecordConfig(recCNAME2) dc2.AddRecordConfig(recMX2) - errs2 := checkCNAMEs(dc2) + errs2 := checkCNAMEs(dc2, nil) if len(errs2) == 0 { t.Error("Expected error for non-proxied CNAME + MX, got none") } @@ -355,7 +355,7 @@ func TestCheckDuplicates(t *testing.T) { dc.AddTestRC(t, "@", 0, dnsv2.TypeNS, "ns3.foo.com.") // NOTE: The comparison ignores ttl. Therefore we don't test that. - errs := checkDuplicates(dc.Records) + errs := checkDuplicates(dc.Records, nil) if len(errs) != 0 { t.Errorf("Expected duplicate NOT found but found %q", errs) } @@ -368,7 +368,7 @@ func TestCheckDuplicates_dup_a(t *testing.T) { dc.AddTestRC(t, "@", 0, dnsv2.TypeA, "1.1.1.1") dc.AddTestRC(t, "@", 0, dnsv2.TypeA, "1.1.1.1") - errs := checkDuplicates(dc.Records) + errs := checkDuplicates(dc.Records, nil) if len(errs) == 0 { t.Error("Expect duplicate found but found none") } @@ -383,7 +383,7 @@ func TestCheckDuplicates_dup_ns(t *testing.T) { dc.AddTestRC(t, "@", 0, dnsv2.TypeNS, "ns2.foo.com.") dc.AddTestRC(t, "@", 0, dnsv2.TypeNS, "ns2.foo.com.") - errs := checkDuplicates(dc.Records) + errs := checkDuplicates(dc.Records, nil) if len(errs) == 0 { t.Error("Expect duplicate found but found none") } diff --git a/pkg/providers/providers.go b/pkg/providers/providers.go index 4f33d86f55..1bac7c732a 100644 --- a/pkg/providers/providers.go +++ b/pkg/providers/providers.go @@ -7,6 +7,7 @@ import ( "log" "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/diff2" ) // Registrar is an interface for a domain registrar. It can return a list of needed corrections to be applied in the future. Implement this only if the provider is a "registrar" (i.e. can update the NS records of the parent to a domain). @@ -50,11 +51,29 @@ type DspInitializerWithOptions func(map[string]string, json.RawMessage, CreateOp // detailing records that this provider can not support. type RecordAuditor func(models.Records) []error +// RecordIdentityFunc returns a function that produces additional identity +// text for a record: text that distinguishes two records which have the same +// label, rType and RDATA but are stored by the provider as separate objects +// (e.g. DNSPod record lines, Route 53 routing policies). It is used by the +// diff engine and by validation-time duplicate detection. +type RecordIdentityFunc func(models.Records) diff2.ComparableFunc + // DspFuncs lists functions registered with a provider. type DspFuncs struct { Initializer DspInitializer InitializerWithOptions DspInitializerWithOptions RecordAuditor RecordAuditor + RecordIdentity RecordIdentityFunc +} + +// GetRecordIdentity returns the provider's RecordIdentity function, or nil if +// the provider does not declare one. +func GetRecordIdentity(dType string) RecordIdentityFunc { + p, ok := DNSProviderTypes[dType] + if !ok { + return nil + } + return p.RecordIdentity } // DNSProviderTypes stores initializer for each DSP. diff --git a/providers/tencentdns/tencentdnsProvider.go b/providers/tencentdns/tencentdnsProvider.go index 718c0f5868..a8cb21e06f 100644 --- a/providers/tencentdns/tencentdnsProvider.go +++ b/providers/tencentdns/tencentdnsProvider.go @@ -37,8 +37,9 @@ func init() { const providerName = "TENCENTDNS" const providerMaintainer = "@cylonchau" fns := providers.DspFuncs{ - Initializer: newTencentDNSDsp, - RecordAuditor: AuditRecords, + Initializer: newTencentDNSDsp, + RecordAuditor: AuditRecords, + RecordIdentity: recordMetadataComparable, } providers.RegisterDomainServiceProviderType(providerName, fns, features) providers.RegisterRegistrarType(providerName, newTencentDNSReg) From e1321aa508c810b1361e1d2d1903ce2373f04377 Mon Sep 17 00:00:00 2001 From: Alice39s Date: Wed, 16 Sep 2026 22:37:18 +0900 Subject: [PATCH 2/4] fix: make provider record identity a pure function Validation runs before the provider has fetched the zone, so the identity function cannot depend on a record set. It takes a single record now, and the tencentdns implementation reads only that record: the line ID, falling back to the line name. Weight is no longer part of it, because the service keys records on name, line, type and value and rejects two records that differ only by weight. --- pkg/normalize/validate.go | 2 +- pkg/providers/providers.go | 14 +++++++------- providers/tencentdns/tencentdnsProvider.go | 19 ++++++++++++++++++- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/pkg/normalize/validate.go b/pkg/normalize/validate.go index 6e5781f4c4..e50571d490 100644 --- a/pkg/normalize/validate.go +++ b/pkg/normalize/validate.go @@ -652,7 +652,7 @@ func domainRecordIdentity(d *models.DomainConfig) func(*models.RecordConfig) str continue } if f := providers.GetRecordIdentity(provider.ProviderType); f != nil { - return f(d.Records) + return f } } return nil diff --git a/pkg/providers/providers.go b/pkg/providers/providers.go index 1bac7c732a..21a79bf412 100644 --- a/pkg/providers/providers.go +++ b/pkg/providers/providers.go @@ -7,7 +7,6 @@ import ( "log" "github.com/DNSControl/dnscontrol/v5/models" - "github.com/DNSControl/dnscontrol/v5/pkg/diff2" ) // Registrar is an interface for a domain registrar. It can return a list of needed corrections to be applied in the future. Implement this only if the provider is a "registrar" (i.e. can update the NS records of the parent to a domain). @@ -51,12 +50,13 @@ type DspInitializerWithOptions func(map[string]string, json.RawMessage, CreateOp // detailing records that this provider can not support. type RecordAuditor func(models.Records) []error -// RecordIdentityFunc returns a function that produces additional identity -// text for a record: text that distinguishes two records which have the same -// label, rType and RDATA but are stored by the provider as separate objects -// (e.g. DNSPod record lines, Route 53 routing policies). It is used by the -// diff engine and by validation-time duplicate detection. -type RecordIdentityFunc func(models.Records) diff2.ComparableFunc +// RecordIdentityFunc returns the identity text of a record: text that +// distinguishes two records which share a label, rType and RDATA but are stored +// by the provider as separate objects (DNSPod record lines, Route 53 routing +// policies). Validation uses it for duplicate detection, and validation runs +// before the provider has read the zone, so the function must depend only on +// the record it is given. +type RecordIdentityFunc func(*models.RecordConfig) string // DspFuncs lists functions registered with a provider. type DspFuncs struct { diff --git a/providers/tencentdns/tencentdnsProvider.go b/providers/tencentdns/tencentdnsProvider.go index a8cb21e06f..8b9b8d8be0 100644 --- a/providers/tencentdns/tencentdnsProvider.go +++ b/providers/tencentdns/tencentdnsProvider.go @@ -39,7 +39,7 @@ func init() { fns := providers.DspFuncs{ Initializer: newTencentDNSDsp, RecordAuditor: AuditRecords, - RecordIdentity: recordMetadataComparable, + RecordIdentity: recordIdentity, } providers.RegisterDomainServiceProviderType(providerName, fns, features) providers.RegisterRegistrarType(providerName, newTencentDNSReg) @@ -243,6 +243,23 @@ func recordMetadataComparable(existingRecords models.Records) diff2.ComparableFu } } +// recordIdentity returns the identity text used by validation-time duplicate +// detection. Validation runs before the provider has read the zone, so this +// reads nothing but the record itself: the line ID when set, otherwise the line +// name. Weight stays out, because it is not part of the key the service uses. +func recordIdentity(rc *models.RecordConfig) string { + if rc.Metadata == nil { + return "" + } + if lineID := rc.Metadata[metaRecordLineID]; lineID != "" { + return "line_id=" + lineID + } + if line := rc.Metadata[metaRecordLine]; line != "" { + return "line=" + line + } + return "" +} + func (p *tencentdnsProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, existingRecords models.Records) ([]*models.Correction, int, error) { var corrections []*models.Correction From 92409648be86d3b35dd7860d6e30f9282a5c0558 Mon Sep 17 00:00:00 2001 From: Alice39s Date: Wed, 16 Sep 2026 22:38:48 +0900 Subject: [PATCH 3/4] docs: document the record identity contract Providers that store per-line records now have a documented hook: what it does, the rule that it must depend only on the record it is given, and what happens when a domain has several providers. The tencentdns page states that duplicate detection uses the key the service uses, and that mixing a line name with a line ID describes the same line twice. Tests cover weight, line names on their own, and the record-only signature. --- .../advanced-features/writing-providers.md | 22 +++++++++++++ documentation/provider/tencentdns.md | 2 ++ pkg/normalize/validate_identity_test.go | 31 +++++++++++++++++++ .../tencentdns/tencentdnsProvider_test.go | 14 +++++++++ 4 files changed, 69 insertions(+) diff --git a/documentation/advanced-features/writing-providers.md b/documentation/advanced-features/writing-providers.md index d22f5be00b..ada936d3f0 100644 --- a/documentation/advanced-features/writing-providers.md +++ b/documentation/advanced-features/writing-providers.md @@ -295,6 +295,28 @@ Enable optional capabilities in the `nameProvider.go` file and run the integrati FYI: If a provider's capabilities changes, run `go generate` to update the documentation. +### Record identity for providers with per-line records + +Some providers store the same name and type several times, splitting the answers by record line, region or routing policy. DNSPod lines, Huawei Cloud lines, Gcore GeoDNS and Route 53 routing policies all work that way. Those records share a label, type and RDATA, so validation would report them as duplicates. + +Such a provider can declare `RecordIdentity` in its `DspFuncs`: + +{% code title="nameProvider.go" %} +```go +fns := providers.DspFuncs{ + Initializer: newNameDsp, + RecordAuditor: AuditRecords, + RecordIdentity: recordIdentity, +} +``` +{% endcode %} + +`recordIdentity` takes one `models.RecordConfig` and returns the text that makes the record distinct, for example `"line_id=5=2"`. Validation appends that text to the key it uses for duplicate detection. A provider that does not declare it keeps the default rules. + +The function runs during validation, before the provider has read the zone, so it must depend only on the record it is given. Anything that needs the live zone belongs in the comparable function that the provider passes to `diff2.ByRecord`, not here. Turning a configured line name into a line ID is an example: only the zone knows the mapping. + +When a domain has several providers, the first one that declares an identity function is used. + ## Step 13: Automated code tests We use a number of automated code-checking systems. Please run your code through all of them and fix all warnings and errors. Some of the automated fixes may not alway sbe perfect. Therefore, it is best to commit your code before running these and verify that you agree with the changes. diff --git a/documentation/provider/tencentdns.md b/documentation/provider/tencentdns.md index f509e4e45a..00cb2eba18 100644 --- a/documentation/provider/tencentdns.md +++ b/documentation/provider/tencentdns.md @@ -114,6 +114,8 @@ D("example.com", REG_TENCENT, DnsProvider(DSP_TENCENT), ``` {% endcode %} +Duplicate detection compares the key DNSPod itself uses: name, line, type and value. The line ID is preferred, with the line name as a fallback. Use one style per line, because `tencentdns_line: "电信"` and `tencentdns_line_id: "10=1"` describe the same line in two ways, and validation has no zone to match them with. + ### Why use `ALIAS` for DNSPod DNSPod does not natively support the `ALIAS` record type. diff --git a/pkg/normalize/validate_identity_test.go b/pkg/normalize/validate_identity_test.go index 07d003568a..a341ce1b72 100644 --- a/pkg/normalize/validate_identity_test.go +++ b/pkg/normalize/validate_identity_test.go @@ -105,3 +105,34 @@ func TestTwoCNAMEsOnOneLineRemainAnError(t *testing.T) { t.Fatalf("unexpected first error: %v", errs[0]) } } + +func TestWeightDoesNotSplitTheIdentity(t *testing.T) { + dc := lineDomain("TENCENTDNS") + for _, weight := range []string{"10", "20"} { + r := dc.MustNewRecordConfig("weighted.sgs", 60, dnsv2.TypeA, "203.0.113.10") + r.Metadata["tencentdns_line_id"] = "5=2" + r.Metadata["tencentdns_weight"] = weight + dc.AddRecordConfig(r) + } + + errs := validateDomain(t, dc) + if len(errs) == 0 { + t.Fatal("expected duplicate detection: the service keys records without weight") + } + if !strings.Contains(errs[0].Error(), "exact duplicate record found") { + t.Fatalf("unexpected first error: %v", errs[0]) + } +} + +func TestLineNamesAloneAlsoCarryIdentity(t *testing.T) { + dc := lineDomain("TENCENTDNS") + for _, line := range []string{"电信", "联通"} { + r := dc.MustNewRecordConfig("named.sgs", 60, dnsv2.TypeA, "203.0.113.10") + r.Metadata["tencentdns_line"] = line + dc.AddRecordConfig(r) + } + + if errs := validateDomain(t, dc); len(errs) != 0 { + t.Fatalf("expected no validation errors, got %v", errs) + } +} diff --git a/providers/tencentdns/tencentdnsProvider_test.go b/providers/tencentdns/tencentdnsProvider_test.go index e8856aa025..c070727ae4 100644 --- a/providers/tencentdns/tencentdnsProvider_test.go +++ b/providers/tencentdns/tencentdnsProvider_test.go @@ -325,6 +325,20 @@ func makeLineRecord(domain, target string, metadata map[string]string) *models.R return rc } +// The validation identity must read nothing but the record it is given, and it +// must leave weight out: the service keys records on name, line, type and value. +func TestRecordIdentityReadsOnlyTheRecord(t *testing.T) { + assert.Equal(t, "", recordIdentity(makeLineRecord("example.com", "1.2.3.4", nil))) + assert.Equal(t, "line_id=5=2", recordIdentity(makeLineRecord("example.com", "1.2.3.4", + map[string]string{metaRecordLineID: "5=2"}))) + assert.Equal(t, "line=电信", recordIdentity(makeLineRecord("example.com", "1.2.3.4", + map[string]string{metaRecordLine: "电信"}))) + assert.Equal(t, "line_id=5=2", recordIdentity(makeLineRecord("example.com", "1.2.3.4", + map[string]string{metaRecordLineID: "5=2", metaRecordWeight: "10"}))) + assert.Equal(t, "line_id=5=2", recordIdentity(makeLineRecord("example.com", "1.2.3.4", + map[string]string{metaRecordLineID: "5=2", metaRecordWeight: "20"}))) +} + func TestMinTTLForGrade(t *testing.T) { packages := []*dnspod.PackageDetailItem{ { From 039e2fc8edbb516b25260ab8ad0036932a0a6929 Mon Sep 17 00:00:00 2001 From: Alice39s Date: Wed, 16 Sep 2026 23:24:02 +0900 Subject: [PATCH 4/4] fix: tighten the record identity boundaries Validation now treats a record without line metadata as the default line, so it matches an explicit line ID of 0. The default line also has one name and one ID. The tests describe a generic zone with RFC 5737 addresses, and the provider without an identity function is registered instead of unknown. The provider docs note where validation cannot see the zone. --- .../advanced-features/writing-providers.md | 8 +- documentation/provider/tencentdns.md | 12 ++- pkg/normalize/validate_identity_test.go | 100 ++++++++++++++---- providers/tencentdns/tencentdnsProvider.go | 22 ++-- .../tencentdns/tencentdnsProvider_test.go | 19 ++-- 5 files changed, 116 insertions(+), 45 deletions(-) diff --git a/documentation/advanced-features/writing-providers.md b/documentation/advanced-features/writing-providers.md index ada936d3f0..2721b3de0f 100644 --- a/documentation/advanced-features/writing-providers.md +++ b/documentation/advanced-features/writing-providers.md @@ -297,7 +297,7 @@ FYI: If a provider's capabilities changes, run `go generate` to update the docum ### Record identity for providers with per-line records -Some providers store the same name and type several times, splitting the answers by record line, region or routing policy. DNSPod lines, Huawei Cloud lines, Gcore GeoDNS and Route 53 routing policies all work that way. Those records share a label, type and RDATA, so validation would report them as duplicates. +Some providers store the same name and type several times, splitting the answers by record line, region or routing policy. DNSPod lines, Huawei Cloud lines, Gcore GeoDNS and ClouDNS geodns work that way. Those records share a label, type and RDATA, so validation would report them as duplicates. Such a provider can declare `RecordIdentity` in its `DspFuncs`: @@ -311,11 +311,13 @@ fns := providers.DspFuncs{ ``` {% endcode %} -`recordIdentity` takes one `models.RecordConfig` and returns the text that makes the record distinct, for example `"line_id=5=2"`. Validation appends that text to the key it uses for duplicate detection. A provider that does not declare it keeps the default rules. +`recordIdentity` takes one `models.RecordConfig` and returns the text that makes the record distinct, for example `"line_id=10=1"`. Validation appends that text to the key it uses for duplicate detection. A provider that does not declare it keeps the default rules. The function runs during validation, before the provider has read the zone, so it must depend only on the record it is given. Anything that needs the live zone belongs in the comparable function that the provider passes to `diff2.ByRecord`, not here. Turning a configured line name into a line ID is an example: only the zone knows the mapping. -When a domain has several providers, the first one that declares an identity function is used. +When a domain has several providers, the first one that declares an identity function is used. The exception applies to the whole domain, so a second provider on the same domain inherits it; two providers that both store per-line records should declare the same rule. + +`dnscontrol check` does not read `creds.json`, so a provider declared as `NewDnsProvider("name")` has no type at that point and its identity function does not run. Use the two-argument form when `check` should see it. ## Step 13: Automated code tests diff --git a/documentation/provider/tencentdns.md b/documentation/provider/tencentdns.md index 00cb2eba18..751c8187ea 100644 --- a/documentation/provider/tencentdns.md +++ b/documentation/provider/tencentdns.md @@ -102,20 +102,24 @@ Available line names, IDs, and weighted-routing features depend on the domain's ### Per-line answers for one name -Each line stores its own record, so one name and type may appear once per line. Records that share a target are stored separately, which is how a name returns a different edge per region: +Each line stores its own record, so one name and type may appear once per line. Records that share a target are still stored as separate records: {% code title="dnsconfig.js" %} ```javascript D("example.com", REG_TENCENT, DnsProvider(DSP_TENCENT), - CNAME("www", "edge-hkg.example.net.", {tencentdns_line_id: "5=2"}), - CNAME("www", "edge-hkg.example.net.", {tencentdns_line_id: "5=5"}), - CNAME("www", "edge-lax.example.net.", {tencentdns_line_id: "5=4"}) + CNAME("www", "origin.example.net.", {tencentdns_line_id: "10=1"}), + CNAME("www", "origin.example.net.", {tencentdns_line_id: "10=3"}), + CNAME("www", "origin.example.net.", {tencentdns_line_id: "10=2"}) ); ``` {% endcode %} Duplicate detection compares the key DNSPod itself uses: name, line, type and value. The line ID is preferred, with the line name as a fallback. Use one style per line, because `tencentdns_line: "电信"` and `tencentdns_line_id: "10=1"` describe the same line in two ways, and validation has no zone to match them with. +A record without line metadata answers on the default line, so it is the same record as one that sets `tencentdns_line_id: "0"` explicitly. + +Because validation cannot resolve a line name, a pair that describes one line in both styles passes `check` and is rejected by the service when the change is pushed. + ### Why use `ALIAS` for DNSPod DNSPod does not natively support the `ALIAS` record type. diff --git a/pkg/normalize/validate_identity_test.go b/pkg/normalize/validate_identity_test.go index a341ce1b72..0ed329e8aa 100644 --- a/pkg/normalize/validate_identity_test.go +++ b/pkg/normalize/validate_identity_test.go @@ -7,24 +7,29 @@ import ( dnsv2 "codeberg.org/miekg/dns" "github.com/DNSControl/dnscontrol/v5/models" + "github.com/DNSControl/dnscontrol/v5/pkg/providers" _ "github.com/DNSControl/dnscontrol/v5/providers/tencentdns" ) -const lineZone = "smart-cluster.hats-saas.top" +const lineZone = "example.com" -// Per-line answers for one name: four lines point at the same target. +// A provider that stores one record per line and declares no identity function +// keeps the duplicate rules that existed before the hook. +const plainProviderType = "TEST_NO_IDENTITY" + +func init() { + providers.RegisterDomainServiceProviderType(plainProviderType, providers.DspFuncs{}, providers.DocumentationNotes{}) +} + +// Four lines answer one name: two share a target, two point elsewhere. var lineRoutes = []struct { line string target string }{ - {"0", "edge-hkg.sgs.smart-cluster.hats-saas.top."}, - {"7=0", "edge-hkg.sgs.smart-cluster.hats-saas.top."}, - {"5=2", "edge-hkg.sgs.smart-cluster.hats-saas.top."}, - {"5=5", "edge-hkg.sgs.smart-cluster.hats-saas.top."}, - {"5=4", "edge-lax.sgs.smart-cluster.hats-saas.top."}, - {"5=6", "edge-lax.sgs.smart-cluster.hats-saas.top."}, - {"5=3", "edge-lon.sgs.smart-cluster.hats-saas.top."}, - {"5=0", "edge-lon.sgs.smart-cluster.hats-saas.top."}, + {"0", "origin.example.net."}, + {"10=1", "origin.example.net."}, + {"10=3", "backup.example.net."}, + {"10=2", "backup.example.net."}, } func lineDomain(providerType string) *models.DomainConfig { @@ -40,7 +45,7 @@ func lineDomain(providerType string) *models.DomainConfig { func addLineRecords(dc *models.DomainConfig, rType uint16, target string) { for _, route := range lineRoutes { - r := dc.MustNewRecordConfig("*.sgs", 60, rType, target) + r := dc.MustNewRecordConfig("edge", 60, rType, target) r.Metadata["tencentdns_line_id"] = route.line dc.AddRecordConfig(r) } @@ -56,7 +61,7 @@ func validateDomain(t *testing.T, dc *models.DomainConfig) []error { func TestPerLineCNAMEsShareOneName(t *testing.T) { dc := lineDomain("TENCENTDNS") for _, route := range lineRoutes { - r := dc.MustNewRecordConfig("*.sgs", 60, dnsv2.TypeCNAME, route.target) + r := dc.MustNewRecordConfig("edge", 60, dnsv2.TypeCNAME, route.target) r.Metadata["tencentdns_line_id"] = route.line dc.AddRecordConfig(r) } @@ -68,7 +73,7 @@ func TestPerLineCNAMEsShareOneName(t *testing.T) { func TestPerLineAddressesMayShareOneTarget(t *testing.T) { dc := lineDomain("TENCENTDNS") - addLineRecords(dc, dnsv2.TypeA, "109.105.193.70") + addLineRecords(dc, dnsv2.TypeA, "192.0.2.10") if errs := validateDomain(t, dc); len(errs) != 0 { t.Fatalf("expected no validation errors, got %v", errs) @@ -76,8 +81,14 @@ func TestPerLineAddressesMayShareOneTarget(t *testing.T) { } func TestPerLineRecordsStillFailWithoutProviderIdentity(t *testing.T) { - dc := lineDomain("ROUTE53") - addLineRecords(dc, dnsv2.TypeA, "109.105.193.70") + if _, ok := providers.DNSProviderTypes[plainProviderType]; !ok { + t.Fatalf("test setup: %s is not registered", plainProviderType) + } + if providers.GetRecordIdentity(plainProviderType) != nil { + t.Fatalf("test setup: %s declares an identity function", plainProviderType) + } + dc := lineDomain(plainProviderType) + addLineRecords(dc, dnsv2.TypeA, "192.0.2.10") errs := validateDomain(t, dc) if len(errs) == 0 { @@ -90,10 +101,10 @@ func TestPerLineRecordsStillFailWithoutProviderIdentity(t *testing.T) { func TestTwoCNAMEsOnOneLineRemainAnError(t *testing.T) { dc := lineDomain("TENCENTDNS") - first := dc.MustNewRecordConfig("*.sgs", 60, dnsv2.TypeCNAME, lineRoutes[0].target) + first := dc.MustNewRecordConfig("edge", 60, dnsv2.TypeCNAME, lineRoutes[0].target) first.Metadata["tencentdns_line_id"] = lineRoutes[0].line dc.AddRecordConfig(first) - second := dc.MustNewRecordConfig("*.sgs", 60, dnsv2.TypeCNAME, "other.example.net.") + second := dc.MustNewRecordConfig("edge", 60, dnsv2.TypeCNAME, "other.example.net.") second.Metadata["tencentdns_line_id"] = lineRoutes[0].line dc.AddRecordConfig(second) @@ -109,8 +120,8 @@ func TestTwoCNAMEsOnOneLineRemainAnError(t *testing.T) { func TestWeightDoesNotSplitTheIdentity(t *testing.T) { dc := lineDomain("TENCENTDNS") for _, weight := range []string{"10", "20"} { - r := dc.MustNewRecordConfig("weighted.sgs", 60, dnsv2.TypeA, "203.0.113.10") - r.Metadata["tencentdns_line_id"] = "5=2" + r := dc.MustNewRecordConfig("weighted", 60, dnsv2.TypeA, "203.0.113.10") + r.Metadata["tencentdns_line_id"] = "10=1" r.Metadata["tencentdns_weight"] = weight dc.AddRecordConfig(r) } @@ -127,7 +138,7 @@ func TestWeightDoesNotSplitTheIdentity(t *testing.T) { func TestLineNamesAloneAlsoCarryIdentity(t *testing.T) { dc := lineDomain("TENCENTDNS") for _, line := range []string{"电信", "联通"} { - r := dc.MustNewRecordConfig("named.sgs", 60, dnsv2.TypeA, "203.0.113.10") + r := dc.MustNewRecordConfig("named", 60, dnsv2.TypeA, "203.0.113.10") r.Metadata["tencentdns_line"] = line dc.AddRecordConfig(r) } @@ -136,3 +147,52 @@ func TestLineNamesAloneAlsoCarryIdentity(t *testing.T) { t.Fatalf("expected no validation errors, got %v", errs) } } + +// A record without line metadata answers on the default line, so it is the same +// record as one that names the default line explicitly. +func TestDefaultLineMatchesAnExplicitDefaultLine(t *testing.T) { + dc := lineDomain("TENCENTDNS") + dc.AddRecordConfig(dc.MustNewRecordConfig("default-host", 60, dnsv2.TypeA, "192.0.2.10")) + explicit := dc.MustNewRecordConfig("default-host", 60, dnsv2.TypeA, "192.0.2.10") + explicit.Metadata["tencentdns_line_id"] = "0" + dc.AddRecordConfig(explicit) + + errs := validateDomain(t, dc) + if len(errs) == 0 { + t.Fatal("expected duplicate detection: no line means the default line") + } + if !strings.Contains(errs[0].Error(), "exact duplicate record found") { + t.Fatalf("unexpected first error: %v", errs[0]) + } +} + +// The default line has both a name and an ID, and both describe one record. +func TestDefaultLineNameMatchesTheDefaultLineID(t *testing.T) { + dc := lineDomain("TENCENTDNS") + dc.AddRecordConfig(dc.MustNewRecordConfig("default-name", 60, dnsv2.TypeA, "192.0.2.10")) + named := dc.MustNewRecordConfig("default-name", 60, dnsv2.TypeA, "192.0.2.10") + named.Metadata["tencentdns_line"] = "默认" + dc.AddRecordConfig(named) + + errs := validateDomain(t, dc) + if len(errs) == 0 { + t.Fatal("expected duplicate detection: the default line has one name and one ID") + } +} + +// Validation runs before the provider reads the zone, so it cannot resolve a +// line name into a line ID. A configuration that describes one line in both +// styles passes here, and the service rejects the duplicate at push time. +func TestMixedLineStylesPassValidation(t *testing.T) { + dc := lineDomain("TENCENTDNS") + byName := dc.MustNewRecordConfig("mixed", 60, dnsv2.TypeA, "192.0.2.10") + byName.Metadata["tencentdns_line"] = "电信" + dc.AddRecordConfig(byName) + byID := dc.MustNewRecordConfig("mixed", 60, dnsv2.TypeA, "192.0.2.10") + byID.Metadata["tencentdns_line_id"] = "10=1" + dc.AddRecordConfig(byID) + + if errs := validateDomain(t, dc); len(errs) != 0 { + t.Fatalf("expected no validation errors, got %v", errs) + } +} diff --git a/providers/tencentdns/tencentdnsProvider.go b/providers/tencentdns/tencentdnsProvider.go index 8b9b8d8be0..5e4853ba99 100644 --- a/providers/tencentdns/tencentdnsProvider.go +++ b/providers/tencentdns/tencentdnsProvider.go @@ -248,16 +248,20 @@ func recordMetadataComparable(existingRecords models.Records) diff2.ComparableFu // reads nothing but the record itself: the line ID when set, otherwise the line // name. Weight stays out, because it is not part of the key the service uses. func recordIdentity(rc *models.RecordConfig) string { - if rc.Metadata == nil { - return "" - } - if lineID := rc.Metadata[metaRecordLineID]; lineID != "" { - return "line_id=" + lineID - } - if line := rc.Metadata[metaRecordLine]; line != "" { - return "line=" + line + if rc.Metadata != nil { + if lineID := rc.Metadata[metaRecordLineID]; lineID != "" { + return "line_id=" + lineID + } + if line := rc.Metadata[metaRecordLine]; line != "" { + // The default line has one name and one ID. + if line == defaultRecordLine { + return "line_id=" + defaultRecordLineID + } + return "line=" + line + } } - return "" + // A record without line metadata answers on the default line. + return "line_id=" + defaultRecordLineID } func (p *tencentdnsProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, existingRecords models.Records) ([]*models.Correction, int, error) { diff --git a/providers/tencentdns/tencentdnsProvider_test.go b/providers/tencentdns/tencentdnsProvider_test.go index c070727ae4..49b2967c08 100644 --- a/providers/tencentdns/tencentdnsProvider_test.go +++ b/providers/tencentdns/tencentdnsProvider_test.go @@ -325,18 +325,19 @@ func makeLineRecord(domain, target string, metadata map[string]string) *models.R return rc } -// The validation identity must read nothing but the record it is given, and it -// must leave weight out: the service keys records on name, line, type and value. +// The validation identity must read nothing but the record it is given. A +// record without line metadata answers on the default line, and weight stays +// out because the service keys records without it. func TestRecordIdentityReadsOnlyTheRecord(t *testing.T) { - assert.Equal(t, "", recordIdentity(makeLineRecord("example.com", "1.2.3.4", nil))) - assert.Equal(t, "line_id=5=2", recordIdentity(makeLineRecord("example.com", "1.2.3.4", - map[string]string{metaRecordLineID: "5=2"}))) + assert.Equal(t, "line_id=0", recordIdentity(makeLineRecord("example.com", "1.2.3.4", nil))) + assert.Equal(t, "line_id=10=1", recordIdentity(makeLineRecord("example.com", "1.2.3.4", + map[string]string{metaRecordLineID: "10=1"}))) assert.Equal(t, "line=电信", recordIdentity(makeLineRecord("example.com", "1.2.3.4", map[string]string{metaRecordLine: "电信"}))) - assert.Equal(t, "line_id=5=2", recordIdentity(makeLineRecord("example.com", "1.2.3.4", - map[string]string{metaRecordLineID: "5=2", metaRecordWeight: "10"}))) - assert.Equal(t, "line_id=5=2", recordIdentity(makeLineRecord("example.com", "1.2.3.4", - map[string]string{metaRecordLineID: "5=2", metaRecordWeight: "20"}))) + assert.Equal(t, "line_id=10=1", recordIdentity(makeLineRecord("example.com", "1.2.3.4", + map[string]string{metaRecordLineID: "10=1", metaRecordWeight: "10"}))) + assert.Equal(t, "line_id=10=1", recordIdentity(makeLineRecord("example.com", "1.2.3.4", + map[string]string{metaRecordLineID: "10=1", metaRecordWeight: "20"}))) } func TestMinTTLForGrade(t *testing.T) {