From f5be32173c828ba179178a976747813ddf3fc7c4 Mon Sep 17 00:00:00 2001 From: Jim Ryan Date: Tue, 17 Dec 2024 16:03:30 +0000 Subject: [PATCH 1/5] Add support for targets in virtual server external dns spec --- .../bases/k8s.nginx.org_virtualservers.yaml | 5 + deploy/crds.yaml | 5 + internal/externaldns/sync.go | 66 ++++--------- pkg/apis/configuration/v1/types.go | 2 + .../configuration/validation/virtualserver.go | 14 ++- .../externaldns/validation/externaldns.go | 54 +++++++++++ .../validation/externaldns_test.go | 94 +++++++++++++++++++ ...server-and-virtualserverroute-resources.md | 3 + 8 files changed, 195 insertions(+), 48 deletions(-) diff --git a/config/crd/bases/k8s.nginx.org_virtualservers.yaml b/config/crd/bases/k8s.nginx.org_virtualservers.yaml index 49adfc3d41..7f6bafa84b 100644 --- a/config/crd/bases/k8s.nginx.org_virtualservers.yaml +++ b/config/crd/bases/k8s.nginx.org_virtualservers.yaml @@ -97,6 +97,11 @@ spec: type: integer recordType: type: string + targets: + description: Targets stores an optional list of specified targets + items: + type: string + type: array type: object gunzip: type: boolean diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 60fff48700..453b8933b5 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -1415,6 +1415,11 @@ spec: type: integer recordType: type: string + targets: + description: Targets stores an optional list of specified targets + items: + type: string + type: array type: object gunzip: type: boolean diff --git a/internal/externaldns/sync.go b/internal/externaldns/sync.go index 092ea49f1c..f23c6a7584 100644 --- a/internal/externaldns/sync.go +++ b/internal/externaldns/sync.go @@ -2,24 +2,21 @@ package externaldns import ( "context" - "errors" "fmt" "github.com/google/go-cmp/cmp" nl "github.com/nginxinc/kubernetes-ingress/internal/logger" vsapi "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1" extdnsapi "github.com/nginxinc/kubernetes-ingress/pkg/apis/externaldns/v1" + externaldns_validation "github.com/nginxinc/kubernetes-ingress/pkg/apis/externaldns/validation" clientset "github.com/nginxinc/kubernetes-ingress/pkg/client/clientset/versioned" extdnslisters "github.com/nginxinc/kubernetes-ingress/pkg/client/listers/externaldns/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime/schema" - validators "k8s.io/apimachinery/pkg/util/validation" - "k8s.io/apimachinery/pkg/util/validation/field" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/record" - netutils "k8s.io/utils/net" ) const ( @@ -45,16 +42,17 @@ func SyncFnFor(rec record.EventRecorder, client clientset.Interface, ig map[stri } l := nl.LoggerFromContext(ctx) - if vs.Status.ExternalEndpoints == nil { + if vs.Status.ExternalEndpoints == nil && len(vs.Spec.ExternalDNS.Targets) == 0 { // It can take time for the external endpoints to sync - kick it back to the queue nl.Info(l, "Failed to determine external endpoints - retrying") return fmt.Errorf("failed to determine external endpoints") } - targets, recordType, err := getValidTargets(ctx, vs.Status.ExternalEndpoints) + targets, recordType, err := getValidTargets(ctx, vs.Status.ExternalEndpoints, vs.Spec.ExternalDNS.Targets) if err != nil { - nl.Error(l, "Invalid external endpoint") - rec.Eventf(vs, corev1.EventTypeWarning, reasonBadConfig, "Invalid external endpoint") + errorText := fmt.Sprintf("Invalid targets: %v", err) + nl.Error(l, errorText) + rec.Event(vs, corev1.EventTypeWarning, reasonBadConfig, errorText) return err } @@ -103,47 +101,23 @@ func SyncFnFor(rec record.EventRecorder, client clientset.Interface, ig map[stri } } -func getValidTargets(ctx context.Context, endpoints []vsapi.ExternalEndpoint) (extdnsapi.Targets, string, error) { - var targets extdnsapi.Targets - var recordType string - var recordA bool - var recordCNAME bool - var recordAAAA bool - var err error - l := nl.LoggerFromContext(ctx) - nl.Debugf(l, "Going through endpoints %v", endpoints) - for _, e := range endpoints { - if e.IP != "" { - nl.Debugf(l, "IP is defined: %v", e.IP) - if errMsg := validators.IsValidIP(field.NewPath(""), e.IP); len(errMsg) > 0 { - continue - } - ip := netutils.ParseIPSloppy(e.IP) - if ip.To4() != nil { - recordA = true - } else { - recordAAAA = true +func getValidTargets(ctx context.Context, endpoints []vsapi.ExternalEndpoint, chosenTargets extdnsapi.Targets) (extdnsapi.Targets, string, error) { + var finalTargets []string + if len(chosenTargets) > 0 { + // if targets are defined on the vs.spec.externaldns.targets, use these + finalTargets = chosenTargets + } else { + // Otherwise, get targets from vs.Status.ExternalEndpoints + for _, e := range endpoints { + if e.IP != "" { + finalTargets = append(finalTargets, e.IP) + } else if e.Hostname != "" { + finalTargets = append(finalTargets, e.Hostname) } - targets = append(targets, e.IP) - } else if e.Hostname != "" { - nl.Debugf(l, "Hostname is defined: %v", e.Hostname) - targets = append(targets, e.Hostname) - recordCNAME = true } } - if len(targets) == 0 { - return targets, recordType, errors.New("valid targets not defined") - } - if recordA { - recordType = recordTypeA - } else if recordAAAA { - recordType = recordTypeAAAA - } else if recordCNAME { - recordType = recordTypeCNAME - } else { - err = errors.New("recordType could not be determined") - } - return targets, recordType, err + + return externaldns_validation.ValidateTargetsAndDetermineRecordType(finalTargets) } func buildDNSEndpoint(ctx context.Context, extdnsLister extdnslisters.DNSEndpointLister, vs *vsapi.VirtualServer, targets extdnsapi.Targets, recordType string) (*extdnsapi.DNSEndpoint, *extdnsapi.DNSEndpoint, error) { diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index cac87569ab..2252c81d7f 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -68,6 +68,8 @@ type VirtualServerListener struct { type ExternalDNS struct { Enable bool `json:"enable"` RecordType string `json:"recordType,omitempty"` + // Targets stores a list of specified targets + Targets []string `json:"targets,omitempty"` // TTL for the record RecordTTL int64 `json:"recordTTL,omitempty"` // Labels stores labels defined for the Endpoint diff --git a/pkg/apis/configuration/validation/virtualserver.go b/pkg/apis/configuration/validation/virtualserver.go index 77d0a89aa1..f41cbd24ae 100644 --- a/pkg/apis/configuration/validation/virtualserver.go +++ b/pkg/apis/configuration/validation/virtualserver.go @@ -9,6 +9,7 @@ import ( "github.com/dlclark/regexp2" "github.com/nginxinc/kubernetes-ingress/internal/configs" v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1" + externaldns "github.com/nginxinc/kubernetes-ingress/pkg/apis/externaldns/validation" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" @@ -202,10 +203,19 @@ func (vsv *VirtualServerValidator) validateExternalDNS(ed *v1.ExternalDNS, field // valid, externalDNS is not required return nil } + allErrs := field.ErrorList{} if !vsv.isExternalDNSEnabled { - return field.ErrorList{field.Forbidden(fieldPath, "field requires externalDNS enablement")} + allErrs = append(allErrs, field.Forbidden(fieldPath, "field requires externalDNS enablement")) } - return nil + + if len(ed.Targets) > 0 { + _, _, err := externaldns.ValidateTargetsAndDetermineRecordType(ed.Targets) + if err != nil { + allErrs = append(allErrs, field.Forbidden(fieldPath.Child("targets"), err.Error())) + } + } + + return allErrs } func validateTLSRedirect(redirect *v1.TLSRedirect, fieldPath *field.Path) field.ErrorList { diff --git a/pkg/apis/externaldns/validation/externaldns.go b/pkg/apis/externaldns/validation/externaldns.go index fa0e7db0eb..0507217c10 100644 --- a/pkg/apis/externaldns/validation/externaldns.go +++ b/pkg/apis/externaldns/validation/externaldns.go @@ -10,8 +10,62 @@ import ( v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/externaldns/v1" "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" + netutils "k8s.io/utils/net" ) +func ValidateTargetsAndDetermineRecordType(targets []string) ([]string, string, error) { + var recordA, recordAAAA, recordCNAME bool + + for _, t := range targets { + if errMsg := validation.IsValidIP(field.NewPath(""), t); len(errMsg) == 0 { + ip := netutils.ParseIPSloppy(t) + if ip.To4() != nil { + recordA = true + } else { + recordAAAA = true + } + } else { + if err := isFullyQualifiedDomainName(t); err != nil { + return nil, "", fmt.Errorf("%w: target %q is invalid: %v", ErrTypeInvalid, t, err) + } + recordCNAME = true + } + } + + if err := isUnique(targets); err != nil { + return nil, "", err + } + + count := 0 + if recordA { + count++ + } + if recordAAAA { + count++ + } + if recordCNAME { + count++ + } + + if count > 1 { + return nil, "", errors.New("multiple record types (A, AAAA, CNAME) detected; must be homogeneous") + } + + var recordType string + switch { + case recordA: + recordType = "A" + case recordAAAA: + recordType = "AAAA" + case recordCNAME: + recordType = "CNAME" + default: + return nil, "", errors.New("could not determine record type from targets") + } + + return targets, recordType, nil +} + // ValidateDNSEndpoint validates if all DNSEndpoint fields are valid. func ValidateDNSEndpoint(dnsendpoint *v1.DNSEndpoint) error { return validateDNSEndpointSpec(&dnsendpoint.Spec) diff --git a/pkg/apis/externaldns/validation/externaldns_test.go b/pkg/apis/externaldns/validation/externaldns_test.go index 85db15c6b6..9e1dc90052 100644 --- a/pkg/apis/externaldns/validation/externaldns_test.go +++ b/pkg/apis/externaldns/validation/externaldns_test.go @@ -2,12 +2,106 @@ package validation_test import ( "errors" + "strings" "testing" v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/externaldns/v1" "github.com/nginxinc/kubernetes-ingress/pkg/apis/externaldns/validation" ) +func TestValidateTargetsAndDetermineRecordType(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + targets []string + wantErr bool + wantErrMsg string + wantType string + }{ + { + name: "single IPv4 target", + targets: []string{"10.10.10.10"}, + wantType: "A", + }, + { + name: "single IPv6 target", + targets: []string{"2001:db8::ff00:42:8329"}, + wantType: "AAAA", + }, + { + name: "single CNAME (hostname) target", + targets: []string{"example.com"}, + wantType: "CNAME", + }, + { + name: "multiple IPv4 targets", + targets: []string{"192.168.1.1", "10.10.10.10"}, + wantType: "A", + }, + { + name: "multiple IPv6 targets", + targets: []string{"2001:db8::1", "2001:db8::2"}, + wantType: "AAAA", + }, + { + name: "multiple hostnames", + targets: []string{"foo.example.com", "bar.example.com"}, + wantType: "CNAME", + }, + { + name: "mixed IPv4 and IPv6", + targets: []string{"192.168.1.1", "2001:db8::1"}, + wantErr: true, + wantErrMsg: "multiple record types", + }, + { + name: "mixed IPv4 and CNAME", + targets: []string{"192.168.1.1", "example.com"}, + wantErr: true, + wantErrMsg: "multiple record types", + }, + { + name: "invalid hostname", + targets: []string{"not_a_valid_hostname"}, + wantErr: true, + wantErrMsg: "invalid", + }, + { + name: "empty targets", + targets: []string{}, + wantErr: true, + wantErrMsg: "determine record type", + }, + { + name: "duplicate targets", + targets: []string{"example.com", "example.com"}, + wantErr: true, + wantErrMsg: "expected unique targets", + }, + } + + for _, tc := range tests { + tc := tc // address gosec G601 + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + _, recordType, err := validation.ValidateTargetsAndDetermineRecordType(tc.targets) + if tc.wantErr && err == nil { + t.Fatalf("expected an error, got none") + } + if !tc.wantErr && err != nil { + t.Fatalf("expected no error, got %v", err) + } + if tc.wantErr && err != nil && tc.wantErrMsg != "" && !strings.Contains(err.Error(), tc.wantErrMsg) { + t.Errorf("expected error message containing %q, got %q", tc.wantErrMsg, err.Error()) + } + if !tc.wantErr && recordType != tc.wantType { + t.Errorf("expected record type %q, got %q", tc.wantType, recordType) + } + }) + } +} + func TestValidateDNSEndpoint(t *testing.T) { t.Parallel() tt := []struct { diff --git a/site/content/configuration/virtualserver-and-virtualserverroute-resources.md b/site/content/configuration/virtualserver-and-virtualserverroute-resources.md index f7695fb3a3..e85141c241 100644 --- a/site/content/configuration/virtualserver-and-virtualserverroute-resources.md +++ b/site/content/configuration/virtualserver-and-virtualserverroute-resources.md @@ -163,6 +163,9 @@ enable: true |``providerSpecific`` | Configure provider specific properties which holds the name and value of a configuration which is specific to individual DNS providers. | [[]ProviderSpecific](#virtualserverexternaldnsproviderspecific) | No | |``recordTTL`` | TTL for the DNS record. This defaults to 0 if not defined. See [the ExternalDNS TTL documentation for provider-specific defaults](https://kubernetes-sigs.github.io/external-dns/v0.14.2/ttl/#providers) | ``int64`` | No | |``recordType`` | The record Type that should be created, e.g. "A", "AAAA", "CNAME". This is automatically computed based on the external endpoints if not defined. | ``string`` | No | +|``targets`` | An optional list of IP addresses or hostnames to serve as DNS targets within the DNSEndpoint. All specified targets must be of the same record type (all IPv4, all IPv6, or all hostnames). | ``[]string`` | No | + + {{}} ### VirtualServer.ExternalDNS.ProviderSpecific From 05f647d5cd796cc332f14c8d75fef086dfb02e50 Mon Sep 17 00:00:00 2001 From: Jim Ryan Date: Tue, 17 Dec 2024 16:05:12 +0000 Subject: [PATCH 2/5] remove space from table --- .../virtualserver-and-virtualserverroute-resources.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/site/content/configuration/virtualserver-and-virtualserverroute-resources.md b/site/content/configuration/virtualserver-and-virtualserverroute-resources.md index e85141c241..e78f72bfb3 100644 --- a/site/content/configuration/virtualserver-and-virtualserverroute-resources.md +++ b/site/content/configuration/virtualserver-and-virtualserverroute-resources.md @@ -164,8 +164,6 @@ enable: true |``recordTTL`` | TTL for the DNS record. This defaults to 0 if not defined. See [the ExternalDNS TTL documentation for provider-specific defaults](https://kubernetes-sigs.github.io/external-dns/v0.14.2/ttl/#providers) | ``int64`` | No | |``recordType`` | The record Type that should be created, e.g. "A", "AAAA", "CNAME". This is automatically computed based on the external endpoints if not defined. | ``string`` | No | |``targets`` | An optional list of IP addresses or hostnames to serve as DNS targets within the DNSEndpoint. All specified targets must be of the same record type (all IPv4, all IPv6, or all hostnames). | ``[]string`` | No | - - {{}} ### VirtualServer.ExternalDNS.ProviderSpecific From 89c5363248c87f6fb21559d6a5d7aa2ed4f38fee Mon Sep 17 00:00:00 2001 From: Jim Ryan Date: Tue, 17 Dec 2024 16:50:14 +0000 Subject: [PATCH 3/5] Add tests --- internal/externaldns/sync.go | 56 ++++++++++++++----- internal/externaldns/sync_test.go | 28 +++++++--- .../externaldns/validation/externaldns.go | 15 +++-- 3 files changed, 73 insertions(+), 26 deletions(-) diff --git a/internal/externaldns/sync.go b/internal/externaldns/sync.go index f23c6a7584..862c4af518 100644 --- a/internal/externaldns/sync.go +++ b/internal/externaldns/sync.go @@ -17,6 +17,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/record" + netutils "k8s.io/utils/net" ) const ( @@ -48,7 +49,7 @@ func SyncFnFor(rec record.EventRecorder, client clientset.Interface, ig map[stri return fmt.Errorf("failed to determine external endpoints") } - targets, recordType, err := getValidTargets(ctx, vs.Status.ExternalEndpoints, vs.Spec.ExternalDNS.Targets) + targets, recordType, err := getValidTargets(vs.Status.ExternalEndpoints, vs.Spec.ExternalDNS.Targets) if err != nil { errorText := fmt.Sprintf("Invalid targets: %v", err) nl.Error(l, errorText) @@ -101,23 +102,52 @@ func SyncFnFor(rec record.EventRecorder, client clientset.Interface, ig map[stri } } -func getValidTargets(ctx context.Context, endpoints []vsapi.ExternalEndpoint, chosenTargets extdnsapi.Targets) (extdnsapi.Targets, string, error) { - var finalTargets []string +func getValidTargets(endpoints []vsapi.ExternalEndpoint, chosenTargets extdnsapi.Targets) (extdnsapi.Targets, string, error) { if len(chosenTargets) > 0 { - // if targets are defined on the vs.spec.externaldns.targets, use these - finalTargets = chosenTargets - } else { - // Otherwise, get targets from vs.Status.ExternalEndpoints - for _, e := range endpoints { - if e.IP != "" { - finalTargets = append(finalTargets, e.IP) - } else if e.Hostname != "" { - finalTargets = append(finalTargets, e.Hostname) + return externaldns_validation.ValidateTargetsAndDetermineRecordType(chosenTargets) + } + + var ipv4Targets, ipv6Targets, cnameTargets []string + for _, e := range endpoints { + if e.IP != "" { + ip := netutils.ParseIPSloppy(e.IP) + if ip == nil { + continue + } + if ip.To4() != nil { + ipv4Targets = append(ipv4Targets, e.IP) + } else { + ipv6Targets = append(ipv6Targets, e.IP) + } + } else if e.Hostname != "" { + cnameTargets = append(cnameTargets, e.Hostname) + } + } + + //priority: prefer IPv4 > IPv6 > CNAME + if len(ipv4Targets) > 0 { + if err := externaldns_validation.IsUnique(ipv4Targets); err != nil { + return nil, "", err + } + return ipv4Targets, "A", nil + } else if len(ipv6Targets) > 0 { + if err := externaldns_validation.IsUnique(ipv6Targets); err != nil { + return nil, "", err + } + return ipv6Targets, "AAAA", nil + } else if len(cnameTargets) > 0 { + if err := externaldns_validation.IsUnique(cnameTargets); err != nil { + return nil, "", err + } + for _, h := range cnameTargets { + if err := externaldns_validation.IsFullyQualifiedDomainName(h); err != nil { + return nil, "", fmt.Errorf("%w: target %q is invalid: %v", externaldns_validation.ErrTypeInvalid, h, err) } } + return cnameTargets, "CNAME", nil } - return externaldns_validation.ValidateTargetsAndDetermineRecordType(finalTargets) + return nil, "", fmt.Errorf("no valid external endpoints found") } func buildDNSEndpoint(ctx context.Context, extdnsLister extdnslisters.DNSEndpointLister, vs *vsapi.VirtualServer, targets extdnsapi.Targets, recordType string) (*extdnsapi.DNSEndpoint, *extdnsapi.DNSEndpoint, error) { diff --git a/internal/externaldns/sync_test.go b/internal/externaldns/sync_test.go index 0d85e26a02..f653e097ec 100644 --- a/internal/externaldns/sync_test.go +++ b/internal/externaldns/sync_test.go @@ -26,10 +26,11 @@ func (EventRecorder) AnnotatedEventf(runtime.Object, map[string]string, string, func TestGetValidTargets(t *testing.T) { t.Parallel() tt := []struct { - name string - wantTargets extdnsapi.Targets - wantRecord string - endpoints []vsapi.ExternalEndpoint + name string + wantTargets extdnsapi.Targets + wantRecord string + endpoints []vsapi.ExternalEndpoint + chosenEndpoints []string }{ { name: "from external endpoint with IPv4", @@ -63,21 +64,34 @@ func TestGetValidTargets(t *testing.T) { }, { name: "from external endpoint with multiple targets", - wantTargets: extdnsapi.Targets{"2001:db8:0:0:0:0:2:1", "10.2.3.4"}, + wantTargets: extdnsapi.Targets{"10.2.3.4"}, wantRecord: "A", endpoints: []vsapi.ExternalEndpoint{ { - IP: "2001:db8:0:0:0:0:2:1", + IP: "2001:db8:0:0:0:0:2:1", // This IPv6 record will be ignored, as the priority is IPv4> IPv6> CNAME }, { IP: "10.2.3.4", }, }, }, + { + name: "from external endpoint with multiple targets", + wantTargets: extdnsapi.Targets{"1.2.3.4"}, + wantRecord: "A", + endpoints: []vsapi.ExternalEndpoint{ + { + IP: "10.2.3.4", // This extrenal IP will be ignored, as IPs have been chosen in the VS spec + }, + }, + chosenEndpoints: []string{ + "1.2.3.4", + }, + }, } for _, tc := range tt { t.Run(tc.name, func(t *testing.T) { - targets, recordType, err := getValidTargets(context.Background(), tc.endpoints) + targets, recordType, err := getValidTargets(tc.endpoints, tc.chosenEndpoints) if err != nil { t.Fatal(err) } diff --git a/pkg/apis/externaldns/validation/externaldns.go b/pkg/apis/externaldns/validation/externaldns.go index 0507217c10..ef4c2c3cb8 100644 --- a/pkg/apis/externaldns/validation/externaldns.go +++ b/pkg/apis/externaldns/validation/externaldns.go @@ -13,6 +13,7 @@ import ( netutils "k8s.io/utils/net" ) +// ValidateTargetsAndDetermineRecordType validates chosen endpoints func ValidateTargetsAndDetermineRecordType(targets []string) ([]string, string, error) { var recordA, recordAAAA, recordCNAME bool @@ -25,14 +26,14 @@ func ValidateTargetsAndDetermineRecordType(targets []string) ([]string, string, recordAAAA = true } } else { - if err := isFullyQualifiedDomainName(t); err != nil { + if err := IsFullyQualifiedDomainName(t); err != nil { return nil, "", fmt.Errorf("%w: target %q is invalid: %v", ErrTypeInvalid, t, err) } recordCNAME = true } } - if err := isUnique(targets); err != nil { + if err := IsUnique(targets); err != nil { return nil, "", err } @@ -111,15 +112,16 @@ func validateTargets(targets v1.Targets) error { return fmt.Errorf("%w: target %q is invalid: %s", ErrTypeInvalid, target, errMsg[0]) } default: - if err := isFullyQualifiedDomainName(target); err != nil { + if err := IsFullyQualifiedDomainName(target); err != nil { return fmt.Errorf("%w: target %q is invalid, it should be a valid IP address or hostname", ErrTypeInvalid, target) } } } - return isUnique(targets) + return IsUnique(targets) } -func isUnique(targets v1.Targets) error { +// IsUnique takes a list of targets and makes an error if a target appears more than once +func IsUnique(targets v1.Targets) error { occurred := make(map[string]bool) for _, target := range targets { if occurred[target] { @@ -144,7 +146,8 @@ func validateTTL(ttl v1.TTL) error { return nil } -func isFullyQualifiedDomainName(name string) error { +// IsFullyQualifiedDomainName checks if a string will be valid for a cname dns record +func IsFullyQualifiedDomainName(name string) error { if name == "" { return fmt.Errorf("%w: name not provided", ErrTypeInvalid) } From 3782b4d9fa1e9c3204150d3b983ddff386a0b248 Mon Sep 17 00:00:00 2001 From: Jim Ryan Date: Tue, 17 Dec 2024 16:58:32 +0000 Subject: [PATCH 4/5] Lint/Update text in CRDS --- config/crd/bases/k8s.nginx.org_virtualservers.yaml | 2 +- deploy/crds.yaml | 2 +- internal/externaldns/sync.go | 4 ++-- pkg/apis/externaldns/validation/externaldns.go | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/config/crd/bases/k8s.nginx.org_virtualservers.yaml b/config/crd/bases/k8s.nginx.org_virtualservers.yaml index 7f6bafa84b..18506face5 100644 --- a/config/crd/bases/k8s.nginx.org_virtualservers.yaml +++ b/config/crd/bases/k8s.nginx.org_virtualservers.yaml @@ -98,7 +98,7 @@ spec: recordType: type: string targets: - description: Targets stores an optional list of specified targets + description: Targets stores a list of specified targets items: type: string type: array diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 453b8933b5..680b455560 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -1416,7 +1416,7 @@ spec: recordType: type: string targets: - description: Targets stores an optional list of specified targets + description: Targets stores a list of specified targets items: type: string type: array diff --git a/internal/externaldns/sync.go b/internal/externaldns/sync.go index 862c4af518..d799079ed9 100644 --- a/internal/externaldns/sync.go +++ b/internal/externaldns/sync.go @@ -124,7 +124,7 @@ func getValidTargets(endpoints []vsapi.ExternalEndpoint, chosenTargets extdnsapi } } - //priority: prefer IPv4 > IPv6 > CNAME + // priority: prefer IPv4 > IPv6 > CNAME if len(ipv4Targets) > 0 { if err := externaldns_validation.IsUnique(ipv4Targets); err != nil { return nil, "", err @@ -141,7 +141,7 @@ func getValidTargets(endpoints []vsapi.ExternalEndpoint, chosenTargets extdnsapi } for _, h := range cnameTargets { if err := externaldns_validation.IsFullyQualifiedDomainName(h); err != nil { - return nil, "", fmt.Errorf("%w: target %q is invalid: %v", externaldns_validation.ErrTypeInvalid, h, err) + return nil, "", fmt.Errorf("%w: target %q is invalid: %w", externaldns_validation.ErrTypeInvalid, h, err) } } return cnameTargets, "CNAME", nil diff --git a/pkg/apis/externaldns/validation/externaldns.go b/pkg/apis/externaldns/validation/externaldns.go index ef4c2c3cb8..ccfc3d6065 100644 --- a/pkg/apis/externaldns/validation/externaldns.go +++ b/pkg/apis/externaldns/validation/externaldns.go @@ -27,7 +27,7 @@ func ValidateTargetsAndDetermineRecordType(targets []string) ([]string, string, } } else { if err := IsFullyQualifiedDomainName(t); err != nil { - return nil, "", fmt.Errorf("%w: target %q is invalid: %v", ErrTypeInvalid, t, err) + return nil, "", fmt.Errorf("%w: target %q is invalid: %w", ErrTypeInvalid, t, err) } recordCNAME = true } From 310db1ad28eb0fa07e3ed742130c5d882dd3edd2 Mon Sep 17 00:00:00 2001 From: Jim Ryan Date: Tue, 17 Dec 2024 17:01:45 +0000 Subject: [PATCH 5/5] Update generated code --- pkg/apis/configuration/v1/zz_generated.deepcopy.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/apis/configuration/v1/zz_generated.deepcopy.go b/pkg/apis/configuration/v1/zz_generated.deepcopy.go index b617f3cb93..79636dd509 100644 --- a/pkg/apis/configuration/v1/zz_generated.deepcopy.go +++ b/pkg/apis/configuration/v1/zz_generated.deepcopy.go @@ -309,6 +309,11 @@ func (in *ErrorPageReturn) DeepCopy() *ErrorPageReturn { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ExternalDNS) DeepCopyInto(out *ExternalDNS) { *out = *in + if in.Targets != nil { + in, out := &in.Targets, &out.Targets + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.Labels != nil { in, out := &in.Labels, &out.Labels *out = make(map[string]string, len(*in))