From 888f16d2d33f53dbda5e9d66b57acb94278f5ebd Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Thu, 5 Mar 2015 15:16:50 -0600 Subject: [PATCH 1/2] helper/schema: allow Schema attrs to be Deprecated Deprecated fields show a customizable warning message to the user when they are used in a Terraform config. This is a tool that provider authors can use for user feedback as they evolve their Schemas. fixes #957 --- helper/schema/schema.go | 28 ++++++++--- helper/schema/schema_test.go | 91 +++++++++++++++++++++++++++++++++--- 2 files changed, 106 insertions(+), 13 deletions(-) diff --git a/helper/schema/schema.go b/helper/schema/schema.go index e05acd395a..54df8e0a1d 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -113,6 +113,13 @@ type Schema struct { // // NOTE: This currently does not work. ComputedWhen []string + + // When Deprecated is set, this field is deprecated. + // + // A deprecated field still works, but will probably stop working in near + // future. This string is the message shown to the user with instructions on + // how to address the deprecation. + Deprecated string } // SchemaDefaultFunc is a function called to return a default value for @@ -877,7 +884,7 @@ func (m schemaMap) validate( raw, err = schema.DefaultFunc() if err != nil { return nil, []error{fmt.Errorf( - "%s, error loading default: %s", k, err)} + "%q, error loading default: %s", k, err)} } // We're okay as long as we had a value set @@ -886,7 +893,7 @@ func (m schemaMap) validate( if !ok { if schema.Required { return nil, []error{fmt.Errorf( - "%s: required field is not set", k)} + "%q: required field is not set", k)} } return nil, nil @@ -895,7 +902,7 @@ func (m schemaMap) validate( if !schema.Required && !schema.Optional { // This is a computed-only field return nil, []error{fmt.Errorf( - "%s: this field cannot be set", k)} + "%q: this field cannot be set", k)} } return m.validateType(k, raw, schema, c) @@ -1066,16 +1073,25 @@ func (m schemaMap) validateType( raw interface{}, schema *Schema, c *terraform.ResourceConfig) ([]string, []error) { + var ws []string + var es []error switch schema.Type { case TypeSet: fallthrough case TypeList: - return m.validateList(k, raw, schema, c) + ws, es = m.validateList(k, raw, schema, c) case TypeMap: - return m.validateMap(k, raw, schema, c) + ws, es = m.validateMap(k, raw, schema, c) default: - return m.validatePrimitive(k, raw, schema, c) + ws, es = m.validatePrimitive(k, raw, schema, c) } + + if schema.Deprecated != "" { + ws = append(ws, fmt.Sprintf( + "%q: [DEPRECATED] %s", k, schema.Deprecated)) + } + + return ws, es } // Zero returns the zero value for a type. diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 81089121c9..b64aa29904 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -2584,11 +2584,11 @@ func TestSchemaMap_InternalValidate(t *testing.T) { func TestSchemaMap_Validate(t *testing.T) { cases := map[string]struct { - Schema map[string]*Schema - Config map[string]interface{} - Vars map[string]string - Warn bool - Err bool + Schema map[string]*Schema + Config map[string]interface{} + Vars map[string]string + Err bool + Warnings []string }{ "Good": { Schema: map[string]*Schema{ @@ -3019,6 +3019,83 @@ func TestSchemaMap_Validate(t *testing.T) { Err: true, }, + + "Deprecated attribute usage generates warning, but not error": { + Schema: map[string]*Schema{ + "old_news": &Schema{ + Type: TypeString, + Optional: true, + Deprecated: "please use 'new_news' instead", + }, + }, + + Config: map[string]interface{}{ + "old_news": "extra extra!", + }, + + Err: false, + + Warnings: []string{ + "\"old_news\": [DEPRECATED] please use 'new_news' instead", + }, + }, + + "Deprecated generates no warnings if attr not used": { + Schema: map[string]*Schema{ + "old_news": &Schema{ + Type: TypeString, + Optional: true, + Deprecated: "please use 'new_news' instead", + }, + }, + + Err: false, + + Warnings: nil, + }, + + "Deprecated works with set/list type": { + Schema: map[string]*Schema{ + "old_news": &Schema{ + Type: TypeSet, + Optional: true, + Elem: &Schema{Type: TypeString}, + Deprecated: "please use 'new_news' instead", + }, + }, + + Config: map[string]interface{}{ + "old_news": []interface{}{"extra extra!"}, + }, + + Err: false, + + Warnings: []string{ + "\"old_news\": [DEPRECATED] please use 'new_news' instead", + }, + }, + + "Deprecated works with map type": { + Schema: map[string]*Schema{ + "old_news": &Schema{ + Type: TypeMap, + Optional: true, + Deprecated: "please use 'new_news' instead", + }, + }, + + Config: map[string]interface{}{ + "old_news": map[string]interface{}{ + "foo": "bar", + }, + }, + + Err: false, + + Warnings: []string{ + "\"old_news\": [DEPRECATED] please use 'new_news' instead", + }, + }, } for tn, tc := range cases { @@ -3050,8 +3127,8 @@ func TestSchemaMap_Validate(t *testing.T) { t.FailNow() } - if (len(ws) > 0) != tc.Warn { - t.Fatalf("%q: ws: %#v", tn, ws) + if !reflect.DeepEqual(ws, tc.Warnings) { + t.Fatalf("%q: warnings:\n\nexpected: %#v\ngot:%#v", tn, tc.Warnings, ws) } } } From ef70c8cae51fdf9c33976cebbd2b14017be12e05 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Thu, 5 Mar 2015 15:33:56 -0600 Subject: [PATCH 2/2] helper/schema: allow Schema attrs to be Removed Removed fields show a customizable error message to the user when they are used in a Terraform config. This is a tool that provider authors can use for user feedback as they evolve their Schemas. refs #957 --- helper/schema/schema.go | 15 +++++++++++- helper/schema/schema_test.go | 47 ++++++++++++++++-------------------- 2 files changed, 35 insertions(+), 27 deletions(-) diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 54df8e0a1d..33c8b83105 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -114,12 +114,20 @@ type Schema struct { // NOTE: This currently does not work. ComputedWhen []string - // When Deprecated is set, this field is deprecated. + // When Deprecated is set, this attribute is deprecated. // // A deprecated field still works, but will probably stop working in near // future. This string is the message shown to the user with instructions on // how to address the deprecation. Deprecated string + + // When Removed is set, this attribute has been removed from the schema + // + // Removed attributes can be left in the Schema to generate informative error + // messages for the user when they show up in resource configurations. + // This string is the message shown to the user with instructions on + // what do to about the removed attribute. + Removed string } // SchemaDefaultFunc is a function called to return a default value for @@ -1091,6 +1099,11 @@ func (m schemaMap) validateType( "%q: [DEPRECATED] %s", k, schema.Deprecated)) } + if schema.Removed != "" { + es = append(es, fmt.Errorf( + "%q: [REMOVED] %s", k, schema.Removed)) + } + return ws, es } diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index b64aa29904..2c9e89f63e 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -2588,6 +2588,7 @@ func TestSchemaMap_Validate(t *testing.T) { Config map[string]interface{} Vars map[string]string Err bool + Errors []error Warnings []string }{ "Good": { @@ -3054,47 +3055,35 @@ func TestSchemaMap_Validate(t *testing.T) { Warnings: nil, }, - "Deprecated works with set/list type": { + "Removed attribute usage generates error": { Schema: map[string]*Schema{ - "old_news": &Schema{ - Type: TypeSet, - Optional: true, - Elem: &Schema{Type: TypeString}, - Deprecated: "please use 'new_news' instead", + "long_gone": &Schema{ + Type: TypeString, + Optional: true, + Removed: "no longer supported by Cloud API", }, }, Config: map[string]interface{}{ - "old_news": []interface{}{"extra extra!"}, + "long_gone": "still here!", }, - Err: false, - - Warnings: []string{ - "\"old_news\": [DEPRECATED] please use 'new_news' instead", + Err: true, + Errors: []error{ + fmt.Errorf("\"long_gone\": [REMOVED] no longer supported by Cloud API"), }, }, - "Deprecated works with map type": { + "Removed generates no errors if attr not used": { Schema: map[string]*Schema{ - "old_news": &Schema{ - Type: TypeMap, - Optional: true, - Deprecated: "please use 'new_news' instead", - }, - }, - - Config: map[string]interface{}{ - "old_news": map[string]interface{}{ - "foo": "bar", + "long_gone": &Schema{ + Type: TypeString, + Optional: true, + Removed: "no longer supported by Cloud API", }, }, Err: false, - - Warnings: []string{ - "\"old_news\": [DEPRECATED] please use 'new_news' instead", - }, }, } @@ -3130,5 +3119,11 @@ func TestSchemaMap_Validate(t *testing.T) { if !reflect.DeepEqual(ws, tc.Warnings) { t.Fatalf("%q: warnings:\n\nexpected: %#v\ngot:%#v", tn, tc.Warnings, ws) } + + if tc.Errors != nil { + if !reflect.DeepEqual(es, tc.Errors) { + t.Fatalf("%q: errors:\n\nexpected: %q\ngot: %q", tn, tc.Errors, es) + } + } } }