Actively disallow reserved field names in schema (#15522)

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{}{},

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{} {

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)
+		}
+	}
+}

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.
 //

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)
 			}