diff --git a/states/module.go b/states/module.go index 760625e05..fbef01c7a 100644 --- a/states/module.go +++ b/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! diff --git a/states/sync.go b/states/sync.go index d0923b150..610fc07aa 100644 --- a/states/sync.go +++ b/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. diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 640a2041b..2b96af22d 100644 --- a/terraform/context_apply_test.go +++ b/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)) + } +} diff --git a/terraform/eval_apply.go b/terraform/eval_apply.go index c5b6a4cad..5832fcd63 100644 --- a/terraform/eval_apply.go +++ b/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{ diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go index 669ff6687..a639d23b9 100644 --- a/terraform/eval_diff.go +++ b/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 diff --git a/terraform/eval_variable.go b/terraform/eval_variable.go index 98dfab1ab..72734b788 100644 --- a/terraform/eval_variable.go +++ b/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