Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Become IDNA aware in Plan and DomainFilter #4689

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions endpoint/domain_filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import (
"regexp"
"sort"
"strings"

log "github.com/sirupsen/logrus"
"golang.org/x/net/idna"
)

type MatchAllDomainFilters []DomainFilterInterface
Expand Down Expand Up @@ -69,7 +72,7 @@ type domainFilterSerde struct {
func prepareFilters(filters []string) []string {
var fs []string
for _, filter := range filters {
if domain := strings.ToLower(strings.TrimSuffix(strings.TrimSpace(filter), ".")); domain != "" {
if domain := normalizeDomain(strings.TrimSpace(filter)); domain != "" {
fs = append(fs, domain)
}
}
Expand Down Expand Up @@ -109,7 +112,7 @@ func matchFilter(filters []string, domain string, emptyval bool) bool {
return emptyval
}

strippedDomain := strings.ToLower(strings.TrimSuffix(domain, "."))
strippedDomain := normalizeDomain(domain)
for _, filter := range filters {
if filter == "" {
continue
Expand All @@ -133,7 +136,7 @@ func matchFilter(filters []string, domain string, emptyval bool) bool {
// only regex regular expression matches the domain
// Otherwise, if either negativeRegex matches or regex does not match the domain, it returns false
func matchRegex(regex *regexp.Regexp, negativeRegex *regexp.Regexp, domain string) bool {
strippedDomain := strings.ToLower(strings.TrimSuffix(domain, "."))
strippedDomain := normalizeDomain(domain)

if negativeRegex != nil && negativeRegex.String() != "" {
return !negativeRegex.MatchString(strippedDomain)
Expand Down Expand Up @@ -214,7 +217,7 @@ func (df DomainFilter) MatchParent(domain string) bool {
return true
}

strippedDomain := strings.ToLower(strings.TrimSuffix(domain, "."))
strippedDomain := normalizeDomain(domain)
for _, filter := range df.Filters {
if filter == "" || strings.HasPrefix(filter, ".") {
// We don't check parents if the filter is prefixed with "."
Expand All @@ -226,3 +229,13 @@ func (df DomainFilter) MatchParent(domain string) bool {
}
return false
}

// normalizeDomain converts a domain to a canonical form, so that we can filter on it
// it: trim "." suffix, get Unicode version of domain complient with Section 5 of RFC 5891
func normalizeDomain(domain string) string {
s, err := idna.Lookup.ToUnicode(strings.TrimSuffix(domain, "."))
if err != nil {
log.Warnf(`Got error while parsing domain %s: %v`, domain, err)
}
return s
}
135 changes: 133 additions & 2 deletions endpoint/domain_filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,26 @@ var domainFilterTests = []domainFilterTest{
"exclude": {".api.example.org"},
},
},
{
[]string{"æøå.org"},
[]string{"api.æøå.org"},
[]string{"foo.api.æøå.org", "api.æøå.org"},
false,
map[string][]string{
"include": {"æøå.org"},
"exclude": {"api.æøå.org"},
},
},
{
[]string{" æøå.org. "},
[]string{" .api.æøå.org "},
[]string{"foo.api.æøå.org", "bar.baz.api.æøå.org."},
false,
map[string][]string{
"include": {"æøå.org"},
"exclude": {".api.æøå.org"},
},
},
{
[]string{"example.org."},
[]string{"api.example.org"},
Expand Down Expand Up @@ -297,6 +317,16 @@ var domainFilterTests = []domainFilterTest{
"exclude": {"foo-bar.example.org"},
},
},
{
[]string{"sTOnks📈.ORG", "API.xn--StonkS-u354e.ORG"},
[]string{"Foo-Bar.stoNks📈.Org"},
[]string{"FoOoo.Api.Stonks📈.Org"},
true,
map[string][]string{
"include": {"api.stonks📈.org", "stonks📈.org"},
"exclude": {"foo-bar.stonks📈.org"},
},
},
{
[]string{"eXaMPle.ORG", "API.example.ORG"},
[]string{"api.example.org"},
Expand Down Expand Up @@ -347,6 +377,25 @@ var regexDomainFilterTests = []regexDomainFilterTest{
"regexInclude": "(?:foo|bar)\\.org$",
},
},
{
regexp.MustCompile("(?:😍|🤩)\\.org$"),
regexp.MustCompile(""),
[]string{"😍.org", "xn--r28h.org", "🤩.org", "example.😍.org", "example.🤩.org", "a.example.xn--r28h.org", "a.example.🤩.org"},
true,
map[string]string{
"regexInclude": "(?:😍|🤩)\\.org$",
},
},
{
regexp.MustCompile("(?:😍|🤩)\\.org$"),
regexp.MustCompile("^example\\.(?:😍|🤩)\\.org$"),
[]string{"example.😍.org", "example.🤩.org"},
false,
map[string]string{
"regexInclude": "(?:😍|🤩)\\.org$",
"regexExclude": "^example\\.(?:😍|🤩)\\.org$",
},
},
{
regexp.MustCompile("(?:foo|bar)\\.org$"),
regexp.MustCompile("^example\\.(?:foo|bar)\\.org$"),
Expand Down Expand Up @@ -479,8 +528,8 @@ func TestPrepareFiltersStripsWhitespaceAndDotSuffix(t *testing.T) {
nil,
},
{
[]string{" foo ", " bar. ", "baz."},
[]string{"foo", "bar", "baz"},
[]string{" foo ", " bar. ", "baz.", "xn--bar-zna"},
[]string{"foo", "bar", "baz", "øbar"},
},
{
[]string{"foo.bar", " foo.bar. ", " foo.bar.baz ", " foo.bar.baz. "},
Expand Down Expand Up @@ -714,6 +763,24 @@ func TestDomainFilterMatchParent(t *testing.T) {
"include": {"a.example.com", "b.example.com"},
},
},
{
[]string{"a.xn--c1yn36f.æøå.", "b.點看.xn--5cab8c", "c.點看.æøå"},
[]string{},
[]string{"xn--c1yn36f.xn--5cab8c"},
true,
map[string][]string{
"include": {"a.點看.æøå", "b.點看.æøå", "c.點看.æøå"},
},
},
{
[]string{"punycode.xn--c1yn36f.local", "å.點看.local.", "ø.點看.local"},
[]string{},
[]string{"點看.local"},
true,
map[string][]string{
"include": {"punycode.點看.local", "å.點看.local", "ø.點看.local"},
},
},
{
[]string{"a.example.com"},
[]string{},
Expand Down Expand Up @@ -769,3 +836,67 @@ func TestDomainFilterMatchParent(t *testing.T) {
})
}
}

func TestDomainFilterNormalizeDomain(t *testing.T) {
records := []struct {
dnsName string
expect string
}{
{
"3AAAA.FOO.BAR.COM",
"3aaaa.foo.bar.com",
},
{
"example.foo.com.",
"example.foo.com",
},
{
"example123.foo.com",
"example123.foo.com",
},
{
"foo.com.",
"foo.com",
},
{
"foo123.COM",
"foo123.com",
},
{
"my-exaMple3.FOO.BAR.COM",
"my-example3.foo.bar.com",
},
{
"my-example1214.FOO-1235.BAR-foo.COM",
"my-example1214.foo-1235.bar-foo.com",
},
{
"my-example-my-example-1214.FOO-1235.BAR-foo.COM",
"my-example-my-example-1214.foo-1235.bar-foo.com",
},
{
"xn--c1yn36f.org.",
"點看.org",
},
{
"xn--nordic--w1a.xn--xn--kItty-pd34d-hn01b3542b.com",
"nordic-ø.xn--kitty-點看pd34d.com",
},
{
"xn--nordic--w1a.xn--kItty-pd34d.com",
"nordic-ø.kitty😸.com",
},
{
"nordic-ø.kitty😸.COM",
"nordic-ø.kitty😸.com",
},
{
"xn--nordic--w1a.kiTTy😸.com.",
"nordic-ø.kitty😸.com",
},
}
for _, r := range records {
gotName := normalizeDomain(r.dnsName)
assert.Equal(t, r.expect, gotName)
}
}
8 changes: 6 additions & 2 deletions plan/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

"github.com/google/go-cmp/cmp"
log "github.com/sirupsen/logrus"
"golang.org/x/net/idna"

"sigs.k8s.io/external-dns/endpoint"
)
Expand Down Expand Up @@ -333,9 +334,12 @@ func filterRecordsForPlan(records []*endpoint.Endpoint, domainFilter endpoint.Ma
}

// normalizeDNSName converts a DNS name to a canonical form, so that we can use string equality
// it: removes space, converts to lower case, ensures there is a trailing dot
// it: removes space, get ASCII version of dnsName complient with Section 5 of RFC 5891, ensures there is a trailing dot
func normalizeDNSName(dnsName string) string {
s := strings.TrimSpace(strings.ToLower(dnsName))
s, err := idna.Lookup.ToASCII(strings.TrimSpace(dnsName))
if err != nil {
log.Warnf(`Got error while parsing DNSName %s: %v`, dnsName, err)
}
if !strings.HasSuffix(s, ".") {
s += "."
}
Expand Down
20 changes: 20 additions & 0 deletions plan/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,26 @@ func TestNormalizeDNSName(t *testing.T) {
"my-example-my-example-1214.FOO-1235.BAR-foo.COM",
"my-example-my-example-1214.foo-1235.bar-foo.com.",
},
{
"點看.org.",
"xn--c1yn36f.org.",
},
{
"nordic-ø.xn--kitty-點看pd34d.com",
"xn--nordic--w1a.xn--xn--kitty-pd34d-hn01b3542b.com.",
},
{
"nordic-ø.kitty😸.com.",
"xn--nordic--w1a.xn--kitty-pd34d.com.",
},
{
" nordic-ø.kitty😸.COM",
"xn--nordic--w1a.xn--kitty-pd34d.com.",
},
{
"xn--nordic--w1a.kitty😸.com.",
"xn--nordic--w1a.xn--kitty-pd34d.com.",
},
}
for _, r := range records {
gotName := normalizeDNSName(r.dnsName)
Expand Down
Loading