Fix terraform import fidelity: stop reverting, dropping, and rotating real state

go.mod

@@ -8,30 +8,60 @@ require (
 	connectrpc.com/connect v1.18.1
 	github.com/hashicorp/terraform-plugin-framework v1.15.1
 	github.com/hashicorp/terraform-plugin-framework-validators v0.18.0
-	github.com/hashicorp/terraform-plugin-log v0.9.0
+	github.com/hashicorp/terraform-plugin-go v0.29.0
+	github.com/hashicorp/terraform-plugin-log v0.10.0
+	github.com/hashicorp/terraform-plugin-testing v1.14.1
 	google.golang.org/genproto v0.0.0-20250922171735-9219d122eba9
+	google.golang.org/grpc v1.75.1
 	google.golang.org/protobuf v1.36.9
 )
 
 require (
+	github.com/ProtonMail/go-crypto v1.1.6 // indirect
+	github.com/agext/levenshtein v1.2.2 // indirect
+	github.com/apparentlymart/go-textseg/v15 v15.0.0 // indirect
+	github.com/cloudflare/circl v1.6.1 // indirect
 	github.com/fatih/color v1.16.0 // indirect
 	github.com/golang/protobuf v1.5.4 // indirect
+	github.com/google/go-cmp v0.7.0 // indirect
+	github.com/hashicorp/errwrap v1.0.0 // indirect
+	github.com/hashicorp/go-checkpoint v0.5.0 // indirect
+	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
+	github.com/hashicorp/go-cty v1.5.0 // indirect
 	github.com/hashicorp/go-hclog v1.6.3 // indirect
-	github.com/hashicorp/go-plugin v1.6.3 // indirect
+	github.com/hashicorp/go-multierror v1.1.1 // indirect
+	github.com/hashicorp/go-plugin v1.7.0 // indirect
+	github.com/hashicorp/go-retryablehttp v0.7.7 // indirect
 	github.com/hashicorp/go-uuid v1.0.3 // indirect
-	github.com/hashicorp/terraform-plugin-go v0.28.0 // indirect
-	github.com/hashicorp/terraform-registry-address v0.2.5 // indirect
+	github.com/hashicorp/go-version v1.7.0 // indirect
+	github.com/hashicorp/hc-install v0.9.2 // indirect
+	github.com/hashicorp/hcl/v2 v2.24.0 // indirect
+	github.com/hashicorp/logutils v1.0.0 // indirect
+	github.com/hashicorp/terraform-exec v0.24.0 // indirect
+	github.com/hashicorp/terraform-json v0.27.2 // indirect
+	github.com/hashicorp/terraform-plugin-sdk/v2 v2.38.1 // indirect
+	github.com/hashicorp/terraform-registry-address v0.4.0 // indirect
 	github.com/hashicorp/terraform-svchost v0.1.1 // indirect
-	github.com/hashicorp/yamux v0.1.1 // indirect
+	github.com/hashicorp/yamux v0.1.2 // indirect
 	github.com/mattn/go-colorable v0.1.13 // indirect
 	github.com/mattn/go-isatty v0.0.20 // indirect
+	github.com/mitchellh/copystructure v1.2.0 // indirect
 	github.com/mitchellh/go-testing-interface v1.14.1 // indirect
-	github.com/oklog/run v1.0.0 // indirect
+	github.com/mitchellh/go-wordwrap v1.0.1 // indirect
+	github.com/mitchellh/mapstructure v1.5.0 // indirect
+	github.com/mitchellh/reflectwalk v1.0.2 // indirect
+	github.com/oklog/run v1.1.0 // indirect
+	github.com/vmihailenco/msgpack v4.0.4+incompatible // indirect
 	github.com/vmihailenco/msgpack/v5 v5.4.1 // indirect
 	github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect
-	golang.org/x/net v0.43.0 // indirect
-	golang.org/x/sys v0.35.0 // indirect
-	golang.org/x/text v0.28.0 // indirect
+	github.com/zclconf/go-cty v1.17.0 // indirect
+	golang.org/x/crypto v0.45.0 // indirect
+	golang.org/x/mod v0.29.0 // indirect
+	golang.org/x/net v0.47.0 // indirect
+	golang.org/x/sync v0.18.0 // indirect
+	golang.org/x/sys v0.38.0 // indirect
+	golang.org/x/text v0.31.0 // indirect
+	golang.org/x/tools v0.38.0 // indirect
+	google.golang.org/appengine v1.6.8 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20250908214217-97024824d090 // indirect
-	google.golang.org/grpc v1.74.2 // indirect
 )

internal/provider/acceptance_test.go

