From 1ffcfe1dff21af276a3487872fa626d1b6330aeb Mon Sep 17 00:00:00 2001 From: Anton Tolchanov Date: Fri, 8 Dec 2023 14:20:03 +0000 Subject: [PATCH] tailscale_tailnet_key: only recreate reusable keys by default 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 --- docs/resources/tailnet_key.md | 2 + tailscale/resource_tailnet_key.go | 77 ++++++++++++- tailscale/resource_tailnet_key_test.go | 146 ++++++++++++++++++------- 3 files changed, 179 insertions(+), 46 deletions(-) diff --git a/docs/resources/tailnet_key.md b/docs/resources/tailnet_key.md index 991f7271..17b9e37b 100644 --- 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 index 272ec104..0fb3d232 100644 --- 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. +func shouldRecreateIfInvalid(reusable bool, recreateIfInvalid string) bool { + // 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. + 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 index 370ada7e..161531c6 100644 --- 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), }, }) }