diff --git a/internal/controller/dnszone_helpers.go b/internal/controller/dnszone_helpers.go index ff6f507..0fdf521 100644 --- a/internal/controller/dnszone_helpers.go +++ b/internal/controller/dnszone_helpers.go @@ -13,7 +13,15 @@ func normalizeStringSlice(in []string) []string { if len(in) == 0 { return nil } - out := append([]string(nil), in...) + seen := make(map[string]struct{}, len(in)) + out := make([]string, 0, len(in)) + for _, s := range in { + if _, ok := seen[s]; ok { + continue + } + seen[s] = struct{}{} + out = append(out, s) + } sort.Strings(out) return out } diff --git a/internal/controller/dnszone_helpers_test.go b/internal/controller/dnszone_helpers_test.go index c81d2d1..30314eb 100644 --- a/internal/controller/dnszone_helpers_test.go +++ b/internal/controller/dnszone_helpers_test.go @@ -28,6 +28,14 @@ func TestNormalizeStringSlice(t *testing.T) { if reflect.DeepEqual(in, want) { t.Fatalf("expected input slice to remain unsorted") } + + // Duplicates are removed: a class listing each nameserver twice must not + // produce a doubled NS RRset (PowerDNS rejects duplicate records, 422). + dupIn := []string{"ns2", "ns1", "ns2", "ns1", "ns1"} + dupWant := []string{"ns1", "ns2"} + if dupGot := normalizeStringSlice(dupIn); !reflect.DeepEqual(dupGot, dupWant) { + t.Fatalf("expected dedup: got=%v want=%v", dupGot, dupWant) + } } func TestNormalizeDomainNameservers(t *testing.T) { diff --git a/internal/pdns/client.go b/internal/pdns/client.go index 4c0faaa..81747ec 100644 --- a/internal/pdns/client.go +++ b/internal/pdns/client.go @@ -80,6 +80,7 @@ type createZoneRequest struct { func (c *Client) CreateZone(ctx context.Context, zone string, nameservers []string) error { // PDNS expects absolute nameserver hostnames (trailing dot) nsAbs := make([]string, 0, len(nameservers)) + seen := make(map[string]struct{}, len(nameservers)) for _, ns := range nameservers { if ns == "" { continue @@ -87,6 +88,10 @@ func (c *Client) CreateZone(ctx context.Context, zone string, nameservers []stri if ns[len(ns)-1] != '.' { ns += "." } + if _, ok := seen[ns]; ok { + continue + } + seen[ns] = struct{}{} nsAbs = append(nsAbs, ns) } payload := createZoneRequest{ @@ -516,10 +521,15 @@ func buildRRSets(zone string, rs dnsv1alpha1.DNSRecordSet) []rrset { } v := strings.TrimSpace(rec.NS.Content) if v != "" { - r.Records = append(r.Records, rrsetRecord{ - Content: qualifyIfNeeded(v), - Disabled: false, - }) + // Dedupe by qualified content: PowerDNS rejects an RRset with + // duplicate records (422), and duplicates reach here when the + // nameserver source lists an NS twice, including trailing-dot + // variants (ns1.example.net vs ns1.example.net.) that collapse + // to identical content only after qualifyIfNeeded. + content := qualifyIfNeeded(v) + if !recordContentExists(r.Records, content) { + r.Records = append(r.Records, rrsetRecord{Content: content, Disabled: false}) + } } case dnsv1alpha1.RRTypeSOA: @@ -730,6 +740,15 @@ func makeSimpleRRSet(name, typ string, ttl int, values []string) rrset { } } +func recordContentExists(recs []rrsetRecord, content string) bool { + for _, r := range recs { + if r.Content == content { + return true + } + } + return false +} + func qualifyOwner(owner, zone string) string { if owner == "@" || owner == "" { return zone + "." diff --git a/internal/pdns/pdns_test.go b/internal/pdns/pdns_test.go index e0ce57e..00a4ee2 100644 --- a/internal/pdns/pdns_test.go +++ b/internal/pdns/pdns_test.go @@ -677,6 +677,35 @@ func TestApplyRecordSetAuthoritative_PathAndHeaders(t *testing.T) { } } +// Duplicate NS content (including trailing-dot variants) must collapse to a +// single record; PowerDNS rejects an RRset with duplicate records (422). +func TestBuildRRSets_NSDedup(t *testing.T) { + t.Parallel() + rs := dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + RecordType: dnsv1alpha1.RRTypeNS, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "@", NS: &dnsv1alpha1.NSRecordSpec{Content: "ns1.example.net."}}, + {Name: "@", NS: &dnsv1alpha1.NSRecordSpec{Content: "ns1.example.net"}}, // trailing-dot variant + {Name: "@", NS: &dnsv1alpha1.NSRecordSpec{Content: "ns1.example.net."}}, // exact dup + {Name: "@", NS: &dnsv1alpha1.NSRecordSpec{Content: "ns2.example.net."}}, + }, + }, + } + rr := buildRRSets("example.com", rs) + if len(rr) != 1 { + t.Fatalf("expected 1 rrset, got %#v", rr) + } + got := []string{} + for _, rec := range rr[0].Records { + got = append(got, rec.Content) + } + want := []string{"ns1.example.net.", "ns2.example.net."} + if !reflect.DeepEqual(got, want) { + t.Fatalf("NS dedup: got %#v want %#v", got, want) + } +} + // sanity: makeSimpleRRSet keeps values verbatim (used after we normalize) func TestMakeSimpleRRSet(t *testing.T) { t.Parallel()