diff --git a/config/crd/bases/k8s.nginx.org_virtualservers.yaml b/config/crd/bases/k8s.nginx.org_virtualservers.yaml index 49adfc3d41..18506face5 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 a 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..680b455560 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -1415,6 +1415,11 @@ spec: type: integer recordType: type: string + targets: + description: Targets stores a 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..d799079ed9 100644 --- a/internal/externaldns/sync.go +++ b/internal/externaldns/sync.go @@ -2,19 +2,17 @@ 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" @@ -45,16 +43,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(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 +102,52 @@ 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) +func getValidTargets(endpoints []vsapi.ExternalEndpoint, chosenTargets extdnsapi.Targets) (extdnsapi.Targets, string, error) { + if len(chosenTargets) > 0 { + return externaldns_validation.ValidateTargetsAndDetermineRecordType(chosenTargets) + } + + var ipv4Targets, ipv6Targets, cnameTargets []string 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 { + ip := netutils.ParseIPSloppy(e.IP) + if ip == nil { continue } - ip := netutils.ParseIPSloppy(e.IP) if ip.To4() != nil { - recordA = true + ipv4Targets = append(ipv4Targets, e.IP) } else { - recordAAAA = true + ipv6Targets = append(ipv6Targets, e.IP) } - 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 + cnameTargets = append(cnameTargets, e.Hostname) } } - 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") + + // 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: %w", externaldns_validation.ErrTypeInvalid, h, err) + } + } + return cnameTargets, "CNAME", nil } - return targets, recordType, err + + 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/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/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)) 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..ccfc3d6065 100644 --- a/pkg/apis/externaldns/validation/externaldns.go +++ b/pkg/apis/externaldns/validation/externaldns.go @@ -10,8 +10,63 @@ 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" ) +// ValidateTargetsAndDetermineRecordType validates chosen endpoints +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: %w", 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) @@ -57,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] { @@ -90,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) } 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..e78f72bfb3 100644 --- a/site/content/configuration/virtualserver-and-virtualserverroute-resources.md +++ b/site/content/configuration/virtualserver-and-virtualserverroute-resources.md @@ -163,6 +163,7 @@ 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