diff --git a/command/format/diff_test.go b/command/format/diff_test.go index b885791a3..d333315d1 100644 --- a/command/format/diff_test.go +++ b/command/format/diff_test.go @@ -3624,24 +3624,13 @@ func TestResourceChange_nestedMap(t *testing.T) { func TestResourceChange_sensitiveVariable(t *testing.T) { testCases := map[string]testCase{ - "in-place update - creation": { - Action: plans.Update, + "creation": { + Action: plans.Create, Mode: addrs.ManagedResourceMode, - Before: cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal("i-02ae66f368e8518a9"), - "ami": cty.StringVal("ami-BEFORE"), - "root_block_device": cty.MapValEmpty(cty.Object(map[string]cty.Type{ - "volume_type": cty.String, - })), - }), + Before: cty.NullVal(cty.EmptyObject), After: cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("i-02ae66f368e8518a9"), - "ami": cty.StringVal("ami-AFTER"), - "root_block_device": cty.MapVal(map[string]cty.Value{ - "a": cty.ObjectVal(map[string]cty.Value{ - "volume_type": cty.StringVal("gp2"), - }), - }), + "ami": cty.StringVal("ami-123"), }), AfterValMarks: []cty.PathValueMarks{ { @@ -3655,29 +3644,141 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { "id": {Type: cty.String, Optional: true, Computed: true}, "ami": {Type: cty.String, Optional: true}, }, - BlockTypes: map[string]*configschema.NestedBlock{ - "root_block_device": { - Block: configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "volume_type": { - Type: cty.String, - Optional: true, - Computed: true, - }, - }, - }, - Nesting: configschema.NestingMap, - }, + }, + ExpectedOutput: ` # test_instance.example will be created + + resource "test_instance" "example" { + + ami = (sensitive) + + id = "i-02ae66f368e8518a9" + } +`, + }, + "in-place update - before sensitive": { + Action: plans.Update, + Mode: addrs.ManagedResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "ami": cty.StringVal("ami-BEFORE"), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "ami": cty.StringVal("ami-AFTER"), + }), + BeforeValMarks: []cty.PathValueMarks{ + { + Path: cty.Path{cty.GetAttrStep{Name: "ami"}}, + Marks: cty.NewValueMarks("sensitive"), + }}, + RequiredReplace: cty.NewPathSet(), + Tainted: false, + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "ami": {Type: cty.String, Optional: true}, }, }, ExpectedOutput: ` # test_instance.example will be updated in-place ~ resource "test_instance" "example" { ~ ami = (sensitive) id = "i-02ae66f368e8518a9" - - + root_block_device "a" { - + volume_type = "gp2" - } + } +`, + }, + "in-place update - after sensitive, map type": { + Action: plans.Update, + Mode: addrs.ManagedResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "tags": cty.MapVal(map[string]cty.Value{ + "name": cty.StringVal("anna"), + }), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "tags": cty.MapVal(map[string]cty.Value{ + "name": cty.StringVal("bob"), + }), + }), + AfterValMarks: []cty.PathValueMarks{ + { + Path: cty.Path{cty.GetAttrStep{Name: "tags"}, cty.IndexStep{Key: cty.StringVal("name")}}, + Marks: cty.NewValueMarks("sensitive"), + }}, + RequiredReplace: cty.NewPathSet(), + Tainted: false, + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "tags": {Type: cty.Map(cty.String), Optional: true}, + }, + }, + ExpectedOutput: ` # test_instance.example will be updated in-place + ~ resource "test_instance" "example" { + id = "i-02ae66f368e8518a9" + ~ tags = (sensitive) + } +`, + }, + "in-place update - both sensitive": { + Action: plans.Update, + Mode: addrs.ManagedResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "ami": cty.StringVal("ami-BEFORE"), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "ami": cty.StringVal("ami-AFTER"), + }), + BeforeValMarks: []cty.PathValueMarks{ + { + Path: cty.Path{cty.GetAttrStep{Name: "ami"}}, + Marks: cty.NewValueMarks("sensitive"), + }}, + AfterValMarks: []cty.PathValueMarks{ + { + Path: cty.Path{cty.GetAttrStep{Name: "ami"}}, + Marks: cty.NewValueMarks("sensitive"), + }}, + RequiredReplace: cty.NewPathSet(), + Tainted: false, + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "ami": {Type: cty.String, Optional: true}, + }, + }, + ExpectedOutput: ` # test_instance.example will be updated in-place + ~ resource "test_instance" "example" { + ~ ami = (sensitive) + id = "i-02ae66f368e8518a9" + } +`, + }, + "deletion": { + Action: plans.Delete, + Mode: addrs.ManagedResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "ami": cty.StringVal("ami-BEFORE"), + }), + After: cty.NullVal(cty.EmptyObject), + BeforeValMarks: []cty.PathValueMarks{ + { + Path: cty.Path{cty.GetAttrStep{Name: "ami"}}, + Marks: cty.NewValueMarks("sensitive"), + }}, + RequiredReplace: cty.NewPathSet(), + Tainted: false, + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "ami": {Type: cty.String, Optional: true}, + }, + }, + ExpectedOutput: ` # test_instance.example will be destroyed + - resource "test_instance" "example" { + - ami = (sensitive) + - id = "i-02ae66f368e8518a9" -> null } `, }, diff --git a/providers/provider.go b/providers/provider.go index b27e3dbce..2d9db44bb 100644 --- a/providers/provider.go +++ b/providers/provider.go @@ -238,7 +238,7 @@ type PlanResourceChangeResponse struct { // configuration is applied. PlannedState cty.Value - // RequiresReplace is the list of thee attributes that are requiring + // RequiresReplace is the list of the attributes that are requiring // resource replacement. RequiresReplace []cty.Path diff --git a/states/instance_object.go b/states/instance_object.go index 2a9e58327..a9ebf4804 100644 --- a/states/instance_object.go +++ b/states/instance_object.go @@ -98,10 +98,11 @@ func (o *ResourceInstanceObject) Encode(ty cty.Type, schemaVersion uint64) (*Res // and raise an error about that. val := cty.UnknownAsNull(o.Value) - // If it contains marks, dump those now + // If it contains marks, save these in state unmarked := val + var pvm []cty.PathValueMarks if val.ContainsMarked() { - unmarked, _ = val.UnmarkDeep() + unmarked, pvm = val.UnmarkDeepWithPaths() } src, err := ctyjson.Marshal(unmarked, ty) if err != nil { @@ -111,6 +112,7 @@ func (o *ResourceInstanceObject) Encode(ty cty.Type, schemaVersion uint64) (*Res return &ResourceInstanceObjectSrc{ SchemaVersion: schemaVersion, AttrsJSON: src, + AttrSensitivePaths: pvm, Private: o.Private, Status: o.Status, Dependencies: o.Dependencies, diff --git a/states/instance_object_src.go b/states/instance_object_src.go index bf790db04..aeb612eaa 100644 --- a/states/instance_object_src.go +++ b/states/instance_object_src.go @@ -49,6 +49,10 @@ type ResourceInstanceObjectSrc struct { // the recommendations in the AttrsJSON documentation above. AttrsFlat map[string]string + // AttrSensitivePaths is an array of paths to mark as sensitive coming out of + // state, or to save as sensitive paths when saving state + AttrSensitivePaths []cty.PathValueMarks + // These fields all correspond to the fields of the same name on // ResourceInstanceObject. Private []byte @@ -78,6 +82,10 @@ func (os *ResourceInstanceObjectSrc) Decode(ty cty.Type) (*ResourceInstanceObjec } } else { val, err = ctyjson.Unmarshal(os.AttrsJSON, ty) + // Mark the value with paths if applicable + if os.AttrSensitivePaths != nil { + val = val.MarkWithPaths(os.AttrSensitivePaths) + } if err != nil { return nil, err } diff --git a/states/state_deepcopy.go b/states/state_deepcopy.go index f6a3919c3..93e96d756 100644 --- a/states/state_deepcopy.go +++ b/states/state_deepcopy.go @@ -144,6 +144,12 @@ func (obj *ResourceInstanceObjectSrc) DeepCopy() *ResourceInstanceObjectSrc { copy(attrsJSON, obj.AttrsJSON) } + var attrPaths []cty.PathValueMarks + if obj.AttrSensitivePaths != nil { + attrPaths = make([]cty.PathValueMarks, len(obj.AttrSensitivePaths)) + copy(attrPaths, obj.AttrSensitivePaths) + } + var private []byte if obj.Private != nil { private = make([]byte, len(obj.Private)) @@ -164,6 +170,7 @@ func (obj *ResourceInstanceObjectSrc) DeepCopy() *ResourceInstanceObjectSrc { Private: private, AttrsFlat: attrsFlat, AttrsJSON: attrsJSON, + AttrSensitivePaths: attrPaths, Dependencies: dependencies, CreateBeforeDestroy: obj.CreateBeforeDestroy, } diff --git a/states/state_test.go b/states/state_test.go index 9f37ec3a1..6b3eb815e 100644 --- a/states/state_test.go +++ b/states/state_test.go @@ -230,7 +230,14 @@ func TestStateDeepCopy(t *testing.T) { Status: ObjectReady, SchemaVersion: 1, AttrsJSON: []byte(`{"woozles":"confuzles"}`), - Private: []byte("private data"), + // Sensitive path at "woozles" + AttrSensitivePaths: []cty.PathValueMarks{ + { + Path: cty.Path{cty.GetAttrStep{Name: "woozles"}}, + Marks: cty.NewValueMarks("sensitive"), + }, + }, + Private: []byte("private data"), Dependencies: []addrs.ConfigResource{ { Module: addrs.RootModule, diff --git a/states/statefile/version4.go b/states/statefile/version4.go index 450062a5a..8d32eb7ad 100644 --- a/states/statefile/version4.go +++ b/states/statefile/version4.go @@ -7,6 +7,7 @@ import ( "sort" version "github.com/hashicorp/go-version" + "github.com/zclconf/go-cty/cty" ctyjson "github.com/zclconf/go-cty/cty/json" "github.com/hashicorp/terraform/addrs" @@ -151,6 +152,24 @@ func prepareStateV4(sV4 *stateV4) (*File, tfdiags.Diagnostics) { } } + // Sensitive paths + if isV4.AttributeSensitivePaths != nil { + paths, pathsDiags := unmarshalPaths([]byte(isV4.AttributeSensitivePaths)) + diags = diags.Append(pathsDiags) + if pathsDiags.HasErrors() { + continue + } + + var pvm []cty.PathValueMarks + for _, path := range paths { + pvm = append(pvm, cty.PathValueMarks{ + Path: path, + Marks: cty.NewValueMarks("sensitive"), + }) + } + obj.AttrSensitivePaths = pvm + } + { // Status raw := isV4.Status @@ -452,16 +471,27 @@ func appendInstanceObjectStateV4(rs *states.Resource, is *states.ResourceInstanc } } + // Extract paths from path value marks + var paths []cty.Path + for _, vm := range obj.AttrSensitivePaths { + paths = append(paths, vm.Path) + } + + // Marshal paths to JSON + attributeSensitivePaths, pathsDiags := marshalPaths(paths) + diags = diags.Append(pathsDiags) + return append(isV4s, instanceObjectStateV4{ - IndexKey: rawKey, - Deposed: string(deposed), - Status: status, - SchemaVersion: obj.SchemaVersion, - AttributesFlat: obj.AttrsFlat, - AttributesRaw: obj.AttrsJSON, - PrivateRaw: privateRaw, - Dependencies: deps, - CreateBeforeDestroy: obj.CreateBeforeDestroy, + IndexKey: rawKey, + Deposed: string(deposed), + Status: status, + SchemaVersion: obj.SchemaVersion, + AttributesFlat: obj.AttrsFlat, + AttributesRaw: obj.AttrsJSON, + AttributeSensitivePaths: attributeSensitivePaths, + PrivateRaw: privateRaw, + Dependencies: deps, + CreateBeforeDestroy: obj.CreateBeforeDestroy, }), diags } @@ -505,9 +535,10 @@ type instanceObjectStateV4 struct { Status string `json:"status,omitempty"` Deposed string `json:"deposed,omitempty"` - SchemaVersion uint64 `json:"schema_version"` - AttributesRaw json.RawMessage `json:"attributes,omitempty"` - AttributesFlat map[string]string `json:"attributes_flat,omitempty"` + SchemaVersion uint64 `json:"schema_version"` + AttributesRaw json.RawMessage `json:"attributes,omitempty"` + AttributesFlat map[string]string `json:"attributes_flat,omitempty"` + AttributeSensitivePaths json.RawMessage `json:"sensitive_attributes,omitempty,"` PrivateRaw []byte `json:"private,omitempty"` @@ -577,3 +608,135 @@ func (si sortInstancesV4) Less(i, j int) bool { } return false } + +// pathStep is an intermediate representation of a cty.PathStep to facilitate +// consistent JSON serialization. The Value field can either be a cty.Value of +// dynamic type (for index steps), or a string (for get attr steps). +type pathStep struct { + Type string `json:"type"` + Value json.RawMessage `json:"value"` +} + +const ( + indexPathStepType = "index" + getAttrPathStepType = "get_attr" +) + +func unmarshalPaths(buf []byte) ([]cty.Path, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + var jsonPaths [][]pathStep + + err := json.Unmarshal(buf, &jsonPaths) + if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Error unmarshaling path steps", + err.Error(), + )) + } + + paths := make([]cty.Path, 0, len(jsonPaths)) + +unmarshalOuter: + for _, jsonPath := range jsonPaths { + var path cty.Path + for _, jsonStep := range jsonPath { + switch jsonStep.Type { + case indexPathStepType: + key, err := ctyjson.Unmarshal(jsonStep.Value, cty.DynamicPseudoType) + if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Error unmarshaling path step", + fmt.Sprintf("Failed to unmarshal index step key: %s", err), + )) + continue unmarshalOuter + } + path = append(path, cty.IndexStep{Key: key}) + case getAttrPathStepType: + var name string + if err := json.Unmarshal(jsonStep.Value, &name); err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Error unmarshaling path step", + fmt.Sprintf("Failed to unmarshal get attr step name: %s", err), + )) + continue unmarshalOuter + } + path = append(path, cty.GetAttrStep{Name: name}) + default: + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Unsupported path step", + fmt.Sprintf("Unsupported path step type %q", jsonStep.Type), + )) + continue unmarshalOuter + } + } + paths = append(paths, path) + } + + return paths, diags +} + +func marshalPaths(paths []cty.Path) ([]byte, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + + // cty.Path is a slice of cty.PathSteps, so our representation of a slice + // of paths is a nested slice of our intermediate pathStep struct + jsonPaths := make([][]pathStep, 0, len(paths)) + +marshalOuter: + for _, path := range paths { + jsonPath := make([]pathStep, 0, len(path)) + for _, step := range path { + var jsonStep pathStep + switch s := step.(type) { + case cty.IndexStep: + key, err := ctyjson.Marshal(s.Key, cty.DynamicPseudoType) + if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Error marshaling path step", + fmt.Sprintf("Failed to marshal index step key %#v: %s", s.Key, err), + )) + continue marshalOuter + } + jsonStep.Type = indexPathStepType + jsonStep.Value = key + case cty.GetAttrStep: + name, err := json.Marshal(s.Name) + if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Error marshaling path step", + fmt.Sprintf("Failed to marshal get attr step name %s: %s", s.Name, err), + )) + continue marshalOuter + } + jsonStep.Type = getAttrPathStepType + jsonStep.Value = name + default: + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Unsupported path step", + fmt.Sprintf("Unsupported path step %#v (%t)", step, step), + )) + continue marshalOuter + } + jsonPath = append(jsonPath, jsonStep) + } + jsonPaths = append(jsonPaths, jsonPath) + } + + buf, err := json.Marshal(jsonPaths) + if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Error marshaling path steps", + fmt.Sprintf("Failed to marshal path steps: %s", err), + )) + } + + return buf, diags +} diff --git a/states/statefile/version4_test.go b/states/statefile/version4_test.go index c59ad5c28..f097a6069 100644 --- a/states/statefile/version4_test.go +++ b/states/statefile/version4_test.go @@ -2,7 +2,11 @@ package statefile import ( "sort" + "strings" "testing" + + "github.com/hashicorp/terraform/tfdiags" + "github.com/zclconf/go-cty/cty" ) // This test verifies that modules are sorted before resources: @@ -39,3 +43,216 @@ func TestVersion4_sort(t *testing.T) { } } } + +func TestVersion4_unmarshalPaths(t *testing.T) { + testCases := map[string]struct { + json string + paths []cty.Path + diags []string + }{ + "no paths": { + json: `[]`, + paths: []cty.Path{}, + }, + "attribute path": { + json: `[ + [ + { + "type": "get_attr", + "value": "password" + } + ] +]`, + paths: []cty.Path{cty.GetAttrPath("password")}, + }, + "attribute and string index": { + json: `[ + [ + { + "type": "get_attr", + "value": "triggers" + }, + { + "type": "index", + "value": { + "value": "secret", + "type": "string" + } + } + ] +]`, + paths: []cty.Path{cty.GetAttrPath("triggers").IndexString("secret")}, + }, + "attribute, number index, attribute": { + json: `[ + [ + { + "type": "get_attr", + "value": "identities" + }, + { + "type": "index", + "value": { + "value": 2, + "type": "number" + } + }, + { + "type": "get_attr", + "value": "private_key" + } + ] +]`, + paths: []cty.Path{cty.GetAttrPath("identities").IndexInt(2).GetAttr("private_key")}, + }, + "multiple paths": { + json: `[ + [ + { + "type": "get_attr", + "value": "alpha" + } + ], + [ + { + "type": "get_attr", + "value": "beta" + } + ], + [ + { + "type": "get_attr", + "value": "gamma" + } + ] +]`, + paths: []cty.Path{cty.GetAttrPath("alpha"), cty.GetAttrPath("beta"), cty.GetAttrPath("gamma")}, + }, + "errors": { + json: `[ + [ + { + "type": "get_attr", + "value": 5 + } + ], + [ + { + "type": "index", + "value": "test" + } + ], + [ + { + "type": "invalid_type", + "value": ["this is invalid too"] + } + ] +]`, + paths: []cty.Path{}, + diags: []string{ + "Failed to unmarshal get attr step name", + "Failed to unmarshal index step key", + "Unsupported path step", + }, + }, + "one invalid path, others valid": { + json: `[ + [ + { + "type": "get_attr", + "value": "alpha" + } + ], + [ + { + "type": "invalid_type", + "value": ["this is invalid too"] + } + ], + [ + { + "type": "get_attr", + "value": "gamma" + } + ] +]`, + paths: []cty.Path{cty.GetAttrPath("alpha"), cty.GetAttrPath("gamma")}, + diags: []string{"Unsupported path step"}, + }, + "invalid structure": { + json: `{}`, + paths: []cty.Path{}, + diags: []string{"Error unmarshaling path steps"}, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + paths, diags := unmarshalPaths([]byte(tc.json)) + + if len(tc.diags) == 0 { + if len(diags) != 0 { + t.Errorf("expected no diags, got: %#v", diags) + } + } else { + if got, want := len(diags), len(tc.diags); got != want { + t.Fatalf("got %d diags, want %d\n%s", got, want, diags.Err()) + } + for i := range tc.diags { + got := tfdiags.Diagnostics{diags[i]}.Err().Error() + if !strings.Contains(got, tc.diags[i]) { + t.Errorf("expected diag %d to contain %q, but was:\n%s", i, tc.diags[i], got) + } + } + } + + if len(paths) != len(tc.paths) { + t.Fatalf("got %d paths, want %d", len(paths), len(tc.paths)) + } + for i, path := range paths { + if !path.Equals(tc.paths[i]) { + t.Errorf("wrong paths\n got: %#v\nwant: %#v", path, tc.paths[i]) + } + } + }) + } +} + +func TestVersion4_marshalPaths(t *testing.T) { + testCases := map[string]struct { + paths []cty.Path + json string + }{ + "no paths": { + paths: []cty.Path{}, + json: `[]`, + }, + "attribute path": { + paths: []cty.Path{cty.GetAttrPath("password")}, + json: `[[{"type":"get_attr","value":"password"}]]`, + }, + "attribute, number index, attribute": { + paths: []cty.Path{cty.GetAttrPath("identities").IndexInt(2).GetAttr("private_key")}, + json: `[[{"type":"get_attr","value":"identities"},{"type":"index","value":{"value":2,"type":"number"}},{"type":"get_attr","value":"private_key"}]]`, + }, + "multiple paths": { + paths: []cty.Path{cty.GetAttrPath("a"), cty.GetAttrPath("b"), cty.GetAttrPath("c")}, + json: `[[{"type":"get_attr","value":"a"}],[{"type":"get_attr","value":"b"}],[{"type":"get_attr","value":"c"}]]`, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + json, diags := marshalPaths(tc.paths) + + if len(diags) != 0 { + t.Fatalf("expected no diags, got: %#v", diags) + } + + if got, want := string(json), tc.json; got != want { + t.Fatalf("wrong JSON output\n got: %s\nwant: %s\n", got, want) + } + }) + } +} diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 182b678b8..255cb91c8 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -11916,3 +11916,82 @@ resource "test_resource" "c" { t.Fatalf("apply errors: %s", diags.Err()) } } + +func TestContext2Apply_variableSensitivity(t *testing.T) { + m := testModuleInline(t, map[string]string{ + "main.tf": ` +terraform { + experiments = [sensitive_variables] +} + +variable "sensitive_var" { + default = "foo" + 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), + }, + }) + + if _, diags := ctx.Plan(); diags.HasErrors() { + t.Fatalf("plan errors: %s", diags.Err()) + } + + state, diags := ctx.Apply() + if diags.HasErrors() { + t.Fatalf("apply errors: %s", diags.Err()) + } + + // Run a second apply with no changes + ctx = testContext2(t, &ContextOpts{ + Config: m, + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + State: state, + }) + + if _, diags := ctx.Plan(); diags.HasErrors() { + t.Fatalf("plan errors: %s", diags.Err()) + } + + state, diags = ctx.Apply() + if diags.HasErrors() { + t.Fatalf("apply errors: %s", diags.Err()) + } + + // Now change the variable value + ctx = testContext2(t, &ContextOpts{ + Config: m, + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + Variables: InputValues{ + "sensitive_var": &InputValue{ + Value: cty.StringVal("bar"), + }, + }, + State: state, + }) + + if _, diags := ctx.Plan(); diags.HasErrors() { + t.Fatalf("plan errors: %s", diags.Err()) + } + + _, diags = ctx.Apply() + if diags.HasErrors() { + t.Fatalf("apply errors: %s", diags.Err()) + } +} diff --git a/terraform/eval_apply.go b/terraform/eval_apply.go index 1a2eef0bd..90040c84c 100644 --- a/terraform/eval_apply.go +++ b/terraform/eval_apply.go @@ -105,21 +105,28 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) { log.Printf("[DEBUG] %s: applying the planned %s change", n.Addr.Absolute(ctx.Path()), change.Action) - // If our config or After value contain any marked values, + // 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() } + + unmarkedBefore := change.Before + if change.Before.ContainsMarked() { + unmarkedBefore, _ = change.Before.UnmarkDeep() + } + unmarkedAfter := change.After + var afterPaths []cty.PathValueMarks if change.After.ContainsMarked() { - unmarkedAfter, _ = change.After.UnmarkDeep() + unmarkedAfter, afterPaths = change.After.UnmarkDeepWithPaths() } resp := provider.ApplyResourceChange(providers.ApplyResourceChangeRequest{ TypeName: n.Addr.Resource.Type, - PriorState: change.Before, + PriorState: unmarkedBefore, Config: unmarkedConfigVal, PlannedState: unmarkedAfter, PlannedPrivate: change.Private, @@ -138,6 +145,11 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) { // incomplete. newVal := resp.NewState + // If we have paths to mark, mark those on this new value + if len(afterPaths) > 0 { + newVal = newVal.MarkWithPaths(afterPaths) + } + 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 diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go index 366e5ddd2..f8fa83a96 100644 --- a/terraform/eval_diff.go +++ b/terraform/eval_diff.go @@ -213,7 +213,14 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { priorVal = cty.NullVal(schema.ImpliedType()) } - proposedNewVal := objchange.ProposedNewObject(schema, priorVal, unmarkedConfigVal) + 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() + } + + proposedNewVal := objchange.ProposedNewObject(schema, unmarkedPriorVal, unmarkedConfigVal) // Call pre-diff hook if !n.Stub { @@ -244,7 +251,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { // we send back this information, we need to process ignore_changes // so that CustomizeDiff will not act on them var ignoreChangeDiags tfdiags.Diagnostics - proposedNewVal, ignoreChangeDiags = n.processIgnoreChanges(priorVal, proposedNewVal) + proposedNewVal, ignoreChangeDiags = n.processIgnoreChanges(unmarkedPriorVal, proposedNewVal) diags = diags.Append(ignoreChangeDiags) if ignoreChangeDiags.HasErrors() { return nil, diags.Err() @@ -253,7 +260,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { resp := provider.PlanResourceChange(providers.PlanResourceChangeRequest{ TypeName: n.Addr.Resource.Type, Config: unmarkedConfigVal, - PriorState: priorVal, + PriorState: unmarkedPriorVal, ProposedNewState: proposedNewVal, PriorPrivate: priorPrivate, ProviderMeta: metaConfigVal, @@ -274,7 +281,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { } // Add the marks back to the planned new value - if configVal.ContainsMarked() { + if len(unmarkedPaths) > 0 { plannedNewVal = plannedNewVal.MarkWithPaths(unmarkedPaths) } @@ -352,7 +359,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { continue } - priorChangedVal, priorPathDiags := hcl.ApplyPath(priorVal, path, nil) + priorChangedVal, priorPathDiags := hcl.ApplyPath(unmarkedPriorVal, path, nil) plannedChangedVal, plannedPathDiags := hcl.ApplyPath(plannedNewVal, path, nil) if plannedPathDiags.HasErrors() && priorPathDiags.HasErrors() { // This means the path was invalid in both the prior and new @@ -384,7 +391,10 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { plannedChangedVal = cty.NullVal(priorChangedVal.Type()) } - eqV := plannedChangedVal.Equals(priorChangedVal) + // Unmark for this value for the equality test. If only sensitivity has changed, + // this does not require an Update or Replace + unmarkedPlannedChangedVal, _ := plannedChangedVal.UnmarkDeep() + eqV := unmarkedPlannedChangedVal.Equals(priorChangedVal) if !eqV.IsKnown() || eqV.False() { reqRep.Add(path) } @@ -394,7 +404,10 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { } } - eqV := plannedNewVal.Equals(priorVal) + // Unmark for this test for equality. If only sensitivity has changed, + // this does not require an Update or Replace + unmarkedPlannedNewVal, _ := plannedNewVal.UnmarkDeep() + eqV := unmarkedPlannedNewVal.Equals(unmarkedPriorVal) eq := eqV.IsKnown() && eqV.True() var action plans.Action @@ -432,11 +445,11 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { nullPriorVal := cty.NullVal(schema.ImpliedType()) // create a new proposed value from the null state and the config - proposedNewVal = objchange.ProposedNewObject(schema, nullPriorVal, configVal) + proposedNewVal = objchange.ProposedNewObject(schema, nullPriorVal, unmarkedConfigVal) resp = provider.PlanResourceChange(providers.PlanResourceChangeRequest{ TypeName: n.Addr.Resource.Type, - Config: configVal, + Config: unmarkedConfigVal, PriorState: nullPriorVal, ProposedNewState: proposedNewVal, PriorPrivate: plannedPrivate, @@ -453,6 +466,11 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { } plannedNewVal = resp.PlannedState plannedPrivate = resp.PlannedPrivate + + if len(unmarkedPaths) > 0 { + plannedNewVal = plannedNewVal.MarkWithPaths(unmarkedPaths) + } + for _, err := range plannedNewVal.Type().TestConformance(schema.ImpliedType()) { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, diff --git a/terraform/eval_refresh.go b/terraform/eval_refresh.go index d56291c19..021da2b5d 100644 --- a/terraform/eval_refresh.go +++ b/terraform/eval_refresh.go @@ -80,6 +80,13 @@ func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) { // Refresh! priorVal := state.Value + + // Unmarked before sending to provider + var priorPaths []cty.PathValueMarks + if priorVal.ContainsMarked() { + priorVal, priorPaths = priorVal.UnmarkDeepWithPaths() + } + req := providers.ReadResourceRequest{ TypeName: n.Addr.Resource.Type, PriorState: priorVal, @@ -129,6 +136,11 @@ func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) { return nil, err } + // Mark the value if necessary + if len(priorPaths) > 0 { + newState.Value = newState.Value.MarkWithPaths(priorPaths) + } + if n.Output != nil { *n.Output = newState }