From 6ef2c9c070feef561e93f8bcb03cfc6014c58471 Mon Sep 17 00:00:00 2001 From: Ivan Ka <5395690+ivankatliarchuk@users.noreply.github.com> Date: Mon, 16 Mar 2026 08:11:39 +0000 Subject: [PATCH] refactor(source): move SuitableType to endpoint package (#6239) * refactore(source): move SuitableType to endpiont package Signed-off-by: ivan katliarchuk * refactore(source): move SuitableType to endpiont package Signed-off-by: ivan katliarchuk * refactore(source): move SuitableType to endpiont package Signed-off-by: ivan katliarchuk * refactore(source): move SuitableType to endpiont package Signed-off-by: ivan katliarchuk * refactore(source): move SuitableType to endpiont package --------- Signed-off-by: ivan katliarchuk --- endpoint/OWNERS | 4 +++ endpoint/utils_test.go | 31 ++++++++++++++++++ provider/pihole/clientV6.go | 31 ++---------------- provider/pihole/clientV6_test.go | 56 -------------------------------- source/compatibility.go | 2 +- source/fqdn/fqdn.go | 13 ++------ source/gateway.go | 4 +-- source/node.go | 4 +-- source/pod.go | 12 +++---- source/service.go | 6 ++-- source/unstructured.go | 4 +-- source/utils.go | 18 ---------- source/utils_test.go | 31 ------------------ 13 files changed, 54 insertions(+), 162 deletions(-) create mode 100644 endpoint/OWNERS diff --git a/endpoint/OWNERS b/endpoint/OWNERS new file mode 100644 index 000000000..7f66e247c --- /dev/null +++ b/endpoint/OWNERS @@ -0,0 +1,4 @@ +# See the OWNERS docs at https://go.k8s.io/owners + +labels: +- endpoint diff --git a/endpoint/utils_test.go b/endpoint/utils_test.go index 7401c31a6..0dd426d45 100644 --- a/endpoint/utils_test.go +++ b/endpoint/utils_test.go @@ -35,6 +35,37 @@ func (m *mockObjectMetaAccessor) GetObjectMeta() metav1.Object { } } +func TestSuitableType(t *testing.T) { + tests := []struct { + target string + expected string + }{ + // IPv4 + {"192.168.1.1", RecordTypeA}, + {"255.255.255.255", RecordTypeA}, + {"0.0.0.0", RecordTypeA}, + // IPv6 + {"2001:0db8:85a3:0000:0000:8a2e:0370:7334", RecordTypeAAAA}, + {"2001:db8:85a3::8a2e:370:7334", RecordTypeAAAA}, + {"::ffff:192.168.20.3", RecordTypeAAAA}, // IPv4-mapped IPv6 + {"::1", RecordTypeAAAA}, + {"::", RecordTypeAAAA}, + // CNAME (hostname or invalid) + {"example.com", RecordTypeCNAME}, + {"", RecordTypeCNAME}, + {"256.256.256.256", RecordTypeCNAME}, + {"192.168.0.1/22", RecordTypeCNAME}, + {"192.168.1", RecordTypeCNAME}, + {"abc.def.ghi.jkl", RecordTypeCNAME}, + } + + for _, tt := range tests { + t.Run(tt.target, func(t *testing.T) { + assert.Equal(t, tt.expected, SuitableType(tt.target)) + }) + } +} + func TestHasEmptyEndpoints(t *testing.T) { tests := []struct { name string diff --git a/provider/pihole/clientV6.go b/provider/pihole/clientV6.go index ab616ddcf..4808df5d3 100644 --- a/provider/pihole/clientV6.go +++ b/provider/pihole/clientV6.go @@ -25,7 +25,6 @@ import ( "fmt" "io" "net/http" - "net/netip" "net/url" "strconv" "strings" @@ -117,32 +116,6 @@ func (p *piholeClientV6) getConfigValue(ctx context.Context, rtype string) ([]st return results, nil } -/** - * isValidIPv4 checks if the given IP address is a valid IPv4 address. - * It returns true if the IP address is valid, false otherwise. - * If the IP address is in IPv6 format, it will return false. - */ -func isValidIPv4(ip string) bool { - addr, err := netip.ParseAddr(ip) - if err != nil { - return false - } - return addr.Is4() -} - -/** - * isValidIPv6 checks if the given IP address is a valid IPv6 address. - * It returns true if the IP address is valid, false otherwise. - * If the IP address is in IPv6 with dual format y:y:y:y:y:y:x.x.x.x. , it will return true. - */ -func isValidIPv6(ip string) bool { - addr, err := netip.ParseAddr(ip) - if err != nil { - return false - } - return addr.Is6() -} - func (p *piholeClientV6) listRecords(ctx context.Context, rtype string) ([]*endpoint.Endpoint, error) { results, err := p.getConfigValue(ctx, rtype) if err != nil { @@ -166,12 +139,12 @@ func (p *piholeClientV6) listRecords(ctx context.Context, rtype string) ([]*endp switch rtype { case endpoint.RecordTypeA: // PiHole return A and AAAA records. Filter to only keep the A records - if !isValidIPv4(Target) { + if endpoint.SuitableType(Target) != endpoint.RecordTypeA { continue } case endpoint.RecordTypeAAAA: // PiHole return A and AAAA records. Filter to only keep the AAAA records - if !isValidIPv6(Target) { + if endpoint.SuitableType(Target) != endpoint.RecordTypeAAAA { continue } case endpoint.RecordTypeCNAME: diff --git a/provider/pihole/clientV6_test.go b/provider/pihole/clientV6_test.go index ff48e48e1..2ee498256 100644 --- a/provider/pihole/clientV6_test.go +++ b/provider/pihole/clientV6_test.go @@ -30,62 +30,6 @@ import ( "sigs.k8s.io/external-dns/endpoint" ) -func TestIsValidIPv4(t *testing.T) { - tests := []struct { - ip string - expected bool - }{ - {"192.168.1.1", true}, - {"255.255.255.255", true}, - {"0.0.0.0", true}, - {"", false}, - {"256.256.256.256", false}, - {"192.168.0.1/22", false}, - {"192.168.1", false}, - {"abc.def.ghi.jkl", false}, - {"::ffff:192.168.20.3", false}, - } - - for _, test := range tests { - t.Run(test.ip, func(t *testing.T) { - if got := isValidIPv4(test.ip); got != test.expected { - t.Errorf("isValidIPv4(%s) = %v; want %v", test.ip, got, test.expected) - } - }) - } -} - -func TestIsValidIPv6(t *testing.T) { - tests := []struct { - ip string - expected bool - }{ - {"2001:0db8:85a3:0000:0000:8a2e:0370:7334", true}, - {"2001:db8:85a3::8a2e:370:7334", true}, - // IPv6 dual, the format is y:y:y:y:y:y:x.x.x.x. - {"::ffff:192.168.20.3", true}, - {"::1", true}, - {"::", true}, - {"2001:db8::", true}, - {"", false}, - {":", false}, - {"::ffff:", false}, - {"192.168.20.3", false}, - {"2001:db8:85a3:0:0:8a2e:370:7334:1234", false}, - {"2001:db8:85a3::8a2e:370g:7334", false}, - {"2001:db8:85a3::8a2e:370:7334::", false}, - {"2001:db8:85a3::8a2e:370:7334::1", false}, - } - - for _, test := range tests { - t.Run(test.ip, func(t *testing.T) { - if got := isValidIPv6(test.ip); got != test.expected { - t.Errorf("isValidIPv6(%s) = %v; want %v", test.ip, got, test.expected) - } - }) - } -} - func newTestServerV6(t *testing.T, hdlr http.HandlerFunc) *httptest.Server { t.Helper() svr := httptest.NewServer(hdlr) diff --git a/source/compatibility.go b/source/compatibility.go index 7c392e076..7b91ef7b8 100644 --- a/source/compatibility.go +++ b/source/compatibility.go @@ -157,7 +157,7 @@ func legacyEndpointsFromDNSControllerNodePortService(svc *v1.Service, sc *servic continue } for _, address := range node.Status.Addresses { - recordType := suitableType(address.Address) + recordType := endpoint.SuitableType(address.Address) // IPv6 addresses are labeled as NodeInternalIP despite being usable externally as well. if isExternal && (address.Type == v1.NodeExternalIP || (sc.exposeInternalIPv6 && address.Type == v1.NodeInternalIP && recordType == endpoint.RecordTypeAAAA)) { endpoints = append(endpoints, endpoint.NewEndpoint(hostname, recordType, address.Address)) diff --git a/source/fqdn/fqdn.go b/source/fqdn/fqdn.go index 571b352f7..39e185dd6 100644 --- a/source/fqdn/fqdn.go +++ b/source/fqdn/fqdn.go @@ -21,7 +21,6 @@ import ( "encoding/json" "fmt" "maps" - "net/netip" "reflect" "slices" "strings" @@ -107,20 +106,12 @@ func replace(oldValue, newValue, target string) string { // isIPv6String reports whether the target string is an IPv6 address, // including IPv4-mapped IPv6 addresses. func isIPv6String(target string) bool { - netIP, err := netip.ParseAddr(target) - if err != nil { - return false - } - return netIP.Is6() + return endpoint.SuitableType(target) == endpoint.RecordTypeAAAA } // isIPv4String reports whether the target string is an IPv4 address. func isIPv4String(target string) bool { - netIP, err := netip.ParseAddr(target) - if err != nil { - return false - } - return netIP.Is4() + return endpoint.SuitableType(target) == endpoint.RecordTypeA } // hasKey checks if a key exists in a map. This is needed because Go templates' diff --git a/source/gateway.go b/source/gateway.go index fb34b2f86..c7f1b1423 100644 --- a/source/gateway.go +++ b/source/gateway.go @@ -19,7 +19,6 @@ package source import ( "context" "fmt" - "net/netip" "sort" "strings" "text/template" @@ -650,8 +649,7 @@ func gwHost(host string) (string, bool) { // isIPAddr returns whether s in an IP address. func isIPAddr(s string) bool { - _, err := netip.ParseAddr(s) - return err == nil + return endpoint.SuitableType(s) != endpoint.RecordTypeCNAME } // isDNS1123Domain returns whether s is a valid domain name according to RFC 1123. diff --git a/source/node.go b/source/node.go index 800c20237..2c27e3236 100644 --- a/source/node.go +++ b/source/node.go @@ -195,7 +195,7 @@ func (ns *nodeSource) endpointsForDNSNames(node *v1.Node, dnsNames []string) ([] log.Debugf("adding endpoint with %d targets", len(addrs)) for _, addr := range addrs { - ep := endpoint.NewEndpointWithTTL(dns, suitableType(addr), ttl, addr) + ep := endpoint.NewEndpointWithTTL(dns, endpoint.SuitableType(addr), ttl, addr) ep.WithLabel(endpoint.ResourceLabelKey, fmt.Sprintf("node/%s", node.Name)) log.Debugf("adding endpoint %s target %s", ep, addr) endpoints = append(endpoints, ep) @@ -217,7 +217,7 @@ func (ns *nodeSource) nodeAddresses(node *v1.Node) ([]string, error) { for _, addr := range node.Status.Addresses { // IPv6 InternalIP addresses have special handling. // Refer to https://github.com/kubernetes-sigs/external-dns/pull/5192 for more details. - if addr.Type == v1.NodeInternalIP && suitableType(addr.Address) == endpoint.RecordTypeAAAA { + if addr.Type == v1.NodeInternalIP && endpoint.SuitableType(addr.Address) == endpoint.RecordTypeAAAA { internalIpv6Addresses = append(internalIpv6Addresses, addr.Address) } addresses[addr.Type] = append(addresses[addr.Type], addr.Address) diff --git a/source/pod.go b/source/pod.go index 6d50a1a67..2fe0d7863 100644 --- a/source/pod.go +++ b/source/pod.go @@ -230,7 +230,7 @@ func (ps *podSource) addInternalHostnameAnnotationEndpoints(endpointMap map[endp domainList := annotations.SplitHostnameAnnotation(domainAnnotation) for _, domain := range domainList { if len(targets) == 0 { - addToEndpointMap(endpointMap, pod, domain, suitableType(pod.Status.PodIP), pod.Status.PodIP) + addToEndpointMap(endpointMap, pod, domain, endpoint.SuitableType(pod.Status.PodIP), pod.Status.PodIP) } else { addTargetsToEndpointMap(endpointMap, pod, targets, domain) } @@ -254,7 +254,7 @@ func (ps *podSource) addKopsDNSControllerEndpoints(endpointMap map[endpoint.Endp if domainAnnotation, ok := pod.Annotations[kopsDNSControllerInternalHostnameAnnotationKey]; ok { domainList := annotations.SplitHostnameAnnotation(domainAnnotation) for _, domain := range domainList { - addToEndpointMap(endpointMap, pod, domain, suitableType(pod.Status.PodIP), pod.Status.PodIP) + addToEndpointMap(endpointMap, pod, domain, endpoint.SuitableType(pod.Status.PodIP), pod.Status.PodIP) } } @@ -269,7 +269,7 @@ func (ps *podSource) addPodSourceDomainEndpoints(endpointMap map[endpoint.Endpoi if ps.podSourceDomain != "" { domain := pod.Name + "." + ps.podSourceDomain if len(targets) == 0 { - addToEndpointMap(endpointMap, pod, domain, suitableType(pod.Status.PodIP), pod.Status.PodIP) + addToEndpointMap(endpointMap, pod, domain, endpoint.SuitableType(pod.Status.PodIP), pod.Status.PodIP) } addTargetsToEndpointMap(endpointMap, pod, targets, domain) } @@ -283,7 +283,7 @@ func (ps *podSource) addPodNodeEndpointsToEndpointMap(endpointMap map[endpoint.E } for _, domain := range domainList { for _, address := range node.Status.Addresses { - recordType := suitableType(address.Address) + recordType := endpoint.SuitableType(address.Address) // IPv6 addresses are labeled as NodeInternalIP despite being usable externally as well. if address.Type == corev1.NodeExternalIP || (address.Type == corev1.NodeInternalIP && recordType == endpoint.RecordTypeAAAA) { addToEndpointMap(endpointMap, pod, domain, recordType, address.Address) @@ -307,7 +307,7 @@ func (ps *podSource) hostsFromTemplate(pod *corev1.Pod) (map[endpoint.EndpointKe } key := endpoint.EndpointKey{ DNSName: target, - RecordType: suitableType(address.IP), + RecordType: endpoint.SuitableType(address.IP), RecordTTL: annotations.TTLFromAnnotations(pod.Annotations, fmt.Sprintf("pod/%s", pod.Name)), } result[key] = append(result[key], address.IP) @@ -320,7 +320,7 @@ func (ps *podSource) hostsFromTemplate(pod *corev1.Pod) (map[endpoint.EndpointKe func addTargetsToEndpointMap(endpointMap map[endpoint.EndpointKey][]string, pod *corev1.Pod, targets []string, domainList ...string) { for _, domain := range domainList { for _, target := range targets { - addToEndpointMap(endpointMap, pod, domain, suitableType(target), target) + addToEndpointMap(endpointMap, pod, domain, endpoint.SuitableType(target), target) } } } diff --git a/source/service.go b/source/service.go index 809738dde..5acb7e4ab 100644 --- a/source/service.go +++ b/source/service.go @@ -422,7 +422,7 @@ func (sc *serviceSource) processHeadlessEndpointsFromSlices( for _, target := range targets { key := endpoint.EndpointKey{ DNSName: headlessDomain, - RecordType: suitableType(target), + RecordType: endpoint.SuitableType(target), } targetsByHeadlessDomainAndType[key] = append(targetsByHeadlessDomainAndType[key], target) } @@ -471,7 +471,7 @@ func (sc *serviceSource) getTargetsForDomain( return nil } for _, address := range node.Status.Addresses { - if address.Type == v1.NodeExternalIP || (sc.exposeInternalIPv6 && address.Type == v1.NodeInternalIP && suitableType(address.Address) == endpoint.RecordTypeAAAA) { + if address.Type == v1.NodeExternalIP || (sc.exposeInternalIPv6 && address.Type == v1.NodeInternalIP && endpoint.SuitableType(address.Address) == endpoint.RecordTypeAAAA) { targets = append(targets, address.Address) log.Debugf("Generating matching endpoint %s with NodeExternalIP %s", headlessDomain, address.Address) } @@ -794,7 +794,7 @@ func (sc *serviceSource) extractNodePortTargets(svc *v1.Service) (endpoint.Targe externalIPs = append(externalIPs, address.Address) case v1.NodeInternalIP: internalIPs = append(internalIPs, address.Address) - if suitableType(address.Address) == endpoint.RecordTypeAAAA { + if endpoint.SuitableType(address.Address) == endpoint.RecordTypeAAAA { ipv6IPs = append(ipv6IPs, address.Address) } } diff --git a/source/unstructured.go b/source/unstructured.go index 18247c1bf..817213bfc 100644 --- a/source/unstructured.go +++ b/source/unstructured.go @@ -258,7 +258,7 @@ func (us *unstructuredSource) endpointsFromFQDNTargetTemplate(el *unstructuredWr continue } - endpoints = append(endpoints, endpoint.NewEndpoint(host, suitableType(target), target)) + endpoints = append(endpoints, endpoint.NewEndpoint(host, endpoint.SuitableType(target), target)) } return MergeEndpoints(endpoints), nil @@ -387,7 +387,7 @@ func EndpointsForHostsAndTargets(hostnames, targets []string) []*endpoint.Endpoi // Group and deduplicate targets by record type targetsByType := make(map[string]map[string]struct{}) for _, target := range targets { - recordType := suitableType(target) + recordType := endpoint.SuitableType(target) if targetsByType[recordType] == nil { targetsByType[recordType] = make(map[string]struct{}) } diff --git a/source/utils.go b/source/utils.go index c1543b05e..7b4c238c6 100644 --- a/source/utils.go +++ b/source/utils.go @@ -15,7 +15,6 @@ package source import ( "fmt" - "net/netip" "slices" "strings" @@ -24,23 +23,6 @@ import ( "sigs.k8s.io/external-dns/endpoint" ) -// suitableType returns the DNS resource record type suitable for the target. -// In this case type A/AAAA for IPs and type CNAME for everything else. -func suitableType(target string) string { - netIP, err := netip.ParseAddr(target) - if err != nil { - return endpoint.RecordTypeCNAME - } - switch { - case netIP.Is4(): - return endpoint.RecordTypeA - case netIP.Is6(): - return endpoint.RecordTypeAAAA - default: - return endpoint.RecordTypeCNAME - } -} - // ParseIngress parses an ingress string in the format "namespace/name" or "name". // It returns the namespace and name extracted from the string, or an error if the format is invalid. // If the namespace is not provided, it defaults to an empty string. diff --git a/source/utils_test.go b/source/utils_test.go index 724685d56..162ccdcb5 100644 --- a/source/utils_test.go +++ b/source/utils_test.go @@ -27,37 +27,6 @@ import ( "sigs.k8s.io/external-dns/source/types" ) -func TestSuitableType(t *testing.T) { - tests := []struct { - name string - target string - expected string - }{ - { - name: "valid IPv4 address", - target: "192.168.1.1", - expected: endpoint.RecordTypeA, - }, - { - name: "valid IPv6 address", - target: "2001:0db8:85a3:0000:0000:8a2e:0370:7334", - expected: endpoint.RecordTypeAAAA, - }, - { - name: "invalid IP address, should return CNAME", - target: "example.com", - expected: endpoint.RecordTypeCNAME, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := suitableType(tt.target) - assert.Equal(t, tt.expected, result) - }) - } -} - func TestParseIngress(t *testing.T) { tests := []struct { name string