diff --git a/command/plan_test.go b/command/plan_test.go index 41db72046..0c732115f 100644 --- a/command/plan_test.go +++ b/command/plan_test.go @@ -940,7 +940,7 @@ func TestPlan_init_required(t *testing.T) { t.Fatalf("expected error, got success") } got := output.Stderr() - if !strings.Contains(got, `Plugin reinitialization required. Please run "terraform init".`) { + if !strings.Contains(got, `Error: Could not load plugin`) { t.Fatal("wrong error message in output:", got) } } diff --git a/configs/configschema/decoder_spec.go b/configs/configschema/decoder_spec.go index f758d27d2..41a3fc497 100644 --- a/configs/configschema/decoder_spec.go +++ b/configs/configschema/decoder_spec.go @@ -92,10 +92,10 @@ func (b *Block) DecoderSpec() hcldec.Spec { for name, blockS := range b.BlockTypes { if _, exists := ret[name]; exists { - // This indicates an invalid schema, since it's not valid to - // define both an attribute and a block type of the same name. - // However, we don't raise this here since it's checked by - // InternalValidate. + // This indicates an invalid schema, since it's not valid to define + // both an attribute and a block type of the same name. We assume + // that the provider has already used something like + // InternalValidate to validate their schema. continue } @@ -104,7 +104,7 @@ func (b *Block) DecoderSpec() hcldec.Spec { // We can only validate 0 or 1 for MinItems, because a dynamic block // may satisfy any number of min items while only having a single // block in the config. We cannot validate MaxItems because a - // configuration may have any number of dynamic blocks + // configuration may have any number of dynamic blocks. minItems := 0 if blockS.MinItems > 1 { minItems = 1 @@ -145,10 +145,12 @@ func (b *Block) DecoderSpec() hcldec.Spec { } case NestingSet: // We forbid dynamically-typed attributes inside NestingSet in - // InternalValidate, so we don't do anything special to handle - // that here. (There is no set analog to tuple and object types, - // because cty's set implementation depends on knowing the static - // type in order to properly compute its internal hashes.) + // InternalValidate, so we don't do anything special to handle that + // here. (There is no set analog to tuple and object types, because + // cty's set implementation depends on knowing the static type in + // order to properly compute its internal hashes.) We assume that + // the provider has already used something like InternalValidate to + // validate their schema. ret[name] = &hcldec.BlockSetSpec{ TypeName: name, Nested: childSpec, @@ -174,7 +176,8 @@ func (b *Block) DecoderSpec() hcldec.Spec { } default: // Invalid nesting type is just ignored. It's checked by - // InternalValidate. + // InternalValidate. We assume that the provider has already used + // something like InternalValidate to validate their schema. continue } } @@ -190,9 +193,10 @@ func (a *Attribute) decoderSpec(name string) hcldec.Spec { } if a.NestedType != nil { - // FIXME: a panic() is a bad UX. Fix this, probably by extending - // InternalValidate() to check Attribute schemas as well and calling it - // when we get the schema from the provider in Context(). + // FIXME: a panic() is a bad UX. InternalValidate() can check Attribute + // schemas as well so a fix might be to call it when we get the schema + // from the provider in Context(). Since this could be a breaking + // change, we'd need to communicate well before adding that call. if a.Type != cty.NilType { panic("Invalid attribute schema: NestedType and Type cannot both be set. This is a bug in the provider.") } diff --git a/configs/configschema/internal_validate.go b/configs/configschema/internal_validate.go index 9114e0ab2..2afa724e1 100644 --- a/configs/configschema/internal_validate.go +++ b/configs/configschema/internal_validate.go @@ -11,77 +11,139 @@ import ( var validName = regexp.MustCompile(`^[a-z0-9_]+$`) -// InternalValidate returns an error if the receiving block and its child -// schema definitions have any consistencies with the documented rules for -// valid schema. +// InternalValidate returns an error if the receiving block and its child schema +// definitions have any inconsistencies with the documented rules for valid +// schema. // -// This is intended to be used within unit tests to detect when a given -// schema is invalid. +// This can be used within unit tests to detect when a given schema is invalid, +// and is run when terraform loads provider schemas during NewContext. func (b *Block) InternalValidate() error { if b == nil { return fmt.Errorf("top-level block schema is nil") } - return b.internalValidate("", nil) - + return b.internalValidate("") } -func (b *Block) internalValidate(prefix string, err error) error { +func (b *Block) internalValidate(prefix string) error { + var multiErr *multierror.Error + for name, attrS := range b.Attributes { if attrS == nil { - err = multierror.Append(err, fmt.Errorf("%s%s: attribute schema is nil", prefix, name)) + multiErr = multierror.Append(multiErr, fmt.Errorf("%s%s: attribute schema is nil", prefix, name)) continue } - if !validName.MatchString(name) { - err = multierror.Append(err, fmt.Errorf("%s%s: name may contain only lowercase letters, digits and underscores", prefix, name)) - } - if !attrS.Optional && !attrS.Required && !attrS.Computed { - err = multierror.Append(err, fmt.Errorf("%s%s: must set Optional, Required or Computed", prefix, name)) - } - if attrS.Optional && attrS.Required { - err = multierror.Append(err, fmt.Errorf("%s%s: cannot set both Optional and Required", prefix, name)) - } - if attrS.Computed && attrS.Required { - err = multierror.Append(err, fmt.Errorf("%s%s: cannot set both Computed and Required", prefix, name)) - } - if attrS.Type == cty.NilType { - err = multierror.Append(err, fmt.Errorf("%s%s: Type must be set to something other than cty.NilType", prefix, name)) - } + multiErr = multierror.Append(multiErr, attrS.internalValidate(name, prefix)) } for name, blockS := range b.BlockTypes { if blockS == nil { - err = multierror.Append(err, fmt.Errorf("%s%s: block schema is nil", prefix, name)) + multiErr = multierror.Append(multiErr, fmt.Errorf("%s%s: block schema is nil", prefix, name)) continue } if _, isAttr := b.Attributes[name]; isAttr { - err = multierror.Append(err, fmt.Errorf("%s%s: name defined as both attribute and child block type", prefix, name)) + multiErr = multierror.Append(multiErr, fmt.Errorf("%s%s: name defined as both attribute and child block type", prefix, name)) } else if !validName.MatchString(name) { - err = multierror.Append(err, fmt.Errorf("%s%s: name may contain only lowercase letters, digits and underscores", prefix, name)) + multiErr = multierror.Append(multiErr, fmt.Errorf("%s%s: name may contain only lowercase letters, digits and underscores", prefix, name)) } if blockS.MinItems < 0 || blockS.MaxItems < 0 { - err = multierror.Append(err, fmt.Errorf("%s%s: MinItems and MaxItems must both be greater than zero", prefix, name)) + multiErr = multierror.Append(multiErr, fmt.Errorf("%s%s: MinItems and MaxItems must both be greater than zero", prefix, name)) } switch blockS.Nesting { case NestingSingle: switch { case blockS.MinItems != blockS.MaxItems: - err = multierror.Append(err, fmt.Errorf("%s%s: MinItems and MaxItems must match in NestingSingle mode", prefix, name)) + multiErr = multierror.Append(multiErr, fmt.Errorf("%s%s: MinItems and MaxItems must match in NestingSingle mode", prefix, name)) case blockS.MinItems < 0 || blockS.MinItems > 1: - err = multierror.Append(err, fmt.Errorf("%s%s: MinItems and MaxItems must be set to either 0 or 1 in NestingSingle mode", prefix, name)) + multiErr = multierror.Append(multiErr, fmt.Errorf("%s%s: MinItems and MaxItems must be set to either 0 or 1 in NestingSingle mode", prefix, name)) } case NestingGroup: if blockS.MinItems != 0 || blockS.MaxItems != 0 { - err = multierror.Append(err, fmt.Errorf("%s%s: MinItems and MaxItems cannot be used in NestingGroup mode", prefix, name)) + multiErr = multierror.Append(multiErr, fmt.Errorf("%s%s: MinItems and MaxItems cannot be used in NestingGroup mode", prefix, name)) } case NestingList, NestingSet: if blockS.MinItems > blockS.MaxItems && blockS.MaxItems != 0 { - err = multierror.Append(err, fmt.Errorf("%s%s: MinItems must be less than or equal to MaxItems in %s mode", prefix, name, blockS.Nesting)) + multiErr = multierror.Append(multiErr, fmt.Errorf("%s%s: MinItems must be less than or equal to MaxItems in %s mode", prefix, name, blockS.Nesting)) } if blockS.Nesting == NestingSet { ety := blockS.Block.ImpliedType() + if ety.HasDynamicTypes() { + // This is not permitted because the HCL (cty) set implementation + // needs to know the exact type of set elements in order to + // properly hash them, and so can't support mixed types. + multiErr = multierror.Append(multiErr, fmt.Errorf("%s%s: NestingSet blocks may not contain attributes of cty.DynamicPseudoType", prefix, name)) + } + } + case NestingMap: + if blockS.MinItems != 0 || blockS.MaxItems != 0 { + multiErr = multierror.Append(multiErr, fmt.Errorf("%s%s: MinItems and MaxItems must both be 0 in NestingMap mode", prefix, name)) + } + default: + multiErr = multierror.Append(multiErr, fmt.Errorf("%s%s: invalid nesting mode %s", prefix, name, blockS.Nesting)) + } + + subPrefix := prefix + name + "." + multiErr = multierror.Append(multiErr, blockS.Block.internalValidate(subPrefix)) + } + + return multiErr.ErrorOrNil() +} + +// InternalValidate returns an error if the receiving attribute and its child +// schema definitions have any inconsistencies with the documented rules for +// valid schema. +func (a *Attribute) InternalValidate(name string) error { + if a == nil { + return fmt.Errorf("attribute schema is nil") + } + return a.internalValidate(name, "") +} + +func (a *Attribute) internalValidate(name, prefix string) error { + var err *multierror.Error + + /* FIXME: this validation breaks certain existing providers and cannot be enforced without coordination. + if !validName.MatchString(name) { + err = multierror.Append(err, fmt.Errorf("%s%s: name may contain only lowercase letters, digits and underscores", prefix, name)) + } + */ + if !a.Optional && !a.Required && !a.Computed { + err = multierror.Append(err, fmt.Errorf("%s%s: must set Optional, Required or Computed", prefix, name)) + } + if a.Optional && a.Required { + err = multierror.Append(err, fmt.Errorf("%s%s: cannot set both Optional and Required", prefix, name)) + } + if a.Computed && a.Required { + err = multierror.Append(err, fmt.Errorf("%s%s: cannot set both Computed and Required", prefix, name)) + } + + if a.Type == cty.NilType && a.NestedType == nil { + err = multierror.Append(err, fmt.Errorf("%s%s: either Type or NestedType must be defined", prefix, name)) + } + + if a.Type != cty.NilType { + if a.NestedType != nil { + err = multierror.Append(fmt.Errorf("%s: Type and NestedType cannot both be set", name)) + } + } + + if a.NestedType != nil { + switch a.NestedType.Nesting { + case NestingSingle: + switch { + case a.NestedType.MinItems != a.NestedType.MaxItems: + err = multierror.Append(err, fmt.Errorf("%s%s: MinItems and MaxItems must match in NestingSingle mode", prefix, name)) + case a.NestedType.MinItems < 0 || a.NestedType.MinItems > 1: + err = multierror.Append(err, fmt.Errorf("%s%s: MinItems and MaxItems must be set to either 0 or 1 in NestingSingle mode", prefix, name)) + } + case NestingList, NestingSet: + if a.NestedType.MinItems > a.NestedType.MaxItems && a.NestedType.MaxItems != 0 { + err = multierror.Append(err, fmt.Errorf("%s%s: MinItems must be less than or equal to MaxItems in %s mode", prefix, name, a.NestedType.Nesting)) + } + if a.NestedType.Nesting == NestingSet { + ety := a.NestedType.ImpliedType() if ety.HasDynamicTypes() { // This is not permitted because the HCL (cty) set implementation // needs to know the exact type of set elements in order to @@ -90,16 +152,20 @@ func (b *Block) internalValidate(prefix string, err error) error { } } case NestingMap: - if blockS.MinItems != 0 || blockS.MaxItems != 0 { + if a.NestedType.MinItems != 0 || a.NestedType.MaxItems != 0 { err = multierror.Append(err, fmt.Errorf("%s%s: MinItems and MaxItems must both be 0 in NestingMap mode", prefix, name)) } default: - err = multierror.Append(err, fmt.Errorf("%s%s: invalid nesting mode %s", prefix, name, blockS.Nesting)) + err = multierror.Append(err, fmt.Errorf("%s%s: invalid nesting mode %s", prefix, name, a.NestedType.Nesting)) + } + for name, attrS := range a.NestedType.Attributes { + if attrS == nil { + err = multierror.Append(err, fmt.Errorf("%s%s: attribute schema is nil", prefix, name)) + continue + } + err = multierror.Append(err, attrS.internalValidate(name, prefix)) } - - subPrefix := prefix + name + "." - err = blockS.Block.internalValidate(subPrefix, err) } - return err + return err.ErrorOrNil() } diff --git a/configs/configschema/internal_validate_test.go b/configs/configschema/internal_validate_test.go index 512c7adbc..dc70a5fa8 100644 --- a/configs/configschema/internal_validate_test.go +++ b/configs/configschema/internal_validate_test.go @@ -10,172 +10,208 @@ import ( func TestBlockInternalValidate(t *testing.T) { tests := map[string]struct { - Block *Block - ErrCount int + Block *Block + Errs []string }{ "empty": { &Block{}, - 0, + []string{}, }, "valid": { &Block{ Attributes: map[string]*Attribute{ - "foo": &Attribute{ + "foo": { Type: cty.String, Required: true, }, - "bar": &Attribute{ + "bar": { Type: cty.String, Optional: true, }, - "baz": &Attribute{ + "baz": { Type: cty.String, Computed: true, }, - "baz_maybe": &Attribute{ + "baz_maybe": { Type: cty.String, Optional: true, Computed: true, }, }, BlockTypes: map[string]*NestedBlock{ - "single": &NestedBlock{ + "single": { Nesting: NestingSingle, Block: Block{}, }, - "single_required": &NestedBlock{ + "single_required": { Nesting: NestingSingle, Block: Block{}, MinItems: 1, MaxItems: 1, }, - "list": &NestedBlock{ + "list": { Nesting: NestingList, Block: Block{}, }, - "list_required": &NestedBlock{ + "list_required": { Nesting: NestingList, Block: Block{}, MinItems: 1, }, - "set": &NestedBlock{ + "set": { Nesting: NestingSet, Block: Block{}, }, - "set_required": &NestedBlock{ + "set_required": { Nesting: NestingSet, Block: Block{}, MinItems: 1, }, - "map": &NestedBlock{ + "map": { Nesting: NestingMap, Block: Block{}, }, }, }, - 0, + []string{}, }, "attribute with no flags set": { &Block{ Attributes: map[string]*Attribute{ - "foo": &Attribute{ + "foo": { Type: cty.String, }, }, }, - 1, // must set one of the flags + []string{"foo: must set Optional, Required or Computed"}, }, "attribute required and optional": { &Block{ Attributes: map[string]*Attribute{ - "foo": &Attribute{ + "foo": { Type: cty.String, Required: true, Optional: true, }, }, }, - 1, // both required and optional + []string{"foo: cannot set both Optional and Required"}, }, "attribute required and computed": { &Block{ Attributes: map[string]*Attribute{ - "foo": &Attribute{ + "foo": { Type: cty.String, Required: true, Computed: true, }, }, }, - 1, // both required and computed + []string{"foo: cannot set both Computed and Required"}, }, "attribute optional and computed": { &Block{ Attributes: map[string]*Attribute{ - "foo": &Attribute{ + "foo": { Type: cty.String, Optional: true, Computed: true, }, }, }, - 0, + []string{}, }, "attribute with missing type": { &Block{ Attributes: map[string]*Attribute{ - "foo": &Attribute{ + "foo": { Optional: true, }, }, }, - 1, // Type must be set + []string{"foo: either Type or NestedType must be defined"}, }, - "attribute with invalid name": { + /* FIXME: This caused errors when applied to existing providers (oci) + and cannot be enforced without coordination. + + "attribute with invalid name": {&Block{Attributes: + map[string]*Attribute{"fooBar": {Type: cty.String, Optional: + true, + }, + }, + }, + []string{"fooBar: name may contain only lowercase letters, digits and underscores"}, + }, + */ + "attribute with invalid NestedType nesting": { &Block{ Attributes: map[string]*Attribute{ - "fooBar": &Attribute{ - Type: cty.String, + "foo": { + NestedType: &Object{ + Nesting: NestingSingle, + MinItems: 10, + MaxItems: 10, + }, Optional: true, }, }, }, - 1, // name may not contain uppercase letters + []string{"foo: MinItems and MaxItems must be set to either 0 or 1 in NestingSingle mode"}, + }, + "attribute with invalid NestedType attribute": { + &Block{ + Attributes: map[string]*Attribute{ + "foo": { + NestedType: &Object{ + Nesting: NestingSingle, + Attributes: map[string]*Attribute{ + "foo": { + Type: cty.String, + Required: true, + Optional: true, + }, + }, + }, + Optional: true, + }, + }, + }, + []string{"foo: cannot set both Optional and Required"}, }, "block type with invalid name": { &Block{ BlockTypes: map[string]*NestedBlock{ - "fooBar": &NestedBlock{ + "fooBar": { Nesting: NestingSingle, }, }, }, - 1, // name may not contain uppercase letters + []string{"fooBar: name may contain only lowercase letters, digits and underscores"}, }, "colliding names": { &Block{ Attributes: map[string]*Attribute{ - "foo": &Attribute{ + "foo": { Type: cty.String, Optional: true, }, }, BlockTypes: map[string]*NestedBlock{ - "foo": &NestedBlock{ + "foo": { Nesting: NestingSingle, }, }, }, - 1, // "foo" is defined as both attribute and block type + []string{"foo: name defined as both attribute and child block type"}, }, "nested block with badness": { &Block{ BlockTypes: map[string]*NestedBlock{ - "bad": &NestedBlock{ + "bad": { Nesting: NestingSingle, Block: Block{ Attributes: map[string]*Attribute{ - "nested_bad": &Attribute{ + "nested_bad": { Type: cty.String, Required: true, Optional: true, @@ -185,16 +221,16 @@ func TestBlockInternalValidate(t *testing.T) { }, }, }, - 1, // nested_bad is both required and optional + []string{"bad.nested_bad: cannot set both Optional and Required"}, }, "nested list block with dynamically-typed attribute": { &Block{ BlockTypes: map[string]*NestedBlock{ - "bad": &NestedBlock{ + "bad": { Nesting: NestingList, Block: Block{ Attributes: map[string]*Attribute{ - "nested_bad": &Attribute{ + "nested_bad": { Type: cty.DynamicPseudoType, Optional: true, }, @@ -203,16 +239,16 @@ func TestBlockInternalValidate(t *testing.T) { }, }, }, - 0, + []string{}, }, "nested set block with dynamically-typed attribute": { &Block{ BlockTypes: map[string]*NestedBlock{ - "bad": &NestedBlock{ + "bad": { Nesting: NestingSet, Block: Block{ Attributes: map[string]*Attribute{ - "nested_bad": &Attribute{ + "nested_bad": { Type: cty.DynamicPseudoType, Optional: true, }, @@ -221,11 +257,11 @@ func TestBlockInternalValidate(t *testing.T) { }, }, }, - 1, // NestingSet blocks may not contain attributes of cty.DynamicPseudoType + []string{"bad: NestingSet blocks may not contain attributes of cty.DynamicPseudoType"}, }, "nil": { nil, - 1, // block is nil + []string{"top-level block schema is nil"}, }, "nil attr": { &Block{ @@ -233,7 +269,7 @@ func TestBlockInternalValidate(t *testing.T) { "bad": nil, }, }, - 1, // attribute schema is nil + []string{"bad: attribute schema is nil"}, }, "nil block type": { &Block{ @@ -241,18 +277,26 @@ func TestBlockInternalValidate(t *testing.T) { "bad": nil, }, }, - 1, // block schema is nil + []string{"bad: block schema is nil"}, }, } for name, test := range tests { t.Run(name, func(t *testing.T) { errs := multierrorErrors(test.Block.InternalValidate()) - if got, want := len(errs), test.ErrCount; got != want { + if got, want := len(errs), len(test.Errs); got != want { t.Errorf("wrong number of errors %d; want %d", got, want) for _, err := range errs { t.Logf("- %s", err.Error()) } + } else { + if len(errs) > 0 { + for i := range errs { + if errs[i].Error() != test.Errs[i] { + t.Errorf("wrong error: got %s, want %s", errs[i].Error(), test.Errs[i]) + } + } + } } }) } diff --git a/terraform/resource_provider.go b/terraform/resource_provider.go index 56f47aa76..d4bdfcb68 100644 --- a/terraform/resource_provider.go +++ b/terraform/resource_provider.go @@ -1,8 +1,6 @@ package terraform const errPluginInit = ` -Plugin reinitialization required. Please run "terraform init". - Plugins are external binaries that Terraform uses to access and manipulate resources. The configuration provided requires plugins which can't be located, don't satisfy the version constraints, or are otherwise incompatible. @@ -12,4 +10,7 @@ configuration, including providers used in child modules. To see the requirements and constraints, run "terraform providers". %s + +Plugin reinitialization required. Please address the above error(s) and run +"terraform init". ` diff --git a/terraform/schemas.go b/terraform/schemas.go index 46d7c27d8..017856ef5 100644 --- a/terraform/schemas.go +++ b/terraform/schemas.go @@ -142,6 +142,9 @@ func loadProviderSchemas(schemas map[addrs.Provider]*ProviderSchema, config *con } for t, r := range resp.ResourceTypes { + if err := r.Block.InternalValidate(); err != nil { + diags = diags.Append(fmt.Errorf(errProviderSchemaInvalid, name, "resource", t, err)) + } s.ResourceTypes[t] = r.Block s.ResourceTypeSchemaVersions[t] = uint64(r.Version) if r.Version < 0 { @@ -152,6 +155,9 @@ func loadProviderSchemas(schemas map[addrs.Provider]*ProviderSchema, config *con } for t, d := range resp.DataSources { + if err := d.Block.InternalValidate(); err != nil { + diags = diags.Append(fmt.Errorf(errProviderSchemaInvalid, name, "data source", t, err)) + } s.DataSources[t] = d.Block if d.Version < 0 { // We're not using the version numbers here yet, but we'll check @@ -274,3 +280,11 @@ func (ps *ProviderSchema) SchemaForResourceType(mode addrs.ResourceMode, typeNam func (ps *ProviderSchema) SchemaForResourceAddr(addr addrs.Resource) (schema *configschema.Block, version uint64) { return ps.SchemaForResourceType(addr.Mode, addr.Type) } + +const errProviderSchemaInvalid = ` +Internal validation of the provider failed! This is always a bug with the +provider itself, and not a user issue. Please report this bug to the +maintainers of the %q provider: + +%s %s: %s +`