From 61fee6735dffcda9e9e27c1ff5cd2d1f4a9f3981 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Sun, 7 Jun 2015 20:49:15 -0500 Subject: [PATCH 1/4] helper/schema: ValidateFunc Allows provider authors to implement arbitrary per-field validation warnings or errors. --- helper/schema/schema.go | 17 ++++++++++++++++ helper/schema/schema_test.go | 38 +++++++++++++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 729fb0980..8766e10b0 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -131,6 +131,9 @@ type Schema struct { // This string is the message shown to the user with instructions on // what do to about the removed attribute. Removed string + + // ValidateFunc allows individual fields to validate + ValidateFunc SchemaValidateFunc } // SchemaDefaultFunc is a function called to return a default value for @@ -173,6 +176,10 @@ type SchemaSetFunc func(interface{}) int // to be stored in the state. type SchemaStateFunc func(interface{}) string +// SchemaValidateFunc is a function used to validate a single field in the +// schema. +type SchemaValidateFunc func(interface{}) ([]string, []error) + func (s *Schema) GoString() string { return fmt.Sprintf("*%#v", *s) } @@ -1176,6 +1183,16 @@ func (m schemaMap) validateType( "%q: [REMOVED] %s", k, schema.Removed)) } + if schema.ValidateFunc != nil { + ws2, es2 := schema.ValidateFunc(raw) + for _, w := range ws2 { + ws = append(ws, fmt.Sprintf("%q: %s", k, w)) + } + for _, e := range es2 { + es = append(es, fmt.Errorf("%q: %s", k, e)) + } + } + return ws, es } diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 9a77cda2f..e23f51279 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -3399,7 +3399,43 @@ func TestSchemaMap_Validate(t *testing.T) { Err: true, Errors: []error{ - fmt.Errorf("\"optional_att\": conflicts with required_att (\"required-val\")"), + fmt.Errorf(`"optional_att": conflicts with required_att ("required-val")`), + }, + }, + + "Good with ValidateFunc": { + Schema: map[string]*Schema{ + "validate_me": &Schema{ + Type: TypeString, + Required: true, + ValidateFunc: func(v interface{}) (ws []string, es []error) { + return + }, + }, + }, + Config: map[string]interface{}{ + "validate_me": "valid", + }, + Err: false, + }, + + "Bad with ValidateFunc": { + Schema: map[string]*Schema{ + "validate_me": &Schema{ + Type: TypeString, + Required: true, + ValidateFunc: func(v interface{}) (ws []string, es []error) { + es = append(es, fmt.Errorf("something is not right here")) + return + }, + }, + }, + Config: map[string]interface{}{ + "validate_me": "invalid", + }, + Err: true, + Errors: []error{ + fmt.Errorf(`"validate_me": something is not right here`), }, }, } From 37b234e42bbff227db131e8fa1dcfe0196864f0e Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Sun, 7 Jun 2015 20:49:48 -0500 Subject: [PATCH 2/4] provider/aws: validate RDS final_snapshot_identifier fixes #2250 --- builtin/providers/aws/resource_aws_db_instance.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/builtin/providers/aws/resource_aws_db_instance.go b/builtin/providers/aws/resource_aws_db_instance.go index 705b773f9..10cce3fd5 100644 --- a/builtin/providers/aws/resource_aws_db_instance.go +++ b/builtin/providers/aws/resource_aws_db_instance.go @@ -3,6 +3,7 @@ package aws import ( "fmt" "log" + "regexp" "strings" "time" @@ -153,6 +154,20 @@ func resourceAwsDbInstance() *schema.Resource { "final_snapshot_identifier": &schema.Schema{ Type: schema.TypeString, Optional: true, + ValidateFunc: func(v interface{}) (ws []string, es []error) { + fsi := v.(string) + if !regexp.MustCompile(`^[0-9A-Za-z-]+$`).MatchString(fsi) { + es = append(es, fmt.Errorf( + "only alphanumeric characters and hyphens allowed")) + } + if regexp.MustCompile(`--`).MatchString(fsi) { + es = append(es, fmt.Errorf("cannot contain two consecutive hyphens")) + } + if regexp.MustCompile(`-$`).MatchString(fsi) { + es = append(es, fmt.Errorf("cannot end in a hyphen")) + } + return + }, }, "db_subnet_group_name": &schema.Schema{ From 49352db26fc2a198fe5ec002453ce14925f3afeb Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Mon, 8 Jun 2015 08:51:04 -0500 Subject: [PATCH 3/4] helper/schema: skip ValidateFunc on other errors Guarantees that the `interface{}` arg to ValidateFunc is the proper type, allowing implementations to be simpler. Finish the docstring on `ValidateFunc` to call this out. /cc @mitchellh --- helper/schema/schema.go | 7 +++++-- helper/schema/schema_test.go | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 8766e10b0..558871ddb 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -132,7 +132,10 @@ type Schema struct { // what do to about the removed attribute. Removed string - // ValidateFunc allows individual fields to validate + // ValidateFunc allows individual fields to define arbitrary validation + // logic. It is yielded the provided config value as an interface{} that is + // guaranteed to be of the proper Schema type, and it can yield warnings or + // errors based on inspection of that value. ValidateFunc SchemaValidateFunc } @@ -1183,7 +1186,7 @@ func (m schemaMap) validateType( "%q: [REMOVED] %s", k, schema.Removed)) } - if schema.ValidateFunc != nil { + if len(es) == 0 && schema.ValidateFunc != nil { ws2, es2 := schema.ValidateFunc(raw) for _, w := range ws2 { ws = append(ws, fmt.Sprintf("%q: %s", k, w)) diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index e23f51279..36aa2e137 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -3438,6 +3438,23 @@ func TestSchemaMap_Validate(t *testing.T) { fmt.Errorf(`"validate_me": something is not right here`), }, }, + + "ValidateFunc not called when type does not match": { + Schema: map[string]*Schema{ + "number": &Schema{ + Type: TypeInt, + Required: true, + ValidateFunc: func(v interface{}) (ws []string, es []error) { + t.Fatalf("Should not have gotten validate call") + return + }, + }, + }, + Config: map[string]interface{}{ + "number": "NaN", + }, + Err: true, + }, } for tn, tc := range cases { From a4912cc51f380adefc6edea7196cf44f1af4b5bb Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Thu, 11 Jun 2015 07:06:30 -0500 Subject: [PATCH 4/4] helper/schema: limit ValidateFunc to primitives for now I couldn't see a simple path get this working for Maps, Sets, and Lists, so lets land it as a primitive-only schema feature. I think validation on primitives comprises 80% of the use cases anyways. --- helper/schema/schema.go | 28 ++++++++++++++++++---------- helper/schema/schema_test.go | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 558871ddb..0a79e1281 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -136,6 +136,8 @@ type Schema struct { // logic. It is yielded the provided config value as an interface{} that is // guaranteed to be of the proper Schema type, and it can yield warnings or // errors based on inspection of that value. + // + // ValidateFunc currently only works for primitive types. ValidateFunc SchemaValidateFunc } @@ -513,6 +515,13 @@ func (m schemaMap) InternalValidate(topSchemaMap schemaMap) error { } } } + + if v.ValidateFunc != nil { + switch v.Type { + case TypeList, TypeSet, TypeMap: + return fmt.Errorf("ValidateFunc is only supported on primitives.") + } + } } return nil @@ -1128,6 +1137,7 @@ func (m schemaMap) validatePrimitive( return nil, nil } + var decoded interface{} switch schema.Type { case TypeBool: // Verify that we can parse this as the correct type @@ -1135,28 +1145,36 @@ func (m schemaMap) validatePrimitive( if err := mapstructure.WeakDecode(raw, &n); err != nil { return nil, []error{err} } + decoded = n case TypeInt: // Verify that we can parse this as an int var n int if err := mapstructure.WeakDecode(raw, &n); err != nil { return nil, []error{err} } + decoded = n case TypeFloat: // Verify that we can parse this as an int var n float64 if err := mapstructure.WeakDecode(raw, &n); err != nil { return nil, []error{err} } + decoded = n case TypeString: // Verify that we can parse this as a string var n string if err := mapstructure.WeakDecode(raw, &n); err != nil { return nil, []error{err} } + decoded = n default: panic(fmt.Sprintf("Unknown validation type: %#v", schema.Type)) } + if schema.ValidateFunc != nil { + return schema.ValidateFunc(decoded) + } + return nil, nil } @@ -1186,16 +1204,6 @@ func (m schemaMap) validateType( "%q: [REMOVED] %s", k, schema.Removed)) } - if len(es) == 0 && schema.ValidateFunc != nil { - ws2, es2 := schema.ValidateFunc(raw) - for _, w := range ws2 { - ws = append(ws, fmt.Sprintf("%q: %s", k, w)) - } - for _, e := range es2 { - es = append(es, fmt.Errorf("%q: %s", k, e)) - } - } - return ws, es } diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 36aa2e137..5ea108380 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -2796,6 +2796,20 @@ func TestSchemaMap_InternalValidate(t *testing.T) { }, false, }, + + // ValidateFunc on non-primitive + { + map[string]*Schema{ + "foo": &Schema{ + Type: TypeMap, + Required: true, + ValidateFunc: func(v interface{}) (ws []string, es []error) { + return + }, + }, + }, + true, + }, } for i, tc := range cases { @@ -3435,7 +3449,7 @@ func TestSchemaMap_Validate(t *testing.T) { }, Err: true, Errors: []error{ - fmt.Errorf(`"validate_me": something is not right here`), + fmt.Errorf(`something is not right here`), }, }, @@ -3455,6 +3469,23 @@ func TestSchemaMap_Validate(t *testing.T) { }, Err: true, }, + "ValidateFunc gets decoded type": { + Schema: map[string]*Schema{ + "maybe": &Schema{ + Type: TypeBool, + Required: true, + ValidateFunc: func(v interface{}) (ws []string, es []error) { + if _, ok := v.(bool); !ok { + t.Fatalf("Expected bool, got: %#v", v) + } + return + }, + }, + }, + Config: map[string]interface{}{ + "maybe": "true", + }, + }, } for tn, tc := range cases {