From 92f3a835306d1c3e8a993276074a77bb8d1f0cb0 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Fri, 30 Sep 2022 09:19:29 -0400
Subject: [PATCH] special handling for legacy ignore_changes = all

Legacy providers expect Terraform to be able to clean up invalid plans
and computed attributes. Add a special case for the LegacyTypeSystem to
revert `ignore_changes = all` to the complete prior state.
---
 internal/terraform/context_plan_test.go       | 64 +++++++++++++++++++
 .../node_resource_abstract_instance.go        | 13 +++-
 2 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/internal/terraform/context_plan_test.go b/internal/terraform/context_plan_test.go
index ba34e20b81c1..9a2eb5f9cb60 100644
--- a/internal/terraform/context_plan_test.go
+++ b/internal/terraform/context_plan_test.go
@@ -6689,6 +6689,70 @@ resource "test_instance" "a" {
 	}
 }
 
+func TestContext2Plan_legacyProviderIgnoreAll(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+resource "test_instance" "a" {
+  lifecycle {
+    ignore_changes = all
+  }
+  data = "foo"
+}
+`,
+	})
+
+	p := testProvider("test")
+	p.GetProviderSchemaResponse = getProviderSchemaResponseFromProviderSchema(&ProviderSchema{
+		ResourceTypes: map[string]*configschema.Block{
+			"test_instance": {
+				Attributes: map[string]*configschema.Attribute{
+					"id":   {Type: cty.String, Computed: true},
+					"data": {Type: cty.String, Optional: true},
+				},
+			},
+		},
+	})
+	p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) (resp providers.PlanResourceChangeResponse) {
+		plan := req.ProposedNewState.AsValueMap()
+		// Update both the computed id and the configured data.
+		// Legacy providers expect terraform to be able to ignore these.
+
+		plan["id"] = cty.StringVal("updated")
+		plan["data"] = cty.StringVal("updated")
+		resp.PlannedState = cty.ObjectVal(plan)
+		resp.LegacyTypeSystem = true
+		return resp
+	}
+
+	state := states.NewState()
+	root := state.EnsureModule(addrs.RootModuleInstance)
+	root.SetResourceInstanceCurrent(
+		mustResourceInstanceAddr("test_instance.a").Resource,
+		&states.ResourceInstanceObjectSrc{
+			Status:       states.ObjectReady,
+			AttrsJSON:    []byte(`{"id":"orig","data":"orig"}`),
+			Dependencies: []addrs.ConfigResource{},
+		},
+		mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`),
+	)
+
+	ctx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+	})
+	plan, diags := ctx.Plan(m, state, DefaultPlanOpts)
+	if diags.HasErrors() {
+		t.Fatal(diags.Err())
+	}
+
+	for _, c := range plan.Changes.Resources {
+		if c.Action != plans.NoOp {
+			t.Fatalf("expected NoOp plan, got %s\n", c.Action)
+		}
+	}
+}
+
 func TestContext2Plan_dataRemovalNoProvider(t *testing.T) {
 	m := testModuleInline(t, map[string]string{
 		"main.tf": `
diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go
index 5df414da8945..ba1aa3a17c60 100644
--- a/internal/terraform/node_resource_abstract_instance.go
+++ b/internal/terraform/node_resource_abstract_instance.go
@@ -881,7 +881,10 @@ func (n *NodeAbstractResourceInstance) plan(
 		// providers that we must accommodate the behavior for now, so for
 		// ignore_changes to work at all on these values, we will revert the
 		// ignored values once more.
-		plannedNewVal, ignoreChangeDiags = n.processIgnoreChanges(unmarkedPriorVal, plannedNewVal, schema)
+		// A nil schema is passed to processIgnoreChanges to indicate that we
+		// don't want to fixup a config value according to the schema when
+		// ignoring "all", rather we are reverting provider imposed changes.
+		plannedNewVal, ignoreChangeDiags = n.processIgnoreChanges(unmarkedPriorVal, plannedNewVal, nil)
 		diags = diags.Append(ignoreChangeDiags)
 		if ignoreChangeDiags.HasErrors() {
 			return plan, state, keyData, diags
@@ -1160,6 +1163,14 @@ func (n *NodeAbstractResource) processIgnoreChanges(prior, config cty.Value, sch
 	}
 
 	if ignoreAll {
+		// Legacy providers need up to clean up their invalid plans and ensure
+		// no changes are passed though, but that also means making an invalid
+		// config with computed values. In that case we just don't supply a
+		// schema and return the prior val directly.
+		if schema == nil {
+			return prior, nil
+		}
+
 		// If we are trying to ignore all attribute changes, we must filter
 		// computed attributes out from the prior state to avoid sending them
 		// to the provider as if they were included in the configuration.