Merge pull request #1994 from hashicorp/add-key_wo_version-to-tfe_ssh_key

Configs with Write-only `key` value will always converge when used in to tfe_ssh_key

Author: Uk1288

CHANGELOG.md

@@ -11,11 +11,13 @@ BREAKING CHANGES:
 * `r/tfe_organization_run_task`: Added the `hmac_key_wo_version` write-only attribute to allow triggering `hmac_key_wo` updates, by @uk1288 ([#1992](https://github.com/hashicorp/terraform-provider-tfe/pull/1992))
 * `r/tfe_policy_set_parameter`: Added the `value_wo_version` write-only attribute to allow triggering `value_wo` updates, by @uk1288 ([#1984](https://github.com/hashicorp/terraform-provider-tfe/pull/1984))
 * `r/tfe_saml_settings`: Added the `private_key_wo_version` write-only attribute to allow triggering `private_key_wo` updates, by @uk1288 ([#1993](https://github.com/hashicorp/terraform-provider-tfe/pull/1993))
+* `r/tfe_ssh_key`: Added the `key_wo_version` write-only attribute to allow triggering `key_wo` updates, by @uk1288 ([#1994](https://github.com/hashicorp/terraform-provider-tfe/pull/1994))
 
 FEATURES:
 * Adds `tfe_query_run` action, allowing users to invoke remote Terraform queries on HCP Terraform and Terraform Enterprise, by @sebasslash [#1982](https://github.com/hashicorp/terraform-provider-tfe/pull/1982)
 * Adds `d/tfe_current_user` data source for retrieving information about the user associated with the configured API token, by @ShaunakRembhotkar
 
+
 ## v0.74.1
 
 BUG FIXES:

internal/provider/resource_tfe_ssh_key.go

@@ -9,17 +9,17 @@ import (
 	"fmt"
 
 	tfe "github.com/hashicorp/go-tfe"
+	"github.com/hashicorp/terraform-plugin-framework-validators/int64validator"
 	"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
 	"github.com/hashicorp/terraform-plugin-framework/path"
 	"github.com/hashicorp/terraform-plugin-framework/resource"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema"
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/schema/validator"
 	"github.com/hashicorp/terraform-plugin-framework/types"
 	"github.com/hashicorp/terraform-plugin-log/tflog"
-	"github.com/hashicorp/terraform-provider-tfe/internal/provider/helpers"
-	"github.com/hashicorp/terraform-provider-tfe/internal/provider/planmodifiers"
 )
 
 var (
@@ -41,17 +41,21 @@ type modelTFESSHKey struct {
 	Organization types.String `tfsdk:"organization"`
 	Key          types.String `tfsdk:"key"`
 	KeyWO        types.String `tfsdk:"key_wo"`
+	KeyWOVersion types.Int64  `tfsdk:"key_wo_version"`
 }
 
-func modelFromTFESSHKey(organization string, sshKey *tfe.SSHKey, lastValue types.String, isWriteOnly bool) *modelTFESSHKey {
+func modelFromTFESSHKey(organization string, sshKey *tfe.SSHKey, lastValue types.String, keyWOVersion types.Int64) *modelTFESSHKey {
 	m := &modelTFESSHKey{
 		ID:           types.StringValue(sshKey.ID),
 		Name:         types.StringValue(sshKey.Name),
 		Organization: types.StringValue(organization),
 		Key:          lastValue,
+		KeyWOVersion: keyWOVersion,
 	}
 
-	if isWriteOnly {
+	// Don't retrieve values if write-only is being used. Unset the key field before updating the state.
+	isWriteOnlyValue := !keyWOVersion.IsNull()
+	if isWriteOnlyValue {
 		m.Key = types.StringNull()
 	}
 
@@ -115,17 +119,29 @@ func (r *resourceTFESSHKey) Schema(_ context.Context, req resource.SchemaRequest
 					stringvalidator.AtLeastOneOf(path.MatchRoot("key"), path.MatchRoot("key_wo")),
 				},
 			},
-
+			// since the key_wo write-only values are not saved to state, they will not trigger updates on their own.
+			// Instead the key_wo_version responsibility is to trigger updates to the key_wo attribute when version number changes.
 			"key_wo": schema.StringAttribute{
 				Description: "The text of the SSH private key, guaranteed not to be written to state.",
 				Optional:    true,
 				WriteOnly:   true,
 				Sensitive:   true,
 				Validators: []validator.String{
 					stringvalidator.ConflictsWith(path.MatchRoot("key")),
+					stringvalidator.AlsoRequires(path.MatchRoot("key_wo_version")),
 				},
-				PlanModifiers: []planmodifier.String{
-					planmodifiers.NewReplaceForWriteOnlyStringValue("key_wo"),
+			},
+
+			"key_wo_version": schema.Int64Attribute{
+				Optional:    true,
+				Description: "Version of the write-only key to trigger updates",
+				Validators: []validator.Int64{
+					int64validator.ConflictsWith(path.MatchRoot("key")),
+					int64validator.AlsoRequires(path.MatchRoot("key_wo")),
+				},
+				PlanModifiers: []planmodifier.Int64{
+					// must be replaced since the go-tfe update api does not have the `key` attribute at time of writing
+					int64planmodifier.RequiresReplace(),
 				},
 			},
 		},
@@ -153,9 +169,8 @@ func (r *resourceTFESSHKey) Create(ctx context.Context, req resource.CreateReque
 		Name: plan.Name.ValueStringPointer(),
 	}
 
-	// Set Value from `value_wo` if set, otherwise use the normal value
-	isWriteOnly := !config.KeyWO.IsNull()
-	if isWriteOnly {
+	// Set Value from `key_wo` if set, otherwise use the normal value
+	if !config.KeyWO.IsNull() {
 		options.Value = config.KeyWO.ValueStringPointer()
 	} else {
 		options.Value = plan.Key.ValueStringPointer()
@@ -169,13 +184,7 @@ func (r *resourceTFESSHKey) Create(ctx context.Context, req resource.CreateReque
 	}
 
 	// Load the response data into the model
-	result := modelFromTFESSHKey(organization, sshKey, plan.Key, isWriteOnly)
-
-	// Write the hashed private key to the state if it was provided
-	if !config.KeyWO.IsNull() {
-		store := r.writeOnlyValueStore(resp.Private)
-		resp.Diagnostics.Append(store.SetPriorValue(ctx, config.KeyWO)...)
-	}
+	result := modelFromTFESSHKey(organization, sshKey, plan.Key, config.KeyWOVersion)
 
 	if resp.Diagnostics.HasError() {
 		return
@@ -216,15 +225,8 @@ func (r *resourceTFESSHKey) Read(ctx context.Context, req resource.ReadRequest,
 		return
 	}
 
-	// Check if the parameter is write-only
-	isWriteOnly, diags := r.writeOnlyValueStore(resp.Private).PriorValueExists(ctx)
-	resp.Diagnostics.Append(diags...)
-	if diags.HasError() {
-		return
-	}
-
 	// Load the response data into the model
-	result := modelFromTFESSHKey(organization, sshKey, state.Key, isWriteOnly)
+	result := modelFromTFESSHKey(organization, sshKey, state.Key, state.KeyWOVersion)
 
 	// Update state
 	resp.Diagnostics.Append(resp.State.Set(ctx, result)...)
@@ -261,7 +263,7 @@ func (r *resourceTFESSHKey) Update(ctx context.Context, req resource.UpdateReque
 	}
 
 	// Load the response data into the model
-	result := modelFromTFESSHKey(organization, sshKey, plan.Key, !config.KeyWO.IsNull())
+	result := modelFromTFESSHKey(organization, sshKey, plan.Key, config.KeyWOVersion)
 
 	// Update state
 	resp.Diagnostics.Append(resp.State.Set(ctx, result)...)
@@ -298,7 +300,3 @@ func (r *resourceTFESSHKey) Delete(ctx context.Context, req resource.DeleteReque
 		return
 	}
 }
-
-func (r *resourceTFESSHKey) writeOnlyValueStore(private helpers.PrivateState) *helpers.WriteOnlyValueStore {
-	return helpers.NewWriteOnlyValueStore(private, "key_wo")
-}

internal/provider/resource_tfe_ssh_key_test.go

@@ -81,9 +81,30 @@ func TestAccTFESSHKey_update(t *testing.T) {
 	})
 }
 
+func TestAccTFESSHKey_WriteOnlyValidation(t *testing.T) {
+	rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int()
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 func() { testAccPreCheck(t) },
+		ProtoV6ProviderFactories: testAccMuxedProviders,
+		CheckDestroy:             testAccCheckTFESSHKeyDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config:      testAccTFESSHKey_WriteOnlyMissingVersion(rInt),
+				ExpectError: regexp.MustCompile(`Attribute "key_wo_version" must be specified when "key_wo" is specified`),
+			},
+			{
+				Config:      testAccTFESSHKey_keyAndKeyWO(rInt),
+				ExpectError: regexp.MustCompile(`Attribute "key_wo" cannot be specified when "key" is specified`),
+			},
+		},
+	})
+}
+
 func TestAccTFESSHKey_keyWO(t *testing.T) {
 	sshKey := &tfe.SSHKey{}
 	rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int()
+	versionOne, versionTwo := 1, 2
 
 	// Create the value comparer so we can add state values to it during the test steps
 	compareValuesDiffer := statecheck.CompareValue(compare.ValuesDiffer())
@@ -94,11 +115,7 @@ func TestAccTFESSHKey_keyWO(t *testing.T) {
 		CheckDestroy:             testAccCheckTFEOrganizationRunTaskDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config:      testAccTFESSHKey_keyAndKeyWO(rInt),
-				ExpectError: regexp.MustCompile(`Attribute "key_wo" cannot be specified when "key" is specified`),
-			},
-			{
-				Config: testAccTFESSHKey_keyWO(rInt, "SSH-KEY-CONTENT"),
+				Config: testAccTFESSHKey_keyWO(rInt, "SSH-KEY-CONTENT", versionOne),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckTFESSHKeyExists("tfe_ssh_key.foobar", sshKey),
 					testAccCheckTFESSHKeyAttributes(sshKey),
@@ -115,7 +132,7 @@ func TestAccTFESSHKey_keyWO(t *testing.T) {
 				},
 			},
 			{
-				Config: testAccTFESSHKey_keyWO(rInt, "SSH-KEY-CONTENT-UPDATED"),
+				Config: testAccTFESSHKey_keyWO(rInt, "SSH-KEY-CONTENT-UPDATED", versionTwo),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckTFESSHKeyExists("tfe_ssh_key.foobar", sshKey),
 					testAccCheckTFESSHKeyAttributes(sshKey),
@@ -244,7 +261,7 @@ resource "tfe_ssh_key" "foobar" {
 }`, rInt)
 }
 
-func testAccTFESSHKey_keyWO(rInt int, key string) string {
+func testAccTFESSHKey_keyWO(rInt int, key string, versionValue int) string {
 	return fmt.Sprintf(`
 resource "tfe_organization" "foobar" {
   name  = "tst-terraform-%d"
@@ -255,7 +272,8 @@ resource "tfe_ssh_key" "foobar" {
   name         = "ssh-key-test"
   organization = tfe_organization.foobar.id
   key_wo       = "%s"
-}`, rInt, key)
+  key_wo_version = %d
+}`, rInt, key, versionValue)
 }
 
 func testAccTFESSHKey_keyAndKeyWO(rInt int) string {
@@ -272,3 +290,17 @@ resource "tfe_ssh_key" "foobar" {
   key_wo       = "SSH-KEY-CONTENT"
 }`, rInt)
 }
+
+func testAccTFESSHKey_WriteOnlyMissingVersion(rInt int) string {
+	return fmt.Sprintf(`
+resource "tfe_organization" "foobar" {
+  name  = "tst-terraform-%d"
+  email = "admin@company.com"
+}
+
+resource "tfe_ssh_key" "foobar" {
+  name         = "ssh-key-test"
+  organization = tfe_organization.foobar.id
+  key_wo       = "SSH-KEY-CONTENT"
+}`, rInt)
+}

website/docs/r/ssh_key.html.markdown

@@ -22,6 +22,22 @@ resource "tfe_ssh_key" "test" {
 }
 ```
 
+With write-only key:
+
+```hcl
+variable "ssh_key" {
+  type      = string
+  ephemeral = true
+}
+
+resource "tfe_ssh_key" "test" {
+  name           = "my-ssh-key-name"
+  organization   = "my-org-name"
+  key_wo         = var.ssh_key
+  key_wo_version = 1
+}
+```
+
 ## Argument Reference
 
 The following arguments are supported:
@@ -31,7 +47,8 @@ The following arguments are supported:
 * `key` - (Optional) The text of the SSH private key. One of `key` or `key_wo`
   must be provided.
 * `key_wo` - (Optional, [Write-Only](https://developer.hashicorp.com/terraform/language/v1.11.x/resources/ephemeral#write-only-arguments)) The text of the SSH private key, guaranteed not to be
-  written to plan or state artifacts. One of `key` or `key_wo` must be provided.
+  written to plan or state artifacts. One of `key` or `key_wo` must be provided. Must be used with `key_wo_version`.
+* `key_wo_version` - (Optional) Version of the write-only key. This field is used to trigger updates when the write-only key changes. Must be used with `key_wo`. When `key_wo_version` changes, the write-only key will be updated.
 
 ## Attributes Reference