Skip to content

Commit

Permalink
Merge pull request #4582 from gofogo/fix-crash-loop
Browse files Browse the repository at this point in the history 
fix(issue-4448): aws route53 inconsistent domain name handling - octal escapes
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Sep 17, 2024
2 parents 0f0f52d + cfd80f0 commit 05cd406
Show file tree
Hide file tree
Showing 2 changed files with 115 additions and 3 deletions.
29 changes: 27 additions & 2 deletions provider/aws/aws.go
View file
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
View file
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)
})
}
}

0 comments on commit 05cd406

Please sign in to comment.