Skip to content
New issue

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

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

Already on GitHub? # to your account

fix(issue-4448): aws route53 inconsistent domain name handling - octal escapes #4582

Merged
merged 27 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
99290e4
fix(issue-4448): make sure nulls not propagated
ivankatliarchuk Jun 29, 2024
d112219
fix(issue-4448): no longer crash
ivankatliarchuk Jun 29, 2024
761ca1d
fix(issue-4448): single change required to fix
ivankatliarchuk Jun 30, 2024
99827b1
fix(issue-4448): single change required to fix
ivankatliarchuk Jun 30, 2024
df6d68e
fix(issue-4448): added octal decode method
ivankatliarchuk Jun 30, 2024
cc1f4ab
fix(issue-4448): added unittest
ivankatliarchuk Jun 30, 2024
d2f5824
fix(issue-4448): added test TestAWSApplyChanges
ivankatliarchuk Jun 30, 2024
2063478
fix(issue-4448): added tests for A and Alias records
ivankatliarchuk Jun 30, 2024
3351494
fix(issue-4448): aws.go intedation to tabs
ivankatliarchuk Jun 30, 2024
7249c64
fix(issue-4448): aws.go intedation to tabs
ivankatliarchuk Jun 30, 2024
3aa5c91
fix(issue-4448): added another flavour of same function with tests
ivankatliarchuk Jul 5, 2024
fca9914
fix(issue-4448): uncomment test
ivankatliarchuk Jul 5, 2024
b1bfffb
fix(issue-4448): uncomment test
ivankatliarchuk Jul 5, 2024
a5b1d0f
swap with golang function
ivankatliarchuk Jul 9, 2024
eb73202
swap with golang function
ivankatliarchuk Jul 9, 2024
2b14345
issue-4448: fix test
ivankatliarchuk Jul 9, 2024
92ced09
chore(github-actions): added scorecard github action
ivankatliarchuk Aug 12, 2024
828f68f
chore(github-actions): added scorecard github action
ivankatliarchuk Aug 12, 2024
4b0954c
chore(github-actions): test with workflow dispatch
ivankatliarchuk Aug 12, 2024
2238f90
Merge pull request #1 from gofogo/issue-4673
ivankatliarchuk Aug 12, 2024
1936c13
Merge branch 'kubernetes-sigs:master' into master
ivankatliarchuk Sep 14, 2024
3830fb2
wip
ivankatliarchuk Sep 14, 2024
ec402b0
Merge branch 'master' into fix-crash-loop
ivankatliarchuk Sep 14, 2024
d11ddb7
merge with master
ivankatliarchuk Sep 14, 2024
847bd3e
merge with master
ivankatliarchuk Sep 14, 2024
8eaf6df
add back different tabulation
ivankatliarchuk Sep 14, 2024
cfd80f0
remove tabulation
ivankatliarchuk Sep 14, 2024
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
29 changes: 27 additions & 2 deletions provider/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"errors"
"fmt"
"regexp"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -398,6 +399,28 @@ func wildcardUnescape(s string) string {
return strings.Replace(s, "\\052", "*", 1)
}

// See https://docs.aws.amazon.com/Route53/latest/DeveloperGuide/DomainNameFormat.html
// convertOctalToAscii decodes inputs that contain octal escape sequences into their original ASCII characters.
// The function returns converted string where any octal escape sequences have been replaced with their corresponding ASCII characters.
func convertOctalToAscii(input string) string {
if !containsOctalSequence(input) {
return input
}
result, err := strconv.Unquote("\"" + input + "\"")
if err != nil {
return input
}
return result
}

// validateDomainName checks if the domain name contains valid octal escape sequences.
func containsOctalSequence(domain string) bool {
// Pattern to match valid octal escape sequences
octalEscapePattern := `\\[0-3][0-7]{2}`
octalEscapeRegex := regexp.MustCompile(octalEscapePattern)
return octalEscapeRegex.MatchString(domain)
}

