feat: zone_groups, provider HTTP tests, Porkbun bug fix, case-insensitive ZoneDiff - #62
Merged
Conversation
Introduces zone_groups as a way to auto-discover zones from a provider without listing each one explicitly. Explicit zones: entries always take precedence over zone_groups (with a warning on conflict). All four commands (plan, apply, drift, validate) now use IZoneResolver for zone loading.
… coverage Adds ~100 new tests across 5 new files and 1 expanded file: - ZoneResolverTests: 20 tests covering discovery, include/exclude patterns, explicit-zone precedence, case insensitivity, multiple groups, failure resilience - ZoneDiffEdgeCaseTests: 14 tests for empty zones, wildcards, SOA filtering, apex-NS flags, TTL-only changes, mixed create/update/delete - ZoneValidatorBoundaryTests: 18 tests for TTL=0/max, SRV port 0/65535/65536, MX preference 0/65535/65536, CAA flags, empty zones - ZoneYamlSerializerEdgeCaseTests: 15 tests for QuoteScalar (@, backtick, colon, hash, quotes, backslash, empty) and QuoteTxt round-trips - YamlProviderEdgeCaseTests: 15 tests for paths with spaces, hyphens, missing files, non-YAML files, malformed YAML, TTL defaults, \; unescape, key edge cases - ConfigLoaderTests: +8 zone_groups validation tests (parsing, unknown source/target, same source+target, read-only target, empty targets, zone_groups-only config)
DNS names are case-insensitive per RFC 1035. GroupBy now uses ToLowerInvariant() on name and ToUpperInvariant() on type so that records differing only in case are treated as the same RRset. Discovered during test audit — previously ZoneDiff was case-sensitive, which could cause spurious creates/deletes if providers returned names in different cases.
Add FakeHttpHandler + internal constructor overloads to inject HttpClient
in tests, enabling full coverage of HTTP behavior without live credentials.
- Cloudflare: auth header, zones pagination, record parsing, apply plan
- Porkbun: preflight, record parsing (ALIAS→CNAME), apply create/delete
- GoDaddy: sso-key auth, zones pagination (100-item boundary), apply plan
- Route53: AWS Sig V4 auth, XML parsing, zone ID bypass, apply plan
- GCP: Bearer token bypass, managed zone lookup, rrset pagination, apply plan
Also fixes Porkbun GetExistingRecordIds to key on full FQDNs (was using
NormalizeFqdn("www") → "www." which never matched change.RecordName
"www.example.com.", silently turning deletes/updates into creates).
367 tests total, all passing.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
Blockers resolved:
- Remove hardcoded static test count badge from README
- Add Porkbun Update test validating the BuildFqdn fix: confirms that a bare
subdomain ID ("www" from the API) is correctly matched to the FQDN RecordName
("www.example.com.") so updates hit the edit endpoint, not create
Major gaps filled:
- Add ChangeType.Update tests for all 5 providers
- Cloudflare: verifies PATCH is sent to dns_records endpoint
- Porkbun: verifies edit/{id} is called with the correct existing record ID
- GoDaddy: verifies PUT is sent (GoDaddy uses PUT for both create and update)
- Route53: verifies UPSERT action is encoded in the XML body
- GCP: verifies the Changes request contains both additions and deletions
- Add ChangeType.Delete test for Cloudflare and GCP
- Add TXT multi-chunk parsing tests for Route53 and GCP (>255 byte values)
- Add Cloudflare TTL=1 normalization test (ttl:1 → 300)
CI/docs:
- Restore upload-artifact alongside codecov-action (artifact was removed by mistake)
- Add comment explaining fail_ci_if_error: false
- Add comprehensive XML doc to ZoneGroupConfig explaining zone discovery,
failure behaviour (soft-fail with warning, not fatal), and explicit-wins semantics
FakeHttpHandler:
- Add VerifyAllConsumed() helper to catch tests that make fewer HTTP requests
than expected (complements the existing "too many requests" guard)
378 tests total, all passing.
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.
Closes / references
What's in this PR
✨ Features
zone_groupsin config — automatic zone discovery from a source provider without listing each zone explicitly. Supportsinclude_pattern/exclude_patternregex filters. If discovery fails (network/auth error), the group is skipped with a warning and the rest of the sync continues.--zoneflag onplan/apply/drift— target a single zone without touching config.🐛 Bug fix — Porkbun update/delete silently became create (#64)
GetExistingRecordIdswas keying onNormalizeFqdn("www")→"www.", butRecordChange.RecordNameis always a full FQDN"www.example.com.". The key never matched, so every update/delete on a subdomain record silently created a duplicate instead of editing or removing the existing one.Fixed by replacing
NormalizeFqdnwithBuildFqdn(name, domain + ".")— matching the same logic already used inParsePorkbunRecord.🔍 ZoneDiff case-insensitive grouping
DNS record names are grouped by
(name.ToLowerInvariant(), type.ToUpperInvariant())— prevents phantom changes when a provider returns names in different casing.🧪 Provider-level HTTP tests (#63)
FakeHttpHandler— shared helper (~50 lines, zero extra NuGet packages) that queuesHttpResponseMessageobjects and records all requests for assertion. IncludesVerifyAllConsumed()to detect fewer-requests-than-expected.Each provider got an
internalconstructor accepting anHttpClientfor injection. Public constructors are unchanged.378 tests total (+166 from this PR), all CI-safe — no live credentials required.
Coverage per provider:
Special cases tested:
_accessToken = "fake-token"pre-set in internal constructorhostedZoneIdto skip zone discovery📊 CI / Coverage
fail_ci_if_error: falseon Codecov upload — outages should not block CI📝 Docs
ZoneGroupConfigXML doc: explains discovery semantics, soft-fail behaviour, explicit-wins ruleFakeHttpHandlerAPI documented with XML summary commentsTest plan
dotnet test— 378 tests, 0 failuresdotnet format --verify-no-changes— no formatting errors