From 073e07046484f0b26ed9c2bf1b22cffc128c109e Mon Sep 17 00:00:00 2001
From: Liam Cervante <liam.cervante@hashicorp.com>
Date: Tue, 23 Apr 2024 09:06:45 +0200
Subject: [PATCH] terraform apply: restore marks after unknown validation
 (#35048)

---
 internal/terraform/context_apply2_test.go     | 62 +++++++++++++++++++
 .../node_resource_abstract_instance.go        | 18 +++---
 2 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/internal/terraform/context_apply2_test.go b/internal/terraform/context_apply2_test.go
index 71afd4702276..2018a3b6c6e2 100644
--- a/internal/terraform/context_apply2_test.go
+++ b/internal/terraform/context_apply2_test.go
@@ -2917,3 +2917,65 @@ resource "test_object" "obj" {
 	_, diags = ctx.Apply(plan, m, nil)
 	assertNoErrors(t, diags)
 }
+
+func TestContext2Apply_35039(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+resource "test_object" "obj" {
+  list = ["a", "b", "c"]
+}
+`,
+	})
+
+	p := testing_provider.MockProvider{}
+	p.GetProviderSchemaResponse = &providers.GetProviderSchemaResponse{
+		ResourceTypes: map[string]providers.Schema{
+			"test_object": {
+				Block: &configschema.Block{
+					Attributes: map[string]*configschema.Attribute{
+						"output": {
+							Type:     cty.String,
+							Computed: true,
+						},
+						"list": {
+							Type:      cty.List(cty.String),
+							Required:  true,
+							Sensitive: true,
+						},
+					},
+				},
+			},
+		},
+	}
+	p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse {
+		return providers.PlanResourceChangeResponse{
+			PlannedState: cty.ObjectVal(map[string]cty.Value{
+				"output": cty.UnknownVal(cty.String),
+				"list":   req.ProposedNewState.GetAttr("list"),
+			}),
+		}
+	}
+	p.ApplyResourceChangeFn = func(req providers.ApplyResourceChangeRequest) providers.ApplyResourceChangeResponse {
+		return providers.ApplyResourceChangeResponse{
+			// This is a bug, the provider shouldn't return unknown values from
+			// ApplyResourceChange. But, Terraform shouldn't crash in response
+			// to this. It should return a nice error message.
+			NewState: req.PlannedState,
+		}
+	}
+
+	ctx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(&p),
+		},
+	})
+
+	plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables)))
+	assertNoErrors(t, diags)
+
+	// Just don't crash, should report an error about the provider.
+	_, diags = ctx.Apply(plan, m, nil)
+	if len(diags) != 1 {
+		t.Fatalf("expected exactly one diagnostic, but got %d: %s", len(diags), diags)
+	}
+}
diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go
index e8b127885396..a0c75fac0cc3 100644
--- a/internal/terraform/node_resource_abstract_instance.go
+++ b/internal/terraform/node_resource_abstract_instance.go
@@ -2438,15 +2438,6 @@ func (n *NodeAbstractResourceInstance) apply(
 	// incomplete.
 	newVal := resp.NewState
 
-	// If we have paths to mark, mark those on this new value we need to
-	// re-check the value against the schema, because nested computed values
-	// won't be included in afterPaths, which are only what was read from the
-	// After plan value.
-	newVal = newVal.MarkWithPaths(afterPaths)
-	if sensitivePaths := schema.SensitivePaths(newVal, nil); len(sensitivePaths) != 0 {
-		newVal = marks.MarkPaths(newVal, marks.Sensitive, sensitivePaths)
-	}
-
 	if newVal == cty.NilVal {
 		// Providers are supposed to return a partial new value even when errors
 		// occur, but sometimes they don't and so in that case we'll patch that up
@@ -2532,6 +2523,15 @@ func (n *NodeAbstractResourceInstance) apply(
 		newVal = cty.UnknownAsNull(newVal)
 	}
 
+	// If we have paths to mark, mark those on this new value we need to
+	// re-check the value against the schema, because nested computed values
+	// won't be included in afterPaths, which are only what was read from the
+	// After plan value.
+	newVal = newVal.MarkWithPaths(afterPaths)
+	if sensitivePaths := schema.SensitivePaths(newVal, nil); len(sensitivePaths) != 0 {
+		newVal = marks.MarkPaths(newVal, marks.Sensitive, sensitivePaths)
+	}
+
 	if change.Action != plans.Delete && !diags.HasErrors() {
 		// Only values that were marked as unknown in the planned value are allowed
 		// to change during the apply operation. (We do this after the unknown-ness