ResiLien
/
terraform
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #29559 from hashicorp/jbardin/optional-attrs 

Prevent `ObjectWithOptionalAttrs` from escaping
This commit is contained in:
James Bardin 2021-09-13 08:58:11 -04:00 committed by GitHub
parent 65e0c448a0 7f26531d4f
commit 8ed9a270e5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 328 additions and 118 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
internal/backend/unparsed_value_test.go
View File

@ -26,6 +26,7 @@ func TestParseVariableValuesUndeclared(t *testing.T) {
"declared1": {
Name: "declared1",
Type: cty.String,
ConstraintType: cty.String,
ParsingMode: configs.VariableParseLiteral,
DeclRange: hcl.Range{
Filename: "fake.tf",
@ -36,6 +37,7 @@ func TestParseVariableValuesUndeclared(t *testing.T) {
"missing1": {
Name: "missing1",
Type: cty.String,
ConstraintType: cty.String,
ParsingMode: configs.VariableParseLiteral,
DeclRange: hcl.Range{
Filename: "fake.tf",
@ -46,6 +48,7 @@ func TestParseVariableValuesUndeclared(t *testing.T) {
"missing2": {
Name: "missing1",
Type: cty.String,
ConstraintType: cty.String,
ParsingMode: configs.VariableParseLiteral,
Default: cty.StringVal("default for missing2"),
DeclRange: hcl.Range{

6
internal/configs/configschema/decoder_spec.go
View File

@ -121,7 +121,7 @@ func (b *Block) DecoderSpec() hcldec.Spec {
// implied type more complete, but if there are any
// dynamically-typed attributes inside we must use a tuple
// instead, at the expense of our type then not being predictable.
if blockS.Block.ImpliedType().HasDynamicTypes() {
if blockS.Block.specType().HasDynamicTypes() {
ret[name] = &hcldec.BlockTupleSpec{
TypeName: name,
Nested: childSpec,
@ -155,7 +155,7 @@ func (b *Block) DecoderSpec() hcldec.Spec {
// implied type more complete, but if there are any
// dynamically-typed attributes inside we must use a tuple
// instead, at the expense of our type then not being predictable.
if blockS.Block.ImpliedType().HasDynamicTypes() {
if blockS.Block.specType().HasDynamicTypes() {
ret[name] = &hcldec.BlockObjectSpec{
TypeName: name,
Nested: childSpec,
@ -195,7 +195,7 @@ func (a *Attribute) decoderSpec(name string) hcldec.Spec {
panic("Invalid attribute schema: NestedType and Type cannot both be set. This is a bug in the provider.")
}
ty := a.NestedType.ImpliedType()
ty := a.NestedType.specType()
ret.Type = ty
ret.Required = a.Required || a.NestedType.MinItems > 0
return ret

24
internal/configs/configschema/implied_type.go
View File

@ -8,11 +8,23 @@ import (
// ImpliedType returns the cty.Type that would result from decoding a
// configuration block using the receiving block schema.
//
// The type returned from Block.ImpliedType differs from the type returned by
// hcldec.ImpliedType in that there will be no objects with optional
// attributes, since this value is not to be used for the decoding of
// configuration.
//
// ImpliedType always returns a result, even if the given schema is
// inconsistent. Code that creates configschema.Block objects should be
// tested using the InternalValidate method to detect any inconsistencies
// that would cause this method to fall back on defaults and assumptions.
func (b *Block) ImpliedType() cty.Type {
return b.specType().WithoutOptionalAttributesDeep()
}
// specType returns the cty.Type used for decoding a configuration
// block using the receiving block schema. This is the type used internally by
// hcldec to decode configuration.
func (b *Block) specType() cty.Type {
if b == nil {
return cty.EmptyObject
}
@ -41,14 +53,20 @@ func (b *Block) ContainsSensitive() bool {
return false
}
// ImpliedType returns the cty.Type that would result from decoding a NestedType
// Attribute using the receiving block schema.
// ImpliedType returns the cty.Type that would result from decoding a
// NestedType Attribute using the receiving block schema.
//
// ImpliedType always returns a result, even if the given schema is
// inconsistent. Code that creates configschema.Object objects should be tested
// using the InternalValidate method to detect any inconsistencies that would
// cause this method to fall back on defaults and assumptions.
func (o *Object) ImpliedType() cty.Type {
return o.specType().WithoutOptionalAttributesDeep()
}
// specType returns the cty.Type used for decoding a NestedType Attribute using
// the receiving block schema.
func (o *Object) specType() cty.Type {
if o == nil {
return cty.EmptyObject
}
@ -56,7 +74,7 @@ func (o *Object) ImpliedType() cty.Type {
attrTys := make(map[string]cty.Type, len(o.Attributes))
for name, attrS := range o.Attributes {
if attrS.NestedType != nil {
attrTys[name] = attrS.NestedType.ImpliedType()
attrTys[name] = attrS.NestedType.specType()
} else {
attrTys[name] = attrS.Type
}

301
internal/configs/configschema/implied_type_test.go
View File

@ -112,6 +112,36 @@ func TestBlockImpliedType(t *testing.T) {
}),
}),
},
"nested objects with optional attrs": {
&Block{
Attributes: map[string]*Attribute{
"map": {
Optional: true,
NestedType: &Object{
Nesting: NestingMap,
Attributes: map[string]*Attribute{
"optional": {Type: cty.String, Optional: true},
"required": {Type: cty.Number, Required: true},
"computed": {Type: cty.List(cty.Bool), Computed: true},
"optional_computed": {Type: cty.Map(cty.Bool), Optional: true, Computed: true},
},
},
},
},
},
// The ImpliedType from the type-level block should not contain any
// optional attributes.
cty.Object(map[string]cty.Type{
"map": cty.Map(cty.Object(
map[string]cty.Type{
"optional": cty.String,
"required": cty.Number,
"computed": cty.List(cty.Bool),
"optional_computed": cty.Map(cty.Bool),
},
)),
}),
},
}
for name, test := range tests {
@ -137,6 +167,211 @@ func TestObjectImpliedType(t *testing.T) {
&Object{},
cty.EmptyObject,
},
"attributes": {
&Object{
Nesting: NestingSingle,
Attributes: map[string]*Attribute{
"optional": {Type: cty.String, Optional: true},
"required": {Type: cty.Number, Required: true},
"computed": {Type: cty.List(cty.Bool), Computed: true},
"optional_computed": {Type: cty.Map(cty.Bool), Optional: true, Computed: true},
},
},
cty.Object(
map[string]cty.Type{
"optional": cty.String,
"required": cty.Number,
"computed": cty.List(cty.Bool),
"optional_computed": cty.Map(cty.Bool),
},
),
},
"nested attributes": {
&Object{
Nesting: NestingSingle,
Attributes: map[string]*Attribute{
"nested_type": {
NestedType: &Object{
Nesting: NestingSingle,
Attributes: map[string]*Attribute{
"optional": {Type: cty.String, Optional: true},
"required": {Type: cty.Number, Required: true},
"computed": {Type: cty.List(cty.Bool), Computed: true},
"optional_computed": {Type: cty.Map(cty.Bool), Optional: true, Computed: true},
},
},
Optional: true,
},
},
},
cty.Object(map[string]cty.Type{
"nested_type": cty.Object(map[string]cty.Type{
"optional": cty.String,
"required": cty.Number,
"computed": cty.List(cty.Bool),
"optional_computed": cty.Map(cty.Bool),
}),
}),
},
"nested object-type attributes": {
&Object{
Nesting: NestingSingle,
Attributes: map[string]*Attribute{
"nested_type": {
NestedType: &Object{
Nesting: NestingSingle,
Attributes: map[string]*Attribute{
"optional": {Type: cty.String, Optional: true},
"required": {Type: cty.Number, Required: true},
"computed": {Type: cty.List(cty.Bool), Computed: true},
"optional_computed": {Type: cty.Map(cty.Bool), Optional: true, Computed: true},
"object": {
Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"optional": cty.String,
"required": cty.Number,
}, []string{"optional"}),
},
},
},
Optional: true,
},
},
},
cty.Object(map[string]cty.Type{
"nested_type": cty.Object(map[string]cty.Type{
"optional": cty.String,
"required": cty.Number,
"computed": cty.List(cty.Bool),
"optional_computed": cty.Map(cty.Bool),
"object": cty.Object(map[string]cty.Type{"optional": cty.String, "required": cty.Number}),
}),
}),
},
"NestingList": {
&Object{
Nesting: NestingList,
Attributes: map[string]*Attribute{
"foo": {Type: cty.String, Optional: true},
},
},
cty.List(cty.Object(map[string]cty.Type{"foo": cty.String})),
},
"NestingMap": {
&Object{
Nesting: NestingMap,
Attributes: map[string]*Attribute{
"foo": {Type: cty.String},
},
},
cty.Map(cty.Object(map[string]cty.Type{"foo": cty.String})),
},
"NestingSet": {
&Object{
Nesting: NestingSet,
Attributes: map[string]*Attribute{
"foo": {Type: cty.String},
},
},
cty.Set(cty.Object(map[string]cty.Type{"foo": cty.String})),
},
"deeply nested NestingList": {
&Object{
Nesting: NestingList,
Attributes: map[string]*Attribute{
"foo": {
NestedType: &Object{
Nesting: NestingList,
Attributes: map[string]*Attribute{
"bar": {Type: cty.String},
},
},
},
},
},
cty.List(cty.Object(map[string]cty.Type{"foo": cty.List(cty.Object(map[string]cty.Type{"bar": cty.String}))})),
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
got := test.Schema.ImpliedType()
if !got.Equals(test.Want) {
t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, test.Want)
}
})
}
}
func TestObjectContainsSensitive(t *testing.T) {
tests := map[string]struct {
Schema *Object
Want bool
}{
"object contains sensitive": {
&Object{
Attributes: map[string]*Attribute{
"sensitive": {Sensitive: true},
},
},
true,
},
"no sensitive attrs": {
&Object{
Attributes: map[string]*Attribute{
"insensitive": {},
},
},
false,
},
"nested object contains sensitive": {
&Object{
Attributes: map[string]*Attribute{
"nested": {
NestedType: &Object{
Nesting: NestingSingle,
Attributes: map[string]*Attribute{
"sensitive": {Sensitive: true},
},
},
},
},
},
true,
},
"nested obj, no sensitive attrs": {
&Object{
Attributes: map[string]*Attribute{
"nested": {
NestedType: &Object{
Nesting: NestingSingle,
Attributes: map[string]*Attribute{
"public": {},
},
},
},
},
},
false,
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
got := test.Schema.ContainsSensitive()
if got != test.Want {
t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, test.Want)
}
})
}
}
// Nested attribute should return optional object attributes for decoding.
func TestObjectSpecType(t *testing.T) {
tests := map[string]struct {
Schema *Object
Want cty.Type
}{
"attributes": {
&Object{
Nesting: NestingSingle,
@ -265,74 +500,10 @@ func TestObjectImpliedType(t *testing.T) {
for name, test := range tests {
t.Run(name, func(t *testing.T) {
got := test.Schema.ImpliedType()
got := test.Schema.specType()
if !got.Equals(test.Want) {
t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, test.Want)
}
})
}
}
func TestObjectContainsSensitive(t *testing.T) {
tests := map[string]struct {
Schema *Object
Want bool
}{
"object contains sensitive": {
&Object{
Attributes: map[string]*Attribute{
"sensitive": {Sensitive: true},
},
},
true,
},
"no sensitive attrs": {
&Object{
Attributes: map[string]*Attribute{
"insensitive": {},
},
},
false,
},
"nested object contains sensitive": {
&Object{
Attributes: map[string]*Attribute{
"nested": {
NestedType: &Object{
Nesting: NestingSingle,
Attributes: map[string]*Attribute{
"sensitive": {Sensitive: true},
},
},
},
},
},
true,
},
"nested obj, no sensitive attrs": {
&Object{
Attributes: map[string]*Attribute{
"nested": {
NestedType: &Object{
Nesting: NestingSingle,
Attributes: map[string]*Attribute{
"public": {},
},
},
},
},
},
false,
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
got := test.Schema.ContainsSensitive()
if got != test.Want {
t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, test.Want)
}
})
}
}

2
internal/configs/experiments.go
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",

3
internal/configs/module_merge.go
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.

2
internal/configs/module_merge_test.go
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",

14
internal/configs/named_values.go
View File

@ -22,7 +22,13 @@ type Variable struct {
Name string
Description string
Default cty.Value
// 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,

4
internal/plans/objchange/objchange_test.go
View File

@ -1452,12 +1452,12 @@ func TestProposedNew(t *testing.T) {
"map": cty.NullVal(cty.Map(cty.Object(map[string]cty.Type{"bar": cty.String}))),
"set": cty.NullVal(cty.Set(cty.Object(map[string]cty.Type{"bar": cty.String}))),
"nested_map": cty.NullVal(cty.Map(cty.Object(map[string]cty.Type{
"inner": cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"inner": cty.Object(map[string]cty.Type{
"optional": cty.String,
"computed": cty.String,
"optional_computed": cty.String,
"required": cty.String,
}, []string{"computed", "optional", "optional_computed"}),
}),
}))),
}),
},

18
internal/terraform/evaluate.go
View File

@ -237,12 +237,6 @@ func (d *evaluationStateData) GetInputVariable(addr addrs.InputVariable, rng tfd
})
return cty.DynamicVal, diags
}
wantType := cty.DynamicPseudoType
if config.Type != cty.NilType {
wantType = config.Type
}
d.Evaluator.VariableValuesLock.Lock()
defer d.Evaluator.VariableValuesLock.Unlock()
@ -262,15 +256,15 @@ func (d *evaluationStateData) GetInputVariable(addr addrs.InputVariable, rng tfd
if d.Operation == walkValidate {
// Ensure variable sensitivity is captured in the validate walk
if config.Sensitive {
return cty.UnknownVal(wantType).Mark(marks.Sensitive), diags
return cty.UnknownVal(config.Type).Mark(marks.Sensitive), diags
}
return cty.UnknownVal(wantType), diags
return cty.UnknownVal(config.Type), diags
}
moduleAddrStr := d.ModulePath.String()
vals := d.Evaluator.VariableValues[moduleAddrStr]
if vals == nil {
return cty.UnknownVal(wantType), diags
return cty.UnknownVal(config.Type), diags
}
val, isSet := vals[addr.Name]
@ -278,11 +272,11 @@ func (d *evaluationStateData) GetInputVariable(addr addrs.InputVariable, rng tfd
if config.Default != cty.NilVal {
return config.Default, diags
}
return cty.UnknownVal(wantType), diags
return cty.UnknownVal(config.Type), diags
}
var err error
val, err = convert.Convert(val, wantType)
val, err = convert.Convert(val, config.ConstraintType)
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.
@ -294,7 +288,7 @@ func (d *evaluationStateData) GetInputVariable(addr addrs.InputVariable, rng tfd
})
// Stub out our return value so that the semantic checker doesn't
// produce redundant downstream errors.
val = cty.UnknownVal(wantType)
val = cty.UnknownVal(config.Type)
}
// Mark if sensitive

4
internal/terraform/evaluate_test.go
View File

@ -98,12 +98,16 @@ func TestEvaluatorGetInputVariable(t *testing.T) {
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"),
Type: cty.String,
ConstraintType: cty.String,
},
},
},

5
internal/terraform/node_module_variable.go
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)

9
internal/terraform/node_module_variable_test.go
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"
@ -17,6 +18,8 @@ func TestNodeModuleVariablePath(t *testing.T) {
Addr: addrs.RootModuleInstance.InputVariable("foo"),
Config: &configs.Variable{
Name: "foo",
Type: cty.String,
ConstraintType: cty.String,
},
}
@ -32,6 +35,8 @@ func TestNodeModuleVariableReferenceableName(t *testing.T) {
Addr: addrs.InputVariable{Name: "foo"},
Config: &configs.Variable{
Name: "foo",
Type: cty.String,
ConstraintType: cty.String,
},
}
@ -65,6 +70,8 @@ func TestNodeModuleVariableReference(t *testing.T) {
Module: addrs.RootModule.Child("bar"),
Config: &configs.Variable{
Name: "foo",
Type: cty.String,
ConstraintType: cty.String,
},
Expr: &hclsyntax.ScopeTraversalExpr{
Traversal: hcl.Traversal{
@ -91,6 +98,8 @@ func TestNodeModuleVariableReference_grandchild(t *testing.T) {
Module: addrs.RootModule.Child("bar"),
Config: &configs.Variable{
Name: "foo",
Type: cty.String,
ConstraintType: cty.String,
},
Expr: &hclsyntax.ScopeTraversalExpr{
Traversal: hcl.Traversal{

3
internal/terraform/node_root_variable_test.go
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) {
@ -14,6 +15,8 @@ func TestNodeRootVariableExecute(t *testing.T) {
Addr: addrs.InputVariable{Name: "foo"},
Config: &configs.Variable{
Name: "foo",
Type: cty.String,
ConstraintType: cty.String,
},
}

4
internal/terraform/variables.go
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: