diff --git a/command/jsonplan/plan.go b/command/jsonplan/plan.go index 4d8dccd95..8474ceac0 100644 --- a/command/jsonplan/plan.go +++ b/command/jsonplan/plan.go @@ -213,7 +213,11 @@ func (p *plan) marshalResourceChanges(changes *plans.Changes, schemas *terraform if err != nil { return err } - bs := sensitiveAsBool(changeV.Before.MarkWithPaths(rc.BeforeValMarks)) + marks := rc.BeforeValMarks + if schema.ContainsSensitive() { + marks = append(marks, schema.ValueMarks(changeV.Before, nil)...) + } + bs := sensitiveAsBool(changeV.Before.MarkWithPaths(marks)) beforeSensitive, err = ctyjson.Marshal(bs, bs.Type()) if err != nil { return err @@ -238,7 +242,11 @@ func (p *plan) marshalResourceChanges(changes *plans.Changes, schemas *terraform } afterUnknown = unknownAsBool(changeV.After) } - as := sensitiveAsBool(changeV.After.MarkWithPaths(rc.AfterValMarks)) + marks := rc.AfterValMarks + if schema.ContainsSensitive() { + marks = append(marks, schema.ValueMarks(changeV.After, nil)...) + } + as := sensitiveAsBool(changeV.After.MarkWithPaths(marks)) afterSensitive, err = ctyjson.Marshal(as, as.Type()) if err != nil { return err diff --git a/command/show_test.go b/command/show_test.go index e0172ba58..2fcafcc19 100644 --- a/command/show_test.go +++ b/command/show_test.go @@ -343,6 +343,88 @@ func TestShow_json_output(t *testing.T) { } } +func TestShow_json_output_sensitive(t *testing.T) { + td := tempDir(t) + inputDir := "testdata/show-json-sensitive" + testCopyDir(t, inputDir, td) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + providerSource, close := newMockProviderSource(t, map[string][]string{"test": {"1.2.3"}}) + defer close() + + p := showFixtureSensitiveProvider() + ui := new(cli.MockUi) + view, _ := testView(t) + m := Meta{ + testingOverrides: metaOverridesForProvider(p), + Ui: ui, + View: view, + ProviderSource: providerSource, + } + + // init + ic := &InitCommand{ + Meta: m, + } + if code := ic.Run([]string{}); code != 0 { + t.Fatalf("init failed\n%s", ui.ErrorWriter) + } + + // flush init output + ui.OutputWriter.Reset() + + pc := &PlanCommand{ + Meta: m, + } + + args := []string{ + "-out=terraform.plan", + } + + if code := pc.Run(args); code != 0 { + fmt.Println(ui.OutputWriter.String()) + t.Fatalf("wrong exit status %d; want 0\nstderr: %s", code, ui.ErrorWriter.String()) + } + + // flush the plan output from the mock ui + ui.OutputWriter.Reset() + sc := &ShowCommand{ + Meta: m, + } + + args = []string{ + "-json", + "terraform.plan", + } + defer os.Remove("terraform.plan") + + if code := sc.Run(args); code != 0 { + t.Fatalf("wrong exit status %d; want 0\nstderr: %s", code, ui.ErrorWriter.String()) + } + + // compare ui output to wanted output + var got, want plan + + gotString := ui.OutputWriter.String() + json.Unmarshal([]byte(gotString), &got) + + wantFile, err := os.Open("output.json") + if err != nil { + t.Fatalf("err: %s", err) + } + defer wantFile.Close() + byteValue, err := ioutil.ReadAll(wantFile) + if err != nil { + t.Fatalf("err: %s", err) + } + json.Unmarshal([]byte(byteValue), &want) + + if !cmp.Equal(got, want) { + t.Fatalf("wrong result:\n %v\n", cmp.Diff(got, want)) + } +} + // similar test as above, without the plan func TestShow_json_output_state(t *testing.T) { fixtureDir := "testdata/show-json-state" @@ -450,6 +532,32 @@ func showFixtureSchema() *providers.GetProviderSchemaResponse { } } +// showFixtureSensitiveSchema returns a schema suitable for processing the configuration +// in testdata/show. This schema should be assigned to a mock provider +// named "test". It includes a sensitive attribute. +func showFixtureSensitiveSchema() *providers.GetProviderSchemaResponse { + return &providers.GetProviderSchemaResponse{ + Provider: providers.Schema{ + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "region": {Type: cty.String, Optional: true}, + }, + }, + }, + ResourceTypes: map[string]providers.Schema{ + "test_instance": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "ami": {Type: cty.String, Optional: true}, + "password": {Type: cty.String, Optional: true, Sensitive: true}, + }, + }, + }, + }, + } +} + // showFixtureProvider returns a mock provider that is configured for basic // operation with the configuration in testdata/show. This mock has // GetSchemaResponse, PlanResourceChangeFn, and ApplyResourceChangeFn populated, @@ -487,6 +595,43 @@ func showFixtureProvider() *terraform.MockProvider { return p } +// showFixtureSensitiveProvider returns a mock provider that is configured for basic +// operation with the configuration in testdata/show. This mock has +// GetSchemaResponse, PlanResourceChangeFn, and ApplyResourceChangeFn populated, +// with the plan/apply steps just passing through the data determined by +// Terraform Core. It also has a sensitive attribute in the provider schema. +func showFixtureSensitiveProvider() *terraform.MockProvider { + p := testProvider() + p.GetProviderSchemaResponse = showFixtureSensitiveSchema() + p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse { + idVal := req.ProposedNewState.GetAttr("id") + if idVal.IsNull() { + idVal = cty.UnknownVal(cty.String) + } + return providers.PlanResourceChangeResponse{ + PlannedState: cty.ObjectVal(map[string]cty.Value{ + "id": idVal, + "ami": req.ProposedNewState.GetAttr("ami"), + "password": req.ProposedNewState.GetAttr("password"), + }), + } + } + p.ApplyResourceChangeFn = func(req providers.ApplyResourceChangeRequest) providers.ApplyResourceChangeResponse { + idVal := req.PlannedState.GetAttr("id") + if !idVal.IsKnown() { + idVal = cty.StringVal("placeholder") + } + return providers.ApplyResourceChangeResponse{ + NewState: cty.ObjectVal(map[string]cty.Value{ + "id": idVal, + "ami": req.PlannedState.GetAttr("ami"), + "password": req.PlannedState.GetAttr("password"), + }), + } + } + return p +} + // showFixturePlanFile creates a plan file at a temporary location containing a // single change to create or update the test_instance.foo that is included in the "show" // test fixture, returning the location of that plan file. diff --git a/command/testdata/show-json-sensitive/main.tf b/command/testdata/show-json-sensitive/main.tf new file mode 100644 index 000000000..a980e4dc4 --- /dev/null +++ b/command/testdata/show-json-sensitive/main.tf @@ -0,0 +1,21 @@ +provider "test" { + region = "somewhere" +} + +variable "test_var" { + default = "bar" + sensitive = true +} + +resource "test_instance" "test" { + // this variable is sensitive + ami = var.test_var + // the password attribute is sensitive in the showFixtureSensitiveProvider schema. + password = "secret" + count = 3 +} + +output "test" { + value = var.test_var + sensitive = true +} diff --git a/command/testdata/show-json-sensitive/output.json b/command/testdata/show-json-sensitive/output.json new file mode 100644 index 000000000..f625fb316 --- /dev/null +++ b/command/testdata/show-json-sensitive/output.json @@ -0,0 +1,205 @@ +{ + "format_version": "0.1", + "variables": { + "test_var": { + "value": "bar" + } + }, + "planned_values": { + "outputs": { + "test": { + "sensitive": true, + "value": "bar" + } + }, + "root_module": { + "resources": [ + { + "address": "test_instance.test[0]", + "index": 0, + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_name": "registry.terraform.io/hashicorp/test", + "schema_version": 0, + "values": { + "ami": "bar", + "password": "secret" + } + }, + { + "address": "test_instance.test[1]", + "index": 1, + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_name": "registry.terraform.io/hashicorp/test", + "schema_version": 0, + "values": { + "ami": "bar", + "password": "secret" + } + }, + { + "address": "test_instance.test[2]", + "index": 2, + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_name": "registry.terraform.io/hashicorp/test", + "schema_version": 0, + "values": { + "ami": "bar", + "password": "secret" + } + } + ] + } + }, + "prior_state": { + "format_version": "0.1", + "values": { + "outputs": { + "test": { + "sensitive": true, + "value": "bar" + } + }, + "root_module": {} + } + }, + "resource_changes": [ + { + "address": "test_instance.test[0]", + "index": 0, + "mode": "managed", + "type": "test_instance", + "provider_name": "registry.terraform.io/hashicorp/test", + "name": "test", + "change": { + "actions": [ + "create" + ], + "before": null, + "after_unknown": { + "id": true + }, + "after": { + "ami": "bar", + "password": "secret" + }, + "after_sensitive": {"ami": true, "password": true}, + "before_sensitive": false + } + }, + { + "address": "test_instance.test[1]", + "index": 1, + "mode": "managed", + "type": "test_instance", + "provider_name": "registry.terraform.io/hashicorp/test", + "name": "test", + "change": { + "actions": [ + "create" + ], + "before": null, + "after_unknown": { + "id": true + }, + "after": { + "ami": "bar", + "password": "secret" + }, + "after_sensitive": {"ami": true, "password": true}, + "before_sensitive": false + } + }, + { + "address": "test_instance.test[2]", + "index": 2, + "mode": "managed", + "type": "test_instance", + "provider_name": "registry.terraform.io/hashicorp/test", + "name": "test", + "change": { + "actions": [ + "create" + ], + "before": null, + "after_unknown": { + "id": true + }, + "after": { + "ami": "bar", + "password": "secret" + }, + "after_sensitive": {"ami": true, "password": true}, + "before_sensitive": false + } + } + ], + "output_changes": { + "test": { + "actions": [ + "create" + ], + "before": null, + "after": "bar", + "after_unknown": false, + "before_sensitive": true, + "after_sensitive": true + } + }, + "configuration": { + "provider_config": { + "test": { + "name": "test", + "expressions": { + "region": { + "constant_value": "somewhere" + } + } + } + }, + "root_module": { + "outputs": { + "test": { + "expression": { + "references": [ + "var.test_var" + ] + }, + "sensitive": true + } + }, + "resources": [ + { + "address": "test_instance.test", + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_config_key": "test", + "schema_version": 0, + "expressions": { + "ami": { + "references": [ + "var.test_var" + ] + }, + "password": {"constant_value": "secret"} + }, + "count_expression": { + "constant_value": 3 + } + } + ], + "variables": { + "test_var": { + "default": "bar", + "sensitive": true + } + } + } + } +} diff --git a/configs/configschema/marks.go b/configs/configschema/marks.go new file mode 100644 index 000000000..8b468ec9f --- /dev/null +++ b/configs/configschema/marks.go @@ -0,0 +1,57 @@ +package configschema + +import ( + "fmt" + + "github.com/zclconf/go-cty/cty" +) + +// ValueMarks returns a set of path value marks for a given value and path, +// based on the sensitive flag for each attribute within the schema. Nested +// blocks are descended (if present in the given value). +func (b *Block) ValueMarks(val cty.Value, path cty.Path) []cty.PathValueMarks { + var pvm []cty.PathValueMarks + for name, attrS := range b.Attributes { + if attrS.Sensitive { + // Create a copy of the path, with this step added, to add to our PathValueMarks slice + attrPath := make(cty.Path, len(path), len(path)+1) + copy(attrPath, path) + attrPath = append(path, cty.GetAttrStep{Name: name}) + pvm = append(pvm, cty.PathValueMarks{ + Path: attrPath, + Marks: cty.NewValueMarks("sensitive"), + }) + } + } + + for name, blockS := range b.BlockTypes { + // If our block doesn't contain any sensitive attributes, skip inspecting it + if !blockS.Block.ContainsSensitive() { + continue + } + + blockV := val.GetAttr(name) + if blockV.IsNull() || !blockV.IsKnown() { + continue + } + + // Create a copy of the path, with this step added, to add to our PathValueMarks slice + blockPath := make(cty.Path, len(path), len(path)+1) + copy(blockPath, path) + blockPath = append(path, cty.GetAttrStep{Name: name}) + + switch blockS.Nesting { + case NestingSingle, NestingGroup: + pvm = append(pvm, blockS.Block.ValueMarks(blockV, blockPath)...) + case NestingList, NestingMap, NestingSet: + for it := blockV.ElementIterator(); it.Next(); { + idx, blockEV := it.Element() + morePaths := blockS.Block.ValueMarks(blockEV, append(blockPath, cty.IndexStep{Key: idx})) + pvm = append(pvm, morePaths...) + } + default: + panic(fmt.Sprintf("unsupported nesting mode %s", blockS.Nesting)) + } + } + return pvm +} diff --git a/configs/configschema/marks_test.go b/configs/configschema/marks_test.go new file mode 100644 index 000000000..9a21e4518 --- /dev/null +++ b/configs/configschema/marks_test.go @@ -0,0 +1,100 @@ +package configschema + +import ( + "fmt" + "testing" + + "github.com/zclconf/go-cty/cty" +) + +func TestBlockValueMarks(t *testing.T) { + schema := &Block{ + Attributes: map[string]*Attribute{ + "unsensitive": { + Type: cty.String, + Optional: true, + }, + "sensitive": { + Type: cty.String, + Sensitive: true, + }, + }, + + BlockTypes: map[string]*NestedBlock{ + "list": { + Nesting: NestingList, + Block: Block{ + Attributes: map[string]*Attribute{ + "unsensitive": { + Type: cty.String, + Optional: true, + }, + "sensitive": { + Type: cty.String, + Sensitive: true, + }, + }, + }, + }, + }, + } + + for _, tc := range []struct { + given cty.Value + expect cty.Value + }{ + { + cty.UnknownVal(schema.ImpliedType()), + cty.UnknownVal(schema.ImpliedType()), + }, + { + cty.ObjectVal(map[string]cty.Value{ + "sensitive": cty.UnknownVal(cty.String), + "unsensitive": cty.UnknownVal(cty.String), + "list": cty.UnknownVal(schema.BlockTypes["list"].ImpliedType()), + }), + cty.ObjectVal(map[string]cty.Value{ + "sensitive": cty.UnknownVal(cty.String).Mark("sensitive"), + "unsensitive": cty.UnknownVal(cty.String), + "list": cty.UnknownVal(schema.BlockTypes["list"].ImpliedType()), + }), + }, + { + cty.ObjectVal(map[string]cty.Value{ + "sensitive": cty.NullVal(cty.String), + "unsensitive": cty.UnknownVal(cty.String), + "list": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "sensitive": cty.UnknownVal(cty.String), + "unsensitive": cty.UnknownVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "sensitive": cty.NullVal(cty.String), + "unsensitive": cty.NullVal(cty.String), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "sensitive": cty.NullVal(cty.String).Mark("sensitive"), + "unsensitive": cty.UnknownVal(cty.String), + "list": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "sensitive": cty.UnknownVal(cty.String).Mark("sensitive"), + "unsensitive": cty.UnknownVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "sensitive": cty.NullVal(cty.String).Mark("sensitive"), + "unsensitive": cty.NullVal(cty.String), + }), + }), + }), + }, + } { + t.Run(fmt.Sprintf("%#v", tc.given), func(t *testing.T) { + got := tc.given.MarkWithPaths(schema.ValueMarks(tc.given, nil)) + if !got.RawEquals(tc.expect) { + t.Fatalf("\nexpected: %#v\ngot: %#v\n", tc.expect, got) + } + }) + } +} diff --git a/terraform/evaluate.go b/terraform/evaluate.go index 2417d2f64..282569ee1 100644 --- a/terraform/evaluate.go +++ b/terraform/evaluate.go @@ -760,7 +760,7 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc // If our provider schema contains sensitive values, mark those as sensitive afterMarks := change.AfterValMarks if schema.ContainsSensitive() { - afterMarks = append(afterMarks, getValMarks(schema, val, nil)...) + afterMarks = append(afterMarks, schema.ValueMarks(val, nil)...) } instances[key] = val.MarkWithPaths(afterMarks) @@ -789,7 +789,7 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc if schema.ContainsSensitive() { var marks []cty.PathValueMarks val, marks = val.UnmarkDeepWithPaths() - marks = append(marks, getValMarks(schema, val, nil)...) + marks = append(marks, schema.ValueMarks(val, nil)...) val = val.MarkWithPaths(marks) } instances[key] = val @@ -959,50 +959,3 @@ func moduleDisplayAddr(addr addrs.ModuleInstance) string { return addr.String() } } - -func getValMarks(schema *configschema.Block, val cty.Value, path cty.Path) []cty.PathValueMarks { - var pvm []cty.PathValueMarks - for name, attrS := range schema.Attributes { - if attrS.Sensitive { - // Create a copy of the path, with this step added, to add to our PathValueMarks slice - attrPath := make(cty.Path, len(path), len(path)+1) - copy(attrPath, path) - attrPath = append(path, cty.GetAttrStep{Name: name}) - pvm = append(pvm, cty.PathValueMarks{ - Path: attrPath, - Marks: cty.NewValueMarks("sensitive"), - }) - } - } - - for name, blockS := range schema.BlockTypes { - // If our block doesn't contain any sensitive attributes, skip inspecting it - if !blockS.Block.ContainsSensitive() { - continue - } - - blockV := val.GetAttr(name) - if blockV.IsNull() || !blockV.IsKnown() { - continue - } - - // Create a copy of the path, with this step added, to add to our PathValueMarks slice - blockPath := make(cty.Path, len(path), len(path)+1) - copy(blockPath, path) - blockPath = append(path, cty.GetAttrStep{Name: name}) - - switch blockS.Nesting { - case configschema.NestingSingle, configschema.NestingGroup: - pvm = append(pvm, getValMarks(&blockS.Block, blockV, blockPath)...) - case configschema.NestingList, configschema.NestingMap, configschema.NestingSet: - for it := blockV.ElementIterator(); it.Next(); { - idx, blockEV := it.Element() - morePaths := getValMarks(&blockS.Block, blockEV, append(blockPath, cty.IndexStep{Key: idx})) - pvm = append(pvm, morePaths...) - } - default: - panic(fmt.Sprintf("unsupported nesting mode %s", blockS.Nesting)) - } - } - return pvm -} diff --git a/terraform/evaluate_test.go b/terraform/evaluate_test.go index c25429e93..8c57c8e65 100644 --- a/terraform/evaluate_test.go +++ b/terraform/evaluate_test.go @@ -1,7 +1,6 @@ package terraform import ( - "fmt" "sync" "testing" @@ -562,95 +561,3 @@ func evaluatorForModule(stateSync *states.SyncState, changesSync *plans.ChangesS Changes: changesSync, } } - -func TestGetValMarks(t *testing.T) { - schema := &configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "unsensitive": { - Type: cty.String, - Optional: true, - }, - "sensitive": { - Type: cty.String, - Sensitive: true, - }, - }, - - BlockTypes: map[string]*configschema.NestedBlock{ - "list": &configschema.NestedBlock{ - Nesting: configschema.NestingList, - Block: configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "unsensitive": { - Type: cty.String, - Optional: true, - }, - "sensitive": { - Type: cty.String, - Sensitive: true, - }, - }, - }, - }, - }, - } - - for _, tc := range []struct { - given cty.Value - expect cty.Value - }{ - { - cty.UnknownVal(schema.ImpliedType()), - cty.UnknownVal(schema.ImpliedType()), - }, - { - cty.ObjectVal(map[string]cty.Value{ - "sensitive": cty.UnknownVal(cty.String), - "unsensitive": cty.UnknownVal(cty.String), - "list": cty.UnknownVal(schema.BlockTypes["list"].ImpliedType()), - }), - cty.ObjectVal(map[string]cty.Value{ - "sensitive": cty.UnknownVal(cty.String).Mark("sensitive"), - "unsensitive": cty.UnknownVal(cty.String), - "list": cty.UnknownVal(schema.BlockTypes["list"].ImpliedType()), - }), - }, - { - cty.ObjectVal(map[string]cty.Value{ - "sensitive": cty.NullVal(cty.String), - "unsensitive": cty.UnknownVal(cty.String), - "list": cty.ListVal([]cty.Value{ - cty.ObjectVal(map[string]cty.Value{ - "sensitive": cty.UnknownVal(cty.String), - "unsensitive": cty.UnknownVal(cty.String), - }), - cty.ObjectVal(map[string]cty.Value{ - "sensitive": cty.NullVal(cty.String), - "unsensitive": cty.NullVal(cty.String), - }), - }), - }), - cty.ObjectVal(map[string]cty.Value{ - "sensitive": cty.NullVal(cty.String).Mark("sensitive"), - "unsensitive": cty.UnknownVal(cty.String), - "list": cty.ListVal([]cty.Value{ - cty.ObjectVal(map[string]cty.Value{ - "sensitive": cty.UnknownVal(cty.String).Mark("sensitive"), - "unsensitive": cty.UnknownVal(cty.String), - }), - cty.ObjectVal(map[string]cty.Value{ - "sensitive": cty.NullVal(cty.String).Mark("sensitive"), - "unsensitive": cty.NullVal(cty.String), - }), - }), - }), - }, - } { - t.Run(fmt.Sprintf("%#v", tc.given), func(t *testing.T) { - got := tc.given.MarkWithPaths(getValMarks(schema, tc.given, nil)) - if !got.RawEquals(tc.expect) { - t.Fatalf("\nexpected: %#v\ngot: %#v\n", tc.expect, got) - } - }) - } -}