Query the resource version prior to making an update

Storing the resource version in Terraform state doesn't quite do what we'd expect. If we tell Terraform that it is computed, we aren't able to read it during `apply` and thus can't pass it as part of the request. If we tall Terraform that it is not computed, it is assumed to be constant, and Terraform will bark at us when we attempt to mutate it.

To remedy this, we read the namespace just before updating or deleting it to get a recent resource version and use that in the request. If a concurrent request causes the update or delete to fail, the user will need to run `terraform apply` again.

internal/provider/namespace_resource.go

@@ -36,7 +36,6 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/types"
 	"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
-	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/temporalio/terraform-provider-temporalcloud/internal/client"
 	cloudservicev1 "github.com/temporalio/terraform-provider-temporalcloud/proto/go/temporal/api/cloud/cloudservice/v1"
 	namespacev1 "github.com/temporalio/terraform-provider-temporalcloud/proto/go/temporal/api/cloud/namespace/v1"
@@ -58,7 +57,6 @@ type (
 		Regions            types.List   `tfsdk:"regions"`
 		AcceptedClientCA   types.String `tfsdk:"accepted_client_ca"`
 		RetentionDays      types.Int64  `tfsdk:"retention_days"`
-		ResourceVersion    types.String `tfsdk:"resource_version"`
 		CertificateFilters types.List   `tfsdk:"certificate_filters"`
 
 		Timeouts timeouts.Value `tfsdk:"timeouts"`
@@ -134,9 +132,6 @@ func (r *namespaceResource) Schema(ctx context.Context, _ resource.SchemaRequest
 			"retention_days": schema.Int64Attribute{
 				Required: true,
 			},
-			"resource_version": schema.StringAttribute{
-				Computed: true,
-			},
 			"certificate_filters": schema.ListNestedAttribute{
 				Optional: true,
 				NestedObject: schema.NestedAttributeObject{
@@ -220,9 +215,6 @@ func (r *namespaceResource) Create(ctx context.Context, req resource.CreateReque
 		return
 	}
 
-	tflog.Debug(ctx, "responded with namespace model", map[string]any{
-		"resource_version": ns.GetNamespace().GetResourceVersion(),
-	})
 	updateModelFromSpec(ctx, resp.Diagnostics, &plan, ns.Namespace)
 	resp.Diagnostics.Append(resp.State.Set(ctx, plan)...)
 }
@@ -263,6 +255,11 @@ func (r *namespaceResource) Update(ctx context.Context, req resource.UpdateReque
 	if resp.Diagnostics.HasError() {
 		return
 	}
+	resourceVersion, err := getCurrentResourceVersion(ctx, r.client, &plan)
+	if err != nil {
+		resp.Diagnostics.AddError("Failed to get current resource version", err.Error())
+		return
+	}
 	svcResp, err := r.client.UpdateNamespace(ctx, &cloudservicev1.UpdateNamespaceRequest{
 		Namespace: plan.ID.ValueString(),
 		Spec: &namespacev1.NamespaceSpec{
@@ -274,7 +271,7 @@ func (r *namespaceResource) Update(ctx context.Context, req resource.UpdateReque
 				CertificateFilters: certFilters,
 			},
 		},
-		ResourceVersion: plan.ResourceVersion.ValueString(),
+		ResourceVersion: resourceVersion,
 	})
 	if err != nil {
 		resp.Diagnostics.AddError("Failed to update namespace", err.Error())
@@ -312,11 +309,16 @@ func (r *namespaceResource) Delete(ctx context.Context, req resource.DeleteReque
 		return
 	}
 
+	resouceVersion, err := getCurrentResourceVersion(ctx, r.client, &state)
+	if err != nil {
+		resp.Diagnostics.AddError("Failed to get current resource version", err.Error())
+		return
+	}
 	ctx, cancel := context.WithTimeout(ctx, deleteTimeout)
 	defer cancel()
 	svcResp, err := r.client.DeleteNamespace(ctx, &cloudservicev1.DeleteNamespaceRequest{
 		Namespace:       state.ID.ValueString(),
-		ResourceVersion: state.ResourceVersion.ValueString(),
+		ResourceVersion: resouceVersion,
 	})
 	if err != nil {
 		resp.Diagnostics.AddError("Failed to delete namespace", err.Error())
@@ -382,7 +384,6 @@ func updateModelFromSpec(ctx context.Context, diags diag.Diagnostics, state *nam
 	state.CertificateFilters = certificateFilter
 	state.AcceptedClientCA = types.StringValue(ns.GetSpec().GetMtlsAuth().GetAcceptedClientCa())
 	state.RetentionDays = types.Int64Value(int64(ns.GetSpec().GetRetentionDays()))
-	state.ResourceVersion = types.StringValue(ns.GetResourceVersion())
 }
 
 func getCertFiltersFromModel(ctx context.Context, diags diag.Diagnostics, model *namespaceResourceModel) []*namespacev1.CertificateFilterSpec {
@@ -415,6 +416,17 @@ func getCertFiltersFromModel(ctx context.Context, diags diag.Diagnostics, model
 	return certificateFilters
 }
 
+func getCurrentResourceVersion(ctx context.Context, client cloudservicev1.CloudServiceClient, model *namespaceResourceModel) (string, error) {
+	ns, err := client.GetNamespace(ctx, &cloudservicev1.GetNamespaceRequest{
+		Namespace: model.ID.ValueString(),
+	})
+	if err != nil {
+		return "", err
+	}
+
+	return ns.GetNamespace().GetResourceVersion(), nil
+}
+
 func stringOrNull(s string) types.String {
 	if s == "" {
 		return types.StringNull()

internal/provider/namespace_resource_test.go

@@ -2,12 +2,14 @@ package provider
 
 import (
 	"fmt"
+	"math/rand"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
 )
 
 func TestAccBasicNamespace(t *testing.T) {
+	name := fmt.Sprintf("%s-%s", "tf-basic-namespace", randomString(10))
 	config := func(name string, retention int) string {
 		return fmt.Sprintf(`
 provider "temporalcloud" {
@@ -43,21 +45,19 @@ PEM
 		Steps: []resource.TestStep{
 			{
 				// New namespace with retention of 7
-				Config: config("tf-basic-namespace", 7),
+				Config: config(name, 7),
 			},
-			/* Does not work yet: CLD-1971
 			{
-				// Update retention to 14
-				Config: testAccBasicNamespaceConfig("terraform-test", 14),
+				Config: config(name, 14),
 			},
-			*/
 			// Delete testing automatically occurs in TestCase
 		},
 	})
 
 }
 
 func TestAccBasicNamespaceWithCertFilters(t *testing.T) {
+	name := fmt.Sprintf("%s-%s", "tf-cert-filters", randomString(10))
 	config := func(name string, retention int) string {
 		return fmt.Sprintf(`
 provider "temporalcloud" {
@@ -101,15 +101,21 @@ PEM
 		Steps: []resource.TestStep{
 			{
 				// New namespace with retention of 7
-				Config: config("terraform-test", 7),
+				Config: config(name, 7),
 			},
-			/* Does not work yet: CLD-1971
 			{
-				// Update retention to 14
-				Config: testAccBasicNamespaceConfig("terraform-test", 14),
+				Config: config(name, 14),
 			},
-			*/
 			// Delete testing automatically occurs in TestCase
 		},
 	})
 }
+
+func randomString(length int) string {
+	const charset = "abcdefghijklmnopqrstuvwxyz"
+	b := make([]byte, length)
+	for i := range b {
+		b[i] = charset[rand.Intn(len(charset))]
+	}
+	return string(b)
+}