@@ -0,0 +1,57 @@
+package provider
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/hashicorp/terraform-plugin-framework/providerserver"
+	"github.com/hashicorp/terraform-plugin-go/tfprotov6"
+
+	apiv1connect "github.com/devzero-inc/terraform-provider-devzero/internal/gen/api/v1/apiv1connect"
+)
+
+// ---------------------------------------------------------------------------
+// Acceptance-test harness: wires the real provider (via ProtoV6ProviderFactories)
+// against the same in-memory fake backend the unit tests use, so
+// resource.TestCase can drive genuine `terraform import` / `terraform plan` /
+// `terraform apply` cycles without hitting a real DevZero API.
+// ---------------------------------------------------------------------------
+
+const (
+	testAccTeamID = "team-1"
+	testAccToken  = "test-token"
+)
+
+// testAccHarness starts a fake backend and returns the provider factories
+// terraform-plugin-testing needs, plus the backend for direct assertions
+// against what was actually persisted (independent of what Terraform reports).
+func testAccHarness(t *testing.T) (map[string]func() (tfprotov6.ProviderServer, error), *fakeBackend) {
+	t.Helper()
+
+	fake := newFakeBackend(testAccTeamID)
+	mux := http.NewServeMux()
+	mux.Handle(apiv1connect.NewK8SRecommendationServiceHandler(fake))
+	mux.Handle(apiv1connect.NewK8SServiceHandler(fake))
+	mux.Handle(apiv1connect.NewClusterMutationServiceHandler(fake))
+	srv := httptest.NewServer(mux)
+	t.Cleanup(srv.Close)
+
+	factories := map[string]func() (tfprotov6.ProviderServer, error){
+		"devzero": providerserver.NewProtocol6WithError(&DevzeroProvider{version: "test", defaultURL: srv.URL}),
+	}
+	return factories, fake
+}
+
+// testAccProviderConfig renders the provider block every test config needs;
+// the URL is baked into the provider instance itself (see testAccHarness), so
+// only credentials the fake backend expects are supplied here.
+func testAccProviderConfig() string {
+	return fmt.Sprintf(`
+provider "devzero" {
+  team_id = %q
+  token   = %q
+}
+`, testAccTeamID, testAccToken)
+}

internal/provider/cluster_resource.go

