fix: enforce single-value CNAME/ALIAS and flag coexistence conflicts#57
Open
ecv wants to merge 1 commit into
Open
fix: enforce single-value CNAME/ALIAS and flag coexistence conflicts#57ecv wants to merge 1 commit into
ecv wants to merge 1 commit into
Conversation
Two PowerDNS 422 modes from #51 beyond the duplicate-NS blocker: Mode 2 — the operator could send a CNAME or ALIAS RRset with the same content repeated (multi-valued), which PowerDNS rejects ("only one such record allowed" / "duplicate record"). buildRRSets now keeps a single record for these single-valued types (first non-empty entry wins), so a malformed multi-valued input can no longer produce an invalid payload. Mode 3 — an ALIAS written at a name that already holds a conflicting RRset returns 422 "Conflicts with pre-existing RRset". This is a well-formed record that cannot coexist, not a transient provider error. Classify it (pdns.IsConflict) and surface a distinct Programmed=False reason (Conflict) so it is greppable in the DNSRecordSet status and not mistaken for a retryable PDNS error. Pre-flight conflict prevention (predicting coexistence before the PATCH) is deferred: it needs validated PowerDNS coexistence rules — ALIAS at apex legitimately coexists with SOA/NS — plus integration testing. Closes #55 Closes #56 Refs #51 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
DNS RFCs say don't allow coexistence, single CNAME value only, no other records.
|
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.
What
Addresses the two non-blocker 422 modes from #51:
Follows #54 (duplicate-NS, Mode 1) and complements #52 (surface pdns detail in status).
Mode 2 — single-value CNAME/ALIAS
buildRRSetsappended one record per entry for CNAME and ALIAS with no constraint (the CNAME case even had// TODO: ...violation of the RFC...we'll allow it). ADNSRecordSet(or discovery aggregating the same answer from multiple resolvers) with repeated/multiple content produced a multi-valued RRset that PowerDNS rejects:Both cases now keep exactly one record (first non-empty entry wins), so a malformed input can no longer produce an invalid payload.
Mode 3 — flag ALIAS coexistence conflicts distinctly
An ALIAS at a name that already holds another RRset returns
422 "Conflicts with pre-existing RRset". That's a well-formed record that cannot coexist — not a transient provider error. Newpdns.IsConflict()classifies it, and the reconciler sets a distinctProgrammed=FalsereasonConflict(vs genericPDNSError), so it's greppable in the DNSRecordSet status and clearly actionable.Deferred (intentionally)
Pre-flight conflict prevention — checking existing RRsets and refusing/clearing before the PATCH — is not included. Doing it correctly needs the real PowerDNS coexistence rules (e.g. ALIAS at apex legitimately coexists with SOA/NS, so a naive "any other type = conflict" check would wrongly block valid apex ALIAS) plus integration testing against pdns. Left for a follow-up; #56 tracks it. The current PR makes the conflict safe and legible; the 30s-capped reconcile backoff still self-heals once the conflicting record is removed.
Test
internal/pdns—TestBuildRRSets_CNAMEAliasSingleValue(multi/dup collapse to one, first-wins),TestIsConflict(conflict vs non-conflict 422 vs non-422).go build ./...clean;internal/pdns+internal/controllerunit tests pass. (Docker/testcontainers integration test unrelated, not run.)Closes #55
Closes #56
Refs #51
🤖 Generated with Claude Code