From 07cbd54fbc238e7a804c627f0d3b3cdda752560f Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Mon, 10 Jul 2017 21:51:55 -0700 Subject: [PATCH] Actively disallow reserved field names in schema (#15522) --- config/loader_hcl.go | 14 ++++++++ helper/schema/provider.go | 17 +++++++++ helper/schema/provider_test.go | 61 +++++++++++++++++++++++++++++++ helper/schema/resource.go | 17 +++++++++ helper/schema/resource_test.go | 66 +++++++++++++++++++++++++++++++++- 5 files changed, 174 insertions(+), 1 deletion(-) diff --git a/config/loader_hcl.go b/config/loader_hcl.go index 2c30d0567..e85e49355 100644 --- a/config/loader_hcl.go +++ b/config/loader_hcl.go @@ -17,6 +17,20 @@ type hclConfigurable struct { Root *ast.File } +var ReservedResourceFields = []string{ + "connection", + "count", + "depends_on", + "lifecycle", + "provider", + "provisioner", +} + +var ReservedProviderFields = []string{ + "alias", + "version", +} + func (t *hclConfigurable) Config() (*Config, error) { validKeys := map[string]struct{}{ "atlas": struct{}{}, diff --git a/helper/schema/provider.go b/helper/schema/provider.go index d52d2f5f0..fb28b4151 100644 --- a/helper/schema/provider.go +++ b/helper/schema/provider.go @@ -8,6 +8,7 @@ import ( "sync" "github.com/hashicorp/go-multierror" + "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/terraform" ) @@ -89,6 +90,13 @@ func (p *Provider) InternalValidate() error { validationErrors = multierror.Append(validationErrors, err) } + // Provider-specific checks + for k, _ := range sm { + if isReservedProviderFieldName(k) { + return fmt.Errorf("%s is a reserved field name for a provider", k) + } + } + for k, r := range p.ResourcesMap { if err := r.InternalValidate(nil, true); err != nil { validationErrors = multierror.Append(validationErrors, fmt.Errorf("resource %s: %s", k, err)) @@ -104,6 +112,15 @@ func (p *Provider) InternalValidate() error { return validationErrors } +func isReservedProviderFieldName(name string) bool { + for _, reservedName := range config.ReservedProviderFields { + if name == reservedName { + return true + } + } + return false +} + // Meta returns the metadata associated with this provider that was // returned by the Configure call. It will be nil until Configure is called. func (p *Provider) Meta() interface{} { diff --git a/helper/schema/provider_test.go b/helper/schema/provider_test.go index 5b06c5e57..0243be938 100644 --- a/helper/schema/provider_test.go +++ b/helper/schema/provider_test.go @@ -407,3 +407,64 @@ func TestProviderReset(t *testing.T) { t.Fatal(err) } } + +func TestProvider_InternalValidate(t *testing.T) { + cases := []struct { + P *Provider + ExpectedErr error + }{ + { + P: &Provider{ + Schema: map[string]*Schema{ + "foo": { + Type: TypeBool, + Optional: true, + }, + }, + }, + ExpectedErr: nil, + }, + { // Reserved resource fields should be allowed in provider block + P: &Provider{ + Schema: map[string]*Schema{ + "provisioner": { + Type: TypeString, + Optional: true, + }, + "count": { + Type: TypeInt, + Optional: true, + }, + }, + }, + ExpectedErr: nil, + }, + { // Reserved provider fields should not be allowed + P: &Provider{ + Schema: map[string]*Schema{ + "alias": { + Type: TypeString, + Optional: true, + }, + }, + }, + ExpectedErr: fmt.Errorf("%s is a reserved field name for a provider", "alias"), + }, + } + + for i, tc := range cases { + err := tc.P.InternalValidate() + if tc.ExpectedErr == nil { + if err != nil { + t.Fatalf("%d: Error returned (expected no error): %s", i, err) + } + continue + } + if tc.ExpectedErr != nil && err == nil { + t.Fatalf("%d: Expected error (%s), but no error returned", i, tc.ExpectedErr) + } + if err.Error() != tc.ExpectedErr.Error() { + t.Fatalf("%d: Errors don't match. Expected: %#v Given: %#v", i, tc.ExpectedErr, err) + } + } +} diff --git a/helper/schema/resource.go b/helper/schema/resource.go index 03f62cb10..ddba1096d 100644 --- a/helper/schema/resource.go +++ b/helper/schema/resource.go @@ -6,6 +6,7 @@ import ( "log" "strconv" + "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/terraform" ) @@ -394,9 +395,25 @@ func (r *Resource) InternalValidate(topSchemaMap schemaMap, writable bool) error } } + // Resource-specific checks + for k, _ := range tsm { + if isReservedResourceFieldName(k) { + return fmt.Errorf("%s is a reserved field name for a resource", k) + } + } + return schemaMap(r.Schema).InternalValidate(tsm) } +func isReservedResourceFieldName(name string) bool { + for _, reservedName := range config.ReservedResourceFields { + if name == reservedName { + return true + } + } + return false +} + // Data returns a ResourceData struct for this Resource. Each return value // is a separate copy and can be safely modified differently. // diff --git a/helper/schema/resource_test.go b/helper/schema/resource_test.go index 7ecab5b2c..69748e0ed 100644 --- a/helper/schema/resource_test.go +++ b/helper/schema/resource_test.go @@ -706,11 +706,75 @@ func TestResourceInternalValidate(t *testing.T) { true, true, }, + + 8: { // Reserved name at root should be disallowed + &Resource{ + Create: func(d *ResourceData, meta interface{}) error { return nil }, + Read: func(d *ResourceData, meta interface{}) error { return nil }, + Update: func(d *ResourceData, meta interface{}) error { return nil }, + Delete: func(d *ResourceData, meta interface{}) error { return nil }, + Schema: map[string]*Schema{ + "count": { + Type: TypeInt, + Optional: true, + }, + }, + }, + true, + true, + }, + + 9: { // Reserved name at nested levels should be allowed + &Resource{ + Create: func(d *ResourceData, meta interface{}) error { return nil }, + Read: func(d *ResourceData, meta interface{}) error { return nil }, + Update: func(d *ResourceData, meta interface{}) error { return nil }, + Delete: func(d *ResourceData, meta interface{}) error { return nil }, + Schema: map[string]*Schema{ + "parent_list": &Schema{ + Type: TypeString, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "provisioner": { + Type: TypeString, + Optional: true, + }, + }, + }, + }, + }, + }, + true, + false, + }, + + 10: { // Provider reserved name should be allowed in resource + &Resource{ + Create: func(d *ResourceData, meta interface{}) error { return nil }, + Read: func(d *ResourceData, meta interface{}) error { return nil }, + Update: func(d *ResourceData, meta interface{}) error { return nil }, + Delete: func(d *ResourceData, meta interface{}) error { return nil }, + Schema: map[string]*Schema{ + "alias": &Schema{ + Type: TypeString, + Optional: true, + }, + }, + }, + true, + false, + }, } for i, tc := range cases { t.Run(fmt.Sprintf("#%d", i), func(t *testing.T) { - err := tc.In.InternalValidate(schemaMap{}, tc.Writable) + sm := schemaMap{} + if tc.In != nil { + sm = schemaMap(tc.In.Schema) + } + + err := tc.In.InternalValidate(sm, tc.Writable) if err != nil != tc.Err { t.Fatalf("%d: bad: %s", i, err) }