resource/aws_route53_record: Normalizes Resource Identity name to fully-qualified domain name

.ci/semgrep/list/no-resource-read.yml

@@ -9,7 +9,6 @@ rules:
       include:
         - "/internal/service"
       exclude:
-        - "/internal/service/route53/record_list.go"
         - "/internal/service/route53resolver/rule_association_list.go"
         - "/internal/service/s3/bucket_acl_list.go"
         - "/internal/service/s3/bucket_public_access_block_list.go"

internal/service/route53/clean.go

@@ -64,3 +64,29 @@ func fqdn(name string) string {
 		return name + "."
 	}
 }
+
+// denormalizeDomainName removes escaping for the "*" character in the leftmost label of a domain name,
+// and removes the trailing dot if present.
+// The single dot (".") domain name is passed through as-is.
+func denormalizeDomainName(v any) string {
+	switch v := v.(type) {
+	case *string:
+		return denormalizeDomainNameImpl(aws.ToString(v))
+	case string:
+		return denormalizeDomainNameImpl(v)
+	default:
+		return ""
+	}
+}
+
+func denormalizeDomainNameImpl(s string) string {
+	if s == "" || s == "." {
+		return s
+	}
+
+	s = strings.TrimSuffix(s, ".")
+	if after, ok := strings.CutPrefix(s, `\052.`); ok {
+		s = `*.` + after
+	}
+	return s
+}

internal/service/route53/clean_test.go

@@ -23,7 +23,9 @@ func TestNormalizeAliasDomainName(t *testing.T) {
 		{"ipv6.name-123456789.region.elb.amazonaws.com", "ipv6.name-123456789.region.elb.amazonaws.com"},
 		{"NAME-123456789.region.elb.amazonaws.com", "name-123456789.region.elb.amazonaws.com"},
 		{"name-123456789.region.elb.amazonaws.com", "name-123456789.region.elb.amazonaws.com"},
-		{"\\052.example.com", "\\052.example.com"},
+		{"*.example.com", "\\052.example.com"},     // "*" is normalized to octal "\052"
+		{"@.example.com", "\\100.example.com"},     // "@" is normalized to octal "\100"
+		{"\\052.example.com", "\\052.example.com"}, // octal "\052" is preserved
 		{42, ""},
 	}
 
@@ -63,12 +65,15 @@ func TestNormalizeDomainName(t *testing.T) {
 		input  any
 		output string
 	}{
-		{"example.com", "example.com"},
-		{"example.com.", "example.com"},
-		{"www.example.com.", "www.example.com"},
-		{"www.ExAmPlE.COM.", "www.example.com"},
-		{"", ""},
-		{".", "."},
+		{"example.com", "example.com"},             // as-is
+		{"example.com.", "example.com"},            // trailing dot is removed
+		{"www.example.com.", "www.example.com"},    // trailing dot is removed
+		{"www.ExAmPlE.COM.", "www.example.com"},    // case is normalized to lower case
+		{"*.example.com", "\\052.example.com"},     // "*" is normalized to octal "\052"
+		{"@.example.com", "\\100.example.com"},     // "@" is normalized to octal "\100"
+		{"\\052.example.com", "\\052.example.com"}, // octal "\052" is preserved
+		{"", ""},   // as-is
+		{".", "."}, // dot-only is preserved
 		{aws.String("example.com"), "example.com"},
 		{aws.String("example.com."), "example.com"},
 		{aws.String("www.example.com."), "www.example.com"},
@@ -88,3 +93,36 @@ func TestNormalizeDomainName(t *testing.T) {
 		}
 	}
 }
+
+func TestDenormalizeDomainName(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		input  any
+		output string
+	}{
+		{"example.com", "example.com"},             // as-is
+		{"example.com.", "example.com"},            // trailing dot is removed
+		{"www.example.com.", "www.example.com"},    // trailing dot is removed
+		{"\\052.example.com", "*.example.com"},     // octal "\052" ("*") is denormalized
+		{"\\100.example.com", "\\100.example.com"}, // octal "\100" ("@") is preserved
+		{"", ""},
+		{".", "."},
+		{aws.String("example.com"), "example.com"},
+		{aws.String("example.com."), "example.com"},
+		{aws.String("www.example.com."), "www.example.com"},
+		{aws.String(""), ""},
+		{aws.String("."), "."},
+		{(*string)(nil), ""},
+		{42, ""},
+		{nil, ""},
+	}
+
+	for _, tc := range cases {
+		output := denormalizeDomainName(tc.input)
+
+		if got, want := output, tc.output; got != want {
+			t.Errorf("denormalizeDomainName(%q) = %v, want %v", tc.input, got, want)
+		}
+	}
+}

internal/service/route53/record.go

