Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

configs: explicitly nullable variable values #29832

Merged
merged 5 commits into from
Nov 1, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions internal/configs/module_merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ func TestModuleOverrideVariable(t *testing.T) {
Description: "b_override description",
DescriptionSet: true,
Default: cty.StringVal("b_override"),
Nullable: true,
Type: cty.String,
ConstraintType: cty.String,
ParsingMode: VariableParseLiteral,
Expand All @@ -46,6 +47,7 @@ func TestModuleOverrideVariable(t *testing.T) {
Description: "base description",
DescriptionSet: true,
Default: cty.StringVal("b_override partial"),
Nullable: true,
Type: cty.String,
ConstraintType: cty.String,
ParsingMode: VariableParseLiteral,
Expand Down
26 changes: 26 additions & 0 deletions internal/configs/named_values.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ type Variable struct {
DescriptionSet bool
SensitiveSet bool

// Nullable indicates that null is a valid value for this variable. Setting
// Nullable to false means that the module can expect this variable to
// never be null.
Nullable bool

DeclRange hcl.Range
}

Expand Down Expand Up @@ -110,6 +115,15 @@ func decodeVariableBlock(block *hcl.Block, override bool) (*Variable, hcl.Diagno
v.SensitiveSet = true
}

if attr, exists := content.Attributes["nullable"]; exists {
valDiags := gohcl.DecodeExpression(attr.Expr, nil, &v.Nullable)
diags = append(diags, valDiags...)
} else {
// The current default is true, which is subject to change in a future
// language edition.
v.Nullable = true
}

apparentlymart marked this conversation as resolved.
Show resolved Hide resolved
if attr, exists := content.Attributes["default"]; exists {
val, valDiags := attr.Expr.Value(nil)
diags = append(diags, valDiags...)
Expand All @@ -134,6 +148,15 @@ func decodeVariableBlock(block *hcl.Block, override bool) (*Variable, hcl.Diagno
}
}

if !v.Nullable && val.IsNull() {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid default value for variable",
Detail: "A null default value is not valid when nullable=false.",
Subject: attr.Expr.Range().Ptr(),
})
}

v.Default = val
}

Expand Down Expand Up @@ -556,6 +579,9 @@ var variableBlockSchema = &hcl.BodySchema{
{
Name: "sensitive",
},
{
Name: "nullable",
},
},
Blocks: []hcl.BlockHeaderSchema{
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
variable "in" {
type = number
nullable = false
default = null
}
12 changes: 12 additions & 0 deletions internal/configs/testdata/valid-files/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,15 @@ variable "sensitive_value" {
}
sensitive = true
}

variable "nullable" {
type = string
nullable = true
default = "ok"
}

variable "nullable_default_null" {
type = map(string)
nullable = true
default = null
}
33 changes: 33 additions & 0 deletions internal/terraform/context_apply2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -596,3 +596,36 @@ resource "test_object" "x" {
}

}

func TestContext2Apply_nullableVariables(t *testing.T) {
m := testModule(t, "apply-nullable-variables")
state := states.NewState()
ctx := testContext2(t, &ContextOpts{})
plan, diags := ctx.Plan(m, state, &PlanOpts{})
if diags.HasErrors() {
t.Fatalf("plan: %s", diags.Err())
}
state, diags = ctx.Apply(plan, m)
if diags.HasErrors() {
t.Fatalf("apply: %s", diags.Err())
}

outputs := state.Module(addrs.RootModuleInstance).OutputValues
// we check for null outputs be seeing that they don't exists
if _, ok := outputs["nullable_null_default"]; ok {
t.Error("nullable_null_default: expected no output value")
}
if _, ok := outputs["nullable_non_null_default"]; ok {
t.Error("nullable_non_null_default: expected no output value")
}
if _, ok := outputs["nullable_no_default"]; ok {
t.Error("nullable_no_default: expected no output value")
}

if v := outputs["non_nullable_default"].Value; v.AsString() != "ok" {
t.Fatalf("incorrect 'non_nullable_default' output value: %#v\n", v)
}
if v := outputs["non_nullable_no_default"].Value; v.AsString() != "ok" {
t.Fatalf("incorrect 'non_nullable_no_default' output value: %#v\n", v)
}
}
28 changes: 21 additions & 7 deletions internal/terraform/evaluate.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,11 +268,27 @@ func (d *evaluationStateData) GetInputVariable(addr addrs.InputVariable, rng tfd
}

val, isSet := vals[addr.Name]
if !isSet {
if config.Default != cty.NilVal {
return config.Default, diags
}
return cty.UnknownVal(config.Type), diags
switch {
case !isSet:
// The config loader will ensure there is a default if the value is not
// set at all.
jbardin marked this conversation as resolved.
Show resolved Hide resolved
val = config.Default

case val.IsNull() && !config.Nullable && config.Default != cty.NilVal:
// If nullable=false a null value will use the configured default.
val = config.Default

case val.IsNull() && !config.Nullable:
// The value cannot be null, and there is no configured default.
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: `Invalid variable value`,
Detail: fmt.Sprintf(`The resolved value of variable %q cannot be null.`, addr.Name),
Subject: &config.DeclRange,
})
jbardin marked this conversation as resolved.
Show resolved Hide resolved
// Stub out our return value so that the semantic checker doesn't
// produce redundant downstream errors.
val = cty.UnknownVal(config.Type)
}

var err error
Expand All @@ -286,8 +302,6 @@ func (d *evaluationStateData) GetInputVariable(addr addrs.InputVariable, rng tfd
Detail: fmt.Sprintf(`The resolved value of variable %q is not appropriate: %s.`, addr.Name, err),
Subject: &config.DeclRange,
})
// Stub out our return value so that the semantic checker doesn't
// produce redundant downstream errors.
val = cty.UnknownVal(config.Type)
}

Expand Down
28 changes: 28 additions & 0 deletions internal/terraform/testdata/apply-nullable-variables/main.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
module "mod" {
source = "./mod"
nullable_null_default = null
nullable_non_null_default = null
nullable_no_default = null
non_nullable_default = null
non_nullable_no_default = "ok"
}

output "nullable_null_default" {
value = module.mod.nullable_null_default
}

output "nullable_non_null_default" {
value = module.mod.nullable_non_null_default
}

output "nullable_no_default" {
value = module.mod.nullable_no_default
}

output "non_nullable_default" {
value = module.mod.non_nullable_default
}

output "non_nullable_no_default" {
value = module.mod.non_nullable_no_default
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// optional, and this can take null as an input
variable "nullable_null_default" {
// This is implied now as the default, and probably should be implied even
// when nullable=false is the default, so we're leaving this unset for the test.
// nullable = true

default = null
}

// assigning null can still override the default.
variable "nullable_non_null_default" {
nullable = true
default = "ok"
}

// required, and assigning null is valid.
variable "nullable_no_default" {
nullable = true
}


// this combination is invalid
//variable "non_nullable_null_default" {
// nullable = false
// default = null
//}


// assigning null will take the default
variable "non_nullable_default" {
nullable = false
default = "ok"
}

// required, but null is not a valid value
variable "non_nullable_no_default" {
nullable = false
}

output "nullable_null_default" {
value = var.nullable_null_default
}

output "nullable_non_null_default" {
value = var.nullable_non_null_default
}

output "nullable_no_default" {
value = var.nullable_no_default
}

output "non_nullable_default" {
value = var.non_nullable_default
}

output "non_nullable_no_default" {
value = var.non_nullable_no_default
}