From 8b3329e910bb0df7907fadb69a05191b5152a7a7 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Fri, 24 Jan 2025 16:40:42 +0100 Subject: [PATCH] Check type contraints for output values --- internal/terraform/context_apply2_test.go | 47 +++++++++++++++++++ internal/terraform/node_output.go | 36 +++++++++++++- internal/terraform/node_output_test.go | 12 +++-- .../apply-output-type-constraint.tf | 20 ++++++++ 4 files changed, 109 insertions(+), 6 deletions(-) create mode 100644 internal/terraform/testdata/apply-output-type-constraint/apply-output-type-constraint.tf diff --git a/internal/terraform/context_apply2_test.go b/internal/terraform/context_apply2_test.go index d271331946..2d534ea878 100644 --- a/internal/terraform/context_apply2_test.go +++ b/internal/terraform/context_apply2_test.go @@ -4845,3 +4845,50 @@ resource "test_resource" "test" { }) } } + +func TestContext2Apply_outputWithTypeContraint(t *testing.T) { + m := testModule(t, "apply-output-type-constraint") + p := testProvider("aws") + p.PlanResourceChangeFn = testDiffFn + p.ApplyResourceChangeFn = testApplyFn + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p), + }, + }) + + plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + tfdiags.AssertNoErrors(t, diags) + + state, diags := ctx.Apply(plan, m, nil) + if diags.HasErrors() { + t.Fatalf("diags: %s", diags.Err()) + } + + wantValues := map[string]cty.Value{ + "string": cty.StringVal("true"), + "object_default": cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("Bart"), + }), + "object_override": cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("Lisa"), + }), + } + ovs := state.RootOutputValues + for name, want := range wantValues { + os, ok := ovs[name] + if !ok { + t.Errorf("missing output value %q", name) + continue + } + if got := os.Value; !want.RawEquals(got) { + t.Errorf("wrong value for output %q\ngot: %#v\nwant: %#v", name, got, want) + } + } + + for gotName := range ovs { + if _, ok := wantValues[gotName]; !ok { + t.Errorf("unexpected extra output value %q", gotName) + } + } +} diff --git a/internal/terraform/node_output.go b/internal/terraform/node_output.go index eadd281681..078b385861 100644 --- a/internal/terraform/node_output.go +++ b/internal/terraform/node_output.go @@ -8,7 +8,9 @@ import ( "log" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/ext/typeexpr" "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/convert" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" @@ -448,7 +450,7 @@ func (n *NodeApplyableOutput) Execute(ctx EvalContext, op walkOperation) (diags // This has to run before we have a state lock, since evaluation also // reads the state var evalDiags tfdiags.Diagnostics - val, evalDiags = ctx.EvaluateExpr(n.Config.Expr, cty.DynamicPseudoType, nil) + val, evalDiags = evalOutputValue(ctx, n.Config.Expr, n.Config.ConstraintType, n.Config.TypeDefaults) diags = diags.Append(evalDiags) // We'll handle errors below, after we have loaded the module. @@ -546,6 +548,38 @@ If you do intend to export this data, annotate the output value as sensitive by return diags } +// evalOutputValue encapsulates the logic for transforming an author's value +// expression into a valid value of their declared type constraint, or returning +// an error describing why that isn't possible. +func evalOutputValue(ctx EvalContext, expr hcl.Expression, wantType cty.Type, defaults *typeexpr.Defaults) (cty.Value, tfdiags.Diagnostics) { + // We can't pass wantType to EvaluateExpr here because we'll need to + // possibly apply our defaults before attempting type conversion below. + val, diags := ctx.EvaluateExpr(expr, cty.DynamicPseudoType, nil) + if diags.HasErrors() { + return cty.UnknownVal(wantType), diags + } + + if defaults != nil { + val = defaults.Apply(val) + } + + val, err := convert.Convert(val, wantType) + if err != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid output value", + Detail: fmt.Sprintf("The value expression does not match this output value's type constraint: %s.", tfdiags.FormatError(err)), + Subject: expr.Range().Ptr(), + // TODO: Populate EvalContext and Expression, but we can't do that + // as long as we're using the ctx.EvaluateExpr helper above because + // the EvalContext is hidden from us in that case. + }) + return cty.UnknownVal(wantType), diags + } + + return val, diags +} + // dag.GraphNodeDotter impl. func (n *NodeApplyableOutput) DotNode(name string, opts *dag.DotOpts) *dag.DotNode { return &dag.DotNode{ diff --git a/internal/terraform/node_output_test.go b/internal/terraform/node_output_test.go index 042ce190fc..c86a4eeadf 100644 --- a/internal/terraform/node_output_test.go +++ b/internal/terraform/node_output_test.go @@ -25,7 +25,7 @@ func TestNodeApplyableOutputExecute_knownValue(t *testing.T) { ctx.ChecksState = checks.NewState(nil) ctx.DeferralsState = deferring.NewDeferred(false) - config := &configs.Output{Name: "map-output"} + config := &configs.Output{Name: "map-output", ConstraintType: cty.DynamicPseudoType} addr := addrs.OutputValue{Name: config.Name}.Absolute(addrs.RootModuleInstance) node := &NodeApplyableOutput{Config: config, Addr: addr} val := cty.MapVal(map[string]cty.Value{ @@ -58,7 +58,7 @@ func TestNodeApplyableOutputExecute_knownValue(t *testing.T) { func TestNodeApplyableOutputExecute_noState(t *testing.T) { ctx := new(MockEvalContext) - config := &configs.Output{Name: "map-output"} + config := &configs.Output{Name: "map-output", ConstraintType: cty.DynamicPseudoType} addr := addrs.OutputValue{Name: config.Name}.Absolute(addrs.RootModuleInstance) node := &NodeApplyableOutput{Config: config, Addr: addr} val := cty.MapVal(map[string]cty.Value{ @@ -86,6 +86,7 @@ func TestNodeApplyableOutputExecute_invalidDependsOn(t *testing.T) { hcl.TraverseAttr{Name: "bar"}, }, }, + ConstraintType: cty.DynamicPseudoType, } addr := addrs.OutputValue{Name: config.Name}.Absolute(addrs.RootModuleInstance) node := &NodeApplyableOutput{Config: config, Addr: addr} @@ -108,7 +109,7 @@ func TestNodeApplyableOutputExecute_sensitiveValueNotOutput(t *testing.T) { ctx.StateState = states.NewState().SyncWrapper() ctx.ChecksState = checks.NewState(nil) - config := &configs.Output{Name: "map-output"} + config := &configs.Output{Name: "map-output", ConstraintType: cty.DynamicPseudoType} addr := addrs.OutputValue{Name: config.Name}.Absolute(addrs.RootModuleInstance) node := &NodeApplyableOutput{Config: config, Addr: addr} val := cty.MapVal(map[string]cty.Value{ @@ -132,8 +133,9 @@ func TestNodeApplyableOutputExecute_sensitiveValueAndOutput(t *testing.T) { ctx.DeferralsState = deferring.NewDeferred(false) config := &configs.Output{ - Name: "map-output", - Sensitive: true, + Name: "map-output", + Sensitive: true, + ConstraintType: cty.DynamicPseudoType, } addr := addrs.OutputValue{Name: config.Name}.Absolute(addrs.RootModuleInstance) node := &NodeApplyableOutput{Config: config, Addr: addr} diff --git a/internal/terraform/testdata/apply-output-type-constraint/apply-output-type-constraint.tf b/internal/terraform/testdata/apply-output-type-constraint/apply-output-type-constraint.tf new file mode 100644 index 0000000000..604e31bc32 --- /dev/null +++ b/internal/terraform/testdata/apply-output-type-constraint/apply-output-type-constraint.tf @@ -0,0 +1,20 @@ +output "string" { + type = string + value = true +} + +output "object_default" { + type = object({ + name = optional(string, "Bart") + }) + value = {} +} + +output "object_override" { + type = object({ + name = optional(string, "Bart") + }) + value = { + name = "Lisa" + } +}