From 5e2905d2221fa6979fd823c5e2fa53d38519e246 Mon Sep 17 00:00:00 2001 From: Pam Selle <204372+pselle@users.noreply.github.com> Date: Mon, 12 Oct 2020 14:39:09 -0400 Subject: [PATCH 01/10] Mark attributes providers mark as sensitive This updates GetResource so that the value returned has marks where the provider's schema has marked an attribute as sensitive --- terraform/context_apply_test.go | 15 ++++++++++++++- terraform/context_test.go | 5 +++++ terraform/evaluate.go | 30 ++++++++++++++++++++++++++++-- 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 73614c59b..8e0a069df 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -11856,7 +11856,14 @@ variable "sensitive_map" { resource "test_resource" "foo" { value = var.sensitive_map.x -}`, + sensitive_value = "should get marked" +} + +resource "test_resource" "bar" { + value = test_resource.foo.sensitive_value + random = test_resource.foo.id # not sensitive +} +`, }) p := testProvider("test") @@ -11893,6 +11900,12 @@ resource "test_resource" "foo" { fooChangeSrc := plan.Changes.ResourceInstance(addr) verifySensitiveValue(fooChangeSrc.AfterValMarks) + barAddr := mustResourceInstanceAddr("test_resource.bar") + barChangeSrc := plan.Changes.ResourceInstance(barAddr) + if len(barChangeSrc.AfterValMarks) != 1 { + t.Fatalf("there should only be 1 marked path for bar, there are %v", len(barChangeSrc.AfterValMarks)) + } + state, diags := ctx.Apply() if diags.HasErrors() { t.Fatalf("apply errors: %s", diags.Err()) diff --git a/terraform/context_test.go b/terraform/context_test.go index dea83a790..b223c2a1e 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -425,6 +425,11 @@ func testProviderSchema(name string) *ProviderSchema { Type: cty.String, Optional: true, }, + "sensitive_value": { + Type: cty.String, + Sensitive: true, + Optional: true, + }, "random": { Type: cty.String, Optional: true, diff --git a/terraform/evaluate.go b/terraform/evaluate.go index 9d3e565b4..950ee9fbc 100644 --- a/terraform/evaluate.go +++ b/terraform/evaluate.go @@ -727,7 +727,7 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc } // Planned resources are temporarily stored in state with empty values, - // and need to be replaced bu the planned value here. + // and need to be replaced by the planned value here. if is.Current.Status == states.ObjectPlanned { if change == nil { // If the object is in planned status then we should not get @@ -752,6 +752,10 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc continue } + // If our schema contains sensitive values, mark those as sensitive + if schema.ContainsSensitive() { + val = markProviderSensitiveAttributes(schema, val, nil) + } instances[key] = val continue } @@ -768,7 +772,13 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc }) continue } - instances[key] = ios.Value + + val := ios.Value + // If our schema contains sensitive values, mark those as sensitive + if schema.ContainsSensitive() { + val = markProviderSensitiveAttributes(schema, val, nil) + } + instances[key] = val } var ret cty.Value @@ -935,3 +945,19 @@ func moduleDisplayAddr(addr addrs.ModuleInstance) string { return addr.String() } } + +// markProviderSensitiveAttributes returns an updated value +// where attributes that are Sensitive are marked +func markProviderSensitiveAttributes(schema *configschema.Block, val cty.Value, path cty.Path) cty.Value { + var pvm []cty.PathValueMarks + for name, attrS := range schema.Attributes { + if attrS.Sensitive { + path := append(path, cty.GetAttrStep{Name: name}) + pvm = append(pvm, cty.PathValueMarks{ + Path: path, + Marks: cty.NewValueMarks("sensitive"), + }) + } + } + return val.MarkWithPaths(pvm) +} From f60ae7ac08090381d5d68ca9ca480e8659f811fe Mon Sep 17 00:00:00 2001 From: Pam Selle <204372+pselle@users.noreply.github.com> Date: Wed, 14 Oct 2020 17:37:06 -0400 Subject: [PATCH 02/10] Mark sensitive attributes in blocks This implements marking sensitive attributes within blocks when referenced by adding recursive calls to get more paths from blocks' attributes --- terraform/context_apply_test.go | 17 +++++++++++++- terraform/context_test.go | 9 ++++++++ terraform/evaluate.go | 39 ++++++++++++++++++++++++++++----- 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 8e0a069df..6f2c78b19 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -11860,8 +11860,17 @@ resource "test_resource" "foo" { } resource "test_resource" "bar" { - value = test_resource.foo.sensitive_value + value = test_resource.foo.sensitive_value random = test_resource.foo.id # not sensitive + + nesting_single { + value = "abc" + sensitive_value = "xyz" + } +} + +resource "test_resource" "baz" { + value = test_resource.bar.nesting_single.sensitive_value } `, }) @@ -11906,6 +11915,12 @@ resource "test_resource" "bar" { t.Fatalf("there should only be 1 marked path for bar, there are %v", len(barChangeSrc.AfterValMarks)) } + bazAddr := mustResourceInstanceAddr("test_resource.baz") + bazChangeSrc := plan.Changes.ResourceInstance(bazAddr) + if len(bazChangeSrc.AfterValMarks) != 1 { + t.Fatalf("there should only be 1 marked path for baz, there are %v", len(bazChangeSrc.AfterValMarks)) + } + state, diags := ctx.Apply() if diags.HasErrors() { t.Fatalf("apply errors: %s", diags.Err()) diff --git a/terraform/context_test.go b/terraform/context_test.go index b223c2a1e..dde3ec5e9 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -445,6 +445,15 @@ func testProviderSchema(name string) *ProviderSchema { }, Nesting: configschema.NestingSet, }, + "nesting_single": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "value": {Type: cty.String, Optional: true}, + "sensitive_value": {Type: cty.String, Optional: true, Sensitive: true}, + }, + }, + Nesting: configschema.NestingSingle, + }, }, }, name + "_ami_list": { diff --git a/terraform/evaluate.go b/terraform/evaluate.go index 950ee9fbc..adfacc3f7 100644 --- a/terraform/evaluate.go +++ b/terraform/evaluate.go @@ -754,7 +754,7 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc // If our schema contains sensitive values, mark those as sensitive if schema.ContainsSensitive() { - val = markProviderSensitiveAttributes(schema, val, nil) + val = markProviderSensitiveAttributes(schema, val) } instances[key] = val continue @@ -776,7 +776,7 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc val := ios.Value // If our schema contains sensitive values, mark those as sensitive if schema.ContainsSensitive() { - val = markProviderSensitiveAttributes(schema, val, nil) + val = markProviderSensitiveAttributes(schema, val) } instances[key] = val } @@ -948,16 +948,43 @@ func moduleDisplayAddr(addr addrs.ModuleInstance) string { // markProviderSensitiveAttributes returns an updated value // where attributes that are Sensitive are marked -func markProviderSensitiveAttributes(schema *configschema.Block, val cty.Value, path cty.Path) cty.Value { +func markProviderSensitiveAttributes(schema *configschema.Block, val cty.Value) cty.Value { + return val.MarkWithPaths(getValMarks(schema, val, nil)) +} + +func getValMarks(schema *configschema.Block, val cty.Value, path cty.Path) []cty.PathValueMarks { var pvm []cty.PathValueMarks + for name, blockS := range schema.BlockTypes { + blockV := val.GetAttr(name) + 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: + 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...) + } + case configschema.NestingMap: + // TODO + continue + case configschema.NestingSet: + // TODO + continue + default: + panic(fmt.Sprintf("unsupported nesting mode %s", blockS.Nesting)) + } + } + for name, attrS := range schema.Attributes { if attrS.Sensitive { - path := append(path, cty.GetAttrStep{Name: name}) + attrPath := append(path, cty.GetAttrStep{Name: name}) pvm = append(pvm, cty.PathValueMarks{ - Path: path, + Path: attrPath, Marks: cty.NewValueMarks("sensitive"), }) } } - return val.MarkWithPaths(pvm) + return pvm } From e44e03b2831a67183f523a39dbdc641abaa3d1d1 Mon Sep 17 00:00:00 2001 From: Pam Selle <204372+pselle@users.noreply.github.com> Date: Wed, 14 Oct 2020 17:44:50 -0400 Subject: [PATCH 03/10] If our block doesn't contain any sensitive attrs, skip recursing into it --- terraform/evaluate.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/terraform/evaluate.go b/terraform/evaluate.go index adfacc3f7..33b50cb6d 100644 --- a/terraform/evaluate.go +++ b/terraform/evaluate.go @@ -955,6 +955,10 @@ func markProviderSensitiveAttributes(schema *configschema.Block, val cty.Value) func getValMarks(schema *configschema.Block, val cty.Value, path cty.Path) []cty.PathValueMarks { var pvm []cty.PathValueMarks 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) blockPath := append(path, cty.GetAttrStep{Name: name}) switch blockS.Nesting { From ee9ec8a1936456dc1d5ba0c730230bb59eb3cbfa Mon Sep 17 00:00:00 2001 From: Pam Selle <204372+pselle@users.noreply.github.com> Date: Wed, 14 Oct 2020 17:45:57 -0400 Subject: [PATCH 04/10] Reordering so attributes are first (understandability) --- terraform/evaluate.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/terraform/evaluate.go b/terraform/evaluate.go index 33b50cb6d..d5d2680fa 100644 --- a/terraform/evaluate.go +++ b/terraform/evaluate.go @@ -954,6 +954,16 @@ func markProviderSensitiveAttributes(schema *configschema.Block, val cty.Value) 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 { + 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() { @@ -980,15 +990,5 @@ func getValMarks(schema *configschema.Block, val cty.Value, path cty.Path) []cty panic(fmt.Sprintf("unsupported nesting mode %s", blockS.Nesting)) } } - - for name, attrS := range schema.Attributes { - if attrS.Sensitive { - attrPath := append(path, cty.GetAttrStep{Name: name}) - pvm = append(pvm, cty.PathValueMarks{ - Path: attrPath, - Marks: cty.NewValueMarks("sensitive"), - }) - } - } return pvm } From 10cffc477d6c48b3ece0f0a19f9ff7dd8d092a79 Mon Sep 17 00:00:00 2001 From: Pam Selle <204372+pselle@users.noreply.github.com> Date: Thu, 15 Oct 2020 16:48:36 -0400 Subject: [PATCH 05/10] Basic test for GetResource, plus sensitivity --- terraform/evaluate_test.go | 97 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/terraform/evaluate_test.go b/terraform/evaluate_test.go index 3957d283e..d2fe698bc 100644 --- a/terraform/evaluate_test.go +++ b/terraform/evaluate_test.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" + "github.com/hashicorp/terraform/configs/configschema" "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/tfdiags" @@ -124,6 +125,102 @@ func TestEvaluatorGetInputVariable(t *testing.T) { } } +func TestEvaluatorGetResource(t *testing.T) { + stateSync := states.BuildState(func(ss *states.SyncState) { + ss.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"}`), + }, + addrs.AbsProviderConfig{ + Provider: addrs.NewDefaultProvider("test"), + Module: addrs.RootModule, + }, + ) + }).SyncWrapper() + + rc := &configs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_resource", + Name: "foo", + Config: configs.SynthBody("", map[string]cty.Value{ + "id": cty.StringVal("foo"), + }), + Provider: addrs.Provider{ + Hostname: addrs.DefaultRegistryHost, + Namespace: "hashicorp", + Type: "test", + }, + } + + evaluator := &Evaluator{ + Meta: &ContextMeta{ + Env: "foo", + }, + Changes: plans.NewChanges().SyncWrapper(), + Config: &configs.Config{ + Module: &configs.Module{ + ManagedResources: map[string]*configs.Resource{ + "test_resource.foo": rc, + }, + }, + }, + State: stateSync, + Schemas: &Schemas{ + Providers: map[addrs.Provider]*ProviderSchema{ + addrs.NewDefaultProvider("test"): { + Provider: &configschema.Block{}, + ResourceTypes: map[string]*configschema.Block{ + "test_resource": { + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Computed: true, + }, + "value": { + Type: cty.String, + Computed: true, + Sensitive: true, + }, + }, + }, + }, + }, + }, + }, + } + + data := &evaluationStateData{ + Evaluator: evaluator, + } + scope := evaluator.Scope(data, nil) + + want := cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo"), + "value": cty.StringVal("hello").Mark("sensitive"), + }) + + addr := addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_resource", + Name: "foo", + } + got, diags := scope.Data.GetResource(addr, tfdiags.SourceRange{}) + + if len(diags) != 0 { + t.Errorf("unexpected diagnostics %s", spew.Sdump(diags)) + } + + if !got.RawEquals(want) { + t.Errorf("wrong result %#v; want %#v", got, want) + } +} + func TestEvaluatorGetModule(t *testing.T) { // Create a new evaluator with an existing state stateSync := states.BuildState(func(ss *states.SyncState) { From f790332bff9002330a3c2797bd95b0d63196a310 Mon Sep 17 00:00:00 2001 From: Pam Selle <204372+pselle@users.noreply.github.com> Date: Thu, 15 Oct 2020 16:55:11 -0400 Subject: [PATCH 06/10] NestingList support tested --- terraform/evaluate_test.go | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/terraform/evaluate_test.go b/terraform/evaluate_test.go index d2fe698bc..d8a67c446 100644 --- a/terraform/evaluate_test.go +++ b/terraform/evaluate_test.go @@ -135,7 +135,7 @@ func TestEvaluatorGetResource(t *testing.T) { }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), &states.ResourceInstanceObjectSrc{ Status: states.ObjectReady, - AttrsJSON: []byte(`{"id":"foo", "value":"hello"}`), + AttrsJSON: []byte(`{"id":"foo", "value":"hello", "nesting_list": [{"sensitive_value":"abc"}]}`), }, addrs.AbsProviderConfig{ Provider: addrs.NewDefaultProvider("test"), @@ -188,6 +188,17 @@ func TestEvaluatorGetResource(t *testing.T) { Sensitive: true, }, }, + BlockTypes: map[string]*configschema.NestedBlock{ + "nesting_list": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "value": {Type: cty.String, Optional: true}, + "sensitive_value": {Type: cty.String, Optional: true, Sensitive: true}, + }, + }, + Nesting: configschema.NestingList, + }, + }, }, }, }, @@ -201,7 +212,13 @@ func TestEvaluatorGetResource(t *testing.T) { scope := evaluator.Scope(data, nil) want := cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal("foo"), + "id": cty.StringVal("foo"), + "nesting_list": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "sensitive_value": cty.StringVal("abc").Mark("sensitive"), + "value": cty.NullVal(cty.String), + }), + }), "value": cty.StringVal("hello").Mark("sensitive"), }) From a5c5d2c28c461e407cab9fb46ea38d3d73ec988c Mon Sep 17 00:00:00 2001 From: Pam Selle <204372+pselle@users.noreply.github.com> Date: Thu, 15 Oct 2020 17:19:27 -0400 Subject: [PATCH 07/10] Cover NestingMap case --- terraform/evaluate.go | 5 +---- terraform/evaluate_test.go | 13 ++++++++++++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/terraform/evaluate.go b/terraform/evaluate.go index d5d2680fa..9fafff841 100644 --- a/terraform/evaluate.go +++ b/terraform/evaluate.go @@ -974,15 +974,12 @@ func getValMarks(schema *configschema.Block, val cty.Value, path cty.Path) []cty switch blockS.Nesting { case configschema.NestingSingle, configschema.NestingGroup: pvm = append(pvm, getValMarks(&blockS.Block, blockV, blockPath)...) - case configschema.NestingList: + case configschema.NestingList, configschema.NestingMap: 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...) } - case configschema.NestingMap: - // TODO - continue case configschema.NestingSet: // TODO continue diff --git a/terraform/evaluate_test.go b/terraform/evaluate_test.go index d8a67c446..519a9ab2c 100644 --- a/terraform/evaluate_test.go +++ b/terraform/evaluate_test.go @@ -135,7 +135,7 @@ func TestEvaluatorGetResource(t *testing.T) { }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), &states.ResourceInstanceObjectSrc{ Status: states.ObjectReady, - AttrsJSON: []byte(`{"id":"foo", "value":"hello", "nesting_list": [{"sensitive_value":"abc"}]}`), + AttrsJSON: []byte(`{"id":"foo", "nesting_list": [{"sensitive_value":"abc"}], "nesting_map": {"foo":{"foo":"x"}}, "value":"hello"}`), }, addrs.AbsProviderConfig{ Provider: addrs.NewDefaultProvider("test"), @@ -198,6 +198,14 @@ func TestEvaluatorGetResource(t *testing.T) { }, Nesting: configschema.NestingList, }, + "nesting_map": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "foo": {Type: cty.String, Optional: true, Sensitive: true}, + }, + }, + Nesting: configschema.NestingMap, + }, }, }, }, @@ -219,6 +227,9 @@ func TestEvaluatorGetResource(t *testing.T) { "value": cty.NullVal(cty.String), }), }), + "nesting_map": cty.MapVal(map[string]cty.Value{ + "foo": cty.ObjectVal(map[string]cty.Value{"foo": cty.StringVal("x").Mark("sensitive")}), + }), "value": cty.StringVal("hello").Mark("sensitive"), }) From a1a46425bdac39870970b2c68d3d669f8b03310f Mon Sep 17 00:00:00 2001 From: Pam Selle <204372+pselle@users.noreply.github.com> Date: Thu, 15 Oct 2020 17:25:53 -0400 Subject: [PATCH 08/10] Set and single test coverage --- terraform/evaluate.go | 5 +---- terraform/evaluate_test.go | 26 +++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/terraform/evaluate.go b/terraform/evaluate.go index 9fafff841..c8ee60304 100644 --- a/terraform/evaluate.go +++ b/terraform/evaluate.go @@ -974,15 +974,12 @@ func getValMarks(schema *configschema.Block, val cty.Value, path cty.Path) []cty switch blockS.Nesting { case configschema.NestingSingle, configschema.NestingGroup: pvm = append(pvm, getValMarks(&blockS.Block, blockV, blockPath)...) - case configschema.NestingList, configschema.NestingMap: + 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...) } - case configschema.NestingSet: - // TODO - continue default: panic(fmt.Sprintf("unsupported nesting mode %s", blockS.Nesting)) } diff --git a/terraform/evaluate_test.go b/terraform/evaluate_test.go index 519a9ab2c..3f32c5ffa 100644 --- a/terraform/evaluate_test.go +++ b/terraform/evaluate_test.go @@ -135,7 +135,7 @@ func TestEvaluatorGetResource(t *testing.T) { }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), &states.ResourceInstanceObjectSrc{ Status: states.ObjectReady, - AttrsJSON: []byte(`{"id":"foo", "nesting_list": [{"sensitive_value":"abc"}], "nesting_map": {"foo":{"foo":"x"}}, "value":"hello"}`), + AttrsJSON: []byte(`{"id":"foo", "nesting_list": [{"sensitive_value":"abc"}], "nesting_map": {"foo":{"foo":"x"}}, "nesting_set": [{"baz":"abc"}], "nesting_single": {"boop":"abc"}, "value":"hello"}`), }, addrs.AbsProviderConfig{ Provider: addrs.NewDefaultProvider("test"), @@ -206,6 +206,22 @@ func TestEvaluatorGetResource(t *testing.T) { }, Nesting: configschema.NestingMap, }, + "nesting_set": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "baz": {Type: cty.String, Optional: true, Sensitive: true}, + }, + }, + Nesting: configschema.NestingSet, + }, + "nesting_single": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "boop": {Type: cty.String, Optional: true, Sensitive: true}, + }, + }, + Nesting: configschema.NestingSingle, + }, }, }, }, @@ -230,6 +246,14 @@ func TestEvaluatorGetResource(t *testing.T) { "nesting_map": cty.MapVal(map[string]cty.Value{ "foo": cty.ObjectVal(map[string]cty.Value{"foo": cty.StringVal("x").Mark("sensitive")}), }), + "nesting_set": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "baz": cty.StringVal("abc").Mark("sensitive"), + }), + }), + "nesting_single": cty.ObjectVal(map[string]cty.Value{ + "boop": cty.StringVal("abc").Mark("sensitive"), + }), "value": cty.StringVal("hello").Mark("sensitive"), }) From a9823515ec8411b8f51932e0bd69b5ddbdb50aad Mon Sep 17 00:00:00 2001 From: Pam Selle <204372+pselle@users.noreply.github.com> Date: Thu, 15 Oct 2020 17:38:09 -0400 Subject: [PATCH 09/10] Update context apply test --- terraform/context_apply_test.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 6f2c78b19..1a8aec539 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -11905,21 +11905,19 @@ resource "test_resource" "baz" { } addr := mustResourceInstanceAddr("test_resource.foo") - fooChangeSrc := plan.Changes.ResourceInstance(addr) verifySensitiveValue(fooChangeSrc.AfterValMarks) + // Sensitive attributes (defined by the provider) are marked + // as sensitive when referenced from another resource + // "bar" references sensitive resources in "foo" barAddr := mustResourceInstanceAddr("test_resource.bar") barChangeSrc := plan.Changes.ResourceInstance(barAddr) - if len(barChangeSrc.AfterValMarks) != 1 { - t.Fatalf("there should only be 1 marked path for bar, there are %v", len(barChangeSrc.AfterValMarks)) - } + verifySensitiveValue(barChangeSrc.AfterValMarks) bazAddr := mustResourceInstanceAddr("test_resource.baz") bazChangeSrc := plan.Changes.ResourceInstance(bazAddr) - if len(bazChangeSrc.AfterValMarks) != 1 { - t.Fatalf("there should only be 1 marked path for baz, there are %v", len(bazChangeSrc.AfterValMarks)) - } + verifySensitiveValue(bazChangeSrc.AfterValMarks) state, diags := ctx.Apply() if diags.HasErrors() { @@ -11928,6 +11926,12 @@ resource "test_resource" "baz" { fooState := state.ResourceInstance(addr) verifySensitiveValue(fooState.Current.AttrSensitivePaths) + + barState := state.ResourceInstance(barAddr) + verifySensitiveValue(barState.Current.AttrSensitivePaths) + + bazState := state.ResourceInstance(bazAddr) + verifySensitiveValue(bazState.Current.AttrSensitivePaths) } func TestContext2Apply_variableSensitivityChange(t *testing.T) { From 394e60608ccee16c4c979b5843a614bbbb856f63 Mon Sep 17 00:00:00 2001 From: Pam Selle <204372+pselle@users.noreply.github.com> Date: Mon, 19 Oct 2020 15:24:14 -0400 Subject: [PATCH 10/10] Allocate new copies of paths to avoid append drama Create new copies of the Path to avoid possible append related dramas. Also add a test to cover nested block within blocks --- terraform/evaluate.go | 11 +++++++++-- terraform/evaluate_test.go | 28 ++++++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/terraform/evaluate.go b/terraform/evaluate.go index c8ee60304..add559cbc 100644 --- a/terraform/evaluate.go +++ b/terraform/evaluate.go @@ -956,7 +956,10 @@ func getValMarks(schema *configschema.Block, val cty.Value, path cty.Path) []cty var pvm []cty.PathValueMarks for name, attrS := range schema.Attributes { if attrS.Sensitive { - attrPath := append(path, cty.GetAttrStep{Name: name}) + // 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"), @@ -969,8 +972,12 @@ func getValMarks(schema *configschema.Block, val cty.Value, path cty.Path) []cty if !blockS.Block.ContainsSensitive() { 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}) + blockV := val.GetAttr(name) - blockPath := append(path, cty.GetAttrStep{Name: name}) switch blockS.Nesting { case configschema.NestingSingle, configschema.NestingGroup: pvm = append(pvm, getValMarks(&blockS.Block, blockV, blockPath)...) diff --git a/terraform/evaluate_test.go b/terraform/evaluate_test.go index 3f32c5ffa..eb07af314 100644 --- a/terraform/evaluate_test.go +++ b/terraform/evaluate_test.go @@ -135,7 +135,7 @@ func TestEvaluatorGetResource(t *testing.T) { }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), &states.ResourceInstanceObjectSrc{ Status: states.ObjectReady, - AttrsJSON: []byte(`{"id":"foo", "nesting_list": [{"sensitive_value":"abc"}], "nesting_map": {"foo":{"foo":"x"}}, "nesting_set": [{"baz":"abc"}], "nesting_single": {"boop":"abc"}, "value":"hello"}`), + AttrsJSON: []byte(`{"id":"foo", "nesting_list": [{"sensitive_value":"abc"}], "nesting_map": {"foo":{"foo":"x"}}, "nesting_set": [{"baz":"abc"}], "nesting_single": {"boop":"abc"}, "nesting_nesting": {"nesting_list":[{"sensitive_value":"abc"}]}, "value":"hello"}`), }, addrs.AbsProviderConfig{ Provider: addrs.NewDefaultProvider("test"), @@ -222,6 +222,22 @@ func TestEvaluatorGetResource(t *testing.T) { }, Nesting: configschema.NestingSingle, }, + "nesting_nesting": { + Block: configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "nesting_list": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "value": {Type: cty.String, Optional: true}, + "sensitive_value": {Type: cty.String, Optional: true, Sensitive: true}, + }, + }, + Nesting: configschema.NestingList, + }, + }, + }, + Nesting: configschema.NestingSingle, + }, }, }, }, @@ -246,6 +262,14 @@ func TestEvaluatorGetResource(t *testing.T) { "nesting_map": cty.MapVal(map[string]cty.Value{ "foo": cty.ObjectVal(map[string]cty.Value{"foo": cty.StringVal("x").Mark("sensitive")}), }), + "nesting_nesting": cty.ObjectVal(map[string]cty.Value{ + "nesting_list": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "sensitive_value": cty.StringVal("abc").Mark("sensitive"), + "value": cty.NullVal(cty.String), + }), + }), + }), "nesting_set": cty.SetVal([]cty.Value{ cty.ObjectVal(map[string]cty.Value{ "baz": cty.StringVal("abc").Mark("sensitive"), @@ -269,7 +293,7 @@ func TestEvaluatorGetResource(t *testing.T) { } if !got.RawEquals(want) { - t.Errorf("wrong result %#v; want %#v", got, want) + t.Errorf("wrong result:\ngot: %#v\nwant: %#v", got, want) } }