tailscale: do not show diffs when field names change (#274)

tailscale_acl resource currently expects users to provide intended policy
content as a serialized JSON string. This JSON payload is not actually used
directly in the API call to Tailscale - instead, it's deserialized, validated
and serialized again by the `tailsacle-client-go` API library.

Since JSON deserialization in Go matches field names in a case insentive way,
as a result of this re-serialization, spelling of policy field names will be
changed from whatever user specified, to the one configured in the JSON schema
of the API client. For example, `ACLs` might change to `acls`.

Tailscale API will accept field names in a case-insensitive way as well, and
will store the payload verbatim. However during a next change, Terraform will
fetch policy content from Tailscale API, and will show a diff, indicating that
field names got changed. To avoid such spurious diffs, users currently need to
use field names exactly as they are specified in the API client schema, just to
satisfy Terraform.

To handle this better, if we have ACL content in Terraform state, we check
whether it's equivalent to the ACL fetched via API. If it is, we do not replace
local state with the policy fetched from the API. As a result, local changed
are always diffed against their previous version specified by the user (rather
than against re-serialized payload returned from the API), and no spurious
diffs are shown.

Fixes #226

tailscale/resource_acl.go

@@ -34,7 +34,6 @@ func resourceACL() *schema.Resource {
 				Type:             schema.TypeString,
 				Required:         true,
 				ValidateDiagFunc: validateACL,
-				DiffSuppressFunc: suppressACLDiff,
 				Description:      "The JSON-based policy that defines which devices and users are allowed to connect in your network",
 			},
 		},
@@ -48,10 +47,8 @@ func validateACL(i interface{}, p cty.Path) diag.Diagnostics {
 	return nil
 }
 
-func suppressACLDiff(_, old, new string, _ *schema.ResourceData) bool {
-	oldACL, oldErr := unmarshalACL(old)
-	newACL, newErr := unmarshalACL(new)
-	return oldErr == nil && newErr == nil && cmp.Equal(oldACL, newACL)
+func sameACL(a, b tailscale.ACL) bool {
+	return cmp.Equal(a, b)
 }
 
 func resourceACLRead(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
@@ -61,21 +58,29 @@ func resourceACLRead(ctx context.Context, d *schema.ResourceData, m interface{})
 		return diagnosticsError(err, "Failed to fetch ACL")
 	}
 
+	// If we have ACL content in Terraform state that is equivalent to the ACL
+	// fetched via API, keep the local version. This is to have further changes
+	// diffed against previous version specified by the user, avoiding spurious
+	// diffs caused by potentially different spelling of ACL field names.
+	current := d.Get("acl").(string)
+	if current != "" {
+		cur, err := unmarshalACL(current)
+		if err != nil {
+			return diagnosticsError(err, "Failed to unmarshal current ACL")
+		}
+		if sameACL(cur, *acl) {
+			return nil
+		}
+	}
+
 	aclStr, err := json.MarshalIndent(acl, "", "  ")
 	if err != nil {
 		return diagnosticsError(err, "Failed to marshal ACL for")
 	}
 
-	values := map[string]interface{}{
-		"acl": string(aclStr),
+	if err := d.Set("acl", string(aclStr)); err != nil {
+		return diag.FromErr(err)
 	}
-
-	for k, v := range values {
-		if err = d.Set(k, v); err != nil {
-			return diag.FromErr(err)
-		}
-	}
-
 	return nil
 }
 

tailscale/resource_acl_test.go

@@ -1,6 +1,8 @@
 package tailscale_test
 
 import (
+	"encoding/json"
+	"fmt"
 	"net/http"
 	"testing"
 
@@ -79,3 +81,50 @@ func TestProvider_TailscaleACL(t *testing.T) {
 		},
 	})
 }
+
+// TestProvider_TailscaleACLDiffs checks that ACL keys specified with
+// different casing than the one used by the API client do not result
+// in spurious diffs in Terraform plan.
+func TestProvider_TailscaleACLDiffs(t *testing.T) {
+	// policyObject returns a map that, when serialized to JSON,
+	// is a valid Tailscale policy with only the "Hosts" field set.
+	policyObject := func(hostsKey string) map[string]map[string]string {
+		return map[string]map[string]string{
+			hostsKey: {"example": "100.101.102.103"},
+		}
+	}
+	policyHCL := func(hostsKey string) string {
+		j, err := json.MarshalIndent(policyObject(hostsKey), "", " ")
+		if err != nil {
+			t.Fatal(err)
+		}
+		return fmt.Sprintf(
+			`resource "tailscale_acl" "test_acl" {
+				acl = <<EOF
+					%s
+				EOF
+			}`, j)
+	}
+
+	resource.Test(t, resource.TestCase{
+		IsUnitTest:        true,
+		ProviderFactories: testProviderFactories(t),
+		PreCheck: func() {
+			testServer.ResponseCode = http.StatusOK
+		},
+		Steps: []resource.TestStep{
+			testResourceCreated("tailscale_acl.test_acl", policyHCL("hosts")),
+
+			// Now we check that, whatever spelling of "hosts" we use, the
+			// Terraform plan will be empty.
+			{ResourceName: "tailscale_acl.test_acl", Config: policyHCL("hosts"),
+				PreConfig: func() {
+					testServer.ResponseBody = policyObject("HOSTS")
+				},
+			},
+			{ResourceName: "tailscale_acl.test_acl", Config: policyHCL("Hosts")},
+			{ResourceName: "tailscale_acl.test_acl", Config: policyHCL("HoStS")},
+			{ResourceName: "tailscale_acl.test_acl", Config: policyHCL("HOSTS")},
+		},
+	})
+}