tailscale_tailnet_key: only recreate reusable keys by default (#310)

This change partially reverts the behaviour introduced in #287 that currently results in single-use keys being recreated, triggering unnecessary updates to downstream Terraform resources. By default, the provider will now only recreate reusable keys, ignoring invalid single-use keys. This can also be changed now using a new `recreate_if_invalid` attribute. Fixes #306 Signed-off-by: Anton Tolchanov <[email protected]>
tailscale · Dec 12, 2023 · f65978c · f65978c
1 parent 14dd762
commit f65978c
Show file tree

Hide file tree

Showing 3 changed files with 179 additions and 46 deletions.
diff --git a/docs/resources/tailnet_key.md b/docs/resources/tailnet_key.md
@@ -31,6 +31,7 @@ resource "tailscale_tailnet_key" "sample_key" {
 - `ephemeral` (Boolean) Indicates if the key is ephemeral. Defaults to `false`.
 - `expiry` (Number) The expiry of the key in seconds. Defaults to `7776000` (90 days).
 - `preauthorized` (Boolean) Determines whether or not the machines authenticated by the key will be authorized for the tailnet by default. Defaults to `false`.
+- `recreate_if_invalid` (String) Determines whether the key should be created again if it becomes invalid. By default, reusable keys will be recreated, but single-use keys will not. Possible values: 'always', 'never'.
 - `reusable` (Boolean) Indicates if the key is reusable or single-use. Defaults to `false`.
 - `tags` (Set of String) List of tags to apply to the machines authenticated by the key.
 
@@ -39,4 +40,5 @@ resource "tailscale_tailnet_key" "sample_key" {
 - `created_at` (String) The creation timestamp of the key in RFC3339 format
 - `expires_at` (String) The expiry timestamp of the key in RFC3339 format
 - `id` (String) The ID of this resource.
+- `invalid` (Boolean) Indicates whether the key is invalid (e.g. expired, revoked or has been deleted).
 - `key` (String, Sensitive) The authentication key
diff --git a/tailscale/resource_tailnet_key.go b/tailscale/resource_tailnet_key.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"time"
 
+	"github.com/hashicorp/go-cty/cty"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 
@@ -16,6 +17,8 @@ func resourceTailnetKey() *schema.Resource {
 		ReadContext:   resourceTailnetKeyRead,
 		CreateContext: resourceTailnetKeyCreate,
 		DeleteContext: resourceTailnetKeyDelete,
+		UpdateContext: schema.NoopContext,
+		CustomizeDiff: resourceTailnetKeyDiff,
 		Schema: map[string]*schema.Schema{
 			"reusable": {
 				Type:        schema.TypeBool,
@@ -72,6 +75,24 @@ func resourceTailnetKey() *schema.Resource {
 				Description: "A description of the key consisting of alphanumeric characters. Defaults to `\"\"`.",
 				ForceNew:    true,
 			},
+			"invalid": {
+				Type:        schema.TypeBool,
+				Description: "Indicates whether the key is invalid (e.g. expired, revoked or has been deleted).",
+				Computed:    true,
+			},
+			"recreate_if_invalid": {
+				Type:        schema.TypeString,
+				Optional:    true,
+				Description: "Determines whether the key should be created again if it becomes invalid. By default, reusable keys will be recreated, but single-use keys will not. Possible values: 'always', 'never'.",
+				ValidateDiagFunc: func(i interface{}, p cty.Path) diag.Diagnostics {
+					switch i.(string) {
+					case "", "always", "never":
+						return nil
+					default:
+						return diagnosticsError(nil, "unexpected value of recreate_if_invalid: %s", i)
+					}
+				},
+			},
 		},
 	}
 }
@@ -122,6 +143,10 @@ func resourceTailnetKeyCreate(ctx context.Context, d *schema.ResourceData, m int
 		return diagnosticsError(err, "Failed to set expires_at")
 	}
 
+	if err = d.Set("invalid", key.Invalid); err != nil {
+		return diagnosticsError(err, "Failed to set 'invalid'")
+	}
+
 	return nil
 }
 
@@ -140,21 +165,61 @@ func resourceTailnetKeyDelete(ctx context.Context, d *schema.ResourceData, m int
 	}
 }
 
+// shouldRecreateIfInvalid determines if a resource should be recreated when
+// it's invalid, based on the values of `reusable` and `recreate_if_invalid` fields.
+// By default, we automatically recreate reusable keys, but ignore invalid single-use
+// keys, assuming they have successfully been used, and recreating them might trigger
+// unnecessary updates of other Terraform resources that depend on the key.
+func shouldRecreateIfInvalid(reusable bool, recreateIfInvalid string) bool {
+	if recreateIfInvalid == "always" {
+		return true
+	}
+	if recreateIfInvalid == "never" {
+		return false
+	}
+	return reusable
+}
+
+// resourceTailnetKeyDiff makes sure a resource is recreated when a `recreate_if_invalid`
+// field changes in a way that requires it.
+func resourceTailnetKeyDiff(ctx context.Context, d *schema.ResourceDiff, m interface{}) error {
+	old, new := d.GetChange("recreate_if_invalid")
+	if old == new {
+		return nil
+	}
+
+	recreateIfInvalid := shouldRecreateIfInvalid(d.Get("reusable").(bool), d.Get("recreate_if_invalid").(string))
+	if !recreateIfInvalid {
+		return nil
+	}
+
+	client := m.(*tailscale.Client)
+	key, err := client.GetKey(ctx, d.Id())
+	if tailscale.IsNotFound(err) || (err == nil && key.Invalid) {
+		d.ForceNew("recreate_if_invalid")
+	}
+	return nil
+}
+
 func resourceTailnetKeyRead(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
+	recreateIfInvalid := shouldRecreateIfInvalid(d.Get("reusable").(bool), d.Get("recreate_if_invalid").(string))
+
 	client := m.(*tailscale.Client)
 	key, err := client.GetKey(ctx, d.Id())
 
 	switch {
 	case tailscale.IsNotFound(err):
-		d.SetId("")
+		if recreateIfInvalid {
+			d.SetId("")
+		}
 		return nil
 	case err != nil:
 		return diagnosticsError(err, "Failed to fetch key")
 	}
 
-	if key.Invalid == true {
-		// The Tailscale API continues to return keys for some time after they've expired.
-		// Use `invalid` key property to determine if key should be removed from state.
+	// The Tailscale API continues to return keys for some time after they've expired.
+	// Use `invalid` key property to determine if key should be recreated.
+	if key.Invalid && recreateIfInvalid {
 		d.SetId("")
 		return nil
 	}
@@ -180,5 +245,9 @@ func resourceTailnetKeyRead(ctx context.Context, d *schema.ResourceData, m inter
 		return diagnosticsError(err, "Failed to set description")
 	}
 
+	if err = d.Set("invalid", key.Invalid); err != nil {
+		return diagnosticsError(err, "Failed to set 'invalid'")
+	}
+
 	return nil
 }
diff --git a/tailscale/resource_tailnet_key_test.go b/tailscale/resource_tailnet_key_test.go
@@ -2,6 +2,7 @@ package tailscale_test
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"testing"
@@ -41,6 +42,84 @@ func TestProvider_TailscaleTailnetKey(t *testing.T) {
 	})
 }
 
