From 93c190af2d0f9e6b74c911748ff7f364e34cf688 Mon Sep 17 00:00:00 2001 From: Ivan Bakalov Date: Tue, 19 Mar 2024 05:04:19 +0200 Subject: [PATCH] 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 | 1 + go.mod | 2 +- .../registered_domain/resource_test.go | 1 + .../resources/registered_domain/update.go | 8 ++--- .../resources/zone_record_resource.go | 22 ++++++++++-- .../resources/zone_record_resource_test.go | 36 +++++++++++++++++++ tools/sweep/main.go | 4 ++- 7 files changed, 65 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e5461554..9d1a5f30 100644 --- a/CHANGELOG.md +++ b/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: diff --git a/go.mod b/go.mod index 5f21960f..0956d7d0 100644 --- a/go.mod +++ b/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 diff --git a/internal/framework/resources/registered_domain/resource_test.go b/internal/framework/resources/registered_domain/resource_test.go index f0097c6a..e5ef702d 100644 --- a/internal/framework/resources/registered_domain/resource_test.go +++ b/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 }, diff --git a/internal/framework/resources/registered_domain/update.go b/internal/framework/resources/registered_domain/update.go index 14ca696d..ff80b3cb 100644 --- a/internal/framework/resources/registered_domain/update.go +++ b/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 } diff --git a/internal/framework/resources/zone_record_resource.go b/internal/framework/resources/zone_record_resource.go index c010e3df..4ba9f595 100644 --- a/internal/framework/resources/zone_record_resource.go +++ b/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())) } diff --git a/internal/framework/resources/zone_record_resource_test.go b/internal/framework/resources/zone_record_resource_test.go index 21c839a8..a957ca27 100644 --- a/internal/framework/resources/zone_record_resource_test.go +++ b/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" { diff --git a/tools/sweep/main.go b/tools/sweep/main.go index b6458fce..18a5266c 100644 --- a/tools/sweep/main.go +++ b/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) } }