Handle normalized name value for zone_record resource (#207)

This PR ensure that we correctly handle the cases where the `dnsimple_zone_record` `name` attribute has been normalized by the API.

- Cache search logic updated to use normalized value to avoid false negatives.
- Fixed a bug where the `qualified_name` attribute was set to the record `name` in cases where the name was **empty** (root). It is now set to the zone name.

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ENHANCEMENTS:
 
+- **Update Resource:** `dnsimple_zone_record` has been updated to handle cases where the `name` attribute is normalized by the API, resulting in bad state as config differs from state.
 - **Update Resource:** `dnsimple_domain_delegation` now has the trailing dot removed from the `name_servers` attribute entries. This is to align with the API and avoid perma diffs. (dnsimple/terraform-provider-dnsimple#203)
 
 BUG FIXES:

go.mod

@@ -3,8 +3,8 @@ module github.com/terraform-providers/terraform-provider-dnsimple
 require (
 	github.com/dnsimple/dnsimple-go v1.7.0
 	github.com/hashicorp/terraform-plugin-docs v0.18.0
-	github.com/hashicorp/terraform-plugin-framework-timeouts v0.4.1
 	github.com/hashicorp/terraform-plugin-framework v1.6.1
+	github.com/hashicorp/terraform-plugin-framework-timeouts v0.4.1
 	github.com/hashicorp/terraform-plugin-go v0.22.1
 	github.com/hashicorp/terraform-plugin-log v0.9.0
 	github.com/hashicorp/terraform-plugin-mux v0.15.0

internal/framework/resources/registered_domain/resource_test.go

@@ -147,6 +147,7 @@ func TestAccRegisteredDomainResource_RegistrantChange_WithExtendedAttrs(t *testi
 				// We expect the plan to be non-empty because we are creating a registrant change that will not be completed
 				// and we will attempt to converge it by setting the state to completed
 				ExpectNonEmptyPlan: true,
+				PlanOnly:           true,
 			},
 			// Delete testing automatically occurs in TestCase
 		},

internal/framework/resources/registered_domain/update.go

@@ -131,11 +131,11 @@ func (r *RegisteredDomainResource) Update(ctx context.Context, req resource.Upda
 				// user needs to run terraform again to try and converge the registrant change
 
 				// Update the data with the current registrant change
-				registrantChangeResponse, err = r.config.Client.Registrar.GetRegistrantChange(ctx, r.config.AccountID, int(registrantChange.Id.ValueInt64()))
-				if err != nil {
+				registrantChangeResponse, registrantChangeError := r.config.Client.Registrar.GetRegistrantChange(ctx, r.config.AccountID, int(registrantChange.Id.ValueInt64()))
+				if registrantChangeError != nil {
 					resp.Diagnostics.AddError(
 						fmt.Sprintf("failed to read DNSimple Registrant Change Id: %d", registrantChange.Id.ValueInt64()),
-						err.Error(),
+						registrantChangeError.Error(),
 					)
 					return
 				}
@@ -153,7 +153,7 @@ func (r *RegisteredDomainResource) Update(ctx context.Context, req resource.Upda
 				// Exit with warning to prevent the state from being tainted
 				resp.Diagnostics.AddWarning(
 					"failed to converge on registrant change",
-					err.Error(),
+					"the registrant change is not ready, please run terraform apply again to try and converge the registrant change",
 				)
 				return
 			}

internal/framework/resources/zone_record_resource.go

@@ -42,6 +42,7 @@ type ZoneRecordResourceModel struct {
 	ZoneName        types.String `tfsdk:"zone_name"`
 	ZoneId          types.String `tfsdk:"zone_id"`
 	Name            types.String `tfsdk:"name"`
+	NameNormalized  types.String `tfsdk:"name_normalized"`
 	QualifiedName   types.String `tfsdk:"qualified_name"`
 	Type            types.String `tfsdk:"type"`
 	Regions         types.List   `tfsdk:"regions"`
@@ -74,6 +75,9 @@ func (r *ZoneRecordResource) Schema(_ context.Context, _ resource.SchemaRequest,
 			"name": schema.StringAttribute{
 				Required: true,
 			},
+			"name_normalized": schema.StringAttribute{
+				Computed: true,
+			},
 			"qualified_name": schema.StringAttribute{
 				Computed: true,
 			},
@@ -212,7 +216,14 @@ func (r *ZoneRecordResource) Read(ctx context.Context, req resource.ReadRequest,
 			}
 		}
 
-		cacheRecord, ok := r.config.ZoneRecordCache.Find(data.ZoneName.ValueString(), data.Name.ValueString(), data.Type.ValueString(), data.ValueNormalized.ValueString())
+		var lookupName string
+		if data.NameNormalized.IsNull() || data.NameNormalized.IsUnknown() {
+			lookupName = data.Name.ValueString()
+		} else {
+			lookupName = data.NameNormalized.ValueString()
+		}
+
+		cacheRecord, ok := r.config.ZoneRecordCache.Find(data.ZoneName.ValueString(), lookupName, data.Type.ValueString(), data.ValueNormalized.ValueString())
 		if !ok {
 			resp.Diagnostics.AddError(
 				"record not found",
@@ -361,14 +372,19 @@ func (r *ZoneRecordResource) ImportState(ctx context.Context, req resource.Impor
 func (r *ZoneRecordResource) updateModelFromAPIResponse(record *dnsimple.ZoneRecord, data *ZoneRecordResourceModel) {
 	data.Id = types.Int64Value(record.ID)
 	data.ZoneId = types.StringValue(record.ZoneID)
-	data.Name = types.StringValue(record.Name)
+	data.NameNormalized = types.StringValue(record.Name)
 	data.Type = types.StringValue(record.Type)
 	data.ValueNormalized = types.StringValue(record.Content)
 	data.TTL = types.Int64Value(int64(record.TTL))
 	data.Priority = types.Int64Value(int64(record.Priority))
 
+	if data.Name.IsNull() || data.Name.IsUnknown() {
+		// This can happen during a resource import, where the name is not in the state
+		data.Name = types.StringValue(record.Name)
+	}
+
 	if record.Name == "" {
-		data.QualifiedName = data.Name
+		data.QualifiedName = data.ZoneName
 	} else {
 		data.QualifiedName = types.StringValue(fmt.Sprintf("%s.%s", record.Name, data.ZoneName.ValueString()))
 	}

internal/framework/resources/zone_record_resource_test.go

@@ -141,6 +141,31 @@ func TestAccZoneRecordResourceWithNormalizedTXT(t *testing.T) {
 	})
 }
 
+func TestAccZoneRecordResourceWithNormalizedName(t *testing.T) {
+	domainName := os.Getenv("DNSIMPLE_DOMAIN")
+	resourceName := "dnsimple_zone_record.test"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 func() { test_utils.TestAccPreCheck(t) },
+		ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
+		CheckDestroy:             testAccCheckZoneRecordResourceDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccZoneRecordResourceNormalizedNameConfig(domainName),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "zone_name", domainName),
+					resource.TestCheckResourceAttr(resourceName, "name", "@"),
+					resource.TestCheckResourceAttr(resourceName, "name_normalized", ""),
+					resource.TestCheckResourceAttr(resourceName, "qualified_name", domainName),
+					resource.TestCheckResourceAttr(resourceName, "ttl", "3600"),
+					resource.TestCheckResourceAttrSet(resourceName, "id"),
+				),
+			},
+			// Delete testing automatically occurs in TestCase
+		},
+	})
+}
+
 func TestAccZoneRecordResourceWithPrefetch(t *testing.T) {
 	domainName := os.Getenv("DNSIMPLE_DOMAIN")
 	resourceName := "dnsimple_zone_record.test"
@@ -329,6 +354,17 @@ resource "dnsimple_zone_record" "test" {
 }`, domainName, value)
 }
 
+func testAccZoneRecordResourceNormalizedNameConfig(domainName string) string {
+	return fmt.Sprintf(`
+resource "dnsimple_zone_record" "test" {
+	zone_name = %[1]q
+
+	name = "@"
+	value = "terraform value"
+	type = "TXT"
+}`, domainName)
+}
+
 func testAccZoneRecordResourcePriorityConfig(domainName string) string {
 	return fmt.Sprintf(`
 resource "dnsimple_zone_record" "test" {

tools/sweep/main.go

@@ -37,7 +37,9 @@ func main() {
 	for _, record := range records.Data {
 		if !record.SystemRecord {
 			_, err := dnsimpleClient.Zones.DeleteRecord(context.Background(), account, domainName, record.ID)
-			if err != nil {
+			if err != nil && strings.Contains(err.Error(), "404") {
+				// 404 is expected if the record was already deleted
+			} else if err != nil {
 				panic(err)
 			}
 		}