From befa81c74ff50e950cd2651777b062846873f9aa Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 11 Jul 2018 20:59:32 -0400 Subject: [PATCH] add implicit "id" to resource attribute schemas When converting a legacy schemaMap to a configschema, we need to add "id" as a required attribute to top-level resources if it's not declared. The "id" field will be required to interoperate with the legacy helper schema, since the presence of an id was used to indicate the existence of a resource. --- helper/schema/core_schema.go | 24 +++++++++++-- helper/schema/core_schema_test.go | 60 +++++++++++++++++++++---------- helper/schema/provider_test.go | 14 ++++---- 3 files changed, 71 insertions(+), 27 deletions(-) diff --git a/helper/schema/core_schema.go b/helper/schema/core_schema.go index 3e8bdd91e..bdabf1324 100644 --- a/helper/schema/core_schema.go +++ b/helper/schema/core_schema.go @@ -153,9 +153,29 @@ func (s *Schema) coreConfigSchemaType() cty.Type { } } -// CoreConfigSchema is a convenient shortcut for calling CoreConfigSchema -// on the resource's schema. +// CoreConfigSchema is a convenient shortcut for calling CoreConfigSchema on +// the resource's schema. CoreConfigSchema adds the implicitly required "id" +// attribute for top level resources if it doesn't exist. func (r *Resource) CoreConfigSchema() *configschema.Block { + block := r.coreConfigSchema() + + if block.Attributes == nil { + block.Attributes = map[string]*configschema.Attribute{} + } + + // Add the implicitly required "id" field if it doesn't exist + if block.Attributes["id"] == nil { + block.Attributes["id"] = &configschema.Attribute{ + Type: cty.String, + Optional: true, + Computed: true, + } + } + + return block +} + +func (r *Resource) coreConfigSchema() *configschema.Block { return schemaMap(r.Schema).CoreConfigSchema() } diff --git a/helper/schema/core_schema_test.go b/helper/schema/core_schema_test.go index 68f8a9159..197e9b2f3 100644 --- a/helper/schema/core_schema_test.go +++ b/helper/schema/core_schema_test.go @@ -1,16 +1,40 @@ package schema import ( - "reflect" "testing" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/zclconf/go-cty/cty" - "github.com/davecgh/go-spew/spew" - "github.com/hashicorp/terraform/configs/configschema" ) +var ( + ignoreUnexported = cmpopts.IgnoreUnexported(cty.Type{}) + equateEmpty = cmpopts.EquateEmpty() +) + +// add the implicit "id" attribute for test resources +func testResource(block *configschema.Block) *configschema.Block { + if block.Attributes == nil { + block.Attributes = make(map[string]*configschema.Attribute) + } + + if block.BlockTypes == nil { + block.BlockTypes = make(map[string]*configschema.NestedBlock) + } + + if block.Attributes["id"] == nil { + block.Attributes["id"] = &configschema.Attribute{ + Type: cty.String, + Optional: true, + Computed: true, + } + } + return block +} + func TestSchemaMapCoreConfigSchema(t *testing.T) { tests := map[string]struct { Schema map[string]*Schema @@ -18,7 +42,7 @@ func TestSchemaMapCoreConfigSchema(t *testing.T) { }{ "empty": { map[string]*Schema{}, - &configschema.Block{}, + testResource(&configschema.Block{}), }, "primitives": { map[string]*Schema{ @@ -41,7 +65,7 @@ func TestSchemaMapCoreConfigSchema(t *testing.T) { Computed: true, }, }, - &configschema.Block{ + testResource(&configschema.Block{ Attributes: map[string]*configschema.Attribute{ "int": { Type: cty.Number, @@ -63,7 +87,7 @@ func TestSchemaMapCoreConfigSchema(t *testing.T) { }, }, BlockTypes: map[string]*configschema.NestedBlock{}, - }, + }), }, "simple collections": { map[string]*Schema{ @@ -96,7 +120,7 @@ func TestSchemaMapCoreConfigSchema(t *testing.T) { // for pre-existing schemas. }, }, - &configschema.Block{ + testResource(&configschema.Block{ Attributes: map[string]*configschema.Attribute{ "list": { Type: cty.List(cty.Number), @@ -116,7 +140,7 @@ func TestSchemaMapCoreConfigSchema(t *testing.T) { }, }, BlockTypes: map[string]*configschema.NestedBlock{}, - }, + }), }, "incorrectly-specified collections": { // Historically we tolerated setting a type directly as the Elem @@ -140,7 +164,7 @@ func TestSchemaMapCoreConfigSchema(t *testing.T) { Elem: TypeBool, }, }, - &configschema.Block{ + testResource(&configschema.Block{ Attributes: map[string]*configschema.Attribute{ "list": { Type: cty.List(cty.Number), @@ -156,7 +180,7 @@ func TestSchemaMapCoreConfigSchema(t *testing.T) { }, }, BlockTypes: map[string]*configschema.NestedBlock{}, - }, + }), }, "sub-resource collections": { map[string]*Schema{ @@ -184,7 +208,7 @@ func TestSchemaMapCoreConfigSchema(t *testing.T) { }, }, }, - &configschema.Block{ + testResource(&configschema.Block{ Attributes: map[string]*configschema.Attribute{}, BlockTypes: map[string]*configschema.NestedBlock{ "list": { @@ -203,7 +227,7 @@ func TestSchemaMapCoreConfigSchema(t *testing.T) { Block: configschema.Block{}, }, }, - }, + }), }, "nested attributes and blocks": { map[string]*Schema{ @@ -233,7 +257,7 @@ func TestSchemaMapCoreConfigSchema(t *testing.T) { }, }, }, - &configschema.Block{ + testResource(&configschema.Block{ Attributes: map[string]*configschema.Attribute{}, BlockTypes: map[string]*configschema.NestedBlock{ "foo": &configschema.NestedBlock{ @@ -255,7 +279,7 @@ func TestSchemaMapCoreConfigSchema(t *testing.T) { MinItems: 1, // because schema is Required }, }, - }, + }), }, "sensitive": { map[string]*Schema{ @@ -265,7 +289,7 @@ func TestSchemaMapCoreConfigSchema(t *testing.T) { Sensitive: true, }, }, - &configschema.Block{ + testResource(&configschema.Block{ Attributes: map[string]*configschema.Attribute{ "string": { Type: cty.String, @@ -274,15 +298,15 @@ func TestSchemaMapCoreConfigSchema(t *testing.T) { }, }, BlockTypes: map[string]*configschema.NestedBlock{}, - }, + }), }, } for name, test := range tests { t.Run(name, func(t *testing.T) { got := schemaMap(test.Schema).CoreConfigSchema() - if !reflect.DeepEqual(got, test.Want) { - t.Errorf("wrong result\ngot: %swant: %s", spew.Sdump(got), spew.Sdump(test.Want)) + if !cmp.Equal(got, test.Want, equateEmpty, ignoreUnexported) { + cmp.Diff(got, test.Want, equateEmpty, ignoreUnexported) } }) } diff --git a/helper/schema/provider_test.go b/helper/schema/provider_test.go index 603bb5da9..2a45a97d7 100644 --- a/helper/schema/provider_test.go +++ b/helper/schema/provider_test.go @@ -6,7 +6,7 @@ import ( "testing" "time" - "github.com/davecgh/go-spew/spew" + "github.com/google/go-cmp/cmp" "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/config" @@ -61,7 +61,7 @@ func TestProviderGetSchema(t *testing.T) { BlockTypes: map[string]*configschema.NestedBlock{}, }, ResourceTypes: map[string]*configschema.Block{ - "foo": &configschema.Block{ + "foo": testResource(&configschema.Block{ Attributes: map[string]*configschema.Attribute{ "bar": &configschema.Attribute{ Type: cty.String, @@ -69,10 +69,10 @@ func TestProviderGetSchema(t *testing.T) { }, }, BlockTypes: map[string]*configschema.NestedBlock{}, - }, + }), }, DataSources: map[string]*configschema.Block{ - "baz": &configschema.Block{ + "baz": testResource(&configschema.Block{ Attributes: map[string]*configschema.Attribute{ "bur": &configschema.Attribute{ Type: cty.String, @@ -80,7 +80,7 @@ func TestProviderGetSchema(t *testing.T) { }, }, BlockTypes: map[string]*configschema.NestedBlock{}, - }, + }), }, } got, err := p.GetSchema(&terraform.ProviderSchemaRequest{ @@ -91,8 +91,8 @@ func TestProviderGetSchema(t *testing.T) { t.Fatalf("unexpected error %s", err) } - if !reflect.DeepEqual(got, want) { - t.Errorf("wrong result\ngot: %swant: %s", spew.Sdump(got), spew.Sdump(want)) + if !cmp.Equal(got, want, equateEmpty, ignoreUnexported) { + t.Error("wrong result:\n", cmp.Diff(got, want, equateEmpty, ignoreUnexported)) } }