[QT-601] Nest scenario step variables in eval context (#107)

Previously we'd expose any know scenario step variables at the top-level
of a steps entry in the eval context. This leads to problems if a
Terraform module has both an input and output with the same name.
Consider the following scenario:

```hcl
module "cluster" {
  source = "./modules/cluster"
}

module "worker" {
  source = "./modules/worker"
}

variable "addr" {
  type = string
  default = "http://192.168.0.1"
}

scenario "boundary" {
  step "cluster" {
    module = module.cluster
    variables {
      addr = var.addr
    }
  }

  step "worker" {
    module = module.worker
    variables {
      upstream_addr = step.cluster.addr
    }
  }

  step "worker_downstream" {
    module = module.worker
    variables {
      upstream_addr = step.worker.upstream_addr
    }
  }
}
```

We expect expect `step.cluster.addr` to be the known value of of
`var.addr`. But what happens with step worker here is the problem.
`step.worker.upstream_addr` should be module call to `step.cluster.addr`
but because we've exposed `step.cluster.addr` in the eval context we
don't do a module call, and instead inherit the value from `var.addr`.
The same happens with `step.worker_downstream.upstream_addr`. Instead of
being a module call to `step.worker.upstream_addr` we inherit the know
value of `var.addr`.

With this change that behavior is now default. If you wish to use the
prior behavior (getting a steps known variables) you can access them
with the `variables` key, e.g. `step.cluster.variables.addr`.

- [x] The commit message includes an explanation of the changes
- [x] Manual validation of the changes have been performed (if possible)
- [x] New or modified code has requisite test coverage (if possible)
- [x] I have performed a self-review of the changes
- [x] I have made necessary changes and/or pull requests for documentation
- [x] I have written useful comments in the code

Signed-off-by: Ryan Cragun <[email protected]>

acceptance/scenarios/build_pgo/enos-sample.hcl

@@ -39,10 +39,10 @@ scenario "upgrade" {
 
   step "upgrade" {
     module = module.upgrade
-  }
 
-  variables {
-    az = matrix.distro
+    variables {
+      az = matrix.distro
+    }
   }
 }
 
@@ -79,10 +79,10 @@ scenario "replication" {
 
   step "replication" {
     module = module.replication
-  }
 
-  variables {
-    az = matrix.distro
+    variables {
+      az = matrix.distro
+    }
   }
 }
 

acceptance/scenarios/sample_list/enos-sample.hcl

@@ -14,10 +14,10 @@ scenario "upgrade" {
 
   step "upgrade" {
     module = module.upgrade
-  }
 
-  variables {
-    az = matrix.az
+    variables {
+      az = matrix.az
+    }
   }
 }
 
@@ -29,10 +29,9 @@ scenario "replication" {
 
   step "replication" {
     module = module.replication
-  }
-
-  variables {
-    az = matrix.az
+    variables {
+      az = matrix.az
+    }
   }
 }
 

acceptance/scenarios/scenario_generate_step_vars/enos.hcl

@@ -63,12 +63,12 @@ scenario "step_vars" {
 
   output "module_default" {
     description = "a known value inherited through module defaults"
-    value       = step.infra_default.az
+    value       = step.infra_default.variables.az
   }
 
   output "step_known" {
     description = "a known value set at a step"
-    value       = step.infra_west.az
+    value       = step.infra_west.variables.az
   }
 
   output "step_reference_output_ref" {

acceptance/scenarios/scenario_generate_step_vars/modules/target/main.tf

@@ -2,6 +2,10 @@ variable "ami" {
   type = string
 }
 
+output "ami" {
+  value = var.ami
+}
+
 output "ips" {
   value = ["127.0.0.1"]
 }

internal/flightplan/decoder_test.go

@@ -183,7 +183,12 @@ func testRequireEqualFP(t *testing.T, fp, expected *FlightPlan) {
 					eAttr := expected.ScenarioBlocks[i].Scenarios[j].Steps[is].Module.Attrs[isa]
 					aAttr := fp.ScenarioBlocks[i].Scenarios[j].Steps[is].Module.Attrs[isa]
 
-					require.True(t, eAttr.Type().Equals(aAttr.Type()))
+					require.Truef(
+						t, eAttr.Type().Equals(aAttr.Type()),
+						fmt.Sprintf("expected %s type to have type %s, got %s",
+							isa, eAttr.Type().GoString(), aAttr.Type().GoString(),
+						),
+					)
 					if !eAttr.IsNull() {
 						testMostlyEqualStepVar(t, eAttr, aAttr)
 					}
@@ -209,7 +214,11 @@ func testMostlyEqualStepVar(t *testing.T, expected cty.Value, got cty.Value) {
 	aVal, diags := StepVariableFromVal(got)
 	require.False(t, diags.HasErrors(), diags.Error())
 	require.EqualValues(t, eVal.Value, aVal.Value)
-	require.Len(t, eVal.Traversal, len(aVal.Traversal))
+	require.Lenf(t, eVal.Traversal, len(aVal.Traversal),
+		"expected %s to have a traversal of: %+v, got: %+v", eVal.Value.GoString(),
+		eVal.Traversal, aVal.Traversal,
+	)
+
 	for i := range eVal.Traversal {
 		if i == 0 {
 			eAttr, ok := eVal.Traversal[i].(hcl.TraverseRoot)

internal/flightplan/scenario_step.go

@@ -821,17 +821,14 @@ func (ss *ScenarioStep) insertIntoCtx(ctx *hcl.EvalContext) hcl.Diagnostics {
 	}
 
 	vals := map[string]cty.Value{
-		"source": cty.StringVal(ss.Module.Source),
-		"name":   cty.StringVal(ss.Name),
+		"source":    cty.StringVal(ss.Module.Source),
+		"name":      cty.StringVal(ss.Name),
+		"variables": cty.ObjectVal(ss.Module.Attrs),
 	}
 	if ss.Module.Version != "" {
 		vals["version"] = cty.StringVal(ss.Module.Version)
 	}
 
-	for k, v := range ss.Module.Attrs {
-		vals[k] = v
-	}
-
 	steps[ss.Name] = cty.ObjectVal(vals)
 	if ctx.Variables == nil {
 		ctx.Variables = map[string]cty.Value{}

internal/flightplan/scenario_step_test.go

@@ -625,24 +625,25 @@ scenario "step_vars" {
   step "one" {
     module = module.one
 
-	variables {
+    variables {
       concrete    = "oneconcrete"
-	  matrixinput = matrix.input
-	}
+      matrixinput = matrix.input
+    }
   }
 
   step "two" {
     module = module.two
 
-	variables {
-      concrete           = "twoconcrete"
-	  inherited_concrete = step.one.concrete
-	  reference          = step.one.reference
-	  oneattr            = step.one.oneattr
-	  matrixconcrete     = matrix.input
-	  matrixinherited    = step.one.matrixinput
-	  fromvariables      = var.something
-	}
+    variables {
+      concrete            = "twoconcrete"
+      reference           = step.one.reference
+      reference_same_name = step.one.concrete
+      inherited_concrete  = step.one.variables.concrete
+      oneattr             = step.one.oneattr
+      matrixconcrete      = matrix.input
+      matrixreference     = step.one.matrixinput
+      fromvariables       = var.something
+    }
   }
 }
 `, modulePath),
@@ -693,14 +694,116 @@ scenario "step_vars" {
 											Name:   "two",
 											Source: modulePath,
 											Attrs: map[string]cty.Value{
-												"twoattr":            testMakeStepVarValue(cty.StringVal("twoattrval")),
-												"concrete":           testMakeStepVarValue(cty.StringVal("twoconcrete")),
-												"inherited_concrete": testMakeStepVarValue(cty.StringVal("oneconcrete")),
-												"reference":          testMakeStepVarTraversal("step", "one", "reference"),
-												"oneattr":            testMakeStepVarValue(cty.StringVal("oneattrval")),
-												"matrixconcrete":     testMakeStepVarValue(cty.StringVal("matrixinput")),
-												"matrixinherited":    testMakeStepVarValue(cty.StringVal("matrixinput")),
-												"fromvariables":      testMakeStepVarValue(cty.StringVal("somethingval")),
+												"twoattr":             testMakeStepVarValue(cty.StringVal("twoattrval")),
+												"concrete":            testMakeStepVarValue(cty.StringVal("twoconcrete")),
+												"inherited_concrete":  testMakeStepVarValue(cty.StringVal("oneconcrete")),
+												"reference":           testMakeStepVarTraversal("step", "one", "reference"),
+												"reference_same_name": testMakeStepVarTraversal("step", "one", "concrete"),
+												"oneattr":             testMakeStepVarTraversal("step", "one", "oneattr"),
+												"matrixconcrete":      testMakeStepVarValue(cty.StringVal("matrixinput")),
+												"matrixreference":     testMakeStepVarTraversal("step", "one", "matrixinput"),
+												"fromvariables":       testMakeStepVarValue(cty.StringVal("somethingval")),
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			desc: "step variables with vars and outputs of same name",
+			hcl: fmt.Sprintf(`
+module "cluster" {
+  source = "%s"
+}
+
+module "worker" {
+  source = "%[1]s"
+}
+
+variable "addr" {
+  type = string
+  default = "http://192.168.0.1"
+}
+
+scenario "boundary" {
+  step "cluster" {
+    module = module.cluster
+
+    variables {
+      addr = var.addr
+    }
+  }
+
+  step "worker" {
+    module = module.worker
+
+    variables {
+      upstream_addr = step.cluster.addr
+    }
+  }
+
+  step "worker_downstream" {
+    module = module.worker
+
+    variables {
+      upstream_addr = step.worker.upstream_addr
+    }
+  }
+}
+`, modulePath),
+			expected: &FlightPlan{
+				TerraformCLIs: []*TerraformCLI{
+					DefaultTerraformCLI(),
+				},
+				Modules: []*Module{
+					{
+						Name:   "cluster",
+						Source: modulePath,
+					},
+					{
+						Name:   "worker",
+						Source: modulePath,
+					},
+				},
+				ScenarioBlocks: DecodedScenarioBlocks{
+					{
+						Name: "boundary",
+						Scenarios: []*Scenario{
+							{
+								Name:         "boundary",
+								TerraformCLI: DefaultTerraformCLI(),
+								Steps: []*ScenarioStep{
+									{
+										Name: "cluster",
+										Module: &Module{
+											Name:   "cluster",
+											Source: modulePath,
+											Attrs: map[string]cty.Value{
+												"addr": testMakeStepVarValue(cty.StringVal("http://192.168.0.1")),
+											},
+										},
+									},
+									{
+										Name: "worker",
+										Module: &Module{
+											Name:   "worker",
+											Source: modulePath,
+											Attrs: map[string]cty.Value{
+												"upstream_addr": testMakeStepVarTraversal("step", "cluster", "addr"),
+											},
+										},
+									},
+									{
+										Name: "worker_downstream",
+										Module: &Module{
+											Name:   "worker",
+											Source: modulePath,
+											Attrs: map[string]cty.Value{
+												"upstream_addr": testMakeStepVarTraversal("step", "worker", "upstream_addr"),
 											},
 										},
 									},