diff --git a/builtin/providers/test/diff_apply_test.go b/builtin/providers/test/diff_apply_test.go index 63a186d36..144e70624 100644 --- a/builtin/providers/test/diff_apply_test.go +++ b/builtin/providers/test/diff_apply_test.go @@ -131,7 +131,7 @@ func TestDiffApply_set(t *testing.T) { "id": "testID", } - attrs, err := diff.Apply(priorAttrs, schema.LegacyResourceSchema(&schema.Resource{Schema: resSchema}).CoreConfigSchema()) + attrs, err := diff.Apply(priorAttrs, (&schema.Resource{Schema: resSchema}).CoreConfigSchema()) if err != nil { t.Fatal(err) } diff --git a/builtin/providers/test/resource_config_mode.go b/builtin/providers/test/resource_config_mode.go index 82e8ff214..da0896e33 100644 --- a/builtin/providers/test/resource_config_mode.go +++ b/builtin/providers/test/resource_config_mode.go @@ -28,21 +28,6 @@ func testResourceConfigMode() *schema.Resource { }, }, }, - "resource_as_attr_dynamic": { - Type: schema.TypeList, - ConfigMode: schema.SchemaConfigModeAttr, - SkipCoreTypeCheck: true, - Optional: true, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "foo": { - Type: schema.TypeString, - Optional: true, - Default: "default", - }, - }, - }, - }, }, } } @@ -53,14 +38,12 @@ func testResourceConfigModeCreate(d *schema.ResourceData, meta interface{}) erro } func testResourceConfigModeRead(d *schema.ResourceData, meta interface{}) error { - for _, k := range []string{"resource_as_attr", "resource_as_attr_dynamic"} { - if l, ok := d.Get(k).([]interface{}); !ok { - return fmt.Errorf("%s should appear as []interface{}, not %T", k, l) - } else { - for i, item := range l { - if _, ok := item.(map[string]interface{}); !ok { - return fmt.Errorf("%s[%d] should appear as map[string]interface{}, not %T", k, i, item) - } + if l, ok := d.Get("resource_as_attr").([]interface{}); !ok { + return fmt.Errorf("resource_as_attr should appear as []interface{}, not %T", l) + } else { + for i, item := range l { + if _, ok := item.(map[string]interface{}); !ok { + return fmt.Errorf("resource_as_attr[%d] should appear as map[string]interface{}, not %T", i, item) } } } diff --git a/builtin/providers/test/resource_config_mode_test.go b/builtin/providers/test/resource_config_mode_test.go index 02d0c575a..46445bed5 100644 --- a/builtin/providers/test/resource_config_mode_test.go +++ b/builtin/providers/test/resource_config_mode_test.go @@ -23,22 +23,12 @@ resource "test_resource_config_mode" "foo" { foo = "resource_as_attr 1" }, ] - resource_as_attr_dynamic = [ - { - foo = "resource_as_attr_dynamic 0" - }, - { - }, - ] } `), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr.#", "2"), resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr.0.foo", "resource_as_attr 0"), resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr.1.foo", "resource_as_attr 1"), - resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr_dynamic.#", "2"), - resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr_dynamic.0.foo", "resource_as_attr_dynamic 0"), - resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr_dynamic.1.foo", "default"), ), }, resource.TestStep{ @@ -58,22 +48,12 @@ resource "test_resource_config_mode" "foo" { resource_as_attr { foo = "resource_as_attr 1" } - resource_as_attr_dynamic = [ - { - foo = "resource_as_attr_dynamic 0" - }, - { - }, - ] } `), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr.#", "2"), resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr.0.foo", "resource_as_attr 0"), resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr.1.foo", "resource_as_attr 1"), - resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr_dynamic.#", "2"), - resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr_dynamic.0.foo", "resource_as_attr_dynamic 0"), - resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr_dynamic.1.foo", "default"), ), }, resource.TestStep{ @@ -84,43 +64,21 @@ resource "test_resource_config_mode" "foo" { foo = "resource_as_attr 0 updated" }, ] - resource_as_attr_dynamic = [ - { - }, - ] } `), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr.#", "1"), resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr.0.foo", "resource_as_attr 0 updated"), - resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr_dynamic.#", "1"), - resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr_dynamic.0.foo", "default"), - ), - }, - resource.TestStep{ - Config: strings.TrimSpace(` -resource "test_resource_config_mode" "foo" { - resource_as_attr_dynamic = [ - { - }, - ] -} - `), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr.#", "1"), - resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr_dynamic.#", "1"), ), }, resource.TestStep{ Config: strings.TrimSpace(` resource "test_resource_config_mode" "foo" { resource_as_attr = [] - resource_as_attr_dynamic = [] } `), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr.#", "0"), - resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr_dynamic.#", "0"), ), }, resource.TestStep{ @@ -130,7 +88,6 @@ resource "test_resource_config_mode" "foo" { `), Check: resource.ComposeTestCheckFunc( resource.TestCheckNoResourceAttr("test_resource_config_mode.foo", "resource_as_attr.#"), - resource.TestCheckNoResourceAttr("test_resource_config_mode.foo", "resource_as_attr_dynamic.#"), ), }, }, diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index b9b420995..510f47f35 100644 --- a/helper/plugin/grpc_provider.go +++ b/helper/plugin/grpc_provider.go @@ -52,7 +52,7 @@ func (s *GRPCProviderServer) GetSchema(_ context.Context, req *proto.GetProvider } resp.Provider = &proto.Schema{ - Block: convert.ConfigSchemaToProto(s.getProviderSchemaBlockForCore()), + Block: convert.ConfigSchemaToProto(s.getProviderSchemaBlock()), } for typ, res := range s.provider.ResourcesMap { @@ -72,46 +72,26 @@ func (s *GRPCProviderServer) GetSchema(_ context.Context, req *proto.GetProvider return resp, nil } -func (s *GRPCProviderServer) getProviderSchemaBlockForCore() *configschema.Block { +func (s *GRPCProviderServer) getProviderSchemaBlock() *configschema.Block { return schema.InternalMap(s.provider.Schema).CoreConfigSchema() } -func (s *GRPCProviderServer) getResourceSchemaBlockForCore(name string) *configschema.Block { +func (s *GRPCProviderServer) getResourceSchemaBlock(name string) *configschema.Block { res := s.provider.ResourcesMap[name] return res.CoreConfigSchema() } -func (s *GRPCProviderServer) getDatasourceSchemaBlockForCore(name string) *configschema.Block { +func (s *GRPCProviderServer) getDatasourceSchemaBlock(name string) *configschema.Block { dat := s.provider.DataSourcesMap[name] return dat.CoreConfigSchema() } -func (s *GRPCProviderServer) getProviderSchemaBlockForShimming() *configschema.Block { - newSchema := map[string]*schema.Schema{} - for attr, s := range s.provider.Schema { - newSchema[attr] = schema.LegacySchema(s) - } - - return schema.InternalMap(newSchema).CoreConfigSchema() -} - -func (s *GRPCProviderServer) getResourceSchemaBlockForShimming(name string) *configschema.Block { - res := s.provider.ResourcesMap[name] - return schema.LegacyResourceSchema(res).CoreConfigSchema() -} - -func (s *GRPCProviderServer) getDatasourceSchemaBlockForShimming(name string) *configschema.Block { - dat := s.provider.DataSourcesMap[name] - return schema.LegacyResourceSchema(dat).CoreConfigSchema() -} - func (s *GRPCProviderServer) PrepareProviderConfig(_ context.Context, req *proto.PrepareProviderConfig_Request) (*proto.PrepareProviderConfig_Response, error) { resp := &proto.PrepareProviderConfig_Response{} - blockForCore := s.getProviderSchemaBlockForCore() - blockForShimming := s.getProviderSchemaBlockForShimming() + schemaBlock := s.getProviderSchemaBlock() - configVal, err := msgpack.Unmarshal(req.Config.Msgpack, blockForCore.ImpliedType()) + configVal, err := msgpack.Unmarshal(req.Config.Msgpack, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -182,7 +162,7 @@ func (s *GRPCProviderServer) PrepareProviderConfig(_ context.Context, req *proto return resp, nil } - configVal, err = blockForShimming.CoerceValue(configVal) + configVal, err = schemaBlock.CoerceValue(configVal) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -194,12 +174,12 @@ func (s *GRPCProviderServer) PrepareProviderConfig(_ context.Context, req *proto return resp, nil } - config := terraform.NewResourceConfigShimmed(configVal, blockForShimming) + config := terraform.NewResourceConfigShimmed(configVal, schemaBlock) warns, errs := s.provider.Validate(config) resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, convert.WarnsAndErrsToProto(warns, errs)) - preparedConfigMP, err := msgpack.Marshal(configVal, blockForCore.ImpliedType()) + preparedConfigMP, err := msgpack.Marshal(configVal, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -213,16 +193,15 @@ func (s *GRPCProviderServer) PrepareProviderConfig(_ context.Context, req *proto func (s *GRPCProviderServer) ValidateResourceTypeConfig(_ context.Context, req *proto.ValidateResourceTypeConfig_Request) (*proto.ValidateResourceTypeConfig_Response, error) { resp := &proto.ValidateResourceTypeConfig_Response{} - blockForCore := s.getResourceSchemaBlockForCore(req.TypeName) - blockForShimming := s.getResourceSchemaBlockForShimming(req.TypeName) + schemaBlock := s.getResourceSchemaBlock(req.TypeName) - configVal, err := msgpack.Unmarshal(req.Config.Msgpack, blockForCore.ImpliedType()) + configVal, err := msgpack.Unmarshal(req.Config.Msgpack, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil } - config := terraform.NewResourceConfigShimmed(configVal, blockForShimming) + config := terraform.NewResourceConfigShimmed(configVal, schemaBlock) warns, errs := s.provider.ValidateResource(req.TypeName, config) resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, convert.WarnsAndErrsToProto(warns, errs)) @@ -233,10 +212,9 @@ func (s *GRPCProviderServer) ValidateResourceTypeConfig(_ context.Context, req * func (s *GRPCProviderServer) ValidateDataSourceConfig(_ context.Context, req *proto.ValidateDataSourceConfig_Request) (*proto.ValidateDataSourceConfig_Response, error) { resp := &proto.ValidateDataSourceConfig_Response{} - blockForCore := s.getDatasourceSchemaBlockForCore(req.TypeName) - blockForShimming := s.getDatasourceSchemaBlockForShimming(req.TypeName) + schemaBlock := s.getDatasourceSchemaBlock(req.TypeName) - configVal, err := msgpack.Unmarshal(req.Config.Msgpack, blockForCore.ImpliedType()) + configVal, err := msgpack.Unmarshal(req.Config.Msgpack, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -248,7 +226,7 @@ func (s *GRPCProviderServer) ValidateDataSourceConfig(_ context.Context, req *pr return resp, nil } - config := terraform.NewResourceConfigShimmed(configVal, blockForShimming) + config := terraform.NewResourceConfigShimmed(configVal, schemaBlock) warns, errs := s.provider.ValidateDataSource(req.TypeName, config) resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, convert.WarnsAndErrsToProto(warns, errs)) @@ -260,31 +238,32 @@ func (s *GRPCProviderServer) UpgradeResourceState(_ context.Context, req *proto. resp := &proto.UpgradeResourceState_Response{} res := s.provider.ResourcesMap[req.TypeName] - blockForCore := s.getResourceSchemaBlockForCore(req.TypeName) - blockForShimming := s.getResourceSchemaBlockForShimming(req.TypeName) + schemaBlock := s.getResourceSchemaBlock(req.TypeName) version := int(req.Version) - var jsonMap map[string]interface{} + jsonMap := map[string]interface{}{} var err error - // if there's a JSON state, we need to decode it. - if len(req.RawState.Json) > 0 { - err = json.Unmarshal(req.RawState.Json, &jsonMap) - if err != nil { - resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) - return resp, nil - } - } - + switch { // We first need to upgrade a flatmap state if it exists. // There should never be both a JSON and Flatmap state in the request. - if req.RawState.Flatmap != nil { + case len(req.RawState.Flatmap) > 0: jsonMap, version, err = s.upgradeFlatmapState(version, req.RawState.Flatmap, res) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil } + // if there's a JSON state, we need to decode it. + case len(req.RawState.Json) > 0: + err = json.Unmarshal(req.RawState.Json, &jsonMap) + if err != nil { + resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) + return resp, nil + } + default: + log.Println("[DEBUG] no state provided to upgrade") + return resp, nil } // complete the upgrade of the JSON states @@ -295,18 +274,18 @@ func (s *GRPCProviderServer) UpgradeResourceState(_ context.Context, req *proto. } // The provider isn't required to clean out removed fields - s.removeAttributes(jsonMap, blockForShimming.ImpliedType()) + s.removeAttributes(jsonMap, schemaBlock.ImpliedType()) // now we need to turn the state into the default json representation, so // that it can be re-decoded using the actual schema. - val, err := schema.JSONMapToStateValue(jsonMap, blockForShimming) + val, err := schema.JSONMapToStateValue(jsonMap, schemaBlock) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil } // encode the final state to the expected msgpack format - newStateMP, err := msgpack.Marshal(val, blockForCore.ImpliedType()) + newStateMP, err := msgpack.Marshal(val, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -329,7 +308,7 @@ func (s *GRPCProviderServer) upgradeFlatmapState(version int, m map[string]strin // first determine if we need to call the legacy MigrateState func requiresMigrate := version < res.SchemaVersion - schemaType := schema.LegacyResourceSchema(res).CoreConfigSchema().ImpliedType() + schemaType := res.CoreConfigSchema().ImpliedType() // if there are any StateUpgraders, then we need to only compare // against the first version there @@ -433,6 +412,11 @@ func (s *GRPCProviderServer) removeAttributes(v interface{}, ty cty.Type) { return } + if ty == cty.DynamicPseudoType { + log.Printf("[DEBUG] ignoring dynamic block: %#v\n", v) + return + } + if !ty.IsObjectType() { // This shouldn't happen, and will fail to decode further on, so // there's no need to handle it here. @@ -468,10 +452,9 @@ func (s *GRPCProviderServer) Stop(_ context.Context, _ *proto.Stop_Request) (*pr func (s *GRPCProviderServer) Configure(_ context.Context, req *proto.Configure_Request) (*proto.Configure_Response, error) { resp := &proto.Configure_Response{} - blockForCore := s.getProviderSchemaBlockForCore() - blockForShimming := s.getProviderSchemaBlockForShimming() + schemaBlock := s.getProviderSchemaBlock() - configVal, err := msgpack.Unmarshal(req.Config.Msgpack, blockForCore.ImpliedType()) + configVal, err := msgpack.Unmarshal(req.Config.Msgpack, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -485,7 +468,7 @@ func (s *GRPCProviderServer) Configure(_ context.Context, req *proto.Configure_R return resp, nil } - config := terraform.NewResourceConfigShimmed(configVal, blockForShimming) + config := terraform.NewResourceConfigShimmed(configVal, schemaBlock) err = s.provider.Configure(config) resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) @@ -496,10 +479,9 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso resp := &proto.ReadResource_Response{} res := s.provider.ResourcesMap[req.TypeName] - blockForCore := s.getResourceSchemaBlockForCore(req.TypeName) - blockForShimming := s.getResourceSchemaBlockForShimming(req.TypeName) + schemaBlock := s.getResourceSchemaBlock(req.TypeName) - stateVal, err := msgpack.Unmarshal(req.CurrentState.Msgpack, blockForCore.ImpliedType()) + stateVal, err := msgpack.Unmarshal(req.CurrentState.Msgpack, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -521,7 +503,7 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso // The old provider API used an empty id to signal that the remote // object appears to have been deleted, but our new protocol expects // to see a null value (in the cty sense) in that case. - newStateMP, err := msgpack.Marshal(cty.NullVal(blockForCore.ImpliedType()), blockForCore.ImpliedType()) + newStateMP, err := msgpack.Marshal(cty.NullVal(schemaBlock.ImpliedType()), schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) } @@ -534,7 +516,7 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso // helper/schema should always copy the ID over, but do it again just to be safe newInstanceState.Attributes["id"] = newInstanceState.ID - newStateVal, err := hcl2shim.HCL2ValueFromFlatmap(newInstanceState.Attributes, blockForShimming.ImpliedType()) + newStateVal, err := hcl2shim.HCL2ValueFromFlatmap(newInstanceState.Attributes, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -543,7 +525,7 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso newStateVal = normalizeNullValues(newStateVal, stateVal, false) newStateVal = copyTimeoutValues(newStateVal, stateVal) - newStateMP, err := msgpack.Marshal(newStateVal, blockForCore.ImpliedType()) + newStateMP, err := msgpack.Marshal(newStateVal, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -568,10 +550,9 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl resp.LegacyTypeSystem = true res := s.provider.ResourcesMap[req.TypeName] - blockForCore := s.getResourceSchemaBlockForCore(req.TypeName) - blockForShimming := s.getResourceSchemaBlockForShimming(req.TypeName) + schemaBlock := s.getResourceSchemaBlock(req.TypeName) - priorStateVal, err := msgpack.Unmarshal(req.PriorState.Msgpack, blockForCore.ImpliedType()) + priorStateVal, err := msgpack.Unmarshal(req.PriorState.Msgpack, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -579,7 +560,7 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl create := priorStateVal.IsNull() - proposedNewStateVal, err := msgpack.Unmarshal(req.ProposedNewState.Msgpack, blockForCore.ImpliedType()) + proposedNewStateVal, err := msgpack.Unmarshal(req.ProposedNewState.Msgpack, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -617,7 +598,7 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl } // turn the proposed state into a legacy configuration - cfg := terraform.NewResourceConfigShimmed(proposedNewStateVal, blockForShimming) + cfg := terraform.NewResourceConfigShimmed(proposedNewStateVal, schemaBlock) diff, err := s.provider.SimpleDiff(info, priorState, cfg) if err != nil { @@ -650,15 +631,15 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl } // now we need to apply the diff to the prior state, so get the planned state - plannedAttrs, err := diff.Apply(priorState.Attributes, blockForShimming) + plannedAttrs, err := diff.Apply(priorState.Attributes, schemaBlock) - plannedStateVal, err := hcl2shim.HCL2ValueFromFlatmap(plannedAttrs, blockForShimming.ImpliedType()) + plannedStateVal, err := hcl2shim.HCL2ValueFromFlatmap(plannedAttrs, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil } - plannedStateVal, err = blockForShimming.CoerceValue(plannedStateVal) + plannedStateVal, err = schemaBlock.CoerceValue(plannedStateVal) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -690,10 +671,10 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl // if this was creating the resource, we need to set any remaining computed // fields if create { - plannedStateVal = SetUnknowns(plannedStateVal, blockForShimming) + plannedStateVal = SetUnknowns(plannedStateVal, schemaBlock) } - plannedMP, err := msgpack.Marshal(plannedStateVal, blockForCore.ImpliedType()) + plannedMP, err := msgpack.Marshal(plannedStateVal, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -746,7 +727,7 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl requiresNew = append(requiresNew, "id") } - requiresReplace, err := hcl2shim.RequiresReplace(requiresNew, blockForShimming.ImpliedType()) + requiresReplace, err := hcl2shim.RequiresReplace(requiresNew, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -767,16 +748,15 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A } res := s.provider.ResourcesMap[req.TypeName] - blockForCore := s.getResourceSchemaBlockForCore(req.TypeName) - blockForShimming := s.getResourceSchemaBlockForShimming(req.TypeName) + schemaBlock := s.getResourceSchemaBlock(req.TypeName) - priorStateVal, err := msgpack.Unmarshal(req.PriorState.Msgpack, blockForCore.ImpliedType()) + priorStateVal, err := msgpack.Unmarshal(req.PriorState.Msgpack, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil } - plannedStateVal, err := msgpack.Unmarshal(req.PlannedState.Msgpack, blockForCore.ImpliedType()) + plannedStateVal, err := msgpack.Unmarshal(req.PlannedState.Msgpack, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -864,13 +844,13 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) } - newStateVal := cty.NullVal(blockForShimming.ImpliedType()) + newStateVal := cty.NullVal(schemaBlock.ImpliedType()) // Always return a null value for destroy. // While this is usually indicated by a nil state, check for missing ID or // attributes in the case of a provider failure. if destroy || newInstanceState == nil || newInstanceState.Attributes == nil || newInstanceState.ID == "" { - newStateMP, err := msgpack.Marshal(newStateVal, blockForCore.ImpliedType()) + newStateMP, err := msgpack.Marshal(newStateVal, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -883,7 +863,7 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A // We keep the null val if we destroyed the resource, otherwise build the // entire object, even if the new state was nil. - newStateVal, err = schema.StateValueFromInstanceState(newInstanceState, blockForShimming.ImpliedType()) + newStateVal, err = schema.StateValueFromInstanceState(newInstanceState, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -893,7 +873,7 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A newStateVal = copyTimeoutValues(newStateVal, plannedStateVal) - newStateMP, err := msgpack.Marshal(newStateVal, blockForCore.ImpliedType()) + newStateMP, err := msgpack.Marshal(newStateVal, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -942,15 +922,14 @@ func (s *GRPCProviderServer) ImportResourceState(_ context.Context, req *proto.I resourceType = req.TypeName } - blockForCore := s.getResourceSchemaBlockForCore(resourceType) - blockForShimming := s.getResourceSchemaBlockForShimming(resourceType) - newStateVal, err := hcl2shim.HCL2ValueFromFlatmap(is.Attributes, blockForShimming.ImpliedType()) + schemaBlock := s.getResourceSchemaBlock(resourceType) + newStateVal, err := hcl2shim.HCL2ValueFromFlatmap(is.Attributes, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil } - newStateMP, err := msgpack.Marshal(newStateVal, blockForCore.ImpliedType()) + newStateMP, err := msgpack.Marshal(newStateVal, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -979,10 +958,9 @@ func (s *GRPCProviderServer) ImportResourceState(_ context.Context, req *proto.I func (s *GRPCProviderServer) ReadDataSource(_ context.Context, req *proto.ReadDataSource_Request) (*proto.ReadDataSource_Response, error) { resp := &proto.ReadDataSource_Response{} - blockForCore := s.getDatasourceSchemaBlockForCore(req.TypeName) - blockForShimming := s.getDatasourceSchemaBlockForShimming(req.TypeName) + schemaBlock := s.getDatasourceSchemaBlock(req.TypeName) - configVal, err := msgpack.Unmarshal(req.Config.Msgpack, blockForCore.ImpliedType()) + configVal, err := msgpack.Unmarshal(req.Config.Msgpack, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -998,7 +976,7 @@ func (s *GRPCProviderServer) ReadDataSource(_ context.Context, req *proto.ReadDa return resp, nil } - config := terraform.NewResourceConfigShimmed(configVal, blockForShimming) + config := terraform.NewResourceConfigShimmed(configVal, schemaBlock) // we need to still build the diff separately with the Read method to match // the old behavior @@ -1015,7 +993,7 @@ func (s *GRPCProviderServer) ReadDataSource(_ context.Context, req *proto.ReadDa return resp, nil } - newStateVal, err := schema.StateValueFromInstanceState(newInstanceState, blockForShimming.ImpliedType()) + newStateVal, err := schema.StateValueFromInstanceState(newInstanceState, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -1023,7 +1001,7 @@ func (s *GRPCProviderServer) ReadDataSource(_ context.Context, req *proto.ReadDa newStateVal = copyTimeoutValues(newStateVal, configVal) - newStateMP, err := msgpack.Marshal(newStateVal, blockForCore.ImpliedType()) + newStateMP, err := msgpack.Marshal(newStateVal, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil diff --git a/helper/plugin/grpc_provider_test.go b/helper/plugin/grpc_provider_test.go index f58778c47..6186cd411 100644 --- a/helper/plugin/grpc_provider_test.go +++ b/helper/plugin/grpc_provider_test.go @@ -154,10 +154,9 @@ func TestUpgradeState_removedAttr(t *testing.T) { r3 := &schema.Resource{ Schema: map[string]*schema.Schema{ "config_mode_attr": { - Type: schema.TypeList, - ConfigMode: schema.SchemaConfigModeAttr, - SkipCoreTypeCheck: true, - Optional: true, + Type: schema.TypeList, + ConfigMode: schema.SchemaConfigModeAttr, + Optional: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "foo": { diff --git a/helper/schema/core_schema.go b/helper/schema/core_schema.go index d563ab8ca..875677eb3 100644 --- a/helper/schema/core_schema.go +++ b/helper/schema/core_schema.go @@ -174,14 +174,6 @@ func (s *Schema) coreConfigSchemaBlock() *configschema.NestedBlock { // coreConfigSchemaType determines the core config schema type that corresponds // to a particular schema's type. func (s *Schema) coreConfigSchemaType() cty.Type { - if s.SkipCoreTypeCheck { - // If we're preparing a schema for Terraform Core and the schema is - // asking us to skip the Core type-check then we'll tell core that this - // attribute is dynamically-typed, so it'll just pass through anything - // and let us validate it on the plugin side. - return cty.DynamicPseudoType - } - switch s.Type { case TypeString: return cty.String diff --git a/helper/schema/core_schema_test.go b/helper/schema/core_schema_test.go index 92fe194af..7d4b32e01 100644 --- a/helper/schema/core_schema_test.go +++ b/helper/schema/core_schema_test.go @@ -445,28 +445,6 @@ func TestSchemaMapCoreConfigSchema(t *testing.T) { BlockTypes: map[string]*configschema.NestedBlock{}, }), }, - "skip core type check": { - map[string]*Schema{ - "list": { - Type: TypeList, - ConfigMode: SchemaConfigModeAttr, - SkipCoreTypeCheck: true, - Optional: true, - Elem: &Resource{ - Schema: map[string]*Schema{}, - }, - }, - }, - testResource(&configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "list": { - Type: cty.DynamicPseudoType, - Optional: true, // Just so we can progress to provider-driven validation and return the error there - }, - }, - BlockTypes: map[string]*configschema.NestedBlock{}, - }), - }, } for name, test := range tests { diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 140cdff22..6a3c15a64 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -95,34 +95,6 @@ type Schema struct { // behavior, and SchemaConfigModeBlock is not permitted. ConfigMode SchemaConfigMode - // SkipCoreTypeCheck, if set, will advertise this attribute to Terraform Core - // has being dynamically-typed rather than deriving a type from the schema. - // This has the effect of making Terraform Core skip all type-checking of - // the value, and thus leaves all type checking up to a combination of this - // SDK and the provider's own code. - // - // This flag does nothing for Terraform versions prior to v0.12, because - // in prior versions there was no Core-side typecheck anyway. - // - // The most practical effect of this flag is to allow object-typed schemas - // (specified with Elem: schema.Resource) to pass through Terraform Core - // even without all of the object type attributes specified, which may be - // useful when using ConfigMode: SchemaConfigModeAttr to achieve - // nested-block-like behaviors while using attribute syntax. - // - // However, by doing so we require type information to be sent and stored - // per-object rather than just once statically in the schema, and so this - // will change the wire serialization of a resource type in state. Changing - // the value of SkipCoreTypeCheck will therefore require a state migration - // if there has previously been any release of the provider compatible with - // Terraform v0.12. - // - // SkipCoreTypeCheck can only be set when ConfigMode is SchemaConfigModeAttr, - // because nested blocks cannot be decoded by Terraform Core without at - // least shallow information about the next level of nested attributes and - // blocks. - SkipCoreTypeCheck bool - // If one of these is set, then this item can come from the configuration. // Both cannot be set. If Optional is set, the value is optional. If // Required is set, the value is required. @@ -735,8 +707,6 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro computedOnly := v.Computed && !v.Optional - isBlock := false - switch v.ConfigMode { case SchemaConfigModeBlock: if _, ok := v.Elem.(*Resource); !ok { @@ -748,7 +718,6 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro if computedOnly { return fmt.Errorf("%s: ConfigMode of block cannot be used for computed schema", k) } - isBlock = true case SchemaConfigModeAttr: // anything goes case SchemaConfigModeAuto: @@ -756,7 +725,6 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro // and that's impossible inside an attribute, we require it to be // explicitly overridden as mode "Attr" for clarity. if _, ok := v.Elem.(*Resource); ok { - isBlock = true if attrsOnly { return fmt.Errorf("%s: in *schema.Resource with ConfigMode of attribute, so must also have ConfigMode of attribute", k) } @@ -765,10 +733,6 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro return fmt.Errorf("%s: invalid ConfigMode value", k) } - if isBlock && v.SkipCoreTypeCheck { - return fmt.Errorf("%s: SkipCoreTypeCheck must be false unless ConfigMode is attribute", k) - } - if v.Computed && v.Default != nil { return fmt.Errorf("%s: Default must be nil if computed", k) } diff --git a/helper/schema/shims.go b/helper/schema/shims.go index d43028e58..203d01704 100644 --- a/helper/schema/shims.go +++ b/helper/schema/shims.go @@ -113,45 +113,3 @@ func JSONMapToStateValue(m map[string]interface{}, block *configschema.Block) (c func StateValueFromInstanceState(is *terraform.InstanceState, ty cty.Type) (cty.Value, error) { return is.AttrsAsObjectValue(ty) } - -// LegacyResourceSchema takes a *Resource and returns a deep copy with 0.12 specific -// features removed. This is used by the shims to get a configschema that -// directly matches the structure of the schema.Resource. -func LegacyResourceSchema(r *Resource) *Resource { - if r == nil { - return nil - } - // start with a shallow copy - newResource := new(Resource) - *newResource = *r - newResource.Schema = map[string]*Schema{} - - for k, s := range r.Schema { - newResource.Schema[k] = LegacySchema(s) - } - - return newResource -} - -// LegacySchema takes a *Schema and returns a deep copy with some 0.12-specific -// features disabled. This is used by the shims to get a configschema that -// better reflects the given schema.Resource, without any adjustments we -// make for when sending a schema to Terraform Core. -func LegacySchema(s *Schema) *Schema { - if s == nil { - return nil - } - // start with a shallow copy - newSchema := new(Schema) - *newSchema = *s - newSchema.SkipCoreTypeCheck = false - - switch e := newSchema.Elem.(type) { - case *Schema: - newSchema.Elem = LegacySchema(e) - case *Resource: - newSchema.Elem = LegacyResourceSchema(e) - } - - return newSchema -} diff --git a/terraform/eval_state_upgrade.go b/terraform/eval_state_upgrade.go index b7720d71b..e1940005e 100644 --- a/terraform/eval_state_upgrade.go +++ b/terraform/eval_state_upgrade.go @@ -19,26 +19,13 @@ import ( // If any errors occur during upgrade, error diagnostics are returned. In that // case it is not safe to proceed with using the original state object. func UpgradeResourceState(addr addrs.AbsResourceInstance, provider providers.Interface, src *states.ResourceInstanceObjectSrc, currentSchema *configschema.Block, currentVersion uint64) (*states.ResourceInstanceObjectSrc, tfdiags.Diagnostics) { - currentTy := currentSchema.ImpliedType() - - // If the state is currently in flatmap format and the current schema - // contains DynamicPseudoType attributes then we won't be able to convert - // it to JSON without the provider's help even if the schema version matches, - // since only the provider knows how to interpret the dynamic attribute - // value in flatmap format to convert it to JSON. - schemaHasDynamic := currentTy.HasDynamicTypes() - stateIsFlatmap := len(src.AttrsJSON) == 0 - forceProviderUpgrade := schemaHasDynamic && stateIsFlatmap - - if src.SchemaVersion == currentVersion && !forceProviderUpgrade { - // No upgrading required, then. - return src, nil - } if addr.Resource.Resource.Mode != addrs.ManagedResourceMode { // We only do state upgrading for managed resources. return src, nil } + stateIsFlatmap := len(src.AttrsJSON) == 0 + providerType := addr.Resource.Resource.DefaultProviderConfig().Type if src.SchemaVersion > currentVersion { log.Printf("[TRACE] UpgradeResourceState: can't downgrade state for %s from version %d to %d", addr, src.SchemaVersion, currentVersion) @@ -60,7 +47,11 @@ func UpgradeResourceState(addr addrs.AbsResourceInstance, provider providers.Int // v0.12, this also includes translating from legacy flatmap to new-style // representation, since only the provider has enough information to // understand a flatmap built against an older schema. - log.Printf("[TRACE] UpgradeResourceState: upgrading state for %s from version %d to %d using provider %q", addr, src.SchemaVersion, currentVersion, providerType) + if src.SchemaVersion != currentVersion { + log.Printf("[TRACE] UpgradeResourceState: upgrading state for %s from version %d to %d using provider %q", addr, src.SchemaVersion, currentVersion, providerType) + } else { + log.Printf("[TRACE] UpgradeResourceState: schema version of %s is still %d; calling provider %q for any other minor fixups", addr, currentVersion, providerType) + } req := providers.UpgradeResourceStateRequest{ TypeName: addr.Resource.Resource.Type, diff --git a/terraform/provider_mock.go b/terraform/provider_mock.go index 8c5824089..4ae346d7d 100644 --- a/terraform/provider_mock.go +++ b/terraform/provider_mock.go @@ -6,6 +6,7 @@ import ( "sync" "github.com/zclconf/go-cty/cty" + ctyjson "github.com/zclconf/go-cty/cty/json" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/config/hcl2shim" @@ -191,6 +192,10 @@ func (p *MockProvider) UpgradeResourceState(r providers.UpgradeResourceStateRequ p.Lock() defer p.Unlock() + schemas := p.getSchema() + schema := schemas.ResourceTypes[r.TypeName] + schemaType := schema.Block.ImpliedType() + p.UpgradeResourceStateCalled = true p.UpgradeResourceStateRequest = r @@ -198,7 +203,28 @@ func (p *MockProvider) UpgradeResourceState(r providers.UpgradeResourceStateRequ return p.UpgradeResourceStateFn(r) } - return p.UpgradeResourceStateResponse + resp := p.UpgradeResourceStateResponse + + if resp.UpgradedState == cty.NilVal { + switch { + case r.RawStateFlatmap != nil: + v, err := hcl2shim.HCL2ValueFromFlatmap(r.RawStateFlatmap, schemaType) + if err != nil { + resp.Diagnostics = resp.Diagnostics.Append(err) + return resp + } + resp.UpgradedState = v + case len(r.RawStateJSON) > 0: + v, err := ctyjson.Unmarshal(r.RawStateJSON, schemaType) + + if err != nil { + resp.Diagnostics = resp.Diagnostics.Append(err) + return resp + } + resp.UpgradedState = v + } + } + return resp } func (p *MockProvider) Configure(r providers.ConfigureRequest) providers.ConfigureResponse {