@@ -20,7 +20,6 @@ import (
 var _ resource.Resource = &ClusterResource{}
 var _ resource.ResourceWithConfigure = &ClusterResource{}
 var _ resource.ResourceWithImportState = &ClusterResource{}
-var _ resource.ResourceWithModifyPlan = &ClusterResource{}
 
 func NewClusterResource() resource.Resource {
 	return &ClusterResource{}
@@ -91,33 +90,6 @@ func (r *ClusterResource) Configure(ctx context.Context, req resource.ConfigureR
 	r.client = client
 }
 
-func (r *ClusterResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) {
-	// If the resource is being created, skip forcing a rotation during plan
-	if req.State.Raw.IsNull() {
-		return
-	}
-
-	var data ClusterResourceModel
-	resp.Diagnostics.Append(req.State.Get(ctx, &data)...)
-	if resp.Diagnostics.HasError() {
-		return
-	}
-
-	// If the prior token is empty, mark the planned token as unknown so that
-	// Terraform plans an apply which will rotate the token during Update.
-	if data.Token.IsNull() || data.Token.ValueString() == "" {
-		// Only attempt to set if plan is available
-		if !req.Plan.Raw.IsNull() {
-			// Set token to unknown in plan
-			err := resp.Plan.SetAttribute(ctx, path.Root("token"), types.StringUnknown())
-			if err != nil {
-				resp.Diagnostics.AddError("Plan Error", fmt.Sprintf("Unable to mark token unknown in plan: %s", err))
-				return
-			}
-		}
-	}
-}
-
 func (r *ClusterResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
 	var data ClusterResourceModel
 
@@ -227,23 +199,9 @@ func (r *ClusterResource) Update(ctx context.Context, req resource.UpdateRequest
 	}
 	data.Name = types.StringValue(updatedName)
 
-	// If prior token was empty, rotate it now and persist the new token in state
-	if data.Token.IsNull() || data.Token.IsUnknown() || data.Token.ValueString() == "" {
-		resetReq := &apiv1.ResetClusterTokenRequest{
-			TeamId:    r.client.TeamId,
-			ClusterId: data.Id.ValueString(),
-		}
-		resetResp, err := r.client.ClusterMutationClient.ResetClusterToken(ctx, connect.NewRequest(resetReq))
-		if err != nil {
-			resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to reset cluster token, got error: %s", err))
-			return
-		}
-		if resetResp.Msg.Token == "" {
-			resp.Diagnostics.AddError("Client Error", "Cluster token reset returned empty token")
-			return
-		}
-		data.Token = types.StringValue(resetResp.Msg.Token)
-	}
+	// token is write-once: the API only ever returns it from CreateCluster,
+	// never from GetCluster, so it's simply carried forward from state here
+	// (real value if set at create, null if the resource was imported).
 
 	// Save updated data into Terraform state
 	resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)

internal/provider/cluster_resource_import_test.go

@@ -0,0 +1,117 @@
+package provider
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"connectrpc.com/connect"
+	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
+	"github.com/hashicorp/terraform-plugin-testing/terraform"
+
+	apiv1 "github.com/devzero-inc/terraform-provider-devzero/internal/gen/api/v1"
+)
+
+// TestAccClusterResource_ImportRoundTrip proves full import fidelity for
+// devzero_cluster: import hydrates every readable attribute, the plan right
+// after import is clean, and editing+reapplying an imported resource updates
+// the real resource.
+//
+// The cluster is seeded directly against the fake backend (not via a
+// Terraform apply) and imported as the test's first step. This is
+// deliberate: a Terraform-driven Create never exercises Read at all (it
+// writes state straight from the API response), so an import test built on
+// top of an in-suite apply would still pass even if Read silently dropped
+// fields. Importing a backend object Terraform never created is the only
+// way to actually exercise "Read from scratch" the way `terraform import`
+// really uses it, and it matches terraform-plugin-testing's own documented
+// pattern for import-as-first-step (see ImportStatePersist's doc comment).
+func TestAccClusterResource_ImportRoundTrip(t *testing.T) {
+	factories, fake := testAccHarness(t)
+
+	seeded, err := fake.CreateCluster(context.Background(), connect.NewRequest(&apiv1.CreateClusterRequest{
+		TeamId:      testAccTeamID,
+		ClusterName: "acc-cluster",
+	}))
+	if err != nil {
+		t.Fatalf("seeding cluster: %s", err)
+	}
+	clusterID := seeded.Msg.Cluster.Id
+
+	importCfg := testAccProviderConfig() + `
+resource "devzero_cluster" "test" {
+  name = "acc-cluster"
+}
+`
+	renamedCfg := testAccProviderConfig() + `
+resource "devzero_cluster" "test" {
+  name = "acc-cluster-renamed"
+}
+`
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: factories,
+		Steps: []resource.TestStep{
+			{
+				Config:        importCfg,
+				ResourceName:  "devzero_cluster.test",
+				ImportState:   true,
+				ImportStateId: clusterID,
+				// Without this, the imported state is discarded at the end
+				// of the step and step 2 below would silently check a plan
+				// against no state at all rather than the real post-import
+				// state.
+				ImportStatePersist: true,
+				ImportStateCheck: func(states []*terraform.InstanceState) error {
+					if len(states) != 1 {
+						return fmt.Errorf("expected 1 imported instance, got %d", len(states))
+					}
+					if got := states[0].Attributes["name"]; got != "acc-cluster" {
+						return fmt.Errorf("imported name = %q, want %q", got, "acc-cluster")
+					}
+					if got := states[0].Attributes["id"]; got != clusterID {
+						return fmt.Errorf("imported id = %q, want %q", got, clusterID)
+					}
+					return nil
+				},
+			},
+			{
+				// Same config as the import step, applied fresh right
+				// after: this must produce zero changes, proving
+				// `terraform plan` right after `terraform import` is clean.
+				Config:   importCfg,
+				PlanOnly: true,
+			},
+			{
+				Config: renamedCfg,
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr("devzero_cluster.test", "name", "acc-cluster-renamed"),
+					testAccCheckClusterNameInBackend(fake, "devzero_cluster.test", "acc-cluster-renamed"),
+				),
+			},
+		},
+	})
+}
+
+// testAccCheckClusterNameInBackend asserts the fake backend's own record was
+// actually updated, independent of what Terraform's state reports.
+func testAccCheckClusterNameInBackend(fake *fakeBackend, resourceName, wantName string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		rs, ok := s.RootModule().Resources[resourceName]
+		if !ok {
+			return fmt.Errorf("resource %s not found in state", resourceName)
+		}
+		id := rs.Primary.ID
+
+		fake.mu.Lock()
+		defer fake.mu.Unlock()
+		c, ok := fake.clusters[id]
+		if !ok {
+			return fmt.Errorf("cluster %s not found in fake backend", id)
+		}
+		if c.CustomName != wantName {
+			return fmt.Errorf("fake backend cluster %s has name %q, want %q", id, c.CustomName, wantName)
+		}
+		return nil
+	}
+}