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

docs/resources/acl.md

@@ -11,19 +11,25 @@ description: |-
 
 The acl resource allows you to configure a Tailscale ACL. See https://tailscale.com/kb/1018/acls for more information. Note that this resource will completely overwrite existing ACL contents for a given tailnet.
 
-If tests are defined in the ACL (the top-level "tests" section), ACL validation will occur before creation and update operations are applied.
+Plan steps only fail if the policy syntax is invalid. 
+
+You can run the plan operation in [debug mode](https://developer.hashicorp.com/terraform/internals/debugging) or use the [Tailscale validation endpoint](https://tailscale.com/api#tag/policyfile/post/tailnet/{tailnet}/acl/validate) 
+to see more detailed pre-flight validation messages from the Tailscale API. For example if you have defined tests in the ACL (the top-level "tests" section), 
+ACL validation during a plan will output whether the tests passed or not.
+
+Look for `ACL validation unsuccessful` or `ACL validation successful` in the debug logs to see more in-depth validation results. 
 
 ## Example Usage
 
 ```terraform
 resource "tailscale_acl" "as_json" {
   acl = jsonencode({
-    acls : [
+    grants : [
       {
         // Allow all users access to all ports.
-        action = "accept",
-        users  = ["*"],
-        ports  = ["*:*"],
+        src = ["*"],
+        dst = ["*"],
+        ip  = ["*"],
       },
     ],
   })
@@ -33,12 +39,12 @@ resource "tailscale_acl" "as_hujson" {
   acl = <<EOF
   {
     // Comments in HuJSON policy are preserved when the policy is applied.
-    "acls": [
+    "grants": [
       {
         // Allow all users access to all ports.
-        action = "accept",
-        users  = ["*"],
-        ports  = ["*:*"],
+        "src": ["*"],
+        "dst": ["*"],
+        "ip":  ["*"],
       },
     ],
   }

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())