tailscale/resource_acl: relax policy validation during plan steps

Some resources like SCIM groups can be created and added as a reference to the policy file in the same run. The eager validation causes otherwise valid runs to fail at the plan step because the references do not exist yet.

We are relaxing the plan validation logic to avoid such false positives. The full validation results are still available in debug logs in case this is useful. Note that the validation is done in full during an apply before the policy file is changed so this does not permit a tailnet policy to be in an invalid state.

Fixes #546

Signed-off-by: mcoulombe <[email protected]>

Author: mcoulombe

tailscale/resource_acl.go

@@ -6,9 +6,11 @@ package tailscale
 import (
 	"context"
 	"fmt"
+	"regexp"
 	"strings"
 
 	"github.com/hashicorp/go-cty/cty"
+	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 
@@ -25,6 +27,8 @@ If tests are defined in the ACL (the top-level "tests" section), ACL validation
 // TODO: use an exported variable when https://github.com/hashicorp/terraform-plugin-sdk/issues/803 has been addressed.
 const UnknownVariableValue = "74D93920-ED26-11E3-AC10-0800200C9A66"
 
+var syntaxErrorMessage = regexp.MustCompile(`^ACL validation failed: line \d+, column \d+:`)
+
 func resourceACL() *schema.Resource {
 	return &schema.Resource{
 		Description:   resourceACLDescription,
@@ -42,7 +46,17 @@ func resourceACL() *schema.Resource {
 			if rd.Get("acl").(string) == "" {
 				return nil
 			}
-			return client.PolicyFile().Validate(ctx, rd.Get("acl").(string))
+			err := client.PolicyFile().Validate(ctx, rd.Get("acl").(string))
+			if err != nil && syntaxErrorMessage.MatchString(err.Error()) {
+				return err
+			} else if err != nil {
+				// non-syntax errors are logged but do not fail the plan operation
+				tflog.Debug(ctx, "ACL validation unsuccessful due to advisory error", map[string]interface{}{"error": err})
+				return nil
+			}
+
+			tflog.Debug(ctx, "ACL validation successful")
+			return nil
 		},
 		Schema: map[string]*schema.Schema{
 			"acl": {

tailscale/resource_acl_test.go

@@ -8,6 +8,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"regexp"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -292,6 +293,63 @@ func TestAccACL_resetOnDestroy(t *testing.T) {
 	})
 }
 
+func TestAccACLValidation(t *testing.T) {
+	const resourceName = "tailscale_acl.test_acl_validation"
+
+	const testACLInvalidSyntax = `
+		resource "tailscale_acl" "test_acl" {
+			acl = <<EOF
+			{
+                "grants": [
+					{
+						"src": ["group:scim-that-does-not-exist-yet"], 
+						"dst": ["*"], 
+						"ip": ["*"]
+					},
+    			// ] <- Commented out to create invalid syntax, invalid JSON should fail the plan
+			}
+			EOF
+		}`
+	const testACLUndefinedReferences = `
+		resource "tailscale_acl" "test_acl" {
+			acl = <<EOF
+			{
+                "grants": [
+					{
+						"src": ["group:scim-that-does-not-exist-yet"], // <- Undefined reference do not fail plan because they could be created in the same run
+						"dst": ["*"], 
+						"ip": ["*"]
+					},
+    			]
+			}
+			EOF
+		}`
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:          func() { testAccPreCheck(t) },
+		ProviderFactories: testAccProviderFactories(t),
+		Steps: []resource.TestStep{
+			{
+				ResourceName: resourceName,
+				Config:       testACLInvalidSyntax,
+				PlanOnly:     true,
+				ExpectError:  regexp.MustCompile("Error: ACL is not a valid HuJSON string"),
+			},
+			{
+				ResourceName:       resourceName,
+				Config:             testACLUndefinedReferences,
+				PlanOnly:           true,
+				ExpectNonEmptyPlan: true,
+			},
+			{
+				ResourceName: resourceName,
+				Config:       testACLUndefinedReferences,
+				ExpectError:  regexp.MustCompile("Error: Failed to set ACL"), // Apply should still fail if the pre-requisite resource is not created before the ACLs are applied
+			},
+		},
+	})
+}
+
 func checkProperties(expected *tailscale.ACL) func(client *tailscale.Client, rs *terraform.ResourceState) error {
 	return func(client *tailscale.Client, rs *terraform.ResourceState) error {
 		actual, err := client.PolicyFile().Get(context.Background())