Apply should not communicate w provider if only sensitivity changes

If sensitivity changes, we have an update plan,
but should avoid communicating with the provider
on the apply, as the values are equal (and otherwise
a NoOp plan)

terraform/context_apply_test.go

@@ -12088,3 +12088,105 @@ resource "test_resource" "foo" {
 	fooState := state.ResourceInstance(addr)
 	verifySensitiveValue(fooState.Current.AttrSensitivePaths)
 }
+
+func TestContext2Apply_variableSensitivityChange(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+terraform {
+	experiments = [sensitive_variables]
+}
+
+variable "sensitive_var" {
+	default = "hello"
+	sensitive = true
+}
+
+resource "test_resource" "foo" {
+	value = var.sensitive_var
+}`,
+	})
+
+	p := testProvider("test")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+		State: states.BuildState(func(s *states.SyncState) {
+			s.SetResourceInstanceCurrent(
+				addrs.Resource{
+					Mode: addrs.ManagedResourceMode,
+					Type: "test_resource",
+					Name: "foo",
+				}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
+				&states.ResourceInstanceObjectSrc{
+					Status:    states.ObjectReady,
+					AttrsJSON: []byte(`{"id":"foo", "value":"hello"}`),
+					// No AttrSensitivePaths present
+				},
+				addrs.AbsProviderConfig{
+					Provider: addrs.NewDefaultProvider("test"),
+					Module:   addrs.RootModule,
+				},
+			)
+		}),
+	})
+
+	_, diags := ctx.Plan()
+	assertNoErrors(t, diags)
+
+	addr := mustResourceInstanceAddr("test_resource.foo")
+
+	state, diags := ctx.Apply()
+	assertNoErrors(t, diags)
+
+	fooState := state.ResourceInstance(addr)
+
+	got := fooState.Current.AttrSensitivePaths[0]
+	want := cty.PathValueMarks{
+		Path:  cty.GetAttrPath("value"),
+		Marks: cty.NewValueMarks("sensitive"),
+	}
+
+	if !got.Equal(want) {
+		t.Fatalf("wrong value marks; got:\n%#v\n\nwant:\n%#v\n", got, want)
+	}
+
+	m2 := testModuleInline(t, map[string]string{
+		"main.tf": `
+terraform {
+	experiments = [sensitive_variables]
+}
+
+variable "sensitive_var" {
+	default = "hello"
+	sensitive = false
+}
+
+resource "test_resource" "foo" {
+	value = var.sensitive_var
+}`,
+	})
+
+	ctx2 := testContext2(t, &ContextOpts{
+		Config: m2,
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+		State: state,
+	})
+
+	_, diags = ctx2.Plan()
+	assertNoErrors(t, diags)
+
+	stateWithoutSensitive, diags := ctx.Apply()
+	assertNoErrors(t, diags)
+
+	fooState2 := stateWithoutSensitive.ResourceInstance(addr)
+	if len(fooState2.Current.AttrSensitivePaths) > 0 {
+		t.Fatalf("wrong number of sensitive paths, expected 0, got, %v", len(fooState2.Current.AttrSensitivePaths))
+	}
+}

terraform/eval_apply.go

@@ -109,20 +109,17 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) {
 	// If our config, Before or After value contain any marked values,
 	// ensure those are stripped out before sending
 	// this to the provider
-	unmarkedConfigVal := configVal
-	if configVal.ContainsMarked() {
-		unmarkedConfigVal, _ = configVal.UnmarkDeep()
-	}
+	unmarkedConfigVal, _ := configVal.UnmarkDeep()
+	unmarkedBefore, beforePaths := change.Before.UnmarkDeepWithPaths()
+	unmarkedAfter, afterPaths := change.After.UnmarkDeepWithPaths()
 
-	unmarkedBefore := change.Before
-	if change.Before.ContainsMarked() {
-		unmarkedBefore, _ = change.Before.UnmarkDeep()
-	}
-
-	unmarkedAfter := change.After
-	var afterPaths []cty.PathValueMarks
-	if change.After.ContainsMarked() {
-		unmarkedAfter, afterPaths = change.After.UnmarkDeepWithPaths()
+	// If we have an Update action, our before and after values are equal,
+	// and only differ on their sensitivity, the newVal is the after val
+	// and we should not communicate with the provider or perform further action.
+	eqV := unmarkedBefore.Equals(unmarkedAfter)
+	eq := eqV.IsKnown() && eqV.True()
+	if change.Action == plans.Update && eq && (len(beforePaths) != len(afterPaths)) {
+		return nil, diags.ErrWithWarnings()
 	}
 
 	resp := provider.ApplyResourceChange(providers.ApplyResourceChangeRequest{

terraform/eval_diff.go

@@ -216,7 +216,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 	// Store the paths for the config val to re-markafter
 	// we've sent things over the wire.
 	unmarkedConfigVal, unmarkedPaths := configValIgnored.UnmarkDeepWithPaths()
-	unmarkedPriorVal, priorPaths := priorVal.UnmarkDeep()
+	unmarkedPriorVal, priorPaths := priorVal.UnmarkDeepWithPaths()
 
 	proposedNewVal := objchange.ProposedNewObject(schema, unmarkedPriorVal, unmarkedConfigVal)
 
@@ -393,11 +393,6 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 		action = plans.Create
 	case eq:
 		action = plans.NoOp
-		// If we plan to write or delete sensitive paths from state,
-		// this is an Update action
-		if len(priorPaths) != len(unmarkedPaths) {
-			action = plans.Update
-		}
 	case !reqRep.Empty():
 		// If there are any "requires replace" paths left _after our filtering
 		// above_ then this is a replace action.
@@ -487,6 +482,12 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 		priorVal = priorValTainted
 	}
 
+	// If we plan to write or delete sensitive paths from state,
+	// this is an Update action
+	if action == plans.NoOp && len(priorPaths) != len(unmarkedPaths) {
+		action = plans.Update
+	}
+
 	// As a special case, if we have a previous diff (presumably from the plan
 	// phases, whereas we're now in the apply phase) and it was for a replace,
 	// we've already deleted the original object from state by the time we