From fcdd92bd2d6f91537bbd973b23efab3761061055 Mon Sep 17 00:00:00 2001 From: Dylan Ratcliffe Date: Sat, 30 Nov 2024 14:35:41 +0000 Subject: [PATCH 1/2] Fixed record mapping for `aws_route53_record` resources Fixes #682 --- adapters/route53-resource-record-set.go | 36 +++++++++++++++++++++---- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/adapters/route53-resource-record-set.go b/adapters/route53-resource-record-set.go index 129d4c3d..c3fe7511 100644 --- a/adapters/route53-resource-record-set.go +++ b/adapters/route53-resource-record-set.go @@ -3,6 +3,7 @@ package adapters import ( "context" "errors" + "fmt" "strings" "github.com/aws/aws-sdk-go-v2/service/route53" @@ -17,21 +18,46 @@ func resourceRecordSetGetFunc(ctx context.Context, client *route53.Client, scope } // ResourceRecordSetSearchFunc Search func that accepts a hosted zone or a -// terraform ID in the format {hostedZone}_{recordName}_{type} +// terraform ID in the format {hostedZone}_{recordName}_{type}. Unfortunately +// the "name" means the record name within the scope of the hosted zone, not the +// full FQDN. This is something that Terraform does to match the AWS GUI, where +// you specify a name like "foo" and then you end up with a record like +// "foo.example.com.". That record has a "name" attribute, but it's set to +// "foo.example.com.". +// +// Because of this behaviour we need to construct the full name, rather than +// just the half-name. You can see that the terraform provider itself also does +// this in `findResourceRecordSetByFourPartKey`: +// https://github.com/hashicorp/terraform-provider-aws/blob/main/internal/service/route53/record.go#L786-L825 func resourceRecordSetSearchFunc(ctx context.Context, client *route53.Client, scope, query string) ([]*types.ResourceRecordSet, error) { splits := strings.Split(query, "_") var out *route53.ListResourceRecordSetsOutput var err error if len(splits) == 3 { - // In this case we have a terraform ID + var zoneResp *route53.GetHostedZoneOutput + // In this case we have a terraform ID. We have to get the details of the hosted zone first + zoneResp, err = client.GetHostedZone(ctx, &route53.GetHostedZoneInput{ + Id: &splits[0], + }) + if err != nil { + return nil, err + } + if zoneResp.HostedZone == nil { + return nil, fmt.Errorf("hosted zone %s not found", splits[0]) + } + + // Calculate the full FQDN based on the hosted zone name and the record name + fullName := splits[1] + "." + *zoneResp.HostedZone.Name + var max int32 = 1 - out, err = client.ListResourceRecordSets(ctx, &route53.ListResourceRecordSetsInput{ + req := route53.ListResourceRecordSetsInput{ HostedZoneId: &splits[0], - StartRecordName: &splits[1], + StartRecordName: &fullName, StartRecordType: types.RRType(splits[2]), MaxItems: &max, - }) + } + out, err = client.ListResourceRecordSets(ctx, &req) } else { // In this case we have a hosted zone ID out, err = client.ListResourceRecordSets(ctx, &route53.ListResourceRecordSetsInput{ From faac95300dc1ac6d7a94a46e94fafc705fda89e0 Mon Sep 17 00:00:00 2001 From: Dylan Ratcliffe Date: Sat, 30 Nov 2024 14:56:00 +0000 Subject: [PATCH 2/2] Fixed tests --- adapters/route53-resource-record-set.go | 25 +++++++++++++++----- adapters/route53-resource-record-set_test.go | 16 +++++++++---- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/adapters/route53-resource-record-set.go b/adapters/route53-resource-record-set.go index c3fe7511..9d3376a4 100644 --- a/adapters/route53-resource-record-set.go +++ b/adapters/route53-resource-record-set.go @@ -35,26 +35,39 @@ func resourceRecordSetSearchFunc(ctx context.Context, client *route53.Client, sc var out *route53.ListResourceRecordSetsOutput var err error if len(splits) == 3 { + hostedZoneID := splits[0] + recordName := splits[1] + recordType := splits[2] + var zoneResp *route53.GetHostedZoneOutput // In this case we have a terraform ID. We have to get the details of the hosted zone first zoneResp, err = client.GetHostedZone(ctx, &route53.GetHostedZoneInput{ - Id: &splits[0], + Id: &hostedZoneID, }) if err != nil { return nil, err } if zoneResp.HostedZone == nil { - return nil, fmt.Errorf("hosted zone %s not found", splits[0]) + return nil, fmt.Errorf("hosted zone %s not found", hostedZoneID) } - // Calculate the full FQDN based on the hosted zone name and the record name - fullName := splits[1] + "." + *zoneResp.HostedZone.Name + // If the name is the same as the FQDN of the hosted zone, we don't have + // to append it otherwise it'll be in there twice. It seems that NS and + // MX records sometimes have the full FQDN in the name + zoneFQDN := strings.TrimSuffix(*zoneResp.HostedZone.Name, ".") + var fullName string + if recordName == zoneFQDN { + fullName = recordName + } else { + // Calculate the full FQDN based on the hosted zone name and the record name + fullName = recordName + "." + *zoneResp.HostedZone.Name + } var max int32 = 1 req := route53.ListResourceRecordSetsInput{ - HostedZoneId: &splits[0], + HostedZoneId: &hostedZoneID, StartRecordName: &fullName, - StartRecordType: types.RRType(splits[2]), + StartRecordType: types.RRType(recordType), MaxItems: &max, } out, err = client.ListResourceRecordSets(ctx, &req) diff --git a/adapters/route53-resource-record-set_test.go b/adapters/route53-resource-record-set_test.go index 546367d2..75308844 100644 --- a/adapters/route53-resource-record-set_test.go +++ b/adapters/route53-resource-record-set_test.go @@ -157,12 +157,17 @@ func TestNewRoute53ResourceRecordSetAdapter(t *testing.T) { t.Errorf("expected %d items, got %d", numItems, len(items)) } - if len(items) > 0 { - item := items[0] + for _, item := range items { + // Only use CNAME records + typ, _ := item.GetAttributes().Get("Type") + if typ != "CNAME" { + continue + } // Construct a terraform style ID - name, _ := item.GetAttributes().Get("Name") - typ, _ := item.GetAttributes().Get("Type") + fqdn, _ := item.GetAttributes().Get("Name") + sections := strings.Split(fqdn.(string), ".") + name := sections[0] search = fmt.Sprintf("%s_%s_%s", rawZone, name, typ) items, err := adapter.Search(context.Background(), zoneSource.Scopes()[0], search, true) @@ -173,5 +178,8 @@ func TestNewRoute53ResourceRecordSetAdapter(t *testing.T) { if len(items) != 1 { t.Errorf("expected 1 item, got %d", len(items)) } + + // Only need to test this once + break } }