Skip to content

Commit c30c045

Browse files
dilyevskyclaude
dilyevsky
and
claude
committed
[apiserver] validate A/AAAA IP formats and fix silent misclassification
Skip unparseable IPs in DomainToDomainRecords instead of silently bucketing them as A records. Add format validation in DomainRecord.validate() to reject non-IPv4 values in dns.a and non-IPv6 values in dns.aaaa. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c3acaeb commit c30c045

File tree

7 files changed

+148
-61
lines changed

7 files changed

+148
-61
lines changed

‎api/core/v1alpha3/domainrecord_types.go‎

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package v1alpha3
33
import (
44
"context"
55
"fmt"
6-
"net"
76
"strings"
87

98
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -85,10 +84,13 @@ type DomainRecordTarget struct {
8584
// DomainRecordTargetDNS specifies DNS record data. Exactly one field must be populated.
8685
// The populated field determines the DNS record type.
8786
type DomainRecordTargetDNS struct {
88-
// IPs holds A/AAAA record addresses.
89-
// The record type (A vs AAAA) is determined by the IP address format.
87+
// A holds IPv4 A record addresses.
9088
// +optional
91-
IPs []string `json:"ips,omitempty"`
89+
A []string `json:"a,omitempty"`
90+
91+
// AAAA holds IPv6 AAAA record addresses.
92+
// +optional
93+
AAAA []string `json:"aaaa,omitempty"`
9294

9395
// FQDN holds a CNAME record target.
9496
// +optional
@@ -146,8 +148,11 @@ func (d *DomainRecordTargetDNS) DNSFieldKey() string {
146148
if d == nil {
147149
return ""
148150
}
149-
if len(d.IPs) > 0 {
150-
return "ips"
151+
if len(d.A) > 0 {
152+
return "a"
153+
}
154+
if len(d.AAAA) > 0 {
155+
return "aaaa"
151156
}
152157
if d.FQDN != nil {
153158
return "fqdn"
@@ -191,7 +196,10 @@ func (d *DomainRecordTargetDNS) PopulatedFieldCount() int {
191196
return 0
192197
}
193198
count := 0
194-
if len(d.IPs) > 0 {
199+
if len(d.A) > 0 {
200+
count++
201+
}
202+
if len(d.AAAA) > 0 {
195203
count++
196204
}
197205
if d.FQDN != nil {
@@ -346,8 +354,11 @@ func domainRecordTargetString(r *DomainRecord) string {
346354
}
347355
if r.Spec.Target.DNS != nil {
348356
dns := r.Spec.Target.DNS
349-
if len(dns.IPs) > 0 {
350-
return formatMultiValue(dns.IPs, 40)
357+
if len(dns.A) > 0 {
358+
return formatMultiValue(dns.A, 40)
359+
}
360+
if len(dns.AAAA) > 0 {
361+
return formatMultiValue(dns.AAAA, 40)
351362
}
352363
if dns.FQDN != nil {
353364
return truncateString(*dns.FQDN, 40)
@@ -386,33 +397,31 @@ func domainRecordTargetString(r *DomainRecord) string {
386397
return ""
387398
}
388399

389-
// domainRecordTypeString returns the DNS record type for display.
390-
func domainRecordTypeString(r *DomainRecord) string {
391-
if r.Status.Type != "" {
392-
return r.Status.Type
393-
}
400+
// deriveRecordType returns the DNS record type string for a DomainRecord.
401+
func deriveRecordType(r *DomainRecord) string {
394402
if r.Spec.Target.Ref != nil {
395403
return "Ref"
396404
}
397405
if r.Spec.Target.DNS != nil {
398-
dns := r.Spec.Target.DNS
399-
if len(dns.IPs) > 0 {
400-
if ip := net.ParseIP(dns.IPs[0]); ip != nil && ip.To4() == nil {
401-
return "AAAA"
402-
}
403-
return "A"
404-
}
405-
if dns.FQDN != nil {
406+
if r.Spec.Target.DNS.FQDN != nil {
406407
return "CNAME"
407408
}
408-
key := dns.DNSFieldKey()
409+
key := r.Spec.Target.DNS.DNSFieldKey()
409410
if key != "" {
410411
return strings.ToUpper(key)
411412
}
412413
}
413414
return ""
414415
}
415416

417+
// domainRecordTypeString returns the DNS record type for display.
418+
func domainRecordTypeString(r *DomainRecord) string {
419+
if r.Status.Type != "" {
420+
return r.Status.Type
421+
}
422+
return deriveRecordType(r)
423+
}
424+
416425
// domainRecordTTL returns the effective TTL for display.
417426
func domainRecordTTL(r *DomainRecord) int32 {
418427
if r.Spec.TTL != nil {

‎api/core/v1alpha3/domainrecord_validate.go‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package v1alpha3
33
import (
44
"context"
55
"fmt"
6+
"net"
67
"strings"
78

89
"k8s.io/apimachinery/pkg/runtime"
@@ -22,13 +23,21 @@ func (r *DomainRecord) Default() {
2223

2324
var _ resourcestrategy.PrepareForCreater = &DomainRecord{}
2425

25-
// PrepareForCreate derives metadata.name from spec when the user omits it.
26+
// PrepareForCreate derives metadata.name from spec when the user omits it
27+
// and sets status.type from the populated DNS field.
2628
func (r *DomainRecord) PrepareForCreate(ctx context.Context) {
2729
r.GenerateName = ""
28-
if r.Name != "" {
29-
return // user provided a name; Validate will check it
30+
if r.Name == "" {
31+
r.Name = r.derivedName()
3032
}
31-
r.Name = r.derivedName()
33+
r.Status.Type = deriveRecordType(r)
34+
}
35+
36+
var _ resourcestrategy.PrepareForUpdater = &DomainRecord{}
37+
38+
// PrepareForUpdate re-derives status.type on updates.
39+
func (r *DomainRecord) PrepareForUpdate(ctx context.Context, old runtime.Object) {
40+
r.Status.Type = deriveRecordType(r)
3241
}
3342

3443
var _ resourcestrategy.Validater = &DomainRecord{}
@@ -155,6 +164,27 @@ func (r *DomainRecord) validate() field.ErrorList {
155164
if count > 1 {
156165
errs = append(errs, field.Forbidden(targetPath.Child("dns"), "exactly one DNS field must be populated, found multiple"))
157166
}
167+
168+
dns := r.Spec.Target.DNS
169+
dnsPath := targetPath.Child("dns")
170+
for i, addr := range dns.A {
171+
ip := net.ParseIP(addr)
172+
if ip == nil || ip.To4() == nil {
173+
errs = append(errs, field.Invalid(
174+
dnsPath.Child("a").Index(i), addr,
175+
"must be a valid IPv4 address",
176+
))
177+
}
178+
}
179+
for i, addr := range dns.AAAA {
180+
ip := net.ParseIP(addr)
181+
if ip == nil || ip.To4() != nil {
182+
errs = append(errs, field.Invalid(
183+
dnsPath.Child("aaaa").Index(i), addr,
184+
"must be a valid IPv6 address",
185+
))
186+
}
187+
}
158188
}
159189

160190
// tls is only valid when target.ref is set.

‎api/core/v1alpha3/zz_generated.deepcopy.go‎

Lines changed: 7 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/generated/zz_generated.openapi.go‎

Lines changed: 17 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/apiserver/admission/domainrecord_reflection.go‎

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"io"
7+
"net"
78

89
kerrors "k8s.io/apimachinery/pkg/api/errors"
910
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -492,7 +493,10 @@ func dnsTargetEqual(a, b *corev1alpha3.DomainRecordTargetDNS) bool {
492493
if a == nil || b == nil {
493494
return false
494495
}
495-
if !stringSliceEqual(a.IPs, b.IPs) {
496+
if !stringSliceEqual(a.A, b.A) {
497+
return false
498+
}
499+
if !stringSliceEqual(a.AAAA, b.AAAA) {
496500
return false
497501
}
498502
if !stringPtrEqual(a.FQDN, b.FQDN) {
@@ -581,11 +585,32 @@ func DomainToDomainRecords(domain *corev1alpha3.Domain) []corev1alpha3.DomainRec
581585
dns := domain.Spec.Target.DNS
582586

583587
if dns.IPs != nil && len(dns.IPs.Addresses) > 0 {
584-
dr := newDNSRecord(domain, "ips", dns.IPs.TTL)
585-
dr.Spec.Target.DNS = &corev1alpha3.DomainRecordTargetDNS{
586-
IPs: append([]string(nil), dns.IPs.Addresses...),
588+
var v4, v6 []string
589+
for _, addr := range dns.IPs.Addresses {
590+
ip := net.ParseIP(addr)
591+
if ip == nil {
592+
continue
593+
}
594+
if ip.To4() == nil {
595+
v6 = append(v6, addr)
596+
} else {
597+
v4 = append(v4, addr)
598+
}
599+
}
600+
if len(v4) > 0 {
601+
dr := newDNSRecord(domain, "a", dns.IPs.TTL)
602+
dr.Spec.Target.DNS = &corev1alpha3.DomainRecordTargetDNS{
603+
A: v4,
604+
}
605+
records = append(records, dr)
606+
}
607+
if len(v6) > 0 {
608+
dr := newDNSRecord(domain, "aaaa", dns.IPs.TTL)
609+
dr.Spec.Target.DNS = &corev1alpha3.DomainRecordTargetDNS{
610+
AAAA: v6,
611+
}
612+
records = append(records, dr)
587613
}
588-
records = append(records, dr)
589614
}
590615
if dns.FQDN != nil {
591616
dr := newDNSRecord(domain, "fqdn", dns.FQDN.TTL)
@@ -760,11 +785,14 @@ func DomainRecordsToDomainSpec(records []corev1alpha3.DomainRecord) corev1alpha3
760785
}
761786

762787
dns := r.Spec.Target.DNS
763-
if len(dns.IPs) > 0 {
764-
spec.Target.DNS.IPs = &corev1alpha3.DNSAddressRecords{
765-
Addresses: append([]string(nil), dns.IPs...),
766-
TTL: copyInt32Ptr(r.Spec.TTL),
788+
if len(dns.A) > 0 || len(dns.AAAA) > 0 {
789+
if spec.Target.DNS.IPs == nil {
790+
spec.Target.DNS.IPs = &corev1alpha3.DNSAddressRecords{
791+
TTL: copyInt32Ptr(r.Spec.TTL),
792+
}
767793
}
794+
spec.Target.DNS.IPs.Addresses = append(spec.Target.DNS.IPs.Addresses, dns.A...)
795+
spec.Target.DNS.IPs.Addresses = append(spec.Target.DNS.IPs.Addresses, dns.AAAA...)
768796
}
769797
if dns.FQDN != nil {
770798
fqdn := *dns.FQDN

0 commit comments

Comments
 (0)