Trim trailing dot (root) from name servers in the `dnsimple_domain_de…

…legation` resource (#203)

It was brought up some time ago that having fully qualified name server entries (i.e. with a . at the end) would result in perma diff since our API normalizes the entries and returns the normalized entries creating the difference. Since this is a known behaviour it made sense for the provider to acknowledge it and handle this case.

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## main
 
+ENHANCEMENTS:
+
+- **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)
+
 NOTES:
 
 - This release is no longer compatible with Terraform versions < 1.3. This is due to the new protocol changes in the underlying terraform framework. If you are using Terraform 1.3 or later, you should be unaffected by this change.

internal/framework/modifiers/setplanmodifier.go

@@ -0,0 +1,45 @@
+package modifiers
+
+import (
+	"context"
+	"strings"
+
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
+	"github.com/hashicorp/terraform-plugin-framework/types"
+)
+
+type setTrimSuffix struct {
+}
+
+func SetTrimSuffixValue() planmodifier.Set {
+	return setTrimSuffix{}
+}
+
+func (m setTrimSuffix) Description(context.Context) string {
+	return "Trim suffix from configuration value when comparing with state"
+}
+
+func (m setTrimSuffix) MarkdownDescription(ctx context.Context) string {
+	return m.Description(ctx)
+}
+
+func (m setTrimSuffix) PlanModifySet(ctx context.Context, req planmodifier.SetRequest, resp *planmodifier.SetResponse) {
+	if req.ConfigValue.IsNull() || req.PlanValue.IsUnknown() || req.PlanValue.IsNull() {
+		return
+	}
+
+	var planValue []string
+	resp.Diagnostics.Append(req.PlanValue.ElementsAs(ctx, &planValue, false)...)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	for i, element := range planValue {
+		planValue[i] = strings.TrimSuffix(element, ".")
+	}
+
+	serializedPlanValue, diags := types.SetValueFrom(ctx, types.StringType, planValue)
+	resp.Diagnostics.Append(diags...)
+
+	resp.PlanValue = serializedPlanValue
+}

internal/framework/modifiers/setplanmodifier_test.go

@@ -0,0 +1,84 @@
+package modifiers_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/hashicorp/terraform-plugin-framework/path"
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
+	"github.com/hashicorp/terraform-plugin-framework/types"
+	"github.com/stretchr/testify/assert"
+	"github.com/terraform-providers/terraform-provider-dnsimple/internal/framework/modifiers"
+)
+
+func TestSetTrimSuffixValue(t *testing.T) {
+	t.Parallel()
+
+	serializeList := func(elements []string) types.Set {
+		listValue, _ := types.SetValueFrom(context.Background(), types.StringType, elements)
+		return listValue
+	}
+
+	type testCase struct {
+		plannedValue  types.Set
+		stateValue    types.Set
+		configValue   types.Set
+		expectedValue types.Set
+		expectError   bool
+	}
+	tests := map[string]testCase{
+		"trim `.` in string value ": {
+			plannedValue:  serializeList([]string{"beta.alpha.", "gamma.omega"}),
+			stateValue:    types.SetNull(types.StringType),
+			configValue:   serializeList([]string{"beta.alpha.", "gamma.omega"}),
+			expectedValue: serializeList([]string{"beta.alpha", "gamma.omega"}),
+		},
+		"when state is Null": {
+			plannedValue:  serializeList([]string{"beta.alpha.", "gamma.omega."}),
+			stateValue:    types.SetNull(types.StringType),
+			configValue:   serializeList([]string{"beta.alpha.", "gamma.omega."}),
+			expectedValue: serializeList([]string{"beta.alpha", "gamma.omega"}),
+		},
+		"when plan is Null": {
+			plannedValue:  types.SetNull(types.StringType),
+			stateValue:    types.SetNull(types.StringType),
+			configValue:   serializeList([]string{"beta.alpha.", "gamma.omega."}),
+			expectedValue: types.SetNull(types.StringType),
+		},
+		"when plan is Unknown": {
+			plannedValue:  types.SetUnknown(types.StringType),
+			stateValue:    types.SetNull(types.StringType),
+			configValue:   serializeList([]string{"beta.alpha.", "gamma.omega."}),
+			expectedValue: types.SetUnknown(types.StringType),
+		},
+	}
+
+	for name, test := range tests {
+		name, test := name, test
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			ctx := context.Background()
+			request := planmodifier.SetRequest{
+				Path:        path.Root("test"),
+				PlanValue:   test.plannedValue,
+				StateValue:  test.stateValue,
+				ConfigValue: test.configValue,
+			}
+			response := planmodifier.SetResponse{
+				PlanValue: request.PlanValue,
+			}
+			modifiers.SetTrimSuffixValue().PlanModifySet(ctx, request, &response)
+
+			if !response.Diagnostics.HasError() && test.expectError {
+				t.Fatal("expected error, got no error")
+			}
+
+			if response.Diagnostics.HasError() && !test.expectError {
+				t.Fatalf("got unexpected error: %s", response.Diagnostics)
+			}
+
+			assert.Equal(t, test.expectedValue, response.PlanValue)
+		})
+	}
+}

internal/framework/resources/domain_delegation_resource.go

@@ -15,6 +15,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/types"
 	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/terraform-providers/terraform-provider-dnsimple/internal/framework/common"
+	"github.com/terraform-providers/terraform-provider-dnsimple/internal/framework/modifiers"
 	"github.com/terraform-providers/terraform-provider-dnsimple/internal/framework/utils"
 )
 
@@ -60,6 +61,9 @@ func (r *DomainDelegationResource) Schema(_ context.Context, _ resource.SchemaRe
 			"name_servers": schema.SetAttribute{
 				Required:    true,
 				ElementType: types.StringType,
+				PlanModifiers: []planmodifier.Set{
+					modifiers.SetTrimSuffixValue(),
+				},
 			},
 		},
 	}

internal/framework/resources/domain_delegation_resource_test.go

@@ -43,6 +43,18 @@ func TestAccDomainDelegationResource(t *testing.T) {
 					resource.TestCheckTypeSetElemAttr(resourceName, "name_servers.*", "ns-1556.awsdns-02.co.uk"),
 				),
 			},
+			{
+				Config:             testAccDomainDelegationResourceConfigWithSuffix(domainId),
+				ExpectNonEmptyPlan: false,
+				PlanOnly:           true,
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttrSet(resourceName, "id"),
+					resource.TestCheckResourceAttr(resourceName, "domain", domainId),
+					resource.TestCheckResourceAttr(resourceName, "name_servers.#", "2"),
+					resource.TestCheckTypeSetElemAttr(resourceName, "name_servers.*", "ns-998.awsdns-60.net"),
+					resource.TestCheckTypeSetElemAttr(resourceName, "name_servers.*", "ns-1556.awsdns-02.co.uk"),
+				),
+			},
 			{
 				ResourceName:      resourceName,
 				ImportStateIdFunc: testAccDomainDelegationImportStateIDFunc(resourceName),
@@ -89,3 +101,11 @@ resource "dnsimple_domain_delegation" "test" {
 	name_servers = ["ns-1556.awsdns-02.co.uk", "ns-998.awsdns-60.net"]
 }`, domainId)
 }
+
+func testAccDomainDelegationResourceConfigWithSuffix(domainId string) string {
+	return fmt.Sprintf(`
+resource "dnsimple_domain_delegation" "test" {
+	domain = %[1]q
+	name_servers = ["ns-1556.awsdns-02.co.uk.", "ns-998.awsdns-60.net"]
+}`, domainId)
+}