configs: add ConstraintType to config.Variable

In order to handle optional attributes, the Variable type needs to keep
track of the type constraint for decoding and conversion, as well as the
concrete type for creating values and type comparison.

Since the Type field is referenced throughout the codebase, and for
future refactoring if the handling of optional attributes changes
significantly, the constraint is now loaded into an entirely new field
called ConstraintType. This prevents types containing
ObjectWithOptionalAttrs from escaping the decode/conversion codepaths
into the rest of the codebase.
This commit is contained in:
James Bardin 2021-09-10 10:58:44 -04:00
parent 43123d284e
commit 53a73a8ab6
11 changed files with 70 additions and 34 deletions

View File

@ -24,9 +24,10 @@ func TestParseVariableValuesUndeclared(t *testing.T) {
}
decls := map[string]*configs.Variable{
"declared1": {
Name: "declared1",
Type: cty.String,
ParsingMode: configs.VariableParseLiteral,
Name: "declared1",
Type: cty.String,
ConstraintType: cty.String,
ParsingMode: configs.VariableParseLiteral,
DeclRange: hcl.Range{
Filename: "fake.tf",
Start: hcl.Pos{Line: 2, Column: 1, Byte: 0},
@ -34,9 +35,10 @@ func TestParseVariableValuesUndeclared(t *testing.T) {
},
},
"missing1": {
Name: "missing1",
Type: cty.String,
ParsingMode: configs.VariableParseLiteral,
Name: "missing1",
Type: cty.String,
ConstraintType: cty.String,
ParsingMode: configs.VariableParseLiteral,
DeclRange: hcl.Range{
Filename: "fake.tf",
Start: hcl.Pos{Line: 3, Column: 1, Byte: 0},
@ -44,10 +46,11 @@ func TestParseVariableValuesUndeclared(t *testing.T) {
},
},
"missing2": {
Name: "missing1",
Type: cty.String,
ParsingMode: configs.VariableParseLiteral,
Default: cty.StringVal("default for missing2"),
Name: "missing1",
Type: cty.String,
ConstraintType: cty.String,
ParsingMode: configs.VariableParseLiteral,
Default: cty.StringVal("default for missing2"),
DeclRange: hcl.Range{
Filename: "fake.tf",
Start: hcl.Pos{Line: 4, Column: 1, Byte: 0},

View File

@ -198,7 +198,7 @@ func checkModuleExperiments(m *Module) hcl.Diagnostics {
if !m.ActiveExperiments.Has(experiments.ModuleVariableOptionalAttrs) {
for _, v := range m.Variables {
if typeConstraintHasOptionalAttrs(v.Type) {
if typeConstraintHasOptionalAttrs(v.ConstraintType) {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Optional object type attributes are experimental",

View File

@ -51,6 +51,7 @@ func (v *Variable) merge(ov *Variable) hcl.Diagnostics {
}
if ov.Type != cty.NilType {
v.Type = ov.Type
v.ConstraintType = ov.ConstraintType
}
if ov.ParsingMode != 0 {
v.ParsingMode = ov.ParsingMode
@ -67,7 +68,7 @@ func (v *Variable) merge(ov *Variable) hcl.Diagnostics {
// constraint but the converted value cannot. In practice, this situation
// should be rare since most of our conversions are interchangable.
if v.Default != cty.NilVal {
val, err := convert.Convert(v.Default, v.Type)
val, err := convert.Convert(v.Default, v.ConstraintType)
if err != nil {
// What exactly we'll say in the error message here depends on whether
// it was Default or Type that was overridden here.

View File

@ -25,6 +25,7 @@ func TestModuleOverrideVariable(t *testing.T) {
DescriptionSet: true,
Default: cty.StringVal("b_override"),
Type: cty.String,
ConstraintType: cty.String,
ParsingMode: VariableParseLiteral,
DeclRange: hcl.Range{
Filename: "testdata/valid-modules/override-variable/primary.tf",
@ -46,6 +47,7 @@ func TestModuleOverrideVariable(t *testing.T) {
DescriptionSet: true,
Default: cty.StringVal("b_override partial"),
Type: cty.String,
ConstraintType: cty.String,
ParsingMode: VariableParseLiteral,
DeclRange: hcl.Range{
Filename: "testdata/valid-modules/override-variable/primary.tf",

View File

@ -22,7 +22,13 @@ type Variable struct {
Name string
Description string
Default cty.Value
Type cty.Type
// Type is the concrete type of the variable value.
Type cty.Type
// ConstraintType is used for decoding and type conversions, and may
// contain nested ObjectWithOptionalAttr types.
ConstraintType cty.Type
ParsingMode VariableParsingMode
Validations []*VariableValidation
Sensitive bool
@ -45,6 +51,7 @@ func decodeVariableBlock(block *hcl.Block, override bool) (*Variable, hcl.Diagno
// or not they are set when we merge.
if !override {
v.Type = cty.DynamicPseudoType
v.ConstraintType = cty.DynamicPseudoType
v.ParsingMode = VariableParseLiteral
}
@ -92,7 +99,8 @@ func decodeVariableBlock(block *hcl.Block, override bool) (*Variable, hcl.Diagno
if attr, exists := content.Attributes["type"]; exists {
ty, parseMode, tyDiags := decodeVariableType(attr.Expr)
diags = append(diags, tyDiags...)
v.Type = ty
v.ConstraintType = ty
v.Type = ty.WithoutOptionalAttributesDeep()
v.ParsingMode = parseMode
}
@ -112,9 +120,9 @@ func decodeVariableBlock(block *hcl.Block, override bool) (*Variable, hcl.Diagno
// attribute above.
// However, we can't do this if we're in an override file where
// the type might not be set; we'll catch that during merge.
if v.Type != cty.NilType {
if v.ConstraintType != cty.NilType {
var err error
val, err = convert.Convert(val, v.Type)
val, err = convert.Convert(val, v.ConstraintType)
if err != nil {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,

View File

@ -238,7 +238,16 @@ func (d *evaluationStateData) GetInputVariable(addr addrs.InputVariable, rng tfd
return cty.DynamicVal, diags
}
// wantType is the concrete value type to be returned.
wantType := cty.DynamicPseudoType
// converstionType is the type used for conversion, which may include
// optional attributes.
conversionType := cty.DynamicPseudoType
if config.ConstraintType != cty.NilType {
conversionType = config.ConstraintType
}
if config.Type != cty.NilType {
wantType = config.Type
}
@ -282,7 +291,7 @@ func (d *evaluationStateData) GetInputVariable(addr addrs.InputVariable, rng tfd
}
var err error
val, err = convert.Convert(val, wantType)
val, err = convert.Convert(val, conversionType)
if err != nil {
// We should never get here because this problem should've been caught
// during earlier validation, but we'll do something reasonable anyway.

View File

@ -95,15 +95,19 @@ func TestEvaluatorGetInputVariable(t *testing.T) {
Module: &configs.Module{
Variables: map[string]*configs.Variable{
"some_var": {
Name: "some_var",
Sensitive: true,
Default: cty.StringVal("foo"),
Name: "some_var",
Sensitive: true,
Default: cty.StringVal("foo"),
Type: cty.String,
ConstraintType: cty.String,
},
// Avoid double marking a value
"some_other_var": {
Name: "some_other_var",
Sensitive: true,
Default: cty.StringVal("bar"),
Name: "some_other_var",
Sensitive: true,
Default: cty.StringVal("bar"),
Type: cty.String,
ConstraintType: cty.String,
},
},
},

View File

@ -200,7 +200,6 @@ func (n *nodeModuleVariable) DotNode(name string, opts *dag.DotOpts) *dag.DotNod
// validation, and we will not have any expansion module instance
// repetition data.
func (n *nodeModuleVariable) evalModuleCallArgument(ctx EvalContext, validateOnly bool) (map[string]cty.Value, error) {
wantType := n.Config.Type
name := n.Addr.Variable.Name
expr := n.Expr
@ -238,7 +237,7 @@ func (n *nodeModuleVariable) evalModuleCallArgument(ctx EvalContext, validateOnl
// now we can do our own local type conversion and produce an error message
// with better context if it fails.
var convErr error
val, convErr = convert.Convert(val, wantType)
val, convErr = convert.Convert(val, n.Config.ConstraintType)
if convErr != nil {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
@ -251,7 +250,7 @@ func (n *nodeModuleVariable) evalModuleCallArgument(ctx EvalContext, validateOnl
})
// We'll return a placeholder unknown value to avoid producing
// redundant downstream errors.
val = cty.UnknownVal(wantType)
val = cty.UnknownVal(n.Config.Type)
}
vals := make(map[string]cty.Value)

View File

@ -7,6 +7,7 @@ import (
"github.com/go-test/deep"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hclsyntax"
"github.com/zclconf/go-cty/cty"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/configs"
@ -16,7 +17,9 @@ func TestNodeModuleVariablePath(t *testing.T) {
n := &nodeModuleVariable{
Addr: addrs.RootModuleInstance.InputVariable("foo"),
Config: &configs.Variable{
Name: "foo",
Name: "foo",
Type: cty.String,
ConstraintType: cty.String,
},
}
@ -31,7 +34,9 @@ func TestNodeModuleVariableReferenceableName(t *testing.T) {
n := &nodeExpandModuleVariable{
Addr: addrs.InputVariable{Name: "foo"},
Config: &configs.Variable{
Name: "foo",
Name: "foo",
Type: cty.String,
ConstraintType: cty.String,
},
}
@ -64,7 +69,9 @@ func TestNodeModuleVariableReference(t *testing.T) {
Addr: addrs.InputVariable{Name: "foo"},
Module: addrs.RootModule.Child("bar"),
Config: &configs.Variable{
Name: "foo",
Name: "foo",
Type: cty.String,
ConstraintType: cty.String,
},
Expr: &hclsyntax.ScopeTraversalExpr{
Traversal: hcl.Traversal{
@ -90,7 +97,9 @@ func TestNodeModuleVariableReference_grandchild(t *testing.T) {
Addr: addrs.InputVariable{Name: "foo"},
Module: addrs.RootModule.Child("bar"),
Config: &configs.Variable{
Name: "foo",
Name: "foo",
Type: cty.String,
ConstraintType: cty.String,
},
Expr: &hclsyntax.ScopeTraversalExpr{
Traversal: hcl.Traversal{

View File

@ -5,6 +5,7 @@ import (
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/configs"
"github.com/zclconf/go-cty/cty"
)
func TestNodeRootVariableExecute(t *testing.T) {
@ -13,7 +14,9 @@ func TestNodeRootVariableExecute(t *testing.T) {
n := &NodeRootVariable{
Addr: addrs.InputVariable{Name: "foo"},
Config: &configs.Variable{
Name: "foo",
Name: "foo",
Type: cty.String,
ConstraintType: cty.String,
},
}

View File

@ -262,10 +262,8 @@ func checkInputVariables(vcs map[string]*configs.Variable, vs InputValues) tfdia
continue
}
wantType := vc.Type
// A given value is valid if it can convert to the desired type.
_, err := convert.Convert(val.Value, wantType)
_, err := convert.Convert(val.Value, vc.ConstraintType)
if err != nil {
switch val.SourceType {
case ValueFromConfig, ValueFromAutoFile, ValueFromNamedFile: