DNSControl v5 Release - #4414
Draft
TomOnTime wants to merge 279 commits into
Draft
Conversation
TomOnTime
marked this pull request as draft
July 2, 2026 19:30
Member
|
I tested this with PORKBUN and breaking. Can we remove |
This was referenced Jul 4, 2026
…ords (#4758) ## The bug `makeChanges()` passed `dom.Records` to `AuditRecords()`. `dom` is a copy of the `DomainConfig` built by `getDomainConfigWithNameservers()`, which calls `nameservers.AddNSRecords()` to synthesize one apex `NS` record per delegated nameserver. Those records are present in every single test case, so a provider whose auditor rejects apex `NS` records rejects *every* test case in the suite — including the record-less `Clean Slate` ones. This was latent until #4707 made `rejectif.NsAtApex` actually match the apex label (it had compared against `""` instead of `"@"`, so it never fired). DOMAINNAMESHOP registers that rule, so its entire integration run silently became a no-op: | | before #4707 | after #4707 | |---|---|---| | subtests executed | 230 (229 pass, 1 fail) | **0** | | skipped by the audit | 0 | **230** | | skipped by group filters | 72 | 72 | Zero API calls were made against the provider, and the run still reported `ok`. WEBSUPPORT has the same problem via its `rejectNS` rule, which rejects `NS` records unconditionally. ## Why production is unaffected That asymmetry is the point. `AuditRecords()` runs in `pkg/normalize/validate.go` on the user's records, whereas `AddNSRecords()` only runs later, in `generateDelegationCorrections()` (`commands/ppreviewPush.go`). In production the audit never sees synthesized apex NS records — only `NS()` records the user wrote themselves, which is exactly what these rules are meant to catch. So this is a test-harness bug, not a provider bug, and `NsAtApex` itself is correct as fixed in #4707. ## The fix Audit only the records the test case contributes, so the harness matches production ordering. Two details worth noting for review: - The audited slice shares its pointers with `dom.Records`, so it holds the same record objects the test goes on to push. - `Auditor.Audit()` only ever appends to a named return, so it yields `nil` for the empty slice a `Clean Slate` case produces. Those cases run rather than skipping on an empty error list. ## Verification Full run against DOMAINNAMESHOP (bdns.no): **230 subtests run and pass, 0 failures.** The 72 group-level filter skips are unchanged. The only tests this rule still skips are the two in `27:NS only APEX`: ``` ***SKIPPED(PROVIDER DOES NOT SUPPORT '[NS records not supported at apex]' ::"27:NS only APEX") ***SKIPPED(PROVIDER DOES NOT SUPPORT '[NS records not supported at apex NS records not supported at apex]' ::"27:NS only APEX") ``` Those are the only test cases in the entire suite carrying an apex NS record of their own (`ns("@", ...)` appears exactly twice, at `integration_test.go:511-512`), and they are precisely the tests #4707 set out to make skip. ## Note for WEBSUPPORT (@mtmn) This un-skips WEBSUPPORT's suite too, but its `26:NS` group will then genuinely fail rather than skip, since `rejectNS` rejects all NS records and WEBSUPPORT isn't in that group's `not()` list. Left alone here — needs a separate look from someone who can run against that provider. Related to #4748. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01WoVVephnrZDEmU4fzzWUzJ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Skip test as requested in #4748 (comment)
#4664 (commit: d5268bb) introduced a regression preventing successful integration tests harness run against release_candidate_v5. This PR addresses the regression. ``` --- FAIL: TestDNSProviders (370.74s) --- FAIL: TestDNSProviders/dnscontrol-test-zone-001.com (370.40s) --- FAIL: TestDNSProviders/dnscontrol-test-zone-001.com/04:CNAME:Create_a_CNAME (0.55s) --- FAIL: TestDNSProviders/dnscontrol-test-zone-001.com/05:CNAME-short:Create_a_CNAME (0.83s) --- FAIL: TestDNSProviders/dnscontrol-test-zone-001.com/17:TypeChangeHard:Create_a_CNAME (0.49s) --- FAIL: TestDNSProviders/dnscontrol-test-zone-001.com/22:CNAME:Record_pointing_to_@ (0.97s) --- FAIL: TestDNSProviders/dnscontrol-test-zone-001.com/26:NS:NS_for_subdomain (0.44s) --- FAIL: TestDNSProviders/dnscontrol-test-zone-001.com/33:IDNA:Internationalized_CNAME_Target (0.43s) --- FAIL: TestDNSProviders/dnscontrol-test-zone-001.com/34:IDNAs_in_CNAME_targets:IDN_CNAME_AND_Target (0.46s) --- FAIL: TestDNSProviders/dnscontrol-test-zone-001.com/43:PTR:Create_PTR_record (0.47s) --- FAIL: TestDNSProviders/dnscontrol-test-zone-001.com/55:ALIAS_on_apex:ALIAS_at_root (0.41s) --- FAIL: TestDNSProviders/dnscontrol-test-zone-001.com/56:ALIAS_to_nonfqdn:ALIAS_at_root (0.87s) --- FAIL: TestDNSProviders/dnscontrol-test-zone-001.com/57:ALIAS_on_subdomain:ALIAS_at_subdomain (0.41s) --- FAIL: TestDNSProviders/dnscontrol-test-zone-001.com/91:IGNORE_main:Create_some_records (2.29s) --- FAIL: TestDNSProviders/dnscontrol-test-zone-001.com/92:IGNORE_apex:Create_some_records (2.24s) --- FAIL: TestDNSProviders/dnscontrol-test-zone-001.com/94:IGNORE_wilds:Create_some_records (2.17s) --- FAIL: TestDNSProviders/dnscontrol-test-zone-001.com/96:IGNORE_TARGET_b2285:Create_some_records (0.52s) FAIL FAIL github.com/DNSControl/dnscontrol/v5/integrationTest 372.969s ```
Skip NullMX tests for Vercel and save us some headroom within rate limits.
Hi @TomOnTime here is tencentdns integration tests fix ``` === RUN TestMakeTests --- PASS: TestMakeTests (0.03s) === RUN TestDualProviders Testing Profile="TENCENTDNS" (TYPE="TENCENTDNS") provider_test.go:50: Clearing everything provider_test.go:44: #1: - DELETE final.drdm88.com TXT "TestDNSProviders was successful!" line_id=0 ttl=600 provider_test.go:62: Adding test nameservers provider_test.go:44: #1: + CREATE drdm88.com NS ns1.example.com. line_id=0 ttl=600 provider_test.go:44: #2: + CREATE drdm88.com NS ns2.example.com. line_id=0 ttl=600 provider_test.go:65: Running again to ensure stability provider_test.go:81: Removing test nameservers provider_test.go:44: #1: - DELETE drdm88.com NS ns1.example.com. line_id=0 ttl=600 provider_test.go:44: #2: - DELETE drdm88.com NS ns2.example.com. line_id=0 ttl=600 --- PASS: TestDualProviders (9.05s) === RUN TestNameserverDots Testing Profile="TENCENTDNS" (TYPE="TENCENTDNS") === RUN TestNameserverDots/No_trailing_dot_in_nameserver --- PASS: TestNameserverDots (2.86s) --- PASS: TestNameserverDots/No_trailing_dot_in_nameserver (0.00s) === RUN TestDuplicateNameservers Testing Profile="TENCENTDNS" (TYPE="TENCENTDNS") provider_test.go:150: Skipping. Deduplication logic is not implemented for this provider. --- SKIP: TestDuplicateNameservers (1.10s) === RUN TestARecordingIsNamedAfterTheProvidersPackageNotItsType --- PASS: TestARecordingIsNamedAfterTheProvidersPackageNotItsType (0.00s) === RUN TestRecordingDirResolvesRecordDirFromTheModuleRoot --- PASS: TestRecordingDirResolvesRecordDirFromTheModuleRoot (0.00s) PASS ok github.com/DNSControl/dnscontrol/v5/integrationTest 726.367s ``` ### 1. ALIAS Record Failures (Tests 55: `ALIAS on apex`, 56: `ALIAS to nonfqdn`, 57: `ALIAS on subdomain`) * **Original Error log**: ``` --- FAIL: TestDNSProviders/drdm88.com/55:ALIAS_on_apex:ALIAS_at_root (2.91s) --- FAIL: TestDNSProviders/drdm88.com/56:ALIAS_to_nonfqdn:ALIAS_at_root (3.21s) --- FAIL: TestDNSProviders/drdm88.com/57:ALIAS_on_subdomain:ALIAS_at_subdomain (2.77s) ``` * **Root Cause**: Tencent Cloud DNSPod does not have a native `ALIAS` record type. The driver previously faked ALIAS support by converting them into CNAME records and declared `CanUseAlias: Can()`. * **Why this fix**: In accordance with DNSControl conventions and aligned with other providers like `alidns` and `huaweicloud`, pseudo-ALIAS conversions should not be implemented in the provider. Setting `CanUseAlias: Cannot()` accurately reflects native capabilities and causes the integration test suite to automatically skip these tests. * **Changes Made**: - `providers/tencentdns/tencentdnsProvider.go`: Set `providers.CanUseAlias: providers.Cannot()`. - `providers/tencentdns/convert.go`: Removed all fake `ALIAS` conversion logic in `nativeToRecord`, `recordToCreateRequest`, and `recordToModifyRequest`. ### 2. TXT Special Character & Escaping Failures (Tests 28: `complex TXT`, 29: `TXT backslashes`) * **Original Error log**: ``` === RUN TestDNSProviders/drdm88.com/28:complex_TXT:TXT_with_1_dq-1interior helpers_integration_test.go:246: + CREATE foodq.drdm88.com TXT "in\"side" line_id=0 ttl=600 helpers_integration_test.go:251: [TencentCloudSDKError] Code=InvalidParameter.RecordValueInvalid, Message=记录的值不正确。, RequestId=201968b0-ecc9-4fa2-81bc-7790d7de45fe --- FAIL: TestDNSProviders/drdm88.com/28:complex_TXT:TXT_with_1_dq-1interior (2.45s) === RUN TestDNSProviders/drdm88.com/28:complex_TXT:TXT_trailing_ws helpers_integration_test.go:246: + CREATE foows1.drdm88.com TXT "trailingws " line_id=0 ttl=600 --- FAIL: TestDNSProviders/drdm88.com/28:complex_TXT:TXT_trailing_ws (3.12s) === RUN TestDNSProviders/drdm88.com/29:TXT_backslashes:TXT_with_backslashs helpers_integration_test.go:246: + CREATE foobs.drdm88.com TXT "1back\\slash" line_id=0 ttl=600 --- FAIL: TestDNSProviders/drdm88.com/29:TXT_backslashes:TXT_with_backslashs (3.24s) ``` * **Root Cause**: Tencent Cloud DNSPod API enforces strict sanitation/rejection on special characters in TXT record values. * **Why this fix**: Per DNSControl's design guidelines, providers should use `AuditRecords` with `rejectif` helpers to explicitly reject unsupported TXT character patterns rather than attempting fragile escaping hacks. The integration test runner automatically skips rejected record types. * **Changes Made**: - `providers/tencentdns/auditrecords.go`: Added `rejectif.TxtHasSingleQuotes`, `rejectif.TxtHasDoubleQuotes`, `rejectif.TxtHasBackslash`, and `rejectif.TxtHasTrailingSpace`. - `providers/tencentdns/auditrecords_test.go`: Added unit tests covering all TXT rejection constraints. ### 3. Non-Chinese Internationalized Domain Name (IDN) Failures (Tests 33: `IDNA`, 34: `IDNAs in CNAME targets`) * **Original Error log**: ``` === RUN TestDNSProviders/drdm88.com/33:IDNA:Create_an_IDNA helpers_integration_test.go:246: + CREATE xn--55qx5d.drdm88.com A 1.2.3.4 line_id=0 ttl=600 + CREATE xn--ndaaa.drdm88.com A 1.2.3.4 line_id=0 ttl=600 helpers_integration_test.go:251: [TencentCloudSDKError] Code=InvalidParameter.SubdomainInvalid, Message=主机记录不正确。, RequestId=... --- FAIL: TestDNSProviders/drdm88.com/33:IDNA (2.89s) ``` * **Root Cause**: Tencent Cloud DNSPod (like Alibaba DNS) only permits ASCII and Chinese characters (CJK Unified Ideographs `U+4E00..U+9FFF` and Extension A `U+3400..U+4DBF`), rejecting other Unicode scripts. * **Why this fix**: Aligned with `alidns` implementation: implemented `labelConstraint` (for record labels) and `targetConstraint` (decoding Punycode targets for CNAME/MX/NS/SRV) in `AuditRecords`. * **Changes Made**: - `providers/tencentdns/auditrecords.go`: Added `isValidTencentDNSString`, `labelConstraint`, and `targetConstraint`. - `providers/tencentdns/auditrecords_test.go`: Added unit tests for IDN label and target constraints. ### 4. Line Routing & Weight Metadata on Free account package / International Site (Test 71: `TENCENTDNS_LINE_WEIGHT`) * **Original Error log**: ``` === RUN TestDNSProviders/drdm88.com/72:TENCENTDNS_LINE_WEIGHT_INTL:create_records_on_the_default_and_regional_lines helpers_integration_test.go:246: + CREATE tencent-line.drdm88.com A 1.2.3.4 line=China Unicom ttl=600 helpers_integration_test.go:251: [TencentCloudSDKError] Code=InvalidParameter.RecordLineInvalid, Message=记录线路不正确。, RequestId=b6370ad3-7c66-4e01-af90-5dabde5412d7 --- FAIL: TestDNSProviders/drdm88.com/72:TENCENTDNS_LINE_WEIGHT_INTL:create_records_on_the_default_and_regional_lines (1.63s) ``` or change default or “默认” ``` ```text === RUN TestDNSProviders/drdm88.com/55:ALIAS_on_apex:ALIAS_at_root helpers_integration_test.go:246: + CREATE @.drdm88.com CNAME foo.com. line_id=0 ttl=600 helpers_integration_test.go:251: [TencentCloudSDKError] Code=InvalidParameter.ConflictRecord, Message=与已有默认线路记录冲突,无法添加。, RequestId=... --- FAIL: TestDNSProviders/drdm88.com/55:ALIAS_on_apex:ALIAS_at_root (2.41s) === RUN TestDNSProviders/drdm88.com/56:ALIAS_to_nonfqdn:ALIAS_at_root helpers_integration_test.go:246: + CREATE foo.drdm88.com A 1.2.3.4 line_id=0 ttl=600 + CREATE @.drdm88.com CNAME foo.drdm88.com. line_id=0 ttl=600 helpers_integration_test.go:251: [TencentCloudSDKError] Code=InvalidParameter.ConflictRecord, Message=与已有默认线路记录冲突,无法添加。, RequestId=... --- FAIL: TestDNSProviders/drdm88.com/56:ALIAS_to_nonfqdn:ALIAS_at_root (2.35s) === RUN TestDNSProviders/drdm88.com/57:ALIAS_on_subdomain:ALIAS_at_subdomain helpers_integration_test.go:246: + CREATE sub.drdm88.com CNAME foo.com. line_id=0 ttl=600 --- FAIL: TestDNSProviders/drdm88.com/57:ALIAS_on_subdomain:ALIAS_at_subdomain (2.52s) ``` * **Root Cause**: 1. Line names differ between the China site (ISP lines: `电信`, `联通`) and the International site (`Asia`, `Europe`). 2. More importantly, custom line routing (split DNS) is a paid feature in Tencent Cloud DNSPod. Free-tier domains (common in CI/integration testing) only support the default line (`0` / `默认`). * **Why this fix**: Preserved the China site test group (`TENCENTDNS_LINE_WEIGHT`) but dynamically checked `globalCfg["site"]` (loaded from `profiles.json`) using `alltrue(!strings.EqualFold(globalCfg["site"], "intl"))` so that international free-tier test runs automatically skip unsupported line tests. * **Changes Made**: - `integrationTest/helpers_integration_test.go`: Added `globalCfg` populated from `origConfig` in `runTests()`. - `integrationTest/integration_test.go`: Added `alltrue(!strings.EqualFold(globalCfg["site"], "intl"))` to guard `TENCENTDNS_LINE_WEIGHT`. ### 5. MX Preference Value 100 Failure (Test `manyTypesAtOnce`) * **Original Error log**: ``` === RUN TestDNSProviders/drdm88.com/manyTypesAtOnce:CreateManyTypesAtLabel helpers_integration_test.go:246: + CREATE testmx.drdm88.com MX 100 bar.com. line_id=0 ttl=600 helpers_integration_test.go:251: [TencentCloudSDKError] Code=InvalidParameter.RecordValueInvalid, Message=记录的值不正确。, RequestId=... --- FAIL: TestDNSProviders/drdm88.com/manyTypesAtOnce:CreateManyTypesAtLabel ``` Creating an MX record with priority 100 failed during integration tests. * **Root Cause**: Tencent Cloud DNSPod restricts MX priority to `1 ~ 50` for free-tier accounts (while paid plans support `0..65535`). * **Why this fix**: Adjusted the integration test record from priority 100 to 50 so tests pass on free-tier test accounts while avoiding hardcoding a 1..50 restriction in the provider's `AuditRecords` that would block paid plan users. * **Changes Made**: - `integrationTest/integration_test.go`: Changed `mx("testmx", 100, "bar.com.")` to `mx("testmx", 50, "bar.com.")` in `manyTypesAtOnce`. ### 6. Blacklisted Public IP `5.5.5.5` Failure (Tests 02: `Protocol-Wildcard`, 92: `IGNORE_main`, 93: `IGNORE_apex`, 95: `IGNORE_wilds`) * **Original Error log**: ``` === RUN TestDNSProviders/drdm88.com/02:Protocol-Wildcard:Create_wildcard helpers_integration_test.go:246: + CREATE www.drdm88.com A 5.5.5.5 line_id=0 ttl=600 helpers_integration_test.go:251: [TencentCloudSDKError] Code=OperationDenied.IPInBlacklistNotAllowed, Message=抱歉,不允许添加黑名单中的IP。, RequestId=82fc016f-a8ec-4d99-811e-716f479986e1 --- FAIL: TestDNSProviders/drdm88.com/02:Protocol-Wildcard:Create_wildcard (2.44s) ``` * **Root Cause**: Tencent Cloud DNSPod security policy blocks adding `5.5.5.5` as a DNS record target. * **Why this fix**: The integration test only tests wildcard and ignore mechanics; the IP address itself is arbitrary. Replaced `5.5.5.5` with `5.4.5.4` across the affected tests. * **Changes Made**: - `integrationTest/integration_test.go`: Replaced `5.5.5.5` with `5.4.5.4` in wildcard and ignore test suites.
Fixes #4759 Also adds a missing file pkg/prettyzone/sorting_test.go
…ases (#4786) ## What Fixes #4780, a bug found by an LLM inspecting the code. The `hasResolvedLastRound` must indeed be moved one up. I let an LLM generate a testcase for it and loo and behold: it failed to sort the most basic of record set without a good reason. Moving the `hasResolvedLastRound` to the "round" loop and it sorted it no problem. TLDR: good bot. ## Release changelog section BUGFIX: Fix dnssort issue not resolving all records in certain edgecases
Fixes #4781. ## 1. NAPTR — the reported bug The LLM was right. `toReq()` built the Dynu API `replacement` field from the record's `Service` rather than its `Replacement`: ```go naptrTarget := f.Service ``` `rdata.NAPTR` has both a `Service` and a `Replacement` field, so this compiled cleanly and sent the service string as the replacement on every create and update. The read path in `toRc()` was already correct, which is probably why it reads as fine at a glance — only writes were affected. It isn't a silent-corruption bug. Dynu validates `replacement` as a hostname, so a service value like `E2U+sip` is rejected outright and NAPTR records could not be pushed at all. Reverting just this line and running the NAPTR group against a live account: ``` === RUN TestDNSProviders/claudecode.com/42:NAPTR:NAPTR_record provider Dynu API error 505: Invalid host. --- FAIL: TestDNSProviders/claudecode.com/42:NAPTR:NAPTR_record (0.11s) ``` It fails on the first NAPTR case. With the fix, all 10 pass. ## 2. RP — a second bug found while verifying Running the full suite to check the NAPTR output surfaced an unrelated failure in both RP groups, `505: Invalid content.`. `toReq()` passed the RP mailbox and TXT domain name through unchanged, where every other name-valued field in that switch strips the trailing dot first. Dynu validates both as hostnames and rejects the trailing dot. Confirmed directly against the API: ``` mailBox=user.example.com. -> 505 Invalid content. mailBox=user.example.com -> 200 OK ``` DNSControl fully qualifies relative names before they reach the provider, so both fields always arrive with a trailing dot — `rp("foo", "user", "server")` arrives as `user.example.com.` and `server.example.com.`. That makes it unconditional rather than an edge case. The read path already runs both through `ensureTrailingDot()`, so the round trip is symmetric once the write path strips it. I've kept this as a separate commit so it can be dropped independently if you'd rather keep the PR scoped to the reported issue. ## Testing Verified against a live Dynu account and zone. | Check | Result | | --- | --- | | `gofmt -l providers/dynu/` | clean | | `go build ./...` | pass | | `go vet ./providers/dynu/...` | pass | | `go test ./providers/dynu/...` | pass | | `go test ./integrationTest/ -provider DYNU` | **305 passed, 0 failed, 112 skipped** | The suite is fully green with both fixes. Before them it failed on NAPTR and RP. New regression tests, each of which fails without its corresponding fix: - `TestToReqNAPTR` — write path, FQDN and null replacement, and asserts the other five NAPTR fields land in the right place - `TestToReqRP` — write path, absolute and relative names - Two NAPTR cases added to the existing `TestToRc` table — read path, including the null replacement round trip where Dynu represents `.` as an empty string No documentation change: `documentation/provider/dynu.md` already lists NAPTR and RP as supported, which is now actually true. --------- Co-authored-by: Rah Sharma <rah.sharma@dynu.systems> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The labelConstraint function checked rc.GetLabel() directly, which for
punycode labels like xn--ndaaa (ööö) returned pure ASCII and passed
isValidAliDNSString. The AliDNS API however rejects non-Chinese IDN
punycode labels with SubDomainInvalid.RR.
Mirror the same idna.ToUnicode decode that targetConstraint already
uses, so non-Chinese punycode labels are correctly rejected by
AuditRecords and the IDNA integration test
(33:IDNA:Internationalized_name) is skipped instead of hitting the live
API.
Also add unit tests: TestLabelConstraint and
TestAuditRecordsRejectsNonChineseIDNLabel.
<!--
## Before submiting a pull request
Please make sure you've run the following commands from the root
directory.
bin/generate-all.sh
(this runs commands like "go generate", fixes formatting, and so on)
## Release changelog section
Help keep the release changelog clear by pre-naming the proper section
in the GitHub pull request title.
Some examples:
* CICD: Add required GHA permissions for goreleaser
* DOCS: Fixed providers with "contributor support" table
* ROUTE53: Allow R53_ALIAS records to enable target health evaluation
More examples/context can be found in the file .goreleaser.yml under the
'build' > 'changelog' key.
!-->
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Tom Limoncelli <tal@whatexit.org>
…ord handling (#4791) Fixes #4761 The API key we use to test OVH does not have access to the "list all zones" API call. Therefore we need to be able to work without that call. When used, ListZones() returns an empty list and the systems that rely (verifying that a zone exists) assume the zone is valid. This mode is activated by the secret flag `preview --disable-list-zones` (works with `push` too) or when running integration tests, `-disablelistzones`. Example usage: ``` # Testing: go test -timeout 1h -failfast -v -args -verbose -profile OVH -disablelistzones # preview/push: dnscontrol preview --disable-list-zones ``` This issue was raised in #4761 Now that OVH integration tests work, we discovered that TXT records don't parse correctly. Fixed.
Work-around for bug in staticcheck no longer needed.
… verb (#4802) `fmt.Errorf` with no format directive is equivalent to `errors.New` but does a needless `Sprintf` pass (staticcheck **S1028**). **No behavior change.** ### Changes - Rewrite the 36 verb-less `fmt.Errorf("...")` call sites to `errors.New("...")` across 19 files, adjusting imports (`errors` in / `fmt` out) as needed. - Two auditors declared a local `var errors []error` that shadowed the `errors` package; renamed to `errs` so the package call resolves (`providers/openwrt/auditrecords.go`, `providers/unifi/auditrecords.go`). ### Verification `gofmt -l` clean · `go vet ./...` clean · `go build ./...` clean · `golangci-lint run` (v2.13.1) → 0 issues.
…4801) Small, mechanical cleanups from a repo-wide Go style pass. **No behavior change.** ### Changes - **Boolean returns** — collapse `if cond { return true }; return false` into `return cond` (`models/stutter.go`, `pkg/mustbe/hosts.go`, `providers/gcloud/gcloudProvider.go`). - **Regexp hoisting** — move constant `regexp.MustCompile(...)` calls out of frequently-called functions into package-level vars so they compile once (`commands/ppreviewPush.go`, `providers/desec/protocol.go`, `providers/softlayer/softlayerProvider.go`). - **Error paths** — drop a redundant `else` after a terminating branch, lowercase/depunctuate a few error strings, and fix a missing space in an autodns error message (`providers/autodns/api.go`, `providers/route53/route53Provider.go`, `providers/openwrt/api.go`). ### Verification `gofmt -l` clean · `go vet ./...` clean · `go build ./...` clean · `golangci-lint run` (v2.13.1) → 0 issues.
* Bump the `go` directive in `go.mod` from `1.26` to `1.27`. * Update types_generate.go to generate Go 1.27 constructs * Update non-generated files to Go 1.27 constructs
…4805) Fixes #4804, and extends the fix into a full audit of the domain-modifier docs. `commands/types/dnscontrol.d.ts` is generated by `build/generate` from each doc's front-matter (`parameters` + `parameter_types`). The LOC bug in #4804 turned out to be one instance of a broader class, so I cross-checked **every** doc in `documentation/language-reference/domain-modifiers/` against the real signatures (`models.Make*` arg counts, `pkg/js/helpers.js`, and `pkg/privatetypes/types_generate.yaml`). ### LOC (#4804) Added the four missing params — `name`, `ns`, `ew`, `...modifiers`: ```ts declare function LOC(name: string, deg1: number, min1: number, sec1: number, ns: "N" | "S" | "n" | "s", deg2: number, min2: number, sec2: number, ew: "E" | "W" | "e" | "w", altitude: number, size: number, horizontal_precision: number, vertical_precision: number, ...modifiers: RecordModifier[]): DomainModifier; ``` ### Additional mismatches found & fixed **Missing trailing `...modifiers`** (all are `rawrecordBuilder` records that accept trailing `RecordModifier`s, same as LOC): - `NAPTR` - `ADGUARDHOME_A_PASSTHROUGH` - `ADGUARDHOME_AAAA_PASSTHROUGH` - `CF_WORKER_ROUTE` **Wrong/stray params:** - `NAMESERVER` — removed a bogus `...modifiers`; the JS function throws if given more than one argument (`helpers.js:766`). - `NAMESERVER_TTL` — removed stray `target`/`modifiers...` entries from `parameter_types` (not real parameters). **Missing object properties:** - `DMARC_BUILDER` — added `percent` (`pct=`), `failureFormat` (`rf=`), and `reportInterval` (`ri=`), which the builder reads (`helpers.js:1711/1749/1755`) but the docs omitted.
Fixes #4796 * Bug 1: `R53_ALIAS`: Always sets the target to the apex domain. * Bug 2: `R53_EVALUATE_TARGET_HEALTH`: Always sets to "false". Changes: * MakeR53ALIAS was defined in two places (luckily the extra one was unused) * The JS r53AliasOptions function ignored the 3rd argument, which held the non-apex string. * evaluate_target_health was set to "false" no matter what. * Added test cases to 019-r53-alias.js to cover both bugs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Problem: RecordConfig has individual fields for each rtype: MxPreference, SrvPriority, SrvWeight, SrvPort, CaaTag, CaaFlag, DsKeyTag, DsAlgorithm, DnskeyFlags, DnskeyProtocol, etc. etc. etc. This wastes memory, requires bespoke functions for each type, and so on.
Solution: Add a field called .RDATA that is an interface reference to dns.RDATA. This stores a reference to a different struct for each rtype. Thus, saving memory (once the old fields are removed). In the meanwhile, bidirectional conversion from the old and new ways is automatic. Old providers still work. New providers can use the new features.
Resolution
Release Notes
DNSControl v5 is refactoring some of the biggest internal parts of the system. There should be zero user-facing
changes. However, I'm releasing a few "test balloons" to get feedback early.
Extra testing needed!
WORKER_ROUTEand sometimes asCF_WORKER_ROUTE. It's amazing such complex code ever worked. Now we useCF_WORKER_ROUTEexclusively. The changes were core to the worker feature. Pleaes give extra attending and testing.If you maintain a DNS provider, please give this release extra testing!
I only have automated testing for the following providers. All others are at risk of being broken!
AXFRDDNS_DNSSEC AZURE_DNS AZURE_PRIVATE_DNS BIND CLOUDFLAREAPI CNR
DIGITALOCEAN GANDI_V5 GCLOUD HEDNS MYTHICBEASTS NETNOD NS1 ROUTE53
SAKURACLOUD TRANSIP VERCEL
Provider maintainers! Please run the integration tests on this branch and report back any problems. (This is also an excellent opportunity to add your provider to the automated testing list)
Developer notes
Future
Workarounds required for codeberg.org/miekg/dns
The migration to the new DNS package was very smooth. However there were some things I couldn't figure out how to do:
pkg/txtutil/{miekg.go miekg_test.go ddd/ddd.go}. It would be nice to not have to copy those.Changes
models.NewDomainConfig(zone).pkg/normalize/validate.gostill exists and is used, but is slowly being deprecated.rtypecontrolmodule. In the few places it was used, replace with the new RDATA functionality.RPandDSwere reimplemented using the new RDATA system.pkg/dnsrr/dnsrr.goto help migrate away from dnsv1pkg/js/helpers.js: Improve rawRecordBuilder() to be feature-compatible with recordBuilder()pkg/js/helpers.js: Convert to "the new way" for A AAAA CAA CF_REDIRECT CF_SINGLE_REDIRECT CF_TEMP_REDIRECT CNAME DHCID DNAME DNSKEY DS HTTPS LOC MX NAPTR NS OPENPGPKEY PTR R53_ALIAS RP SMIMEA SOA SRV SSHFP SVCB TLSApkg/js/parse_testsupdate fixtures due to new JSON fields. .json files no longer have Unicode chars.mustbefor converting raw data to the types we need.TODO
These may or may not make it into v5: