IntenseWebs/opentofu
3
0
Fork 0
mirror of https://github.com/opentofu/opentofu.git synced 2025-01-18 04:32:59 -06:00
Code Issues Packages Projects Releases Wiki Activity
ddc81a204f
opentofu/internal/terraform/eval_variable_test.go

564 lines
16 KiB
Go
Raw Normal View History

core: Handle root and child module input variables consistently Previously we had a significant discrepancy between these two situations: we wrote the raw root module variables directly into the EvalContext and then applied type conversions only at expression evaluation time, while for child modules we converted and validated the values while visiting the variable graph node and wrote only the _final_ value into the EvalContext. This confusion seems to have been the root cause for #29899, where validation rules for root module variables were being applied at the wrong point in the process, prior to type conversion. To fix that bug and also make similar mistakes less likely in the future, I've made the root module variable handling more like the child module variable handling in the following ways: - The "raw value" (exactly as given by the user) lives only in the graph node representing the variable, which mirrors how the _expression_ for a child module variable lives in its graph node. This means that the flow for the two is the same except that there's no expression evaluation step for root module variables, because they arrive as constant values from the caller. - The set of variable values in the EvalContext is always only "final" values, after type conversion is complete. That in turn means we no longer need to do "just in time" conversion in evaluationStateData.GetInputVariable, and can just return the value exactly as stored, which is consistent with how we handle all other references between objects. This diff is noisier than I'd like because of how much it takes to wire a new argument (the raw variable values) through to the plan graph builder, but those changes are pretty mechanical and the interesting logic lives inside the plan graph builder itself, in NodeRootVariable, and the shared helper functions in eval_variable.go. While here I also took the opportunity to fix a historical API wart in EvalContext, where SetModuleCallArguments was built to take a set of variable values all at once but our current caller always calls with only one at a time. That is now just SetModuleCallArgument singular, to match with the new SetRootModuleArgument to deal with root module variables.
2021-11-10 19:29:45 -06:00
package terraform
import (
"fmt"
"testing"
core: More accurate error message for invalid variable values In earlier Terraform versions we had an extra validation step prior to the graph walk which tried to partially validate root module input variable values (just checking their type constraints) and then return error messages which specified as accurately as possible where the value had originally come from. We're now handling that sort of validation exclusively during the graph walk so that we can share the main logic between both root module and child module variable values, but previously that shared code wasn't able to generate such specific information about where the values had originated, because it was adapted from code originally written to only deal with child module variables. Here then we restore a similar level of detail as before, when we're processing root module variables. For child module variables, we use synthetic InputValue objects which state that the value was declared in the configuration, thus causing us to produce a similar sort of error message as we would've before which includes a source range covering the argument expression in the calling module block.
2021-12-21 20:04:24 -06:00
"github.com/hashicorp/hcl/v2"
core: Handle root and child module input variables consistently Previously we had a significant discrepancy between these two situations: we wrote the raw root module variables directly into the EvalContext and then applied type conversions only at expression evaluation time, while for child modules we converted and validated the values while visiting the variable graph node and wrote only the _final_ value into the EvalContext. This confusion seems to have been the root cause for #29899, where validation rules for root module variables were being applied at the wrong point in the process, prior to type conversion. To fix that bug and also make similar mistakes less likely in the future, I've made the root module variable handling more like the child module variable handling in the following ways: - The "raw value" (exactly as given by the user) lives only in the graph node representing the variable, which mirrors how the _expression_ for a child module variable lives in its graph node. This means that the flow for the two is the same except that there's no expression evaluation step for root module variables, because they arrive as constant values from the caller. - The set of variable values in the EvalContext is always only "final" values, after type conversion is complete. That in turn means we no longer need to do "just in time" conversion in evaluationStateData.GetInputVariable, and can just return the value exactly as stored, which is consistent with how we handle all other references between objects. This diff is noisier than I'd like because of how much it takes to wire a new argument (the raw variable values) through to the plan graph builder, but those changes are pretty mechanical and the interesting logic lives inside the plan graph builder itself, in NodeRootVariable, and the shared helper functions in eval_variable.go. While here I also took the opportunity to fix a historical API wart in EvalContext, where SetModuleCallArguments was built to take a set of variable values all at once but our current caller always calls with only one at a time. That is now just SetModuleCallArgument singular, to match with the new SetRootModuleArgument to deal with root module variables.
2021-11-10 19:29:45 -06:00
"github.com/zclconf/go-cty/cty"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/tfdiags"
)
func TestPrepareFinalInputVariableValue(t *testing.T) {
// This is just a concise way to define a bunch of *configs.Variable
// objects to use in our tests below. We're only going to decode this
// config, not fully evaluate it.
cfgSrc := `
variable "nullable_required" {
}
variable "nullable_optional_default_string" {
default = "hello"
}
variable "nullable_optional_default_null" {
default = null
}
variable "constrained_string_nullable_required" {
type = string
}
variable "constrained_string_nullable_optional_default_string" {
type = string
default = "hello"
}
variable "constrained_string_nullable_optional_default_bool" {
type = string
default = true
}
variable "constrained_string_nullable_optional_default_null" {
type = string
default = null
}
variable "required" {
nullable = false
}
variable "optional_default_string" {
nullable = false
default = "hello"
}
variable "constrained_string_required" {
nullable = false
type = string
}
variable "constrained_string_optional_default_string" {
nullable = false
type = string
default = "hello"
}
variable "constrained_string_optional_default_bool" {
nullable = false
type = string
default = true
}
`
cfg := testModuleInline(t, map[string]string{
"main.tf": cfgSrc,
})
variableConfigs := cfg.Module.Variables
core: More accurate error message for invalid variable values In earlier Terraform versions we had an extra validation step prior to the graph walk which tried to partially validate root module input variable values (just checking their type constraints) and then return error messages which specified as accurately as possible where the value had originally come from. We're now handling that sort of validation exclusively during the graph walk so that we can share the main logic between both root module and child module variable values, but previously that shared code wasn't able to generate such specific information about where the values had originated, because it was adapted from code originally written to only deal with child module variables. Here then we restore a similar level of detail as before, when we're processing root module variables. For child module variables, we use synthetic InputValue objects which state that the value was declared in the configuration, thus causing us to produce a similar sort of error message as we would've before which includes a source range covering the argument expression in the calling module block.
2021-12-21 20:04:24 -06:00
// Because we loaded our pseudo-module from a temporary file, the
// declaration source ranges will have unpredictable filenames. We'll
// fix that here just to make things easier below.
for _, vc := range variableConfigs {
vc.DeclRange.Filename = "main.tf"
}
core: Handle root and child module input variables consistently Previously we had a significant discrepancy between these two situations: we wrote the raw root module variables directly into the EvalContext and then applied type conversions only at expression evaluation time, while for child modules we converted and validated the values while visiting the variable graph node and wrote only the _final_ value into the EvalContext. This confusion seems to have been the root cause for #29899, where validation rules for root module variables were being applied at the wrong point in the process, prior to type conversion. To fix that bug and also make similar mistakes less likely in the future, I've made the root module variable handling more like the child module variable handling in the following ways: - The "raw value" (exactly as given by the user) lives only in the graph node representing the variable, which mirrors how the _expression_ for a child module variable lives in its graph node. This means that the flow for the two is the same except that there's no expression evaluation step for root module variables, because they arrive as constant values from the caller. - The set of variable values in the EvalContext is always only "final" values, after type conversion is complete. That in turn means we no longer need to do "just in time" conversion in evaluationStateData.GetInputVariable, and can just return the value exactly as stored, which is consistent with how we handle all other references between objects. This diff is noisier than I'd like because of how much it takes to wire a new argument (the raw variable values) through to the plan graph builder, but those changes are pretty mechanical and the interesting logic lives inside the plan graph builder itself, in NodeRootVariable, and the shared helper functions in eval_variable.go. While here I also took the opportunity to fix a historical API wart in EvalContext, where SetModuleCallArguments was built to take a set of variable values all at once but our current caller always calls with only one at a time. That is now just SetModuleCallArgument singular, to match with the new SetRootModuleArgument to deal with root module variables.
2021-11-10 19:29:45 -06:00
tests := []struct {
varName string
given cty.Value
want cty.Value
wantErr string
}{
// nullable_required
{
"nullable_required",
cty.NilVal,
cty.UnknownVal(cty.DynamicPseudoType),
`Required variable not set: The variable "nullable_required" is required, but is not set.`,
},
{
"nullable_required",
cty.NullVal(cty.DynamicPseudoType),
cty.NullVal(cty.DynamicPseudoType),
``, // "required" for a nullable variable means only that it must be set, even if it's set to null
},
{
"nullable_required",
cty.StringVal("ahoy"),
cty.StringVal("ahoy"),
``,
},
{
"nullable_required",
cty.UnknownVal(cty.String),
cty.UnknownVal(cty.String),
``,
},
// nullable_optional_default_string
{
"nullable_optional_default_string",
cty.NilVal,
cty.StringVal("hello"), // the declared default value
``,
},
{
"nullable_optional_default_string",
cty.NullVal(cty.DynamicPseudoType),
cty.NullVal(cty.DynamicPseudoType), // nullable variables can be really set to null, masking the default
``,
},
{
"nullable_optional_default_string",
cty.StringVal("ahoy"),
cty.StringVal("ahoy"),
``,
},
{
"nullable_optional_default_string",
cty.UnknownVal(cty.String),
cty.UnknownVal(cty.String),
``,
},
// nullable_optional_default_null
{
"nullable_optional_default_null",
cty.NilVal,
cty.NullVal(cty.DynamicPseudoType), // the declared default value
``,
},
{
"nullable_optional_default_null",
cty.NullVal(cty.String),
cty.NullVal(cty.String), // nullable variables can be really set to null, masking the default
``,
},
{
"nullable_optional_default_null",
cty.StringVal("ahoy"),
cty.StringVal("ahoy"),
``,
},
{
"nullable_optional_default_null",
cty.UnknownVal(cty.String),
cty.UnknownVal(cty.String),
``,
},
// constrained_string_nullable_required
{
"constrained_string_nullable_required",
cty.NilVal,
cty.UnknownVal(cty.String),
`Required variable not set: The variable "constrained_string_nullable_required" is required, but is not set.`,
},
{
"constrained_string_nullable_required",
cty.NullVal(cty.DynamicPseudoType),
cty.NullVal(cty.String), // the null value still gets converted to match the type constraint
``, // "required" for a nullable variable means only that it must be set, even if it's set to null
},
{
"constrained_string_nullable_required",
cty.StringVal("ahoy"),
cty.StringVal("ahoy"),
``,
},
{
"constrained_string_nullable_required",
cty.UnknownVal(cty.String),
cty.UnknownVal(cty.String),
``,
},
// constrained_string_nullable_optional_default_string
{
"constrained_string_nullable_optional_default_string",
cty.NilVal,
cty.StringVal("hello"), // the declared default value
``,
},
{
"constrained_string_nullable_optional_default_string",
cty.NullVal(cty.DynamicPseudoType),
cty.NullVal(cty.String), // nullable variables can be really set to null, masking the default
``,
},
{
"constrained_string_nullable_optional_default_string",
cty.StringVal("ahoy"),
cty.StringVal("ahoy"),
``,
},
{
"constrained_string_nullable_optional_default_string",
cty.UnknownVal(cty.String),
cty.UnknownVal(cty.String),
``,
},
// constrained_string_nullable_optional_default_bool
{
"constrained_string_nullable_optional_default_bool",
cty.NilVal,
cty.StringVal("true"), // the declared default value, automatically converted to match type constraint
``,
},
{
"constrained_string_nullable_optional_default_bool",
cty.NullVal(cty.DynamicPseudoType),
cty.NullVal(cty.String), // nullable variables can be really set to null, masking the default
``,
},
{
"constrained_string_nullable_optional_default_bool",
cty.StringVal("ahoy"),
cty.StringVal("ahoy"),
``,
},
{
"constrained_string_nullable_optional_default_bool",
cty.UnknownVal(cty.String),
cty.UnknownVal(cty.String),
``,
},
// constrained_string_nullable_optional_default_null
{
"constrained_string_nullable_optional_default_null",
cty.NilVal,
cty.NullVal(cty.String),
``,
},
{
"constrained_string_nullable_optional_default_null",
cty.NullVal(cty.DynamicPseudoType),
cty.NullVal(cty.String),
``,
},
{
"constrained_string_nullable_optional_default_null",
cty.StringVal("ahoy"),
cty.StringVal("ahoy"),
``,
},
{
"constrained_string_nullable_optional_default_null",
cty.UnknownVal(cty.String),
cty.UnknownVal(cty.String),
``,
},
// required
{
"required",
cty.NilVal,
cty.UnknownVal(cty.DynamicPseudoType),
`Required variable not set: The variable "required" is required, but is not set.`,
},
{
"required",
cty.NullVal(cty.DynamicPseudoType),
cty.UnknownVal(cty.DynamicPseudoType),
core: More accurate error message for invalid variable values In earlier Terraform versions we had an extra validation step prior to the graph walk which tried to partially validate root module input variable values (just checking their type constraints) and then return error messages which specified as accurately as possible where the value had originally come from. We're now handling that sort of validation exclusively during the graph walk so that we can share the main logic between both root module and child module variable values, but previously that shared code wasn't able to generate such specific information about where the values had originated, because it was adapted from code originally written to only deal with child module variables. Here then we restore a similar level of detail as before, when we're processing root module variables. For child module variables, we use synthetic InputValue objects which state that the value was declared in the configuration, thus causing us to produce a similar sort of error message as we would've before which includes a source range covering the argument expression in the calling module block.
2021-12-21 20:04:24 -06:00
`Required variable not set: Unsuitable value for var.required set from outside of the configuration: required variable may not be set to null.`,
core: Handle root and child module input variables consistently Previously we had a significant discrepancy between these two situations: we wrote the raw root module variables directly into the EvalContext and then applied type conversions only at expression evaluation time, while for child modules we converted and validated the values while visiting the variable graph node and wrote only the _final_ value into the EvalContext. This confusion seems to have been the root cause for #29899, where validation rules for root module variables were being applied at the wrong point in the process, prior to type conversion. To fix that bug and also make similar mistakes less likely in the future, I've made the root module variable handling more like the child module variable handling in the following ways: - The "raw value" (exactly as given by the user) lives only in the graph node representing the variable, which mirrors how the _expression_ for a child module variable lives in its graph node. This means that the flow for the two is the same except that there's no expression evaluation step for root module variables, because they arrive as constant values from the caller. - The set of variable values in the EvalContext is always only "final" values, after type conversion is complete. That in turn means we no longer need to do "just in time" conversion in evaluationStateData.GetInputVariable, and can just return the value exactly as stored, which is consistent with how we handle all other references between objects. This diff is noisier than I'd like because of how much it takes to wire a new argument (the raw variable values) through to the plan graph builder, but those changes are pretty mechanical and the interesting logic lives inside the plan graph builder itself, in NodeRootVariable, and the shared helper functions in eval_variable.go. While here I also took the opportunity to fix a historical API wart in EvalContext, where SetModuleCallArguments was built to take a set of variable values all at once but our current caller always calls with only one at a time. That is now just SetModuleCallArgument singular, to match with the new SetRootModuleArgument to deal with root module variables.
2021-11-10 19:29:45 -06:00
},
{
"required",
cty.StringVal("ahoy"),
cty.StringVal("ahoy"),
``,
},
{
"required",
cty.UnknownVal(cty.String),
cty.UnknownVal(cty.String),
``,
},
// optional_default_string
{
"optional_default_string",
cty.NilVal,
cty.StringVal("hello"), // the declared default value
``,
},
{
"optional_default_string",
cty.NullVal(cty.DynamicPseudoType),
cty.StringVal("hello"), // the declared default value
``,
},
{
"optional_default_string",
cty.StringVal("ahoy"),
cty.StringVal("ahoy"),
``,
},
{
"optional_default_string",
cty.UnknownVal(cty.String),
cty.UnknownVal(cty.String),
``,
},
// constrained_string_required
{
"constrained_string_required",
cty.NilVal,
cty.UnknownVal(cty.String),
`Required variable not set: The variable "constrained_string_required" is required, but is not set.`,
},
{
"constrained_string_required",
cty.NullVal(cty.DynamicPseudoType),
cty.UnknownVal(cty.String),
core: More accurate error message for invalid variable values In earlier Terraform versions we had an extra validation step prior to the graph walk which tried to partially validate root module input variable values (just checking their type constraints) and then return error messages which specified as accurately as possible where the value had originally come from. We're now handling that sort of validation exclusively during the graph walk so that we can share the main logic between both root module and child module variable values, but previously that shared code wasn't able to generate such specific information about where the values had originated, because it was adapted from code originally written to only deal with child module variables. Here then we restore a similar level of detail as before, when we're processing root module variables. For child module variables, we use synthetic InputValue objects which state that the value was declared in the configuration, thus causing us to produce a similar sort of error message as we would've before which includes a source range covering the argument expression in the calling module block.
2021-12-21 20:04:24 -06:00
`Required variable not set: Unsuitable value for var.constrained_string_required set from outside of the configuration: required variable may not be set to null.`,
core: Handle root and child module input variables consistently Previously we had a significant discrepancy between these two situations: we wrote the raw root module variables directly into the EvalContext and then applied type conversions only at expression evaluation time, while for child modules we converted and validated the values while visiting the variable graph node and wrote only the _final_ value into the EvalContext. This confusion seems to have been the root cause for #29899, where validation rules for root module variables were being applied at the wrong point in the process, prior to type conversion. To fix that bug and also make similar mistakes less likely in the future, I've made the root module variable handling more like the child module variable handling in the following ways: - The "raw value" (exactly as given by the user) lives only in the graph node representing the variable, which mirrors how the _expression_ for a child module variable lives in its graph node. This means that the flow for the two is the same except that there's no expression evaluation step for root module variables, because they arrive as constant values from the caller. - The set of variable values in the EvalContext is always only "final" values, after type conversion is complete. That in turn means we no longer need to do "just in time" conversion in evaluationStateData.GetInputVariable, and can just return the value exactly as stored, which is consistent with how we handle all other references between objects. This diff is noisier than I'd like because of how much it takes to wire a new argument (the raw variable values) through to the plan graph builder, but those changes are pretty mechanical and the interesting logic lives inside the plan graph builder itself, in NodeRootVariable, and the shared helper functions in eval_variable.go. While here I also took the opportunity to fix a historical API wart in EvalContext, where SetModuleCallArguments was built to take a set of variable values all at once but our current caller always calls with only one at a time. That is now just SetModuleCallArgument singular, to match with the new SetRootModuleArgument to deal with root module variables.
2021-11-10 19:29:45 -06:00
},
{
"constrained_string_required",
cty.StringVal("ahoy"),
cty.StringVal("ahoy"),
``,
},
{
"constrained_string_required",
cty.UnknownVal(cty.String),
cty.UnknownVal(cty.String),
``,
},
// constrained_string_optional_default_string
{
"constrained_string_optional_default_string",
cty.NilVal,
cty.StringVal("hello"), // the declared default value
``,
},
{
"constrained_string_optional_default_string",
cty.NullVal(cty.DynamicPseudoType),
cty.StringVal("hello"), // the declared default value
``,
},
{
"constrained_string_optional_default_string",
cty.StringVal("ahoy"),
cty.StringVal("ahoy"),
``,
},
{
"constrained_string_optional_default_string",
cty.UnknownVal(cty.String),
cty.UnknownVal(cty.String),
``,
},
// constrained_string_optional_default_bool
{
"constrained_string_optional_default_bool",
cty.NilVal,
cty.StringVal("true"), // the declared default value, automatically converted to match type constraint
``,
},
{
"constrained_string_optional_default_bool",
cty.NullVal(cty.DynamicPseudoType),
cty.StringVal("true"), // the declared default value, automatically converted to match type constraint
``,
},
{
"constrained_string_optional_default_bool",
cty.StringVal("ahoy"),
cty.StringVal("ahoy"),
``,
},
{
"constrained_string_optional_default_bool",
cty.UnknownVal(cty.String),
cty.UnknownVal(cty.String),
``,
},
}
for _, test := range tests {
t.Run(fmt.Sprintf("%s %#v", test.varName, test.given), func(t *testing.T) {
varAddr := addrs.InputVariable{Name: test.varName}.Absolute(addrs.RootModuleInstance)
varCfg := variableConfigs[test.varName]
if varCfg == nil {
t.Fatalf("invalid variable name %q", test.varName)
}
t.Logf(
"test case\nvariable: %s\nconstraint: %#v\ndefault: %#v\nnullable: %#v\ngiven value: %#v",
varAddr,
varCfg.Type,
varCfg.Default,
varCfg.Nullable,
test.given,
)
core: More accurate error message for invalid variable values In earlier Terraform versions we had an extra validation step prior to the graph walk which tried to partially validate root module input variable values (just checking their type constraints) and then return error messages which specified as accurately as possible where the value had originally come from. We're now handling that sort of validation exclusively during the graph walk so that we can share the main logic between both root module and child module variable values, but previously that shared code wasn't able to generate such specific information about where the values had originated, because it was adapted from code originally written to only deal with child module variables. Here then we restore a similar level of detail as before, when we're processing root module variables. For child module variables, we use synthetic InputValue objects which state that the value was declared in the configuration, thus causing us to produce a similar sort of error message as we would've before which includes a source range covering the argument expression in the calling module block.
2021-12-21 20:04:24 -06:00
rawVal := &InputValue{
Value: test.given,
SourceType: ValueFromCaller,
}
core: Handle root and child module input variables consistently Previously we had a significant discrepancy between these two situations: we wrote the raw root module variables directly into the EvalContext and then applied type conversions only at expression evaluation time, while for child modules we converted and validated the values while visiting the variable graph node and wrote only the _final_ value into the EvalContext. This confusion seems to have been the root cause for #29899, where validation rules for root module variables were being applied at the wrong point in the process, prior to type conversion. To fix that bug and also make similar mistakes less likely in the future, I've made the root module variable handling more like the child module variable handling in the following ways: - The "raw value" (exactly as given by the user) lives only in the graph node representing the variable, which mirrors how the _expression_ for a child module variable lives in its graph node. This means that the flow for the two is the same except that there's no expression evaluation step for root module variables, because they arrive as constant values from the caller. - The set of variable values in the EvalContext is always only "final" values, after type conversion is complete. That in turn means we no longer need to do "just in time" conversion in evaluationStateData.GetInputVariable, and can just return the value exactly as stored, which is consistent with how we handle all other references between objects. This diff is noisier than I'd like because of how much it takes to wire a new argument (the raw variable values) through to the plan graph builder, but those changes are pretty mechanical and the interesting logic lives inside the plan graph builder itself, in NodeRootVariable, and the shared helper functions in eval_variable.go. While here I also took the opportunity to fix a historical API wart in EvalContext, where SetModuleCallArguments was built to take a set of variable values all at once but our current caller always calls with only one at a time. That is now just SetModuleCallArgument singular, to match with the new SetRootModuleArgument to deal with root module variables.
2021-11-10 19:29:45 -06:00
got, diags := prepareFinalInputVariableValue(
core: More accurate error message for invalid variable values In earlier Terraform versions we had an extra validation step prior to the graph walk which tried to partially validate root module input variable values (just checking their type constraints) and then return error messages which specified as accurately as possible where the value had originally come from. We're now handling that sort of validation exclusively during the graph walk so that we can share the main logic between both root module and child module variable values, but previously that shared code wasn't able to generate such specific information about where the values had originated, because it was adapted from code originally written to only deal with child module variables. Here then we restore a similar level of detail as before, when we're processing root module variables. For child module variables, we use synthetic InputValue objects which state that the value was declared in the configuration, thus causing us to produce a similar sort of error message as we would've before which includes a source range covering the argument expression in the calling module block.
2021-12-21 20:04:24 -06:00
varAddr, rawVal, varCfg,
core: Handle root and child module input variables consistently Previously we had a significant discrepancy between these two situations: we wrote the raw root module variables directly into the EvalContext and then applied type conversions only at expression evaluation time, while for child modules we converted and validated the values while visiting the variable graph node and wrote only the _final_ value into the EvalContext. This confusion seems to have been the root cause for #29899, where validation rules for root module variables were being applied at the wrong point in the process, prior to type conversion. To fix that bug and also make similar mistakes less likely in the future, I've made the root module variable handling more like the child module variable handling in the following ways: - The "raw value" (exactly as given by the user) lives only in the graph node representing the variable, which mirrors how the _expression_ for a child module variable lives in its graph node. This means that the flow for the two is the same except that there's no expression evaluation step for root module variables, because they arrive as constant values from the caller. - The set of variable values in the EvalContext is always only "final" values, after type conversion is complete. That in turn means we no longer need to do "just in time" conversion in evaluationStateData.GetInputVariable, and can just return the value exactly as stored, which is consistent with how we handle all other references between objects. This diff is noisier than I'd like because of how much it takes to wire a new argument (the raw variable values) through to the plan graph builder, but those changes are pretty mechanical and the interesting logic lives inside the plan graph builder itself, in NodeRootVariable, and the shared helper functions in eval_variable.go. While here I also took the opportunity to fix a historical API wart in EvalContext, where SetModuleCallArguments was built to take a set of variable values all at once but our current caller always calls with only one at a time. That is now just SetModuleCallArgument singular, to match with the new SetRootModuleArgument to deal with root module variables.
2021-11-10 19:29:45 -06:00
)
if test.wantErr != "" {
if !diags.HasErrors() {
t.Errorf("unexpected success\nwant error: %s", test.wantErr)
} else if got, want := diags.Err().Error(), test.wantErr; got != want {
t.Errorf("wrong error\ngot: %s\nwant: %s", got, want)
}
} else {
if diags.HasErrors() {
t.Errorf("unexpected error\ngot: %s", diags.Err().Error())
}
}
// NOTE: should still have returned some reasonable value even if there was an error
if !test.want.RawEquals(got) {
t.Fatalf("wrong result\ngot: %#v\nwant: %#v", got, test.want)
}
})
}
core: More accurate error message for invalid variable values In earlier Terraform versions we had an extra validation step prior to the graph walk which tried to partially validate root module input variable values (just checking their type constraints) and then return error messages which specified as accurately as possible where the value had originally come from. We're now handling that sort of validation exclusively during the graph walk so that we can share the main logic between both root module and child module variable values, but previously that shared code wasn't able to generate such specific information about where the values had originated, because it was adapted from code originally written to only deal with child module variables. Here then we restore a similar level of detail as before, when we're processing root module variables. For child module variables, we use synthetic InputValue objects which state that the value was declared in the configuration, thus causing us to produce a similar sort of error message as we would've before which includes a source range covering the argument expression in the calling module block.
2021-12-21 20:04:24 -06:00
t.Run("SourceType error message variants", func(t *testing.T) {
tests := []struct {
SourceType ValueSourceType
SourceRange tfdiags.SourceRange
WantTypeErr string
WantNullErr string
}{
{
ValueFromUnknown,
tfdiags.SourceRange{},
`Invalid value for input variable: Unsuitable value for var.constrained_string_required set from outside of the configuration: string required.`,
`Required variable not set: Unsuitable value for var.constrained_string_required set from outside of the configuration: required variable may not be set to null.`,
},
{
ValueFromConfig,
tfdiags.SourceRange{
Filename: "example.tf",
Start: tfdiags.SourcePos(hcl.InitialPos),
End: tfdiags.SourcePos(hcl.InitialPos),
},
`Invalid value for input variable: The given value is not suitable for var.constrained_string_required declared at main.tf:32,3-41: string required.`,
`Required variable not set: The given value is not suitable for var.constrained_string_required defined at main.tf:32,3-41: required variable may not be set to null.`,
},
{
ValueFromAutoFile,
tfdiags.SourceRange{
Filename: "example.auto.tfvars",
Start: tfdiags.SourcePos(hcl.InitialPos),
End: tfdiags.SourcePos(hcl.InitialPos),
},
`Invalid value for input variable: The given value is not suitable for var.constrained_string_required declared at main.tf:32,3-41: string required.`,
`Required variable not set: The given value is not suitable for var.constrained_string_required defined at main.tf:32,3-41: required variable may not be set to null.`,
},
{
ValueFromNamedFile,
tfdiags.SourceRange{
Filename: "example.tfvars",
Start: tfdiags.SourcePos(hcl.InitialPos),
End: tfdiags.SourcePos(hcl.InitialPos),
},
`Invalid value for input variable: The given value is not suitable for var.constrained_string_required declared at main.tf:32,3-41: string required.`,
`Required variable not set: The given value is not suitable for var.constrained_string_required defined at main.tf:32,3-41: required variable may not be set to null.`,
},
{
ValueFromCLIArg,
tfdiags.SourceRange{},
`Invalid value for input variable: Unsuitable value for var.constrained_string_required set using -var="constrained_string_required=...": string required.`,
`Required variable not set: Unsuitable value for var.constrained_string_required set using -var="constrained_string_required=...": required variable may not be set to null.`,
},
{
ValueFromEnvVar,
tfdiags.SourceRange{},
`Invalid value for input variable: Unsuitable value for var.constrained_string_required set using the TF_VAR_constrained_string_required environment variable: string required.`,
`Required variable not set: Unsuitable value for var.constrained_string_required set using the TF_VAR_constrained_string_required environment variable: required variable may not be set to null.`,
},
{
ValueFromInput,
tfdiags.SourceRange{},
`Invalid value for input variable: Unsuitable value for var.constrained_string_required set using an interactive prompt: string required.`,
`Required variable not set: Unsuitable value for var.constrained_string_required set using an interactive prompt: required variable may not be set to null.`,
},
{
// NOTE: This isn't actually a realistic case for this particular
// function, because if we have a value coming from a plan then
// we must be in the apply step, and we shouldn't be able to
// get past the plan step if we have invalid variable values,
// and during planning we'll always have other source types.
ValueFromPlan,
tfdiags.SourceRange{},
`Invalid value for input variable: Unsuitable value for var.constrained_string_required set from outside of the configuration: string required.`,
`Required variable not set: Unsuitable value for var.constrained_string_required set from outside of the configuration: required variable may not be set to null.`,
},
{
ValueFromCaller,
tfdiags.SourceRange{},
`Invalid value for input variable: Unsuitable value for var.constrained_string_required set from outside of the configuration: string required.`,
`Required variable not set: Unsuitable value for var.constrained_string_required set from outside of the configuration: required variable may not be set to null.`,
},
}
for _, test := range tests {
t.Run(fmt.Sprintf("%s %s", test.SourceType, test.SourceRange.StartString()), func(t *testing.T) {
varAddr := addrs.InputVariable{Name: "constrained_string_required"}.Absolute(addrs.RootModuleInstance)
varCfg := variableConfigs[varAddr.Variable.Name]
t.Run("type error", func(t *testing.T) {
rawVal := &InputValue{
Value: cty.EmptyObjectVal,
SourceType: test.SourceType,
SourceRange: test.SourceRange,
}
_, diags := prepareFinalInputVariableValue(
varAddr, rawVal, varCfg,
)
if !diags.HasErrors() {
t.Fatalf("unexpected success; want error")
}
if got, want := diags.Err().Error(), test.WantTypeErr; got != want {
t.Errorf("wrong error\ngot: %s\nwant: %s", got, want)
}
})
t.Run("null error", func(t *testing.T) {
rawVal := &InputValue{
Value: cty.NullVal(cty.DynamicPseudoType),
SourceType: test.SourceType,
SourceRange: test.SourceRange,
}
_, diags := prepareFinalInputVariableValue(
varAddr, rawVal, varCfg,
)
if !diags.HasErrors() {
t.Fatalf("unexpected success; want error")
}
if got, want := diags.Err().Error(), test.WantNullErr; got != want {
t.Errorf("wrong error\ngot: %s\nwant: %s", got, want)
}
})
})
}
})
core: Handle root and child module input variables consistently Previously we had a significant discrepancy between these two situations: we wrote the raw root module variables directly into the EvalContext and then applied type conversions only at expression evaluation time, while for child modules we converted and validated the values while visiting the variable graph node and wrote only the _final_ value into the EvalContext. This confusion seems to have been the root cause for #29899, where validation rules for root module variables were being applied at the wrong point in the process, prior to type conversion. To fix that bug and also make similar mistakes less likely in the future, I've made the root module variable handling more like the child module variable handling in the following ways: - The "raw value" (exactly as given by the user) lives only in the graph node representing the variable, which mirrors how the _expression_ for a child module variable lives in its graph node. This means that the flow for the two is the same except that there's no expression evaluation step for root module variables, because they arrive as constant values from the caller. - The set of variable values in the EvalContext is always only "final" values, after type conversion is complete. That in turn means we no longer need to do "just in time" conversion in evaluationStateData.GetInputVariable, and can just return the value exactly as stored, which is consistent with how we handle all other references between objects. This diff is noisier than I'd like because of how much it takes to wire a new argument (the raw variable values) through to the plan graph builder, but those changes are pretty mechanical and the interesting logic lives inside the plan graph builder itself, in NodeRootVariable, and the shared helper functions in eval_variable.go. While here I also took the opportunity to fix a historical API wart in EvalContext, where SetModuleCallArguments was built to take a set of variable values all at once but our current caller always calls with only one at a time. That is now just SetModuleCallArgument singular, to match with the new SetRootModuleArgument to deal with root module variables.
2021-11-10 19:29:45 -06:00
}
Reference in New Issue Copy Permalink