Add additional validation around unknown and null values in test variables (#33861)

* Add additional validation around unknown values in test variables

* address comments
pull/33902/head
Liam Cervante 3 years ago committed by GitHub
parent ffbcaf8bef
commit f8d4664bcd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -1269,22 +1269,75 @@ func (runner *TestFileRunner) prepareInputVariablesForAssertions(config *configs
func (runner *TestFileRunner) ctx(run *moduletest.Run, file *moduletest.File, availableVariables terraform.InputValues) (*hcl.EvalContext, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics
availableRunBlocks := make(map[string]bool)
// First, let's build the set of available run blocks.
availableRunBlocks := make(map[string]*terraform.TestContext)
runs := make(map[string]cty.Value)
for _, run := range file.Runs {
name := run.Name
if _, exists := runner.PriorStates[name]; exists {
attrs := make(map[string]cty.Value)
if ctx, exists := runner.PriorStates[name]; exists {
// We have executed this run block previously, therefore it is
// available as a reference at this point in time.
availableRunBlocks[name] = true
availableRunBlocks[name] = ctx
for name, config := range ctx.Config.Module.Outputs {
output := ctx.State.OutputValue(addrs.AbsOutputValue{
OutputValue: addrs.OutputValue{
Name: name,
},
Module: addrs.RootModuleInstance,
})
var value cty.Value
switch {
case output == nil:
// This means the run block returned null for this output.
// It is likely this will produce an error later if it is
// referenced, but users can actually specify that null
// is an acceptable value for an input variable so we won't
// actually raise a fuss about this at all.
value = cty.NullVal(cty.DynamicPseudoType)
case output.Value.IsNull() || output.Value == cty.NilVal:
// This means the output value was returned as (known after
// apply). If this is referenced it always an error, we
// can't handle this in an appropriate way at all. For now,
// we just mark it as unknown and then later we check and
// resolve all the references. We'll raise an error at that
// point if the user actually attempts to reference a value
// that is unknown.
value = cty.DynamicVal
default:
value = output.Value
}
if config.Sensitive || (output != nil && output.Sensitive) {
value = value.Mark(marks.Sensitive)
}
attrs[name] = value
}
runs[name] = cty.ObjectVal(attrs)
continue
}
// We haven't executed this run block yet, therefore it is not available
// as a reference at this point in time.
availableRunBlocks[name] = false
availableRunBlocks[name] = nil
}
// Second, let's build the set of available variables.
vars := make(map[string]cty.Value)
for name, variable := range availableVariables {
vars[name] = variable.Value
}
// Third, let's do some basic validation over the references.
for _, value := range run.Config.Variables {
refs, refDiags := lang.ReferencesInExpr(addrs.ParseRefFromTestingScope, value)
diags = diags.Append(refDiags)
@ -1294,7 +1347,7 @@ func (runner *TestFileRunner) ctx(run *moduletest.Run, file *moduletest.File, av
for _, ref := range refs {
if addr, ok := ref.Subject.(addrs.Run); ok {
available, exists := availableRunBlocks[addr.Name]
ctx, exists := availableRunBlocks[addr.Name]
if !exists {
// Then this is a made up run block.
@ -1308,7 +1361,7 @@ func (runner *TestFileRunner) ctx(run *moduletest.Run, file *moduletest.File, av
continue
}
if !available {
if ctx == nil {
// This run block exists, but it is after the current run block.
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
@ -1320,12 +1373,50 @@ func (runner *TestFileRunner) ctx(run *moduletest.Run, file *moduletest.File, av
continue
}
// Otherwise, we're good. This is an acceptable reference.
value, valueDiags := ref.Remaining.TraverseRel(runs[addr.Name])
diags = diags.Append(valueDiags)
if valueDiags.HasErrors() {
// This means the reference was invalid somehow, we've
// already added the errors to our diagnostics though so
// we'll just carry on.
continue
}
if !value.IsWhollyKnown() {
// This is not valid, we cannot allow users to pass unknown
// values into run blocks. There's just going to be
// difficult and confusing errors later if this happens.
if ctx.Run.Config.Command == configs.PlanTestCommand {
// Then the user has likely attempted to use an output
// that is (known after apply) due to the referenced
// run block only being a plan command.
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Reference to unknown value",
Detail: fmt.Sprintf("The value for %s is unknown. Run block %q is executing a \"plan\" operation, and the specified output value is only known after apply.", ref.DisplayString(), addr.Name),
Subject: ref.SourceRange.ToHCL().Ptr(),
})
continue
}
// Otherwise, this is a bug in Terraform. We shouldn't be
// producing (known after apply) values during apply
// operations.
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Reference to unknown value",
Detail: fmt.Sprintf("The value for %s is unknown; This is a bug in Terraform, please report it.", ref.DisplayString()),
Subject: ref.SourceRange.ToHCL().Ptr(),
})
}
continue
}
if addr, ok := ref.Subject.(addrs.InputVariable); ok {
if _, exists := availableVariables[addr.Name]; !exists {
if _, exists := vars[addr.Name]; !exists {
// This variable reference doesn't exist.
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
@ -1352,66 +1443,12 @@ func (runner *TestFileRunner) ctx(run *moduletest.Run, file *moduletest.File, av
}
}
return &hcl.EvalContext{
Variables: func() map[string]cty.Value {
blocks := make(map[string]cty.Value)
for run, ctx := range runner.PriorStates {
outputs := make(map[string]cty.Value)
for _, output := range ctx.Config.Module.Outputs {
value := ctx.State.OutputValue(addrs.AbsOutputValue{
Module: addrs.RootModuleInstance,
OutputValue: addrs.OutputValue{
Name: output.Name,
},
})
if value == nil {
// Then this output returned null when the configuration
// executed. For now, we'll just skip this output.
//
// There are several things we could try to do, like
// figure out the type based on the variable that it
// is referencing and wrap it up as cty.Val(...) or we
// could not try and work anything out and return it as
// a cty.NilVal.
//
// Both of these mean the error would be raised later
// as non-optional variables would say they don't have
// a value. By just ignoring it here, we get an error
// quicker that says this output doesn't exist. I think
// that would prompt users to go look at the output and
// realise it might be returning null and make the
// connection. With the other approaches they'd look at
// their variable definitions and think they are
// assigning it a value since we would be telling them
// the output does exist.
//
// Let's do the simple thing now, and see what the
// future holds.
continue
}
// Finally, we can just populate our hcl.EvalContext.
if value.Sensitive || output.Sensitive {
outputs[output.Name] = value.Value.Mark(marks.Sensitive)
continue
}
outputs[output.Name] = value.Value
}
blocks[run] = cty.ObjectVal(outputs)
}
variables := make(map[string]cty.Value)
for name, variable := range availableVariables {
variables[name] = variable.Value
}
return map[string]cty.Value{
"run": cty.ObjectVal(blocks),
"var": cty.ObjectVal(variables),
}
}(),
return &hcl.EvalContext{
Variables: map[string]cty.Value{
"run": cty.ObjectVal(runs),
"var": cty.ObjectVal(vars),
},
}, diags
}

@ -4,6 +4,7 @@
package command
import (
"fmt"
"path"
"strings"
"testing"
@ -1265,3 +1266,163 @@ input must contain the character 'b'
t.Errorf("should have deleted all resources on completion but left %v", provider.ResourceString())
}
}
func TestTest_UnknownAndNulls(t *testing.T) {
tcs := map[string]struct {
code int
stdout string
stderr string
}{
"null_value_in_assert": {
code: 1,
stdout: `main.tftest.hcl... in progress
run "first"... fail
main.tftest.hcl... tearing down
main.tftest.hcl... fail
Failure! 0 passed, 1 failed.
`,
stderr: `
Error: Test assertion failed
on main.tftest.hcl line 8, in run "first":
8: condition = test_resource.resource.value == output.null_output
output.null_output is null
test_resource.resource.value is "bar"
this is always going to fail
`,
},
"null_value_in_vars": {
code: 1,
stdout: `fail.tftest.hcl... in progress
run "first"... pass
run "second"... fail
fail.tftest.hcl... tearing down
fail.tftest.hcl... fail
pass.tftest.hcl... in progress
run "first"... pass
run "second"... pass
pass.tftest.hcl... tearing down
pass.tftest.hcl... pass
Failure! 3 passed, 1 failed.
`,
stderr: `
Error: Required variable not set
on fail.tftest.hcl line 11, in run "second":
11: interesting_input = run.first.null_output
The given value is not suitable for var.interesting_input defined at
main.tf:7,1-29: required variable may not be set to null.
`,
},
"unknown_value_in_assert": {
code: 1,
stdout: `main.tftest.hcl... in progress
run "one"... pass
run "two"... fail
main.tftest.hcl... tearing down
main.tftest.hcl... fail
Failure! 1 passed, 1 failed.
`,
stderr: fmt.Sprintf(`
Error: Unknown condition value
on main.tftest.hcl line 8, in run "two":
8: condition = output.destroy_fail == run.one.destroy_fail
output.destroy_fail is false
Condition expression could not be evaluated at this time. This means you have
executed a %s block with %s and one of the values your
condition depended on is not known until after the plan has been applied.
Either remove this value from your condition, or execute an %s command
from this %s block.
`, "`run`", "`command = plan`", "`apply`", "`run`"),
},
"unknown_value_in_vars": {
code: 1,
stdout: `main.tftest.hcl... in progress
run "one"... pass
run "two"... fail
main.tftest.hcl... tearing down
main.tftest.hcl... fail
Failure! 1 passed, 1 failed.
`,
stderr: `
Error: Reference to unknown value
on main.tftest.hcl line 8, in run "two":
8: destroy_fail = run.one.destroy_fail
The value for run.one.destroy_fail is unknown. Run block "one" is executing a
"plan" operation, and the specified output value is only known after apply.
`,
},
"nested_unknown_values": {
code: 1,
stdout: `main.tftest.hcl... in progress
run "first"... pass
run "second"... pass
run "third"... fail
main.tftest.hcl... tearing down
main.tftest.hcl... fail
Failure! 2 passed, 1 failed.
`,
stderr: `
Error: Reference to unknown value
on main.tftest.hcl line 31, in run "third":
31: input = run.second
The value for run.second is unknown. Run block "second" is executing a "plan"
operation, and the specified output value is only known after apply.
`,
},
}
for name, tc := range tcs {
t.Run(name, func(t *testing.T) {
td := t.TempDir()
testCopyDir(t, testFixturePath(path.Join("test", name)), td)
defer testChdir(t, td)()
provider := testing_command.NewProvider(nil)
view, done := testView(t)
c := &TestCommand{
Meta: Meta{
testingOverrides: metaOverridesForProvider(provider.Provider),
View: view,
},
}
code := c.Run([]string{"-no-color"})
output := done(t)
if code != tc.code {
t.Errorf("expected return code %d but got %d", tc.code, code)
}
expectedOut := tc.stdout
actualOut := output.Stdout()
if diff := cmp.Diff(expectedOut, actualOut); len(diff) > 0 {
t.Errorf("unexpected output\n\nexpected:\n%s\nactual:\n%s\ndiff:\n%s", expectedOut, actualOut, diff)
}
expectedErr := tc.stderr
actualErr := output.Stderr()
if diff := cmp.Diff(expectedErr, actualErr); len(diff) > 0 {
t.Errorf("unexpected output\n\nexpected:\n%s\nactual:\n%s\ndiff:\n%s", expectedErr, actualErr, diff)
}
})
}
}

@ -0,0 +1,19 @@
variable "input" {
type = object({
one = string,
two = string,
})
}
resource "test_resource" "resource" {
value = var.input.two
}
output "one" {
value = test_resource.resource.id
}
output "two" {
value = test_resource.resource.value
}

@ -0,0 +1,33 @@
run "first" {
command = plan
variables {
input = {
one = "one"
two = "two"
}
}
}
run "second" {
command = plan
variables {
input = {
# This should be okay, as run.first.one is unknown but we're not
# referencing it directly.
one = "one"
two = run.first.two
}
}
}
run "third" {
variables {
# This should fail as one of the values in run.second is unknown.
input = run.second
}
}

@ -0,0 +1,17 @@
variable "null_input" {
type = string
default = null
}
variable "interesting_input" {
type = string
}
resource "test_resource" "resource" {
value = var.interesting_input
}
output "null_output" {
value = var.null_input
}

@ -0,0 +1,16 @@
run "first" {
variables {
interesting_input = "bar"
}
assert {
condition = test_resource.resource.value == output.null_output
error_message = "this is always going to fail"
}
assert {
condition = var.null_input == output.null_output
error_message = "this should pass"
}
}

@ -0,0 +1,13 @@
run "first" {
variables {
interesting_input = "bar"
}
}
run "second" {
variables {
// It shouldn't let this happen.
interesting_input = run.first.null_output
}
}

@ -0,0 +1,18 @@
variable "null_input" {
type = string
default = null
}
variable "interesting_input" {
type = string
nullable = false
}
resource "test_resource" "resource" {
value = var.interesting_input
}
output "null_output" {
value = var.null_input
}

@ -0,0 +1,18 @@
run "first" {
variables {
interesting_input = "bar"
}
}
run "second" {
variables {
interesting_input = "bar"
null_input = run.first.null_output
}
assert {
condition = output.null_output == run.first.null_output
error_message = "should have passed"
}
}

@ -0,0 +1,14 @@
variable "destroy_fail" {
type = bool
default = null
nullable = true
}
resource "test_resource" "resource" {
destroy_fail = var.destroy_fail
}
output "destroy_fail" {
value = test_resource.resource.destroy_fail
}

@ -0,0 +1,11 @@
run "one" {
command = plan
}
run "two" {
assert {
condition = output.destroy_fail == run.one.destroy_fail
error_message = "should fail"
}
}

@ -0,0 +1,14 @@
variable "destroy_fail" {
type = bool
default = null
nullable = true
}
resource "test_resource" "resource" {
destroy_fail = var.destroy_fail
}
output "destroy_fail" {
value = test_resource.resource.destroy_fail
}

@ -0,0 +1,10 @@
run "one" {
command = plan
}
run "two" {
variables {
destroy_fail = run.one.destroy_fail
}
}

@ -989,7 +989,7 @@ func (d *evaluationStateData) GetOutput(addr addrs.OutputValue, rng tfdiags.Sour
Value: cty.NilVal,
Sensitive: config.Sensitive,
}
} else if output.Value == cty.NilVal {
} else if output.Value == cty.NilVal || output.Value.IsNull() {
// Then we did get a value but Terraform itself thought it was NilVal
// so we treat this as if the value isn't yet known.
output.Value = cty.DynamicVal

Loading…
Cancel
Save