mirrors/external-dns
1
0
Fork 0
mirror of https://github.com/kubernetes-sigs/external-dns.git synced 2025-12-07 21:01:33 +01:00
Code Issues Packages Projects Releases Wiki Activity

feat!: handle IP address comparison

Browse Source 
Previously there was no distinction between an IP address and any other string
when doing a comparison to determine which is "less" when determining which endpoint to actually create.

This explicitly handles IP addresses and will always prefer
them over non-IP addresses when determining which of two targets is less.
This commit is contained in:
Will Hegedus 2022-04-20 13:08:38 -04:00
parent f3e04dfe8c
commit 0c8f6a69bd
3 changed files with 56 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
endpoint/endpoint.go
View File

@ -18,6 +18,7 @@ package endpoint
import (
"fmt"
"net/netip"
"sort"
"strings"
@ -107,7 +108,21 @@ func (t Targets) IsLess(o Targets) bool {
for i, e := range t {
if e != o[i] {
return e < o[i]
// Explicitly prefers IP addresses (e.g. A records) over FQDNs (e.g. CNAMEs).
// This prevents behavior like `1-2-3-4.example.com` being "less" than `1.2.3.4` when doing lexicographical string comparison.
ipA, _ := netip.ParseAddr(e)
ipB, _ := netip.ParseAddr(o[i])
switch {
case ipA.IsValid() && ipB.IsValid():
return ipA.Less(ipB)
case ipA.IsValid() && !ipB.IsValid():
return true
case !ipA.IsValid() && ipB.IsValid():
return false
default:
return e < o[i]
}
}
}
return false

39
endpoint/endpoint_test.go
View File

@ -76,3 +76,42 @@ func TestSameFailures(t *testing.T) {
}
}
}
func TestIsLess(t *testing.T) {
testsA := []Targets{
{""},
{"1.2.3.4"},
{"1.2.3.4"},
{"example.org", "example.com"},
{"8.8.8.8", "8.8.4.4"},
{"1-2-3-4.example.org", "EXAMPLE.ORG"},
{"1-2-3-4.example.org", "EXAMPLE.ORG", "1.2.3.4"},
{"example.com", "example.org"},
}
testsB := []Targets{
{"", ""},
{"1-2-3-4.example.org"},
{"1.2.3.5"},
{"example.com", "examplea.org"},
{"8.8.8.8"},
{"1.2.3.4", "EXAMPLE.ORG"},
{"1-2-3-4.example.org", "EXAMPLE.ORG"},
{"example.com", "example.org"},
}
expected := []bool{
true,
true,
true,
true,
false,
false,
false,
false,
}
for i, d := range testsA {
if d.IsLess(testsB[i]) != expected[i] {
t.Errorf("%v < %v is expected to be %v", d, testsB[i], expected[i])
}
}
}

2
plan/conflict_test.go
View File

@ -114,7 +114,7 @@ func (suite *ResolverSuite) TestStrictResolver() {
suite.Equal(suite.fooV1Cname, suite.perResource.ResolveCreate([]*endpoint.Endpoint{suite.fooV2Cname, suite.fooV1Cname}), "should pick min one")
//test that perResource resolver preserves resource if it still exists
suite.Equal(suite.bar127A, suite.perResource.ResolveUpdate(suite.bar127A, []*endpoint.Endpoint{suite.bar127AAnother, suite.bar127A}), "should pick min for update when same resource endpoint occurs multiple times (remove after multiple-target support") // TODO:remove this test
suite.Equal(suite.bar127AAnother, suite.perResource.ResolveUpdate(suite.bar127A, []*endpoint.Endpoint{suite.bar127AAnother, suite.bar127A}), "should pick min for update when same resource endpoint occurs multiple times (remove after multiple-target support") // TODO:remove this test
suite.Equal(suite.bar127A, suite.perResource.ResolveUpdate(suite.bar127A, []*endpoint.Endpoint{suite.bar192A, suite.bar127A}), "should pick existing resource")
suite.Equal(suite.fooV2Cname, suite.perResource.ResolveUpdate(suite.fooV2Cname, []*endpoint.Endpoint{suite.fooV2Cname, suite.fooV2CnameDuplicate}), "should pick existing resource even if targets are same")
suite.Equal(suite.fooA5, suite.perResource.ResolveUpdate(suite.fooV1Cname, []*endpoint.Endpoint{suite.fooA5, suite.fooV2Cname}), "should pick new if resource was deleted")