From 4fe76d3f517adab3c35dbb832bb68232c5e46f7b Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Thu, 21 Jan 2021 16:31:49 -0500 Subject: [PATCH] core: Fix crash for marked data source prior value When planning a data source change, the object compatibility verification step should be performed with unmarked values. This value is transient and preserving marks is not necessary, and the object change code is not fully marks-aware. --- terraform/context_plan2_test.go | 85 ++++++++++++++++++++ terraform/node_resource_abstract_instance.go | 7 +- 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/terraform/context_plan2_test.go b/terraform/context_plan2_test.go index ecf2a36f8..cfa89938f 100644 --- a/terraform/context_plan2_test.go +++ b/terraform/context_plan2_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/configs/configschema" "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/states" @@ -59,3 +60,87 @@ resource "test_object" "a" { t.Fatal(diags.Err()) } } + +func TestContext2Plan_noChangeDataSourceSensitiveNestedSet(t *testing.T) { + m := testModuleInline(t, map[string]string{ + "main.tf": ` +variable "bar" { + sensitive = true + default = "baz" +} + +data "test_data_source" "foo" { + foo { + bar = var.bar + } +} +`, + }) + + p := new(MockProvider) + p.GetSchemaResponse = getSchemaResponseFromProviderSchema(&ProviderSchema{ + DataSources: map[string]*configschema.Block{ + "test_data_source": { + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Computed: true, + }, + }, + BlockTypes: map[string]*configschema.NestedBlock{ + "foo": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "bar": {Type: cty.String, Optional: true}, + }, + }, + Nesting: configschema.NestingSet, + }, + }, + }, + }, + }) + + p.ReadDataSourceResponse = &providers.ReadDataSourceResponse{ + State: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("data_id"), + "foo": cty.SetVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{"bar": cty.StringVal("baz")})}), + }), + } + + state := states.NewState() + root := state.EnsureModule(addrs.RootModuleInstance) + root.SetResourceInstanceCurrent( + mustResourceInstanceAddr("data.test_data_source.foo").Resource, + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id":"data_id", "foo":[{"bar":"baz"}]}`), + AttrSensitivePaths: []cty.PathValueMarks{ + { + Path: cty.GetAttrPath("foo"), + Marks: cty.NewValueMarks("sensitive"), + }, + }, + }, + mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`), + ) + + ctx := testContext2(t, &ContextOpts{ + Config: m, + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + State: state, + }) + + plan, diags := ctx.Plan() + if diags.HasErrors() { + t.Fatal(diags.ErrWithWarnings()) + } + + for _, res := range plan.Changes.Resources { + if res.Action != plans.NoOp { + t.Fatalf("expected NoOp, got: %q %s", res.Addr, res.Action) + } + } +} diff --git a/terraform/node_resource_abstract_instance.go b/terraform/node_resource_abstract_instance.go index ad5574c85..c73f60050 100644 --- a/terraform/node_resource_abstract_instance.go +++ b/terraform/node_resource_abstract_instance.go @@ -1415,10 +1415,15 @@ func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, currentSt // if we have a prior value, we can check for any irregularities in the response if !priorVal.IsNull() { + // We drop marks on the values used here as the result is only + // temporarily used for validation. + unmarkedConfigVal, _ := configVal.UnmarkDeep() + unmarkedPriorVal, _ := priorVal.UnmarkDeep() + // While we don't propose planned changes for data sources, we can // generate a proposed value for comparison to ensure the data source // is returning a result following the rules of the provider contract. - proposedVal := objchange.ProposedNewObject(schema, priorVal, configVal) + proposedVal := objchange.ProposedNewObject(schema, unmarkedPriorVal, unmarkedConfigVal) if errs := objchange.AssertObjectCompatible(schema, proposedVal, newVal); len(errs) > 0 { // Resources have the LegacyTypeSystem field to signal when they are // using an SDK which may not produce precise values. While data