diff --git a/builtin/providers/test/provider.go b/builtin/providers/test/provider.go index 155b39422..524200a1a 100644 --- a/builtin/providers/test/provider.go +++ b/builtin/providers/test/provider.go @@ -26,6 +26,7 @@ func Provider() terraform.ResourceProvider { "test_resource_nested": testResourceNested(), "test_resource_nested_set": testResourceNestedSet(), "test_resource_state_func": testResourceStateFunc(), + "test_resource_deprecated": testResourceDeprecated(), }, DataSourcesMap: map[string]*schema.Resource{ "test_data_source": testDataSource(), diff --git a/builtin/providers/test/resource_deprecated.go b/builtin/providers/test/resource_deprecated.go new file mode 100644 index 000000000..a176977b9 --- /dev/null +++ b/builtin/providers/test/resource_deprecated.go @@ -0,0 +1,119 @@ +package test + +import ( + "github.com/hashicorp/terraform/helper/schema" +) + +func testResourceDeprecated() *schema.Resource { + return &schema.Resource{ + Create: testResourceDeprecatedCreate, + Read: testResourceDeprecatedRead, + Update: testResourceDeprecatedUpdate, + Delete: testResourceDeprecatedDelete, + + Schema: map[string]*schema.Schema{ + "map_deprecated": { + Type: schema.TypeMap, + Optional: true, + Deprecated: "deprecated", + }, + "map_removed": { + Type: schema.TypeMap, + Optional: true, + Removed: "removed", + }, + "set_block_deprecated": { + Type: schema.TypeSet, + Optional: true, + MaxItems: 1, + Deprecated: "deprecated", + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "value": { + Type: schema.TypeString, + Required: true, + Deprecated: "deprecated", + }, + "optional": { + Type: schema.TypeString, + ForceNew: true, + Optional: true, + Deprecated: "deprecated", + }, + }, + }, + }, + "set_block_removed": { + Type: schema.TypeSet, + Optional: true, + MaxItems: 1, + Removed: "Removed", + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "optional": { + Type: schema.TypeString, + ForceNew: true, + Optional: true, + Computed: true, + Removed: "removed", + }, + }, + }, + }, + "list_block_deprecated": { + Type: schema.TypeList, + Optional: true, + Deprecated: "deprecated", + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "value": { + Type: schema.TypeString, + Required: true, + Deprecated: "deprecated", + }, + "optional": { + Type: schema.TypeString, + ForceNew: true, + Optional: true, + Deprecated: "deprecated", + }, + }, + }, + }, + "list_block_removed": { + Type: schema.TypeList, + Optional: true, + Removed: "removed", + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "optional": { + Type: schema.TypeString, + ForceNew: true, + Optional: true, + Removed: "removed", + }, + }, + }, + }, + }, + } +} + +func testResourceDeprecatedCreate(d *schema.ResourceData, meta interface{}) error { + d.SetId("testId") + return nil +} + +func testResourceDeprecatedRead(d *schema.ResourceData, meta interface{}) error { + + return nil +} + +func testResourceDeprecatedUpdate(d *schema.ResourceData, meta interface{}) error { + return nil +} + +func testResourceDeprecatedDelete(d *schema.ResourceData, meta interface{}) error { + d.SetId("") + return nil +} diff --git a/builtin/providers/test/resource_deprecated_test.go b/builtin/providers/test/resource_deprecated_test.go new file mode 100644 index 000000000..8817567d9 --- /dev/null +++ b/builtin/providers/test/resource_deprecated_test.go @@ -0,0 +1,71 @@ +package test + +import ( + "regexp" + "strings" + "testing" + + "github.com/hashicorp/terraform/helper/resource" +) + +// an empty config should be ok, because no deprecated/removed fields are set. +func TestResourceDeprecated_empty(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_deprecated" "foo" { +} + `), + }, + }, + }) +} + +// Deprecated fields should still work +func TestResourceDeprecated_deprecatedOK(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_deprecated" "foo" { + map_deprecated = { + "a" = "b", + } + set_block_deprecated { + value = "1" + } + list_block_deprecated { + value = "2" + } +} + `), + }, + }, + }) +} + +// Declaring an empty block should trigger the error +func TestResourceDeprecated_removedBlocks(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_deprecated" "foo" { + set_block_removed { + } + list_block_removed { + } +} + `), + ExpectError: regexp.MustCompile("REMOVED"), + }, + }, + }) +} diff --git a/config/hcl2shim/values.go b/config/hcl2shim/values.go index 000ad7ba8..2c5b2907e 100644 --- a/config/hcl2shim/values.go +++ b/config/hcl2shim/values.go @@ -80,6 +80,11 @@ func ConfigValueFromHCL2Block(v cty.Value, schema *configschema.Block) map[strin case configschema.NestingList, configschema.NestingSet: l := bv.LengthInt() + if l == 0 { + // skip empty collections to better mimic how HCL1 would behave + continue + } + elems := make([]interface{}, 0, l) for it := bv.ElementIterator(); it.Next(); { _, ev := it.Element() @@ -92,6 +97,11 @@ func ConfigValueFromHCL2Block(v cty.Value, schema *configschema.Block) map[strin ret[name] = elems case configschema.NestingMap: + if bv.LengthInt() == 0 { + // skip empty collections to better mimic how HCL1 would behave + continue + } + elems := make(map[string]interface{}) for it := bv.ElementIterator(); it.Next(); { ek, ev := it.Element() diff --git a/config/hcl2shim/values_test.go b/config/hcl2shim/values_test.go index 805155f0b..7c3011da0 100644 --- a/config/hcl2shim/values_test.go +++ b/config/hcl2shim/values_test.go @@ -151,7 +151,7 @@ func TestConfigValueFromHCL2Block(t *testing.T) { }, { cty.ObjectVal(map[string]cty.Value{ - "address": cty.ListValEmpty(cty.EmptyObject), + "address": cty.ListValEmpty(cty.EmptyObject), // should be omitted altogether in result }), &configschema.Block{ BlockTypes: map[string]*configschema.NestedBlock{ @@ -161,9 +161,7 @@ func TestConfigValueFromHCL2Block(t *testing.T) { }, }, }, - map[string]interface{}{ - "address": []interface{}{}, - }, + map[string]interface{}{}, }, { cty.ObjectVal(map[string]cty.Value{ @@ -195,9 +193,7 @@ func TestConfigValueFromHCL2Block(t *testing.T) { }, }, }, - map[string]interface{}{ - "address": []interface{}{}, - }, + map[string]interface{}{}, }, { cty.ObjectVal(map[string]cty.Value{ @@ -229,9 +225,7 @@ func TestConfigValueFromHCL2Block(t *testing.T) { }, }, }, - map[string]interface{}{ - "address": map[string]interface{}{}, - }, + map[string]interface{}{}, }, { cty.NullVal(cty.EmptyObject), @@ -240,8 +234,8 @@ func TestConfigValueFromHCL2Block(t *testing.T) { }, } - for i, test := range tests { - t.Run(fmt.Sprintf("%d-%#v", i, test.Input), func(t *testing.T) { + for _, test := range tests { + t.Run(fmt.Sprintf("%#v", test.Input), func(t *testing.T) { got := ConfigValueFromHCL2Block(test.Input, test.Schema) if !reflect.DeepEqual(got, test.Want) { t.Errorf("wrong result\ninput: %#v\ngot: %#v\nwant: %#v", test.Input, got, test.Want) diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index c9ffa1a23..d3ff07717 100644 --- a/helper/plugin/grpc_provider.go +++ b/helper/plugin/grpc_provider.go @@ -421,6 +421,13 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso return resp, nil } + if newInstanceState != nil { + // here we use the prior state to check for unknown/zero containers values + // when normalizing the flatmap. + stateAttrs := hcl2shim.FlatmapValueFromHCL2(stateVal) + newInstanceState.Attributes = normalizeFlatmapContainers(stateAttrs, newInstanceState.Attributes, true) + } + if newInstanceState == nil || newInstanceState.ID == "" { // The old provider API used an empty id to signal that the remote // object appears to have been deleted, but our new protocol expects @@ -445,6 +452,7 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso } newStateVal = copyTimeoutValues(newStateVal, stateVal) + newStateVal = copyMissingValues(newStateVal, stateVal) newStateMP, err := msgpack.Marshal(newStateVal, block.ImpliedType()) if err != nil { @@ -521,7 +529,7 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl // strip out non-diffs for k, v := range diff.Attributes { - if v.New == v.Old && !v.NewComputed && !v.NewRemoved { + if v.New == v.Old && !v.NewComputed { delete(diff.Attributes, k) } } @@ -705,7 +713,7 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A // strip out non-diffs for k, v := range diff.Attributes { - if v.New == v.Old && !v.NewComputed && !v.NewRemoved && v.NewExtra == "" { + if v.New == v.Old && !v.NewComputed && v.NewExtra == "" { delete(diff.Attributes, k) } } diff --git a/helper/resource/state_shim.go b/helper/resource/state_shim.go index 264928d83..5514426b6 100644 --- a/helper/resource/state_shim.go +++ b/helper/resource/state_shim.go @@ -49,7 +49,7 @@ func shimNewState(newState *states.State, providers map[string]terraform.Resourc resType := res.Addr.Type providerType := res.ProviderConfig.ProviderConfig.Type - resource := getResource(providers, providerType, resType) + resource := getResource(providers, providerType, res.Addr) for key, i := range res.Instances { flatmap, err := shimmedAttributes(i.Current, resource) @@ -116,7 +116,7 @@ func shimNewState(newState *states.State, providers map[string]terraform.Resourc return state, nil } -func getResource(providers map[string]terraform.ResourceProvider, providerName, resourceType string) *schema.Resource { +func getResource(providers map[string]terraform.ResourceProvider, providerName string, addr addrs.Resource) *schema.Resource { p := providers[providerName] if p == nil { panic(fmt.Sprintf("provider %q not found in test step", providerName)) @@ -125,23 +125,24 @@ func getResource(providers map[string]terraform.ResourceProvider, providerName, // this is only for tests, so should only see schema.Providers provider := p.(*schema.Provider) - resource := provider.ResourcesMap[resourceType] - if resource != nil { - return resource - + switch addr.Mode { + case addrs.ManagedResourceMode: + resource := provider.ResourcesMap[addr.Type] + if resource != nil { + return resource + } + case addrs.DataResourceMode: + resource := provider.DataSourcesMap[addr.Type] + if resource != nil { + return resource + } } - resource = provider.DataSourcesMap[resourceType] - if resource != nil { - return resource - } - - panic(fmt.Sprintf("resource %s not found in test step", resourceType)) + panic(fmt.Sprintf("resource %s not found in test step", addr.Type)) } func shimmedAttributes(instance *states.ResourceInstanceObjectSrc, res *schema.Resource) (map[string]string, error) { flatmap := instance.AttrsFlat - if flatmap != nil { return flatmap, nil }