From 0c0ef9a3957812a86dede65b1f73b8c731ab5491 Mon Sep 17 00:00:00 2001 From: Jean-Mathieu Deschenes Date: Sat, 30 Oct 2021 18:41:27 -0400 Subject: [PATCH 1/6] Fixed an issue when importing Route53 Records with underscores * There is an issue when a DNS record contains an underscore. The problem was solved for the case where the underscore was the first character of the DNS Name but did not validate when the underscore was somewhere else in the Name. This problem occurred when trying to import a DNS Record for a DKIM record of Microsoft 365. --- internal/service/route53/record.go | 26 ++++++++++++------------- internal/service/route53/record_test.go | 1 + 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/internal/service/route53/record.go b/internal/service/route53/record.go index ae3deffde5a..a1269133b2f 100644 --- a/internal/service/route53/record.go +++ b/internal/service/route53/record.go @@ -975,22 +975,22 @@ func NormalizeAliasName(alias interface{}) string { func ParseRecordID(id string) [4]string { var recZone, recType, recName, recSet string - parts := strings.SplitN(id, "_", 2) - if len(parts) == 2 { + parts := strings.Split(id, "_") + if len(parts) >= 3 { recZone = parts[0] - firstUnderscore := strings.Index(parts[1][:], "_") - // Handles the case of having a DNS name that starts with _ - if firstUnderscore == 0 { - firstUnderscore = strings.Index(parts[1][1:], "_") + 1 - } - recName, recType = parts[1][0:firstUnderscore], parts[1][firstUnderscore+1:] - if !r53ValidRecordTypes.MatchString(recType) { - firstUnderscore = strings.Index(recType, "_") - if firstUnderscore != -1 { - recType, recSet = recType[0:firstUnderscore], recType[firstUnderscore+1:] + var recTypeIndex int = -1 + for i, maybeRecType := range parts[1:] { + if r53ValidRecordTypes.MatchString(maybeRecType) { + recTypeIndex = i + 1 + break } } + if recTypeIndex > 1 { + recName = strings.Join(parts[1:recTypeIndex], "_") + recName = strings.TrimSuffix(recName, ".") + recType = parts[recTypeIndex] + recSet = strings.Join(parts[recTypeIndex+1:], "_") + } } - recName = strings.TrimSuffix(recName, ".") return [4]string{recZone, recName, recType, recSet} } diff --git a/internal/service/route53/record_test.go b/internal/service/route53/record_test.go index 9d8c98b6fb9..953ddedc2c3 100644 --- a/internal/service/route53/record_test.go +++ b/internal/service/route53/record_test.go @@ -94,6 +94,7 @@ func TestParseRecordId(t *testing.T) { {"ABCDEF__underscore.example.com_A_set1", "ABCDEF", "_underscore.example.com", "A", "set1"}, {"ABCDEF__underscore.example.com_A_set_with1", "ABCDEF", "_underscore.example.com", "A", "set_with1"}, {"ABCDEF__underscore.example.com_A_set_with_1", "ABCDEF", "_underscore.example.com", "A", "set_with_1"}, + {"ABCDEF__underscore._example.com_A_set_with_1", "ABCDEF", "_underscore._example.com", "A", "set_with_1"}, } for _, tc := range cases { From cd2fd2aee7b5a6c39a4d8f08bd19485f425fc9a6 Mon Sep 17 00:00:00 2001 From: Jean-Mathieu Deschenes Date: Sat, 30 Oct 2021 19:31:19 -0400 Subject: [PATCH 2/6] Added changelog --- .changelog/21556.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/21556.txt diff --git a/.changelog/21556.txt b/.changelog/21556.txt new file mode 100644 index 00000000000..1bcb9678090 --- /dev/null +++ b/.changelog/21556.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_route53_record: Fix import with underscores in names +``` From 934476f7170c12806309d9bee58d8c06974baf68 Mon Sep 17 00:00:00 2001 From: Jean-Mathieu Deschenes Date: Mon, 6 Dec 2021 08:48:34 -0500 Subject: [PATCH 3/6] Fixed an issue with the unit test --- internal/service/route53/record.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/service/route53/record.go b/internal/service/route53/record.go index a1269133b2f..57d92b1278d 100644 --- a/internal/service/route53/record.go +++ b/internal/service/route53/record.go @@ -976,8 +976,10 @@ func NormalizeAliasName(alias interface{}) string { func ParseRecordID(id string) [4]string { var recZone, recType, recName, recSet string parts := strings.Split(id, "_") - if len(parts) >= 3 { + if len(parts) > 1 { recZone = parts[0] + } + if len(parts) >= 3 { var recTypeIndex int = -1 for i, maybeRecType := range parts[1:] { if r53ValidRecordTypes.MatchString(maybeRecType) { From a98496bb8108fff8d59baa1f68aede7d923f7137 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 7 Jan 2022 17:35:18 -0700 Subject: [PATCH 4/6] Adds additional test cases --- internal/service/route53/record_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/service/route53/record_test.go b/internal/service/route53/record_test.go index e79f1928d63..3b67bfd5168 100644 --- a/internal/service/route53/record_test.go +++ b/internal/service/route53/record_test.go @@ -96,7 +96,9 @@ func TestParseRecordId(t *testing.T) { {"ABCDEF__underscore.example.com_A_set1", "ABCDEF", "_underscore.example.com", "A", "set1"}, {"ABCDEF__underscore.example.com_A_set_with1", "ABCDEF", "_underscore.example.com", "A", "set_with1"}, {"ABCDEF__underscore.example.com_A_set_with_1", "ABCDEF", "_underscore.example.com", "A", "set_with_1"}, - {"ABCDEF__underscore._example.com_A_set_with_1", "ABCDEF", "_underscore._example.com", "A", "set_with_1"}, + {"ABCDEF_prefix._underscore.example.com_A", "ABCDEF", "prefix._underscore.example.com", "A", ""}, + {"ABCDEF_prefix._underscore.example.com_A_set", "ABCDEF", "prefix._underscore.example.com", "A", "set"}, + {"ABCDEF_prefix._underscore.example.com_A_set_underscore", "ABCDEF", "prefix._underscore.example.com", "A", "set_underscore"}, } for _, tc := range cases { From d16c5023a1c2d373522d88c6929b03c88465c007 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 7 Jan 2022 17:36:04 -0700 Subject: [PATCH 5/6] Names outputs in test errors --- internal/service/route53/record_test.go | 28 +++++++++++++------------ 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/internal/service/route53/record_test.go b/internal/service/route53/record_test.go index 3b67bfd5168..5fa9ccd1bc9 100644 --- a/internal/service/route53/record_test.go +++ b/internal/service/route53/record_test.go @@ -102,19 +102,21 @@ func TestParseRecordId(t *testing.T) { } for _, tc := range cases { - parts := tfroute53.ParseRecordID(tc.Input) - if parts[0] != tc.Zone { - t.Fatalf("input: %s\noutput: %s\nexpected:%s", tc.Input, parts[0], tc.Zone) - } - if parts[1] != tc.Name { - t.Fatalf("input: %s\noutput: %s\nexpected:%s", tc.Input, parts[1], tc.Name) - } - if parts[2] != tc.Type { - t.Fatalf("input: %s\noutput: %s\nexpected:%s", tc.Input, parts[2], tc.Type) - } - if parts[3] != tc.Set { - t.Fatalf("input: %s\noutput: %s\nexpected:%s", tc.Input, parts[3], tc.Set) - } + t.Run(tc.Input, func(t *testing.T) { + parts := tfroute53.ParseRecordID(tc.Input) + if parts[0] != tc.Zone { + t.Fatalf("input: %s\nzone: %s\nexpected:%s", tc.Input, parts[0], tc.Zone) + } + if parts[1] != tc.Name { + t.Fatalf("input: %s\nname: %s\nexpected:%s", tc.Input, parts[1], tc.Name) + } + if parts[2] != tc.Type { + t.Fatalf("input: %s\ntype: %s\nexpected:%s", tc.Input, parts[2], tc.Type) + } + if parts[3] != tc.Set { + t.Fatalf("input: %s\nset: %s\nexpected:%s", tc.Input, parts[3], tc.Set) + } + }) } } From d6da2238912470d8db3b182efc5177caf69cc98a Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 7 Jan 2022 17:54:30 -0700 Subject: [PATCH 6/6] Uses `route53.RRType_Values()` to validate record types --- internal/service/route53/record.go | 33 ++++++++++++------------------ 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/internal/service/route53/record.go b/internal/service/route53/record.go index 57d92b1278d..e808a1acd57 100644 --- a/internal/service/route53/record.go +++ b/internal/service/route53/record.go @@ -6,7 +6,6 @@ import ( "fmt" "log" "math/rand" - "regexp" "strconv" "strings" "time" @@ -30,7 +29,6 @@ const ( var ( r53NoRecordsFound = errors.New("No matching records found") r53NoHostedZoneFound = errors.New("No matching Hosted Zone found") - r53ValidRecordTypes = regexp.MustCompile("^(A|AAAA|CAA|CNAME|MX|NAPTR|NS|PTR|SOA|SPF|SRV|TXT|DS)$") ) func ResourceRecord() *schema.Resource { @@ -65,23 +63,9 @@ func ResourceRecord() *schema.Resource { }, "type": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringInSlice([]string{ - route53.RRTypeSoa, - route53.RRTypeA, - route53.RRTypeTxt, - route53.RRTypeNs, - route53.RRTypeCname, - route53.RRTypeMx, - route53.RRTypeNaptr, - route53.RRTypePtr, - route53.RRTypeSrv, - route53.RRTypeSpf, - route53.RRTypeAaaa, - route53.RRTypeCaa, - route53.RRTypeDs, - }, false), + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringInSlice(route53.RRType_Values(), false), }, "zone_id": { @@ -982,7 +966,7 @@ func ParseRecordID(id string) [4]string { if len(parts) >= 3 { var recTypeIndex int = -1 for i, maybeRecType := range parts[1:] { - if r53ValidRecordTypes.MatchString(maybeRecType) { + if validRecordType(maybeRecType) { recTypeIndex = i + 1 break } @@ -996,3 +980,12 @@ func ParseRecordID(id string) [4]string { } return [4]string{recZone, recName, recType, recSet} } + +func validRecordType(s string) bool { + for _, v := range route53.RRType_Values() { + if v == s { + return true + } + } + return false +}