Merge pull request #26507 from hashicorp/pselle/sensitive-vars-change

Update state when sensitivity changes

states/module.go

@@ -76,16 +76,14 @@ func (ms *Module) RemoveResource(addr addrs.Resource) {
 
 // SetResourceInstanceCurrent saves the given instance object as the current
 // generation of the resource instance with the given address, simultaneously
-// updating the recorded provider configuration address, dependencies, and
-// resource EachMode.
+// updating the recorded provider configuration address and dependencies.
 //
 // Any existing current instance object for the given resource is overwritten.
 // Set obj to nil to remove the primary generation object altogether. If there
 // are no deposed objects then the instance will be removed altogether.
 //
-// The provider address and "each mode" are resource-wide settings and so they
-// are updated for all other instances of the same resource as a side-effect of
-// this call.
+// The provider address is a resource-wide setting and is updated for all other
+// instances of the same resource as a side-effect of this call.
 func (ms *Module) SetResourceInstanceCurrent(addr addrs.ResourceInstance, obj *ResourceInstanceObjectSrc, provider addrs.AbsProviderConfig) {
 	rs := ms.Resource(addr.Resource)
 	// if the resource is nil and the object is nil, don't do anything!

states/sync.go

@@ -316,9 +316,8 @@ func (s *SyncState) MaybeFixUpResourceInstanceAddressForCount(addr addrs.ConfigR
 // concurrently mutated during this call, but may be freely used again once
 // this function returns.
 //
-// The provider address and "each mode" are resource-wide settings and so they
-// are updated for all other instances of the same resource as a side-effect of
-// this call.
+// The provider address is a resource-wide settings and is updated
+// for all other instances of the same resource as a side-effect of this call.
 //
 // If the containing module for this resource or the resource itself are not
 // already tracked in state then they will be added as a side-effect.

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

@@ -3,6 +3,7 @@ package terraform
 import (
 	"fmt"
 	"log"
+	"reflect"
 	"strings"
 
 	multierror "github.com/hashicorp/go-multierror"
@@ -109,20 +110,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 && !reflect.DeepEqual(beforePaths, afterPaths) {
+		return nil, diags.ErrWithWarnings()
 	}
 
 	resp := provider.ApplyResourceChange(providers.ApplyResourceChangeRequest{

terraform/eval_diff.go

@@ -3,6 +3,7 @@ package terraform
 import (
 	"fmt"
 	"log"
+	"reflect"
 	"strings"
 
 	"github.com/hashicorp/hcl/v2"
@@ -212,23 +213,11 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 		return nil, diags.Err()
 	}
 
-	// Create an unmarked version of our config val, defaulting
-	// to the configVal so we don't do the work of unmarking unless
-	// necessary
-	unmarkedConfigVal := configValIgnored
-	var unmarkedPaths []cty.PathValueMarks
-	if configValIgnored.ContainsMarked() {
-		// store the marked values so we can re-mark them later after
-		// we've sent things over the wire.
-		unmarkedConfigVal, unmarkedPaths = configValIgnored.UnmarkDeepWithPaths()
-	}
-
-	unmarkedPriorVal := priorVal
-	if priorVal.ContainsMarked() {
-		// store the marked values so we can re-mark them later after
-		// we've sent things over the wire.
-		unmarkedPriorVal, _ = priorVal.UnmarkDeep()
-	}
+	// Create an unmarked version of our config val and our prior val.
+	// Store the paths for the config val to re-markafter
+	// we've sent things over the wire.
+	unmarkedConfigVal, unmarkedPaths := configValIgnored.UnmarkDeepWithPaths()
+	unmarkedPriorVal, priorPaths := priorVal.UnmarkDeepWithPaths()
 
 	proposedNewVal := objchange.ProposedNewObject(schema, unmarkedPriorVal, unmarkedConfigVal)
 
@@ -395,8 +384,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 		}
 	}
 
-	// Unmark for this test for equality. If only sensitivity has changed,
-	// this does not require an Update or Replace
+	// Unmark for this test for value equality.
 	eqV := unmarkedPlannedNewVal.Equals(unmarkedPriorVal)
 	eq := eqV.IsKnown() && eqV.True()
 
@@ -495,6 +483,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 && !reflect.DeepEqual(priorPaths, 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

terraform/eval_variable.go

@@ -12,7 +12,7 @@ import (
 	"github.com/zclconf/go-cty/cty/convert"
 )
 
-// evalVariableValidations ensures ta all of the configured custom validations
+// evalVariableValidations ensures that all of the configured custom validations
 // for a variable are passing.
 //
 // This must be used only after any side-effects that make the value of the