@@ -25,6 +25,7 @@ import (
 	"github.com/hashicorp/terraform-provider-aws/internal/errs"
 	"github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag"
 	"github.com/hashicorp/terraform-provider-aws/internal/flex"
+	"github.com/hashicorp/terraform-provider-aws/internal/provider/sdkv2/importer"
 	"github.com/hashicorp/terraform-provider-aws/internal/retry"
 	"github.com/hashicorp/terraform-provider-aws/internal/sdkv2"
 	tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices"
@@ -37,13 +38,17 @@ import (
 	when `set_identifier` changes. Other changes to Identity-related attributes do not do this.
 */
 
+// For Resource Identity, the fully qualified `fqdn` is used for the `name` attribute to fully, uniquely identify the resource.
+// The `name` of the resource can be partial or fully-qualified so it does not do this.
+
 // @SDKResource("aws_route53_record", name="Record")
 // @IdentityAttribute("zone_id")
-// @IdentityAttribute("name")
+// @IdentityAttribute("name", resourceAttributeName="fqdn")
 // @IdentityAttribute("type")
 // @IdentityAttribute("set_identifier", optional="true")
 // @MutableIdentity
 // @ImportIDHandler("recordImportID")
+// @CustomImport
 // @Testing(existsType="github.com/aws/aws-sdk-go-v2/service/route53/types;awstypes;awstypes.ResourceRecordSet")
 // @Testing(subdomainTfVar="zoneName;recordName")
 // @Testing(generator=false)
@@ -56,6 +61,21 @@ func resourceRecord() *schema.Resource {
 		UpdateWithoutTimeout: resourceRecordUpdate,
 		DeleteWithoutTimeout: resourceRecordDelete,
 
+		Importer: &schema.ResourceImporter{
+			StateContext: func(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) {
+				if err := importer.Import(ctx, d, meta); err != nil {
+					return nil, err
+				}
+
+				if v, ok := d.GetOk("fqdn"); ok {
+					d.Set(names.AttrName, v)
+					d.SetId(createRecordImportID(d))
+				}
+
+				return []*schema.ResourceData{d}, nil
+			},
+		},
+
 		SchemaVersion: 2,
 		MigrateState:  recordMigrateState,
 
@@ -395,7 +415,7 @@ func resourceRecordRead(ctx context.Context, d *schema.ResourceData, meta any) d
 	var diags diag.Diagnostics
 	conn := meta.(*conns.AWSClient).Route53Client(ctx)
 
-	record, fqdn, err := findResourceRecordSetByFourPartKey(ctx, conn, cleanZoneID(d.Get("zone_id").(string)), d.Get(names.AttrName).(string), d.Get(names.AttrType).(string), d.Get("set_identifier").(string))
+	record, err := findResourceRecordSetByFourPartKey(ctx, conn, cleanZoneID(d.Get("zone_id").(string)), d.Get(names.AttrName).(string), d.Get(names.AttrType).(string), d.Get("set_identifier").(string))
 
 	if !d.IsNewResource() && retry.NotFound(err) {
 		log.Printf("[WARN] Route 53 Record (%s) not found, removing from state", d.Id())
@@ -407,6 +427,12 @@ func resourceRecordRead(ctx context.Context, d *schema.ResourceData, meta any) d
 		return sdkdiag.AppendErrorf(diags, "reading Route 53 Record (%s): %s", d.Id(), err)
 	}
 
+	return resourceRecordFlatten(d, record)
+}
+
+func resourceRecordFlatten(d *schema.ResourceData, record *awstypes.ResourceRecordSet) diag.Diagnostics {
+	var diags diag.Diagnostics
+
 	if alias := record.AliasTarget; alias != nil {
 		tfList := []any{map[string]any{
 			"evaluate_target_health": alias.EvaluateTargetHealth,
@@ -437,13 +463,10 @@ func resourceRecordRead(ctx context.Context, d *schema.ResourceData, meta any) d
 			return sdkdiag.AppendErrorf(diags, "setting failover_routing_policy: %s", err)
 		}
 	}
-	// findResourceRecordSetByFourPartKey returns the FQDN in API-normalized form.
-	// For backwards compatibility, restore any '*' as the leftmost label in the domain name.
-	// \052 is the octal representation of '*'.
-	if v := aws.ToString(fqdn); strings.HasPrefix(v, `\052.`) {
-		fqdn = aws.String(`*.` + strings.TrimPrefix(v, `\052.`))
-	}
+
+	fqdn := denormalizeDomainName(record.Name)
 	d.Set("fqdn", fqdn)
+
 	if geoLocation := record.GeoLocation; geoLocation != nil {
 		tfList := []any{map[string]any{
 			"continent":   aws.ToString(geoLocation.ContinentCode),
@@ -710,7 +733,7 @@ func resourceRecordDelete(ctx context.Context, d *schema.ResourceData, meta any)
 	} else {
 		name = d.Get(names.AttrName).(string)
 	}
-	rec, _, err := findResourceRecordSetByFourPartKey(ctx, conn, zoneID, name, d.Get(names.AttrType).(string), d.Get("set_identifier").(string))
+	rec, err := findResourceRecordSetByFourPartKey(ctx, conn, zoneID, name, d.Get(names.AttrType).(string), d.Get("set_identifier").(string))
 
 	if retry.NotFound(err) {
 		return diags
@@ -780,11 +803,11 @@ func recordParseResourceID(id string) [4]string {
 	return [4]string{recZone, recName, recType, recSet}
 }
 
-func findResourceRecordSetByFourPartKey(ctx context.Context, conn *route53.Client, zoneID, recordName, recordType, recordSetID string) (*awstypes.ResourceRecordSet, *string, error) {
+func findResourceRecordSetByFourPartKey(ctx context.Context, conn *route53.Client, zoneID, recordName, recordType, recordSetID string) (*awstypes.ResourceRecordSet, error) {
 	zone, err := findHostedZoneByID(ctx, conn, zoneID)
 
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	name := expandRecordName(recordName, aws.ToString(zone.HostedZone.Name))
@@ -815,10 +838,10 @@ func findResourceRecordSetByFourPartKey(ctx context.Context, conn *route53.Clien
 	})
 
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
-	return output, &name, nil
+	return output, nil
 }
 
 func findResourceRecordSet(ctx context.Context, conn *route53.Client, input *route53.ListResourceRecordSetsInput, morePages tfslices.Predicate[*route53.ListResourceRecordSetsOutput], filter tfslices.Predicate[*awstypes.ResourceRecordSet]) (*awstypes.ResourceRecordSet, error) {

internal/service/route53/record_list.go

@@ -61,6 +61,8 @@ func (l *listResourceRecord) List(ctx context.Context, request list.ListRequest,
 		}
 	}
 
+	ctx = tflog.SetField(ctx, logging.ResourceAttributeKey("zone_id"), query.ZoneID.ValueString())
+
 	tflog.Info(ctx, "Listing Route 53 Records")
 	stream.Results = func(yield func(list.ListResult) bool) {
 		input := &route53.ListResourceRecordSetsInput{
@@ -73,43 +75,45 @@ func (l *listResourceRecord) List(ctx context.Context, request list.ListRequest,
 				return
 			}
 
-			// Create a unique ID for this record
-			recordID := createRecordIDFromResourceRecordSet(query.ZoneID.ValueString(), item)
-			ctx := tflog.SetField(ctx, logging.ResourceAttributeKey(names.AttrID), recordID)
+			name := denormalizeDomainName(item.Name)
+			typ := string(item.Type)
+
+			ctx := tflog.SetField(ctx, logging.ResourceAttributeKey(names.AttrName), name)
+			ctx = tflog.SetField(ctx, logging.ResourceAttributeKey(names.AttrType), typ)
+			if item.SetIdentifier != nil {
+				ctx = tflog.SetField(ctx, logging.ResourceAttributeKey("set_identifier"), aws.ToString(item.SetIdentifier))
+			}
 
 			result := request.NewListResult(ctx)
 			rd := l.ResourceData()
-			rd.SetId(recordID)
+			rd.SetId(createRecordIDFromResourceRecordSet(query.ZoneID.ValueString(), item))
 
 			// Set identity attributes
 			rd.Set("zone_id", query.ZoneID.ValueString())
-			rd.Set(names.AttrName, aws.ToString(item.Name))
-			rd.Set(names.AttrType, string(item.Type))
+			// The Resource Identity `name` attribute is populated from the resource `fqdn` attribute
+			rd.Set("fqdn", name)
+			rd.Set(names.AttrType, typ)
 			if item.SetIdentifier != nil {
 				rd.Set("set_identifier", aws.ToString(item.SetIdentifier))
 			}
 
-			tflog.Info(ctx, "Reading Route 53 Record")
-			diags := resourceRecordRead(ctx, rd, l.Meta())
-			if diags.HasError() {
-				tflog.Error(ctx, "Reading Route 53 Record", map[string]any{
-					names.AttrID: recordID,
-					"diags":      sdkdiag.DiagnosticsString(diags),
-				})
-				continue
-			}
-			if rd.Id() == "" {
-				// Resource is logically deleted
-				continue
+			if request.IncludeResource {
+				diags := resourceRecordFlatten(rd, &item)
+				if diags.HasError() {
+					tflog.Error(ctx, "Reading Route 53 Record", map[string]any{
+						"error": sdkdiag.DiagnosticsString(diags),
+					})
+					continue
+				}
+				rd.Set(names.AttrName, name)
 			}
 
-			result.DisplayName = aws.ToString(item.Name)
+			result.DisplayName = fmt.Sprintf("%s %s", name, typ)
 
 			l.SetResult(ctx, l.Meta(), request.IncludeResource, rd, &result)
 			if result.Diagnostics.HasError() {
 				tflog.Error(ctx, "Setting Route 53 Record result", map[string]any{
-					names.AttrID: recordID,
-					"diags":      result.Diagnostics,
+					"diags": result.Diagnostics,
 				})
 				continue
 			}