+func testTailnetKeyStruct(reusable bool) tailscale.Key {
+	var keyCapabilities tailscale.KeyCapabilities
+	json.Unmarshal([]byte(`
+		{
+			"devices": {
+				"create": {
+					"ephemeral": true,
+					"preauthorized": true,
+					"tags": [
+						"tag:server"
+					]
+				}
+			}
+		}`), &keyCapabilities)
+	keyCapabilities.Devices.Create.Reusable = reusable
+	return tailscale.Key{
+		ID:           "test",
+		Key:          "thisisatestkey",
+		Description:  "Example key",
+		Capabilities: keyCapabilities,
+	}
+}
+
+func setKeyStep(reusable bool, recreateIfInvalid string) resource.TestStep {
+	return resource.TestStep{
+		ResourceName: "tailscale_tailnet_key.example_key",
+		Config: fmt.Sprintf(`
+			resource "tailscale_tailnet_key" "example_key" {
+				reusable = %v
+				recreate_if_invalid = "%s"
+				ephemeral = true
+				preauthorized = true
+				tags = ["tag:server"]
+				expiry = 3600
+				description = "Example key"
+			}
+		`, reusable, recreateIfInvalid),
+		Check: func(s *terraform.State) error {
+			rs, ok := s.RootModule().Resources["tailscale_tailnet_key.example_key"]
+
+			if !ok {
+				return errors.New("key not found")
+			}
+
+			if rs.Primary.ID == "" {
+				return errors.New("no ID set")
+			}
+
+			// Make sure the next API call to the test server returns the key
+			// matching the one we have just set.
+			testServer.ResponseBody = testTailnetKeyStruct(reusable)
+
+			return nil
+		},
+	}
+}
+
+func checkInvalidKeyRecreated(reusable, wantRecreated bool) resource.TestStep {
+	return resource.TestStep{
+		RefreshState:       true,
+		ExpectNonEmptyPlan: true,
+		PreConfig: func() {
+			testServer.ResponseCode = http.StatusOK
+			key := testTailnetKeyStruct(reusable)
+			key.Invalid = true
+			testServer.ResponseBody = key
+		},
+		Check: func(s *terraform.State) error {
+			_, ok := s.RootModule().Resources["tailscale_tailnet_key.example_key"]
+
+			if ok == wantRecreated {
+				return fmt.Errorf("found=%v, wantRecreated=%v", ok, wantRecreated)
+			}
+
+			return nil
+		},
+	}
+}
 func TestProvider_TailscaleTailnetKeyInvalid(t *testing.T) {
 	resource.Test(t, resource.TestCase{
 		IsUnitTest: true,
@@ -53,48 +132,31 @@ func TestProvider_TailscaleTailnetKeyInvalid(t *testing.T) {
 		},
 		ProviderFactories: testProviderFactories(t),
 		Steps: []resource.TestStep{
-			testResourceCreated("tailscale_tailnet_key.example_key", testTailnetKey),
-			{
-				// expect Invalid tailnet key to be re-created
-				RefreshState:       true,
-				ExpectNonEmptyPlan: true,
-				PreConfig: func() {
-					var keyCapabilities tailscale.KeyCapabilities
-					json.Unmarshal([]byte(`
-					{
-						"devices": {
-							"create": {
-								"reusable": true,
-								"ephemeral": true,
-								"preauthorized": true,
-								"tags": [
-									"tag:server"
-								]
-							}
-						}
-					}`), &keyCapabilities)
-
-					testServer.ResponseCode = http.StatusOK
-					testServer.ResponseBody = tailscale.Key{
-						ID:           "test",
-						Key:          "thisisatestkey",
-						Description:  "Example key",
-						Capabilities: keyCapabilities,
-						Invalid:      true, // causes replacement
-					}
-				},
-				Check: func(s *terraform.State) error {
-					_, ok := s.RootModule().Resources["tailscale_tailnet_key.example_key"]
-
-					// an Invalid tailnet key will have be removed from terraform state during the Read operation
-					if ok {
-						// fail here if the resource still exists in state
-						return fmt.Errorf("found: %s", "tailscale_tailnet_key.example_key")
-					}
-
-					return nil
-				},
-			},
+			// Create a reusable key.
+			setKeyStep(true, ""),
+			// Confirm that the reusable key will be recreated when invalid.
+			checkInvalidKeyRecreated(true, true),
+
+			// Now make it a single-use key.
+			setKeyStep(false, ""),
+			// Confirm that the single-use key is not recreated.
+			checkInvalidKeyRecreated(false, false),
+
+			// A single-use key with recreate=always, should be recreated.
+			setKeyStep(false, "always"),
+			checkInvalidKeyRecreated(false, true),
+
+			// A single-use key with recreate=never, should not be recreated.
+			setKeyStep(false, "never"),
+			checkInvalidKeyRecreated(false, false),
+
+			// A reusable key with recreate=always, should be recreated.
+			setKeyStep(true, "always"),
+			checkInvalidKeyRecreated(true, true),
+
+			// A reusable key with recreate=always, should be recreated.
+			setKeyStep(true, "always"),
+			checkInvalidKeyRecreated(true, true),
 		},
 	})
 }