From 1e3d1b07e62746ae127697a9729c471912e4a6e6 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Tue, 12 May 2015 09:45:15 -0500 Subject: [PATCH 1/2] helper/schema: validate ConflictsWith against top-level The runtime impl of ConfictsWith uses Resource.Get(), which makes it work with any other attribute of the resource - the InternalValidate was only checking against the local schemaMap though, preventing subResource from using ConflictsWith properly. It's a lot of wiring and it's a bit ugly, but it's not runtime code, so I'm a bit less concerned about that aspect. This should take care of the problem mentioned in #1909 --- helper/schema/provider.go | 5 +++-- helper/schema/resource.go | 6 ++++-- helper/schema/schema.go | 33 +++++++++++++++++++++++++++++---- 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/helper/schema/provider.go b/helper/schema/provider.go index ff9f991ca..6b364e5a1 100644 --- a/helper/schema/provider.go +++ b/helper/schema/provider.go @@ -61,12 +61,13 @@ func (p *Provider) InternalValidate() error { return errors.New("provider is nil") } - if err := schemaMap(p.Schema).InternalValidate(); err != nil { + sm := schemaMap(p.Schema) + if err := sm.InternalValidate(sm); err != nil { return err } for k, r := range p.ResourcesMap { - if err := r.InternalValidate(); err != nil { + if err := r.InternalValidate(nil); err != nil { return fmt.Errorf("%s: %s", k, err) } } diff --git a/helper/schema/resource.go b/helper/schema/resource.go index 0c640e697..dec94fc7b 100644 --- a/helper/schema/resource.go +++ b/helper/schema/resource.go @@ -220,10 +220,11 @@ func (r *Resource) Refresh( // Provider.InternalValidate() will automatically call this for all of // the resources it manages, so you don't need to call this manually if it // is part of a Provider. -func (r *Resource) InternalValidate() error { +func (r *Resource) InternalValidate(topSchemaMap schemaMap) error { if r == nil { return errors.New("resource is nil") } + tsm := topSchemaMap if r.isTopLevel() { // All non-Computed attributes must be ForceNew if Update is not defined @@ -239,9 +240,10 @@ func (r *Resource) InternalValidate() error { "No Update defined, must set ForceNew on: %#v", nonForceNewAttrs) } } + tsm = schemaMap(r.Schema) } - return schemaMap(r.Schema).InternalValidate() + return schemaMap(r.Schema).InternalValidate(tsm) } // Returns true if the resource is "top level" i.e. not a sub-resource. diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 94ba865a5..67b7ba78b 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -17,6 +17,7 @@ import ( "reflect" "sort" "strconv" + "strings" "github.com/hashicorp/terraform/terraform" "github.com/mitchellh/mapstructure" @@ -410,7 +411,10 @@ func (m schemaMap) Validate(c *terraform.ResourceConfig) ([]string, []error) { // InternalValidate validates the format of this schema. This should be called // from a unit test (and not in user-path code) to verify that a schema // is properly built. -func (m schemaMap) InternalValidate() error { +func (m schemaMap) InternalValidate(topSchemaMap schemaMap) error { + if topSchemaMap == nil { + topSchemaMap = m + } for k, v := range m { if v.Type == TypeInvalid { return fmt.Errorf("%s: Type must be specified", k) @@ -446,11 +450,32 @@ func (m schemaMap) InternalValidate() error { if len(v.ConflictsWith) > 0 { for _, key := range v.ConflictsWith { - if m[key].Required { + parts := strings.Split(key, ".") + sm := topSchemaMap + var target *Schema + for _, part := range parts { + // Skip index fields + if _, err := strconv.Atoi(part); err == nil { + continue + } + + var ok bool + if target, ok = sm[part]; !ok { + return fmt.Errorf("%s: ConflictsWith references unknown attribute (%s)", k, key) + } + + if subResource, ok := target.Elem.(*Resource); ok { + sm = schemaMap(subResource.Schema) + } + } + if target == nil { + return fmt.Errorf("%s: ConflictsWith cannot find target attribute (%s), sm: %#v", k, key, sm) + } + if target.Required { return fmt.Errorf("%s: ConflictsWith cannot contain Required attribute (%s)", k, key) } - if m[key].Computed || len(m[key].ComputedWhen) > 0 { + if target.Computed || len(target.ComputedWhen) > 0 { return fmt.Errorf("%s: ConflictsWith cannot contain Computed(When) attribute (%s)", k, key) } } @@ -473,7 +498,7 @@ func (m schemaMap) InternalValidate() error { switch t := v.Elem.(type) { case *Resource: - if err := t.InternalValidate(); err != nil { + if err := t.InternalValidate(topSchemaMap); err != nil { return err } case *Schema: From bb14bfa6571864e64f66c9c0bd9939246b959597 Mon Sep 17 00:00:00 2001 From: Justin Campbell Date: Tue, 12 May 2015 11:01:02 -0400 Subject: [PATCH 2/2] helper/schema: call InternalValidate w/ schemaMap{} --- helper/schema/resource_test.go | 2 +- helper/schema/schema_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/helper/schema/resource_test.go b/helper/schema/resource_test.go index e406e55b9..8027f5fb1 100644 --- a/helper/schema/resource_test.go +++ b/helper/schema/resource_test.go @@ -334,7 +334,7 @@ func TestResourceInternalValidate(t *testing.T) { } for i, tc := range cases { - err := tc.In.InternalValidate() + err := tc.In.InternalValidate(schemaMap{}) if (err != nil) != tc.Err { t.Fatalf("%d: bad: %s", i, err) } diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index e532a81b9..9a77cda2f 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -2799,7 +2799,7 @@ func TestSchemaMap_InternalValidate(t *testing.T) { } for i, tc := range cases { - err := schemaMap(tc.In).InternalValidate() + err := schemaMap(tc.In).InternalValidate(schemaMap{}) if (err != nil) != tc.Err { if tc.Err { t.Fatalf("%d: Expected error did not occur:\n\n%#v", i, tc.In)