diff --git a/backend/local/backend_apply_test.go b/backend/local/backend_apply_test.go index e7e7a53a9503..f8de90479c3c 100644 --- a/backend/local/backend_apply_test.go +++ b/backend/local/backend_apply_test.go @@ -123,8 +123,7 @@ func TestLocal_applyError(t *testing.T) { b, cleanup := TestLocal(t) defer cleanup() - p := TestLocalProvider(t, b, "test", nil) - p.GetSchemaReturn = &terraform.ProviderSchema{ + schema := &terraform.ProviderSchema{ ResourceTypes: map[string]*configschema.Block{ "test_instance": { Attributes: map[string]*configschema.Attribute{ @@ -134,6 +133,7 @@ func TestLocal_applyError(t *testing.T) { }, }, } + p := TestLocalProvider(t, b, "test", schema) var lock sync.Mutex errored := false diff --git a/backend/local/testing.go b/backend/local/testing.go index 05f7600f9ae7..dccddb5daeed 100644 --- a/backend/local/testing.go +++ b/backend/local/testing.go @@ -6,7 +6,11 @@ import ( "path/filepath" "testing" + "github.com/zclconf/go-cty/cty" + + "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/backend" + "github.com/hashicorp/terraform/configs/configschema" "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/states/statemgr" @@ -38,9 +42,14 @@ func TestLocal(t *testing.T) (*Local, func()) { // function, t.Helper doesn't apply and so the log source // isn't correctly shown in the test log output. This seems // unavoidable as long as this is happening so indirectly. - t.Log(diag.Description().Summary) + desc := diag.Description() + if desc.Detail != "" { + t.Logf("%s: %s", desc.Summary, desc.Detail) + } else { + t.Log(desc.Summary) + } if local.CLI != nil { - local.CLI.Error(diag.Description().Summary) + local.CLI.Error(desc.Summary) } } } @@ -59,10 +68,34 @@ func TestLocal(t *testing.T) (*Local, func()) { func TestLocalProvider(t *testing.T, b *Local, name string, schema *terraform.ProviderSchema) *terraform.MockProvider { // Build a mock resource provider for in-memory operations p := new(terraform.MockProvider) + + if schema == nil { + schema = &terraform.ProviderSchema{} // default schema is empty + } p.GetSchemaReturn = schema + p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse { + rSchema, _ := schema.SchemaForResourceType(addrs.ManagedResourceMode, req.TypeName) + if rSchema == nil { + rSchema = &configschema.Block{} // default schema is empty + } + plannedVals := map[string]cty.Value{} + for name, attrS := range rSchema.Attributes { + val := req.ProposedNewState.GetAttr(name) + if attrS.Computed && val.IsNull() { + val = cty.UnknownVal(attrS.Type) + } + plannedVals[name] = val + } + for name := range rSchema.BlockTypes { + // For simplicity's sake we just copy the block attributes over + // verbatim, since this package's mock providers are all relatively + // simple -- we're testing the backend, not esoteric provider features. + plannedVals[name] = req.ProposedNewState.GetAttr(name) + } + return providers.PlanResourceChangeResponse{ - PlannedState: req.ProposedNewState, + PlannedState: cty.ObjectVal(plannedVals), PlannedPrivate: req.PriorPrivate, } } diff --git a/builtin/providers/test/resource_list_test.go b/builtin/providers/test/resource_list_test.go index 76c82f15a80d..e39b575219cc 100644 --- a/builtin/providers/test/resource_list_test.go +++ b/builtin/providers/test/resource_list_test.go @@ -133,3 +133,58 @@ resource "test_resource_list" "foo" { }, }) } + +func TestResourceList_interpolationChanges(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_list" "foo" { + list_block { + string = "x" + } +} +resource "test_resource_list" "bar" { + list_block { + string = test_resource_list.foo.id + } +} + `), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "test_resource_list.foo", "list_block.0.string", "x", + ), + resource.TestCheckResourceAttr( + "test_resource_list.bar", "list_block.0.string", "testId", + ), + ), + }, + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_list" "baz" { + list_block { + string = "x" + int = 1 + } +} +resource "test_resource_list" "bar" { + list_block { + string = test_resource_list.baz.id + int = 3 + } +} + `), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "test_resource_list.baz", "list_block.0.string", "x", + ), + resource.TestCheckResourceAttr( + "test_resource_list.bar", "list_block.0.string", "testId", + ), + ), + }, + }, + }) +} diff --git a/builtin/providers/test/resource_nested_set_test.go b/builtin/providers/test/resource_nested_set_test.go index e47f1ac5883e..e782bb7fb5e1 100644 --- a/builtin/providers/test/resource_nested_set_test.go +++ b/builtin/providers/test/resource_nested_set_test.go @@ -527,3 +527,56 @@ resource "test_resource_nested_set" "c" { }, }) } + +func TestResourceNestedSet_interpolationChanges(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_nested_set" "foo" { + single { + value = "x" + } +} +resource "test_resource_nested_set" "bar" { + single { + value = test_resource_nested_set.foo.id + } +} + `), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "test_resource_nested_set.foo", "single.#", "1", + ), + resource.TestCheckResourceAttr( + "test_resource_nested_set.bar", "single.#", "1", + ), + ), + }, + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_nested_set" "baz" { + single { + value = "x" + } +} +resource "test_resource_nested_set" "bar" { + single { + value = test_resource_nested_set.baz.id + } +} + `), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "test_resource_nested_set.baz", "single.#", "1", + ), + resource.TestCheckResourceAttr( + "test_resource_nested_set.bar", "single.#", "1", + ), + ), + }, + }, + }) +} diff --git a/command/show_test.go b/command/show_test.go index 746ea1706151..5febe857818b 100644 --- a/command/show_test.go +++ b/command/show_test.go @@ -244,13 +244,29 @@ func showFixtureProvider() *terraform.MockProvider { p := testProvider() p.GetSchemaReturn = showFixtureSchema() p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse { + idVal := req.ProposedNewState.GetAttr("id") + amiVal := req.ProposedNewState.GetAttr("ami") + if idVal.IsNull() { + idVal = cty.UnknownVal(cty.String) + } return providers.PlanResourceChangeResponse{ - PlannedState: req.ProposedNewState, + PlannedState: cty.ObjectVal(map[string]cty.Value{ + "id": idVal, + "ami": amiVal, + }), } } p.ApplyResourceChangeFn = func(req providers.ApplyResourceChangeRequest) providers.ApplyResourceChangeResponse { + idVal := req.PlannedState.GetAttr("id") + amiVal := req.PlannedState.GetAttr("ami") + if !idVal.IsKnown() { + idVal = cty.StringVal("placeholder") + } return providers.ApplyResourceChangeResponse{ - NewState: cty.UnknownAsNull(req.PlannedState), + NewState: cty.ObjectVal(map[string]cty.Value{ + "id": idVal, + "ami": amiVal, + }), } } return p diff --git a/command/test-fixtures/show-json/basic-delete/output.json b/command/test-fixtures/show-json/basic-delete/output.json index 297ded0e7702..837ed5d010e3 100644 --- a/command/test-fixtures/show-json/basic-delete/output.json +++ b/command/test-fixtures/show-json/basic-delete/output.json @@ -18,7 +18,7 @@ "schema_version": 0, "values": { "ami": "bar", - "id": null + "id": "placeholder" } } ] @@ -37,11 +37,11 @@ ], "before": { "ami": "foo", - "id": null + "id": "placeholder" }, "after": { "ami": "bar", - "id": null + "id": "placeholder" }, "after_unknown": { "ami": false, @@ -61,7 +61,7 @@ ], "before": { "ami": "foo", - "id": null + "id": "placeholder" }, "after": null, "after_unknown": false diff --git a/command/test-fixtures/show-json/basic-delete/terraform.tfstate b/command/test-fixtures/show-json/basic-delete/terraform.tfstate index 4cf6ed7fb932..db49d3e6846f 100644 --- a/command/test-fixtures/show-json/basic-delete/terraform.tfstate +++ b/command/test-fixtures/show-json/basic-delete/terraform.tfstate @@ -14,7 +14,8 @@ { "schema_version": 0, "attributes": { - "ami": "foo" + "ami": "foo", + "id": "placeholder" } } ] @@ -28,7 +29,8 @@ { "schema_version": 0, "attributes": { - "ami": "foo" + "ami": "foo", + "id": "placeholder" } } ] diff --git a/command/test-fixtures/show-json/basic-update/output.json b/command/test-fixtures/show-json/basic-update/output.json index d065edaf0168..4f5f115fb9df 100644 --- a/command/test-fixtures/show-json/basic-update/output.json +++ b/command/test-fixtures/show-json/basic-update/output.json @@ -18,7 +18,7 @@ "schema_version": 0, "values": { "ami": "bar", - "id": null + "id": "placeholder" } } ] @@ -37,11 +37,11 @@ ], "before": { "ami": "bar", - "id": null + "id": "placeholder" }, "after": { "ami": "bar", - "id": null + "id": "placeholder" }, "after_unknown": { "ami": false, diff --git a/command/test-fixtures/show-json/basic-update/terraform.tfstate b/command/test-fixtures/show-json/basic-update/terraform.tfstate index f980cdfd5909..dfc796a88408 100644 --- a/command/test-fixtures/show-json/basic-update/terraform.tfstate +++ b/command/test-fixtures/show-json/basic-update/terraform.tfstate @@ -14,7 +14,8 @@ { "schema_version": 0, "attributes": { - "ami": "bar" + "ami": "bar", + "id": "placeholder" } } ] diff --git a/configs/configschema/coerce_value.go b/configs/configschema/coerce_value.go index bae5733df711..877e860f3f44 100644 --- a/configs/configschema/coerce_value.go +++ b/configs/configschema/coerce_value.go @@ -225,7 +225,29 @@ func (b *Block) coerceValue(in cty.Value, path cty.Path) (cty.Value, error) { elems[key.AsString()] = val } } - attrs[typeName] = cty.MapVal(elems) + + // If the attribute values here contain any DynamicPseudoTypes, + // the concrete type must be an object. + useObject := false + switch { + case coll.Type().IsObjectType(): + useObject = true + default: + // It's possible that we were given a map, and need to coerce it to an object + ety := coll.Type().ElementType() + for _, v := range elems { + if !v.Type().Equals(ety) { + useObject = true + break + } + } + } + + if useObject { + attrs[typeName] = cty.ObjectVal(elems) + } else { + attrs[typeName] = cty.MapVal(elems) + } default: attrs[typeName] = cty.MapValEmpty(blockS.ImpliedType()) } diff --git a/configs/configschema/coerce_value_test.go b/configs/configschema/coerce_value_test.go index b7cc5f632df5..3286751a3527 100644 --- a/configs/configschema/coerce_value_test.go +++ b/configs/configschema/coerce_value_test.go @@ -436,6 +436,101 @@ func TestCoerceValue(t *testing.T) { }), ``, }, + "dynamic value attributes": { + &Block{ + BlockTypes: map[string]*NestedBlock{ + "foo": { + Nesting: NestingMap, + Block: Block{ + Attributes: map[string]*Attribute{ + "bar": { + Type: cty.String, + Optional: true, + Computed: true, + }, + "baz": { + Type: cty.DynamicPseudoType, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.ObjectVal(map[string]cty.Value{ + "a": cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("beep"), + }), + "b": cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("boop"), + "baz": cty.NumberIntVal(8), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.ObjectVal(map[string]cty.Value{ + "a": cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("beep"), + "baz": cty.NullVal(cty.DynamicPseudoType), + }), + "b": cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("boop"), + "baz": cty.NumberIntVal(8), + }), + }), + }), + ``, + }, + "dynamic attributes in map": { + // Convert a block represented as a map to an object if a + // DynamicPseudoType causes the element types to mismatch. + &Block{ + BlockTypes: map[string]*NestedBlock{ + "foo": { + Nesting: NestingMap, + Block: Block{ + Attributes: map[string]*Attribute{ + "bar": { + Type: cty.String, + Optional: true, + Computed: true, + }, + "baz": { + Type: cty.DynamicPseudoType, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.MapVal(map[string]cty.Value{ + "a": cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("beep"), + }), + "b": cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("boop"), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.ObjectVal(map[string]cty.Value{ + "a": cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("beep"), + "baz": cty.NullVal(cty.DynamicPseudoType), + }), + "b": cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("boop"), + "baz": cty.NullVal(cty.DynamicPseudoType), + }), + }), + }), + ``, + }, } for name, test := range tests { diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index 8291445c83b8..709441b475f4 100644 --- a/helper/plugin/grpc_provider.go +++ b/helper/plugin/grpc_provider.go @@ -511,23 +511,30 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl // turn the proposed state into a legacy configuration cfg := terraform.NewResourceConfigShimmed(proposedNewStateVal, block) + diff, err := s.provider.SimpleDiff(info, priorState, cfg) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil } + // if this is a new instance, we need to make sure ID is going to be computed + if priorStateVal.IsNull() { + if diff == nil { + diff = terraform.NewInstanceDiff() + } + + diff.Attributes["id"] = &terraform.ResourceAttrDiff{ + NewComputed: true, + } + } + if diff == nil || len(diff.Attributes) == 0 { // schema.Provider.Diff returns nil if it ends up making a diff with no // changes, but our new interface wants us to return an actual change - // description that _shows_ there are no changes. This is usually the - // PriorSate, however if there was no prior state and no diff, then we - // use the ProposedNewState. - if !priorStateVal.IsNull() { - resp.PlannedState = req.PriorState - } else { - resp.PlannedState = req.ProposedNewState - } + // description that _shows_ there are no changes. This is always the + // prior state, because we force a diff above if this is a new instance. + resp.PlannedState = req.PriorState return resp, nil } @@ -561,7 +568,7 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl plannedStateVal = copyTimeoutValues(plannedStateVal, proposedNewStateVal) - // The old SDK code has some inprecisions that cause it to sometimes + // The old SDK code has some imprecisions that cause it to sometimes // generate differences that the SDK itself does not consider significant // but Terraform Core would. To avoid producing weird do-nothing diffs // in that case, we'll check if the provider as produced something we @@ -575,6 +582,12 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl forceNoChanges = true } + // if this was creating the resource, we need to set any remaining computed + // fields + if priorStateVal.IsNull() { + plannedStateVal = SetUnknowns(plannedStateVal, block) + } + plannedMP, err := msgpack.Marshal(plannedStateVal, block.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) diff --git a/helper/plugin/unknown.go b/helper/plugin/unknown.go new file mode 100644 index 000000000000..48e24e5d621c --- /dev/null +++ b/helper/plugin/unknown.go @@ -0,0 +1,131 @@ +package plugin + +import ( + "fmt" + + "github.com/hashicorp/terraform/configs/configschema" + "github.com/zclconf/go-cty/cty" +) + +// SetUnknowns takes a cty.Value, and compares it to the schema setting any null +// values which are computed to unknown. +func SetUnknowns(val cty.Value, schema *configschema.Block) cty.Value { + if !val.IsKnown() { + return val + } + + // If the object was null, we still need to handle the top level attributes + // which might be computed, but we don't need to expand the blocks. + if val.IsNull() { + objMap := map[string]cty.Value{} + allNull := true + for name, attr := range schema.Attributes { + switch { + case attr.Computed: + objMap[name] = cty.UnknownVal(attr.Type) + allNull = false + default: + objMap[name] = cty.NullVal(attr.Type) + } + } + + // If this object has no unknown attributes, then we can leave it null. + if allNull { + return val + } + + return cty.ObjectVal(objMap) + } + + valMap := val.AsValueMap() + newVals := make(map[string]cty.Value) + + for name, attr := range schema.Attributes { + v := valMap[name] + + if attr.Computed && v.IsNull() { + newVals[name] = cty.UnknownVal(attr.Type) + continue + } + + newVals[name] = v + } + + for name, blockS := range schema.BlockTypes { + blockVal := valMap[name] + if blockVal.IsNull() || !blockVal.IsKnown() { + newVals[name] = blockVal + continue + } + + blockValType := blockVal.Type() + blockElementType := blockS.Block.ImpliedType() + + // This switches on the value type here, so we can correctly switch + // between Tuples/Lists and Maps/Objects. + switch { + case blockS.Nesting == configschema.NestingSingle: + // NestingSingle is the only exception here, where we treat the + // block directly as an object + newVals[name] = SetUnknowns(blockVal, &blockS.Block) + + case blockValType.IsSetType(), blockValType.IsListType(), blockValType.IsTupleType(): + listVals := blockVal.AsValueSlice() + newListVals := make([]cty.Value, 0, len(listVals)) + + for _, v := range listVals { + newListVals = append(newListVals, SetUnknowns(v, &blockS.Block)) + } + + switch { + case blockValType.IsSetType(): + switch len(newListVals) { + case 0: + newVals[name] = cty.SetValEmpty(blockElementType) + default: + newVals[name] = cty.SetVal(newListVals) + } + case blockValType.IsListType(): + switch len(newListVals) { + case 0: + newVals[name] = cty.ListValEmpty(blockElementType) + default: + newVals[name] = cty.ListVal(newListVals) + } + case blockValType.IsTupleType(): + newVals[name] = cty.TupleVal(newListVals) + } + + case blockValType.IsMapType(), blockValType.IsObjectType(): + mapVals := blockVal.AsValueMap() + newMapVals := make(map[string]cty.Value) + + for k, v := range mapVals { + newMapVals[k] = SetUnknowns(v, &blockS.Block) + } + + switch { + case blockValType.IsMapType(): + switch len(newMapVals) { + case 0: + newVals[name] = cty.MapValEmpty(blockElementType) + default: + newVals[name] = cty.MapVal(newMapVals) + } + case blockValType.IsObjectType(): + if len(newMapVals) == 0 { + // We need to populate empty values to make a valid object. + for attr, ty := range blockElementType.AttributeTypes() { + newMapVals[attr] = cty.NullVal(ty) + } + } + newVals[name] = cty.ObjectVal(newMapVals) + } + + default: + panic(fmt.Sprintf("failed to set unknown values for nested block %q:%#v", name, blockValType)) + } + } + + return cty.ObjectVal(newVals) +} diff --git a/helper/plugin/unknown_test.go b/helper/plugin/unknown_test.go new file mode 100644 index 000000000000..4214b18497f7 --- /dev/null +++ b/helper/plugin/unknown_test.go @@ -0,0 +1,483 @@ +package plugin + +import ( + "testing" + + "github.com/hashicorp/terraform/configs/configschema" + "github.com/zclconf/go-cty/cty" +) + +func TestSetUnknowns(t *testing.T) { + for n, tc := range map[string]struct { + Schema *configschema.Block + Val cty.Value + Expected cty.Value + }{ + "empty": { + &configschema.Block{}, + cty.EmptyObjectVal, + cty.EmptyObjectVal, + }, + "no prior": { + &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "foo": { + Type: cty.String, + Optional: true, + }, + "bar": { + Type: cty.String, + Computed: true, + }, + }, + BlockTypes: map[string]*configschema.NestedBlock{ + "baz": { + Nesting: configschema.NestingSingle, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "boz": { + Type: cty.String, + Optional: true, + Computed: true, + }, + "biz": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.NullVal(cty.Object(map[string]cty.Type{ + "foo": cty.String, + "bar": cty.String, + "baz": cty.Object(map[string]cty.Type{ + "boz": cty.String, + "biz": cty.String, + }), + })), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.NullVal(cty.String), + "bar": cty.UnknownVal(cty.String), + }), + }, + "null stays null": { + // if the object has no computed attributes, it should stay null + &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "foo": &configschema.Attribute{ + Type: cty.String, + }, + }, + BlockTypes: map[string]*configschema.NestedBlock{ + "baz": { + Nesting: configschema.NestingSet, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "boz": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.NullVal(cty.Object(map[string]cty.Type{ + "foo": cty.String, + "baz": cty.Set(cty.Object(map[string]cty.Type{ + "boz": cty.String, + })), + })), + cty.NullVal(cty.Object(map[string]cty.Type{ + "foo": cty.String, + "baz": cty.Set(cty.Object(map[string]cty.Type{ + "boz": cty.String, + })), + })), + }, + "no prior with set": { + // the set value should remain null + &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "foo": &configschema.Attribute{ + Type: cty.String, + Computed: true, + }, + }, + BlockTypes: map[string]*configschema.NestedBlock{ + "baz": { + Nesting: configschema.NestingSet, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "boz": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.NullVal(cty.Object(map[string]cty.Type{ + "foo": cty.String, + "baz": cty.Set(cty.Object(map[string]cty.Type{ + "boz": cty.String, + })), + })), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.UnknownVal(cty.String), + }), + }, + "prior attributes": { + &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "foo": { + Type: cty.String, + Optional: true, + }, + "bar": { + Type: cty.String, + Computed: true, + }, + "baz": { + Type: cty.String, + Optional: true, + Computed: true, + }, + "boz": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.StringVal("bonjour"), + "bar": cty.StringVal("petit dejeuner"), + "baz": cty.StringVal("grande dejeuner"), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.StringVal("bonjour"), + "bar": cty.StringVal("petit dejeuner"), + "baz": cty.StringVal("grande dejeuner"), + "boz": cty.UnknownVal(cty.String), + }), + }, + "prior nested single": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "foo": { + Nesting: configschema.NestingSingle, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "bar": { + Type: cty.String, + Optional: true, + Computed: true, + }, + "baz": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("beep"), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("beep"), + "baz": cty.UnknownVal(cty.String), + }), + }), + }, + "prior nested list": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "foo": { + Nesting: configschema.NestingList, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "bar": { + Type: cty.String, + Optional: true, + Computed: true, + }, + "baz": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("bap"), + }), + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("blep"), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("bap"), + "baz": cty.UnknownVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("blep"), + "baz": cty.UnknownVal(cty.String), + }), + }), + }), + }, + "prior nested map": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "foo": { + Nesting: configschema.NestingMap, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "bar": { + Type: cty.String, + Optional: true, + Computed: true, + }, + "baz": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.MapVal(map[string]cty.Value{ + "a": cty.ObjectVal(map[string]cty.Value{ + "bar": cty.NullVal(cty.String), + "baz": cty.StringVal("boop"), + }), + "b": cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("blep"), + "baz": cty.NullVal(cty.String), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.MapVal(map[string]cty.Value{ + "a": cty.ObjectVal(map[string]cty.Value{ + "bar": cty.UnknownVal(cty.String), + "baz": cty.StringVal("boop"), + }), + "b": cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("blep"), + "baz": cty.UnknownVal(cty.String), + }), + }), + }), + }, + "prior nested set": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "foo": { + Nesting: configschema.NestingSet, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "bar": { + Type: cty.String, + Optional: true, + }, + "baz": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("blep"), + "baz": cty.NullVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("boop"), + "baz": cty.NullVal(cty.String), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("blep"), + "baz": cty.UnknownVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("boop"), + "baz": cty.UnknownVal(cty.String), + }), + }), + }), + }, + "sets differing only by unknown": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "foo": { + Nesting: configschema.NestingSet, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "bar": { + Type: cty.String, + Optional: true, + }, + "baz": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("boop"), + "baz": cty.NullVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("boop"), + "baz": cty.UnknownVal(cty.String), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("boop"), + "baz": cty.UnknownVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("boop"), + "baz": cty.UnknownVal(cty.String), + }), + }), + }), + }, + "prior nested list with dynamic": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "foo": { + Nesting: configschema.NestingList, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "bar": { + Type: cty.String, + Optional: true, + Computed: true, + }, + "baz": { + Type: cty.DynamicPseudoType, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.TupleVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.NullVal(cty.String), + "baz": cty.NumberIntVal(8), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.TupleVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.UnknownVal(cty.String), + "baz": cty.NumberIntVal(8), + }), + }), + }), + }, + "prior nested map with dynamic": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "foo": { + Nesting: configschema.NestingMap, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "bar": { + Type: cty.String, + Optional: true, + Computed: true, + }, + "baz": { + Type: cty.DynamicPseudoType, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.ObjectVal(map[string]cty.Value{ + "a": cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("beep"), + "baz": cty.NullVal(cty.DynamicPseudoType), + }), + "b": cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("boop"), + "baz": cty.NumberIntVal(8), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.ObjectVal(map[string]cty.Value{ + "a": cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("beep"), + "baz": cty.UnknownVal(cty.DynamicPseudoType), + }), + "b": cty.ObjectVal(map[string]cty.Value{ + "bar": cty.StringVal("boop"), + "baz": cty.NumberIntVal(8), + }), + }), + }), + }, + } { + t.Run(n, func(t *testing.T) { + got := SetUnknowns(tc.Val, tc.Schema) + if !got.RawEquals(tc.Expected) { + t.Fatalf("\nexpected: %#v\ngot: %#v\n", tc.Expected, got) + } + }) + } +} diff --git a/lang/funcs/string.go b/lang/funcs/string.go index cb3d03cbd477..c9ddf19e368d 100644 --- a/lang/funcs/string.go +++ b/lang/funcs/string.go @@ -39,10 +39,18 @@ var JoinFunc = function.New(&function.Spec{ } items := make([]string, 0, l) - for _, list := range listVals { + for ai, list := range listVals { + ei := 0 for it := list.ElementIterator(); it.Next(); { _, val := it.Element() + if val.IsNull() { + if len(listVals) > 1 { + return cty.UnknownVal(cty.String), function.NewArgErrorf(ai+1, "element %d of list %d is null; cannot concatenate null values", ei, ai+1) + } + return cty.UnknownVal(cty.String), function.NewArgErrorf(ai+1, "element %d is null; cannot concatenate null values", ei) + } items = append(items, val.AsString()) + ei++ } } diff --git a/plans/objchange/all_null.go b/plans/objchange/all_null.go new file mode 100644 index 000000000000..b1acb253afe4 --- /dev/null +++ b/plans/objchange/all_null.go @@ -0,0 +1,68 @@ +package objchange + +import ( + "fmt" + + "github.com/hashicorp/terraform/configs/configschema" + "github.com/zclconf/go-cty/cty" +) + +// AllAttributesNull constructs a non-null cty.Value of the object type implied +// by the given schema that has all of its leaf attributes set to null and all +// of its nested block collections set to zero-length. +// +// This simulates what would result from decoding an empty configuration block +// with the given schema, except that it does not produce errors +func AllAttributesNull(schema *configschema.Block) cty.Value { + vals := make(map[string]cty.Value) + ty := schema.ImpliedType() + + for name := range schema.Attributes { + aty := ty.AttributeType(name) + vals[name] = cty.NullVal(aty) + } + + for name, blockS := range schema.BlockTypes { + aty := ty.AttributeType(name) + + switch blockS.Nesting { + case configschema.NestingSingle: + // NestingSingle behaves like an object attribute, which decodes + // as null when it's not present in configuration. + vals[name] = cty.NullVal(aty) + default: + // All other nesting types decode as "empty" when not present, but + // empty values take different forms depending on the type. + switch { + case aty.IsListType(): + vals[name] = cty.ListValEmpty(aty.ElementType()) + case aty.IsSetType(): + vals[name] = cty.SetValEmpty(aty.ElementType()) + case aty.IsMapType(): + vals[name] = cty.MapValEmpty(aty.ElementType()) + case aty.Equals(cty.DynamicPseudoType): + // We use DynamicPseudoType in situations where there's a + // nested attribute of DynamicPseudoType, since the schema + // system cannot predict the final type until it knows exactly + // how many elements there will be. However, since we're + // trying to behave as if there are _no_ elements, we know + // we're producing either an empty tuple or empty object + // and just need to distinguish these two cases. + switch blockS.Nesting { + case configschema.NestingList: + vals[name] = cty.EmptyTupleVal + case configschema.NestingMap: + vals[name] = cty.EmptyObjectVal + } + } + } + + // By the time we get down here we should always have set a value. + // If not, that suggests a missing case in the above switches. + if _, ok := vals[name]; !ok { + panic(fmt.Sprintf("failed to create empty value for nested block %q", name)) + } + } + + return cty.ObjectVal(vals) +} diff --git a/plans/objchange/objchange.go b/plans/objchange/objchange.go index 33e82dd52ff5..15565a400694 100644 --- a/plans/objchange/objchange.go +++ b/plans/objchange/objchange.go @@ -26,11 +26,38 @@ import ( // block where _all_ attributes are computed. func ProposedNewObject(schema *configschema.Block, prior, config cty.Value) cty.Value { if prior.IsNull() { - // In this case, we will treat the prior value as unknown so that - // any computed attributes not overridden in config will show as - // unknown values, rather than null values. - prior = cty.UnknownVal(schema.ImpliedType()) + // In this case, we will construct a synthetic prior value that is + // similar to the result of decoding an empty configuration block, + // which simplifies our handling of the top-level attributes/blocks + // below by giving us one non-null level of object to pull values from. + prior = AllAttributesNull(schema) } + return proposedNewObject(schema, prior, config) +} + +// PlannedDataResourceObject is similar to ProposedNewObject but tailored for +// planning data resources in particular. Specifically, it replaces the values +// of any Computed attributes not set in the configuration with an unknown +// value, which serves as a placeholder for a value to be filled in by the +// provider when the data resource is finally read. +// +// Data resources are different because the planning of them is handled +// entirely within Terraform Core and not subject to customization by the +// provider. This function is, in effect, producing an equivalent result to +// passing the ProposedNewObject result into a provider's PlanResourceChange +// function, assuming a fixed implementation of PlanResourceChange that just +// fills in unknown values as needed. +func PlannedDataResourceObject(schema *configschema.Block, config cty.Value) cty.Value { + // Our trick here is to run the ProposedNewObject logic with an + // entirely-unknown prior value. Because of cty's unknown short-circuit + // behavior, any operation on prior returns another unknown, and so + // unknown values propagate into all of the parts of the resulting value + // that would normally be filled in by preserving the prior state. + prior := cty.UnknownVal(schema.ImpliedType()) + return proposedNewObject(schema, prior, config) +} + +func proposedNewObject(schema *configschema.Block, prior, config cty.Value) cty.Value { if config.IsNull() || !config.IsKnown() { // This is a weird situation, but we'll allow it anyway to free // callers from needing to specifically check for these cases. diff --git a/plans/objchange/objchange_test.go b/plans/objchange/objchange_test.go index b2f6402544b9..91f9df1f21c7 100644 --- a/plans/objchange/objchange_test.go +++ b/plans/objchange/objchange_test.go @@ -44,6 +44,11 @@ func TestProposedNewObject(t *testing.T) { Optional: true, Computed: true, }, + "biz": { + Type: cty.String, + Optional: true, + Computed: true, + }, }, }, }, @@ -55,13 +60,26 @@ func TestProposedNewObject(t *testing.T) { "bar": cty.NullVal(cty.String), "baz": cty.ObjectVal(map[string]cty.Value{ "boz": cty.StringVal("world"), + + // An unknown in the config represents a situation where + // an argument is explicitly set to an expression result + // that is derived from an unknown value. This is distinct + // from leaving it null, which allows the provider itself + // to decide the value during PlanResourceChange. + "biz": cty.UnknownVal(cty.String), }), }), cty.ObjectVal(map[string]cty.Value{ "foo": cty.StringVal("hello"), - "bar": cty.UnknownVal(cty.String), + + // unset computed attributes are null in the proposal; provider + // usually changes them to "unknown" during PlanResourceChange, + // to indicate that the value will be decided during apply. + "bar": cty.NullVal(cty.String), + "baz": cty.ObjectVal(map[string]cty.Value{ "boz": cty.StringVal("world"), + "biz": cty.UnknownVal(cty.String), // explicit unknown preserved from config }), }), }, @@ -468,7 +486,49 @@ func TestProposedNewObject(t *testing.T) { }), cty.ObjectVal(map[string]cty.Value{ "bar": cty.StringVal("bosh"), - "baz": cty.UnknownVal(cty.String), + "baz": cty.NullVal(cty.String), + }), + }), + }), + }, + "sets differing only by unknown": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "multi": { + Nesting: configschema.NestingSet, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "optional": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.NullVal(cty.DynamicPseudoType), + cty.ObjectVal(map[string]cty.Value{ + "multi": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "optional": cty.UnknownVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "optional": cty.UnknownVal(cty.String), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "multi": cty.SetVal([]cty.Value{ + // These remain distinct because unknown values never + // compare equal. They may be consolidated together once + // the values become known, though. + cty.ObjectVal(map[string]cty.Value{ + "optional": cty.UnknownVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "optional": cty.UnknownVal(cty.String), }), }), }), diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index f5eadd40b909..a58035233227 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -1985,8 +1985,12 @@ func TestContext2Plan_dataResourceBecomesComputed(t *testing.T) { } p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse { + fooVal := req.ProposedNewState.GetAttr("foo") return providers.PlanResourceChangeResponse{ - PlannedState: req.ProposedNewState, + PlannedState: cty.ObjectVal(map[string]cty.Value{ + "foo": fooVal, + "computed": cty.UnknownVal(cty.String), + }), PlannedPrivate: req.PriorPrivate, } } @@ -1995,9 +1999,9 @@ func TestContext2Plan_dataResourceBecomesComputed(t *testing.T) { ty := schema.ImpliedType() p.ReadDataSourceResponse = providers.ReadDataSourceResponse{ - State: cty.ObjectVal(map[string]cty.Value{ - "foo": cty.UnknownVal(cty.String), - }), + // This should not be called, because the configuration for the + // data resource contains an unknown value for "foo". + Diagnostics: tfdiags.Diagnostics(nil).Append(fmt.Errorf("ReadDataSource called, but should not have been")), } ctx := testContext2(t, &ContextOpts{ @@ -4986,8 +4990,20 @@ func TestContext2Plan_createBeforeDestroy_depends_datasource(t *testing.T) { }, } p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse { + computedVal := req.ProposedNewState.GetAttr("computed") + if computedVal.IsNull() { + computedVal = cty.UnknownVal(cty.String) + } return providers.PlanResourceChangeResponse{ - PlannedState: req.ProposedNewState, + PlannedState: cty.ObjectVal(map[string]cty.Value{ + "num": req.ProposedNewState.GetAttr("num"), + "computed": computedVal, + }), + } + } + p.ReadDataSourceFn = func(req providers.ReadDataSourceRequest) providers.ReadDataSourceResponse { + return providers.ReadDataSourceResponse{ + Diagnostics: tfdiags.Diagnostics(nil).Append(fmt.Errorf("ReadDataSource called, but should not have been")), } } @@ -5000,11 +5016,20 @@ func TestContext2Plan_createBeforeDestroy_depends_datasource(t *testing.T) { ), }) + // We're skipping ctx.Refresh here, which simulates what happens when + // running "terraform plan -refresh=false". As a result, we don't get our + // usual opportunity to read the data source during the refresh step and + // thus the plan call below is forced to produce a deferred read action. + plan, diags := ctx.Plan() + if p.ReadDataSourceCalled { + t.Errorf("ReadDataSource was called on the provider, but should not have been because we didn't refresh") + } if diags.HasErrors() { t.Fatalf("unexpected errors: %s", diags.Err()) } + seenAddrs := make(map[string]struct{}) for _, res := range plan.Changes.Resources { var schema *configschema.Block switch res.Addr.Resource.Resource.Mode { @@ -5019,6 +5044,8 @@ func TestContext2Plan_createBeforeDestroy_depends_datasource(t *testing.T) { t.Fatal(err) } + seenAddrs[ric.Addr.String()] = struct{}{} + t.Run(ric.Addr.String(), func(t *testing.T) { switch i := ric.Addr.String(); i { case "aws_instance.foo[0]": @@ -5042,6 +5069,10 @@ func TestContext2Plan_createBeforeDestroy_depends_datasource(t *testing.T) { t.Fatalf("resource %s should be read, got %s", ric.Addr, ric.Action) } checkVals(t, objectVal(t, schema, map[string]cty.Value{ + // In a normal flow we would've read an exact value in + // ReadDataSource, but because this test doesn't run + // cty.Refresh we have no opportunity to do that lookup + // and a deferred read is forced. "id": cty.UnknownVal(cty.String), "foo": cty.StringVal("0"), }), ric.After) @@ -5050,6 +5081,10 @@ func TestContext2Plan_createBeforeDestroy_depends_datasource(t *testing.T) { t.Fatalf("resource %s should be read, got %s", ric.Addr, ric.Action) } checkVals(t, objectVal(t, schema, map[string]cty.Value{ + // In a normal flow we would've read an exact value in + // ReadDataSource, but because this test doesn't run + // cty.Refresh we have no opportunity to do that lookup + // and a deferred read is forced. "id": cty.UnknownVal(cty.String), "foo": cty.StringVal("1"), }), ric.After) @@ -5058,6 +5093,16 @@ func TestContext2Plan_createBeforeDestroy_depends_datasource(t *testing.T) { } }) } + + wantAddrs := map[string]struct{}{ + "aws_instance.foo[0]": struct{}{}, + "aws_instance.foo[1]": struct{}{}, + "data.aws_vpc.bar[0]": struct{}{}, + "data.aws_vpc.bar[1]": struct{}{}, + } + if !cmp.Equal(seenAddrs, wantAddrs) { + t.Errorf("incorrect addresses in changeset:\n%s", cmp.Diff(wantAddrs, seenAddrs)) + } } // interpolated lists need to be stored in the original order. diff --git a/terraform/eval_read_data.go b/terraform/eval_read_data.go index e9096909eb0f..d89d02ba9113 100644 --- a/terraform/eval_read_data.go +++ b/terraform/eval_read_data.go @@ -104,7 +104,7 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { return nil, diags.Err() } - proposedNewVal := objchange.ProposedNewObject(schema, priorVal, configVal) + proposedNewVal := objchange.PlannedDataResourceObject(schema, configVal) // If our configuration contains any unknown values then we must defer the // read to the apply phase by producing a "Read" change for this resource, @@ -119,7 +119,11 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { absAddr, ) } - log.Printf("[TRACE] EvalReadData: %s configuration not fully known yet, so deferring to apply phase", absAddr) + if n.ForcePlanRead { + log.Printf("[TRACE] EvalReadData: %s configuration is fully known, but we're forcing a read plan to be created", absAddr) + } else { + log.Printf("[TRACE] EvalReadData: %s configuration not fully known yet, so deferring to apply phase", absAddr) + } err := ctx.Hook(func(h Hook) (HookAction, error) { return h.PreDiff(absAddr, states.CurrentGen, priorVal, proposedNewVal) @@ -227,6 +231,24 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { ), )) } + if !newVal.IsWhollyKnown() { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Provider produced invalid object", + fmt.Sprintf( + "Provider %q produced a value for %s that is not wholly known.\n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.", + n.ProviderAddr.ProviderConfig.Type, absAddr, + ), + )) + + // We'll still save the object, but we need to eliminate any unknown + // values first because we can't serialize them in the state file. + // Note that this may cause set elements to be coalesced if they + // differed only by having unknown values, but we don't worry about + // that here because we're saving the value only for inspection + // purposes; the error we added above will halt the graph walk. + newVal = cty.UnknownAsNull(newVal) + } // Since we've completed the read, we actually have no change to make, but // we'll produce a NoOp one anyway to preserve the usual flow of the