// Records returns the list of records in a given hosted zone.
func (p *AWSProvider) Records(ctx context.Context) (endpoints []*endpoint.Endpoint, _ error) {
zones, err := p.zones(ctx)
Expand Down Expand Up @@ -432,6 +455,8 @@ func (p *AWSProvider) records(ctx context.Context, zones map[string]*profiledZon
continue
}

name := convertOctalToAscii(wildcardUnescape(*r.Name))

var ttl endpoint.TTL
if r.TTL != nil {
ttl = endpoint.TTL(*r.TTL)
Expand All @@ -443,7 +468,7 @@ func (p *AWSProvider) records(ctx context.Context, zones map[string]*profiledZon
targets[idx] = *rr.Value
}

ep := endpoint.NewEndpointWithTTL(wildcardUnescape(*r.Name), string(r.Type), ttl, targets...)
ep := endpoint.NewEndpointWithTTL(name, string(r.Type), ttl, targets...)
if r.Type == endpoint.RecordTypeCNAME {
ep = ep.WithProviderSpecific(providerSpecificAlias, "false")
}
Expand All @@ -456,7 +481,7 @@ func (p *AWSProvider) records(ctx context.Context, zones map[string]*profiledZon
ttl = recordTTL
}
ep := endpoint.
NewEndpointWithTTL(wildcardUnescape(*r.Name), endpoint.RecordTypeA, ttl, *r.AliasTarget.DNSName).
NewEndpointWithTTL(name, endpoint.RecordTypeA, ttl, *r.AliasTarget.DNSName).
WithProviderSpecific(providerSpecificEvaluateTargetHealth, fmt.Sprintf("%t", r.AliasTarget.EvaluateTargetHealth)).
WithProviderSpecific(providerSpecificAlias, "true")
newEndpoints = append(newEndpoints, ep)
Expand Down
89 changes: 88 additions & 1 deletion provider/aws/aws_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,20 @@ func wildcardEscape(s string) string {
return s
}

// Route53 octal escapes https://docs.aws.amazon.com/Route53/latest/DeveloperGuide/DomainNameFormat.html
func specialCharactersEscape(s string) string {
var result strings.Builder
for _, char := range s {
if (char >= 'a' && char <= 'z') || (char >= '0' && char <= '9') || char == '-' || char == '.' {
result.WriteRune(char)
} else {
octalCode := fmt.Sprintf("\\%03o", char)
result.WriteString(octalCode)
}
}
return result.String()
}

func (r *Route53APIStub) ListTagsForResource(ctx context.Context, input *route53.ListTagsForResourceInput, optFns ...func(options *route53.Options)) (*route53.ListTagsForResourceOutput, error) {
if input.ResourceType == route53types.TagResourceTypeHostedzone {
tags := r.zoneTags[*input.ResourceId]
Expand Down Expand Up @@ -352,11 +366,33 @@ func TestAWSRecords(t *testing.T) {
ResourceRecords: []route53types.ResourceRecord{{Value: aws.String("8.8.8.8")}},
},
{
Name: aws.String("*.wildcard-test.zone-2.ext-dns-test-2.teapot.zalan.do."),
Name: aws.String(wildcardEscape("*.wildcard-test.zone-2.ext-dns-test-2.teapot.zalan.do.")),
Type: route53types.RRTypeA,
TTL: aws.Int64(recordTTL),
ResourceRecords: []route53types.ResourceRecord{{Value: aws.String("8.8.8.8")}},
},
{
Name: aws.String(specialCharactersEscape("escape-%!s(<nil>)-codes.zone-2.ext-dns-test-2.teapot.zalan.do.")),
Type: route53types.RRTypeCname,
TTL: aws.Int64(recordTTL),
ResourceRecords: []route53types.ResourceRecord{{Value: aws.String("example")}},
},
{
Name: aws.String(specialCharactersEscape("escape-%!s(<nil>)-codes-a.zone-2.ext-dns-test-2.teapot.zalan.do.")),
Type: route53types.RRTypeA,
TTL: aws.Int64(recordTTL),
ResourceRecords: []route53types.ResourceRecord{{Value: aws.String("1.2.3.4")}},
},
{
Name: aws.String(specialCharactersEscape("escape-%!s(<nil>)-codes-alias.zone-2.ext-dns-test-2.teapot.zalan.do.")),
Type: route53types.RRTypeA,
TTL: aws.Int64(recordTTL),
AliasTarget: &route53types.AliasTarget{
DNSName: aws.String("escape-codes.eu-central-1.elb.amazonaws.com."),
EvaluateTargetHealth: false,
HostedZoneId: aws.String("Z215JYRZR1TBD5"),
},
},
{
Name: aws.String("list-test-alias.zone-1.ext-dns-test-2.teapot.zalan.do."),
Type: route53types.RRTypeA,
Expand Down Expand Up @@ -499,6 +535,9 @@ func TestAWSRecords(t *testing.T) {
endpoint.NewEndpointWithTTL("list-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4"),
endpoint.NewEndpointWithTTL("list-test.zone-2.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "8.8.8.8"),
endpoint.NewEndpointWithTTL("*.wildcard-test.zone-2.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "8.8.8.8"),
endpoint.NewEndpointWithTTL("escape-%!s(<nil>)-codes.zone-2.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeCNAME, endpoint.TTL(recordTTL), "example").WithProviderSpecific(providerSpecificAlias, "false"),
endpoint.NewEndpointWithTTL("escape-%!s(<nil>)-codes-a.zone-2.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4"),
endpoint.NewEndpointWithTTL("escape-%!s(<nil>)-codes-alias.zone-2.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "escape-codes.eu-central-1.elb.amazonaws.com").WithProviderSpecific(providerSpecificEvaluateTargetHealth, "false").WithProviderSpecific(providerSpecificAlias, "true"),
endpoint.NewEndpointWithTTL("list-test-alias.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "foo.eu-central-1.elb.amazonaws.com").WithProviderSpecific(providerSpecificEvaluateTargetHealth, "false").WithProviderSpecific(providerSpecificAlias, "true"),
endpoint.NewEndpointWithTTL("*.wildcard-test-alias.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "foo.eu-central-1.elb.amazonaws.com").WithProviderSpecific(providerSpecificEvaluateTargetHealth, "false").WithProviderSpecific(providerSpecificAlias, "true"),
endpoint.NewEndpointWithTTL("list-test-alias-evaluate.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "foo.eu-central-1.elb.amazonaws.com").WithProviderSpecific(providerSpecificEvaluateTargetHealth, "true").WithProviderSpecific(providerSpecificAlias, "true"),
Expand Down Expand Up @@ -687,6 +726,14 @@ func TestAWSApplyChanges(t *testing.T) {
TTL: aws.Int64(recordTTL),
ResourceRecords: []route53types.ResourceRecord{{Value: aws.String("30 mailhost1.foo.elb.amazonaws.com")}},
},
{
Name: aws.String(specialCharactersEscape("escape-%!s(<nil>)-codes.zone-2.ext-dns-test-2.teapot.zalan.do.")),
Type: route53types.RRTypeA,
TTL: aws.Int64(recordTTL),
ResourceRecords: []route53types.ResourceRecord{{Value: aws.String("1.2.3.4")}},
SetIdentifier: aws.String("no-change"),
Weight: aws.Int64(10),
},
})

createRecords := []*endpoint.Endpoint{
Expand All @@ -712,6 +759,7 @@ func TestAWSApplyChanges(t *testing.T) {
endpoint.NewEndpoint("set-identifier-change.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4").WithSetIdentifier("before").WithProviderSpecific(providerSpecificWeight, "10"),
endpoint.NewEndpoint("set-identifier-no-change.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4").WithSetIdentifier("no-change").WithProviderSpecific(providerSpecificWeight, "10"),
endpoint.NewEndpoint("update-test-mx.zone-2.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeMX, "10 mailhost2.bar.elb.amazonaws.com"),
endpoint.NewEndpoint("escape-%!s(<nil>)-codes.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4").WithSetIdentifier("policy-change").WithSetIdentifier("no-change").WithProviderSpecific(providerSpecificWeight, "10"),
}
updatedRecords := []*endpoint.Endpoint{
endpoint.NewEndpoint("update-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4"),
Expand Down Expand Up @@ -853,6 +901,14 @@ func TestAWSApplyChanges(t *testing.T) {
},
})
validateRecords(t, listAWSRecords(t, provider.clients[defaultAWSProfile], "/hostedzone/zone-2.ext-dns-test-2.teapot.zalan.do."), []route53types.ResourceRecordSet{
{
Name: aws.String("escape-\\045\\041s\\050\\074nil\\076\\051-codes.zone-2.ext-dns-test-2.teapot.zalan.do."),
Type: route53types.RRTypeA,
TTL: aws.Int64(recordTTL),
ResourceRecords: []route53types.ResourceRecord{{Value: aws.String("1.2.3.4")}},
SetIdentifier: aws.String("no-change"),
Weight: aws.Int64(10),
},
{
Name: aws.String("create-test.zone-2.ext-dns-test-2.teapot.zalan.do."),
Type: route53types.RRTypeA,
Expand Down Expand Up @@ -2024,3 +2080,34 @@ func TestRequiresDeleteCreate(t *testing.T) {
assert.False(t, provider.requiresDeleteCreate(oldSetIdentifier, oldSetIdentifier), "actual and expected endpoints don't match. %+v:%+v", oldSetIdentifier, oldSetIdentifier)
assert.True(t, provider.requiresDeleteCreate(oldSetIdentifier, newSetIdentifier), "actual and expected endpoints don't match. %+v:%+v", oldSetIdentifier, newSetIdentifier)
}

func TestConvertOctalToAscii(t *testing.T) {
tests := []struct {
name string
input string
expected string
}{
{
name: "Characters escaped !\"#$%&'()*+,-/:;",
input: "txt-\\041\\042\\043\\044\\045\\046\\047\\050\\051\\052\\053\\054-\\057\\072\\073-test.example.com",
expected: "txt-!\"#$%&'()*+,-/:;-test.example.com",
},
{
name: "Characters escaped <=>?@[\\]^_`{|}~",
input: "txt-\\074\\075\\076\\077\\100\\133\\134\\135\\136_\\140\\173\\174\\175\\176-test2.example.com",
expected: "txt-<=>?@[\\]^_`{|}~-test2.example.com",
},
{
name: "No escaped characters in domain",
input: "txt-awesome-test3.example.com",
expected: "txt-awesome-test3.example.com",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
actual := convertOctalToAscii(tt.input)
assert.Equal(t, tt.expected, actual)
})
}
}