From 4de0b33097bd599fca83b0e9a8c7bb5987c2ceab Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Fri, 8 Mar 2019 16:31:22 -0800 Subject: [PATCH] helper/schema: Honor ConfigMode when building core schema This makes some slight adjustments to the shape of the schema we present to Terraform Core without affecting how it is consumed by the SDK and thus the provider. This mechanism is designed specifically to avoid changing how the schema is interpreted by the SDK itself or by the provider, so that prior behavior can be preserved in Terraform v0.11 mode. This also includes a new rule that Computed-only (i.e. not also Optional) schemas _always_ map to attributes, because that is a better mapping of the intent: they are object values to be used in expressions. Nested blocks conceptually represent nested objects that are in some sense independent of what they are embedded in, and so they cannot themselves be computed. --- builtin/providers/test/provider.go | 1 + .../providers/test/resource_config_mode.go | 60 ++++++++++++++++++ .../test/resource_config_mode_test.go | 61 +++++++++++++++++++ helper/schema/core_schema.go | 32 +++++++--- helper/schema/core_schema_test.go | 15 ++--- 5 files changed, 150 insertions(+), 19 deletions(-) create mode 100644 builtin/providers/test/resource_config_mode.go create mode 100644 builtin/providers/test/resource_config_mode_test.go diff --git a/builtin/providers/test/provider.go b/builtin/providers/test/provider.go index 6c469ba593..65a8fdedbc 100644 --- a/builtin/providers/test/provider.go +++ b/builtin/providers/test/provider.go @@ -33,6 +33,7 @@ func Provider() terraform.ResourceProvider { "test_resource_list_set": testResourceListSet(), "test_resource_map": testResourceMap(), "test_resource_computed_set": testResourceComputedSet(), + "test_resource_config_mode": testResourceConfigMode(), "test_resource_nested_id": testResourceNestedId(), }, DataSourcesMap: map[string]*schema.Resource{ diff --git a/builtin/providers/test/resource_config_mode.go b/builtin/providers/test/resource_config_mode.go new file mode 100644 index 0000000000..c352178041 --- /dev/null +++ b/builtin/providers/test/resource_config_mode.go @@ -0,0 +1,60 @@ +package test + +import ( + "fmt" + + "github.com/hashicorp/terraform/helper/schema" +) + +func testResourceConfigMode() *schema.Resource { + return &schema.Resource{ + Create: testResourceConfigModeCreate, + Read: testResourceConfigModeRead, + Delete: testResourceConfigModeDelete, + Update: testResourceConfigModeUpdate, + + Schema: map[string]*schema.Schema{ + "resource_as_attr": { + Type: schema.TypeList, + ConfigMode: schema.SchemaConfigModeAttr, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "foo": { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + }, + }, + } +} + +func testResourceConfigModeCreate(d *schema.ResourceData, meta interface{}) error { + d.SetId("placeholder") + return testResourceConfigModeRead(d, meta) +} + +func testResourceConfigModeRead(d *schema.ResourceData, meta interface{}) error { + const k = "resource_as_attr" + 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) + } + } + } + return nil +} + +func testResourceConfigModeUpdate(d *schema.ResourceData, meta interface{}) error { + return testResourceConfigModeRead(d, meta) +} + +func testResourceConfigModeDelete(d *schema.ResourceData, meta interface{}) error { + d.SetId("") + return nil +} diff --git a/builtin/providers/test/resource_config_mode_test.go b/builtin/providers/test/resource_config_mode_test.go new file mode 100644 index 0000000000..d7eb71e87b --- /dev/null +++ b/builtin/providers/test/resource_config_mode_test.go @@ -0,0 +1,61 @@ +package test + +import ( + "strings" + "testing" + + "github.com/hashicorp/terraform/helper/resource" +) + +func TestResourceConfigMode(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_config_mode" "foo" { + resource_as_attr = [ + { + foo = "resource_as_attr 0" + }, + { + foo = "resource_as_attr 1" + }, + ] +} + `), + 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.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_config_mode" "foo" { + resource_as_attr = [ + { + foo = "resource_as_attr 0 updated" + }, + ] +} + `), + 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.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_config_mode" "foo" { + resource_as_attr = [] +} + `), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("test_resource_config_mode.foo", "resource_as_attr.#", "0"), + ), + }, + }, + }) +} diff --git a/helper/schema/core_schema.go b/helper/schema/core_schema.go index 727b164b40..875677eb3d 100644 --- a/helper/schema/core_schema.go +++ b/helper/schema/core_schema.go @@ -54,14 +54,27 @@ func (m schemaMap) CoreConfigSchema() *configschema.Block { continue } } - switch schema.Elem.(type) { - case *Schema, ValueType: + switch schema.ConfigMode { + case SchemaConfigModeAttr: ret.Attributes[name] = schema.coreConfigSchemaAttribute() - case *Resource: + case SchemaConfigModeBlock: ret.BlockTypes[name] = schema.coreConfigSchemaBlock() - default: - // Should never happen for a valid schema - panic(fmt.Errorf("invalid Schema.Elem %#v; need *Schema or *Resource", schema.Elem)) + default: // SchemaConfigModeAuto, or any other invalid value + if schema.Computed && !schema.Optional { + // Computed-only schemas are always handled as attributes, + // because they never appear in configuration. + ret.Attributes[name] = schema.coreConfigSchemaAttribute() + continue + } + switch schema.Elem.(type) { + case *Schema, ValueType: + ret.Attributes[name] = schema.coreConfigSchemaAttribute() + case *Resource: + ret.BlockTypes[name] = schema.coreConfigSchemaBlock() + default: + // Should never happen for a valid schema + panic(fmt.Errorf("invalid Schema.Elem %#v; need *Schema or *Resource", schema.Elem)) + } } } @@ -181,9 +194,10 @@ func (s *Schema) coreConfigSchemaType() cty.Type { // common one so we'll just shim it. elemType = (&Schema{Type: set}).coreConfigSchemaType() case *Resource: - // In practice we don't actually use this for normal schema - // construction because we construct a NestedBlock in that - // case instead. See schemaMap.CoreConfigSchema. + // By default we construct a NestedBlock in this case, but this + // behavior is selected either for computed-only schemas or + // when ConfigMode is explicitly SchemaConfigModeBlock. + // See schemaMap.CoreConfigSchema for the exact rules. elemType = set.coreConfigSchema().ImpliedType() default: if set != nil { diff --git a/helper/schema/core_schema_test.go b/helper/schema/core_schema_test.go index b20f164e84..7d4b32e019 100644 --- a/helper/schema/core_schema_test.go +++ b/helper/schema/core_schema_test.go @@ -298,19 +298,14 @@ func TestSchemaMapCoreConfigSchema(t *testing.T) { }, }, testResource(&configschema.Block{ - Attributes: map[string]*configschema.Attribute{}, - BlockTypes: map[string]*configschema.NestedBlock{ + Attributes: map[string]*configschema.Attribute{ "list": { - Nesting: configschema.NestingList, - Block: configschema.Block{}, - MinItems: 0, - MaxItems: 0, + Type: cty.List(cty.EmptyObject), + Computed: true, }, "set": { - Nesting: configschema.NestingSet, - Block: configschema.Block{}, - MinItems: 0, - MaxItems: 0, + Type: cty.Set(cty.EmptyObject), + Computed: true, }, }, }),