diff --git a/.changes/v1.15/ENHANCEMENTS-20250203-175807.yaml b/.changes/v1.15/ENHANCEMENTS-20250203-175807.yaml new file mode 100644 index 000000000000..53bad4ecb532 --- /dev/null +++ b/.changes/v1.15/ENHANCEMENTS-20250203-175807.yaml @@ -0,0 +1,5 @@ +kind: ENHANCEMENTS +body: 'config: `output` blocks now can have an explicit type constraints' +time: 2025-02-03T17:58:07.110141+01:00 +custom: + Issue: "36411" diff --git a/internal/configs/module_merge.go b/internal/configs/module_merge.go index d9bc2abcc726..f0570c037f21 100644 --- a/internal/configs/module_merge.go +++ b/internal/configs/module_merge.go @@ -160,6 +160,11 @@ func (o *Output) merge(oo *Output) hcl.Diagnostics { o.Ephemeral = oo.Ephemeral o.EphemeralSet = oo.EphemeralSet } + if oo.TypeSet { + o.ConstraintType = oo.ConstraintType + o.TypeDefaults = oo.TypeDefaults + o.TypeSet = oo.TypeSet + } // We don't allow depends_on to be overridden because that is likely to // cause confusing misbehavior. diff --git a/internal/configs/module_merge_test.go b/internal/configs/module_merge_test.go index dabed28a8fd8..f6351594ee08 100644 --- a/internal/configs/module_merge_test.go +++ b/internal/configs/module_merge_test.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/zclconf/go-cty/cty" + "github.com/hashicorp/hcl/v2/ext/typeexpr" "github.com/hashicorp/terraform/internal/addrs" ) @@ -413,6 +414,60 @@ func TestModuleOverrideConstVariable(t *testing.T) { } } +func TestModuleOverrideOutputType(t *testing.T) { + type testCase struct { + constraintType cty.Type + typeDefaults *typeexpr.Defaults + typeSet bool + } + cases := map[string]testCase{ + "fully_overridden": { + constraintType: cty.Number, + typeDefaults: nil, + typeSet: true, + }, + "no_override": { + constraintType: cty.String, + typeDefaults: nil, + typeSet: true, + }, + "type_added_by_override": { + constraintType: cty.List(cty.String), + typeDefaults: nil, + typeSet: true, + }, + } + + mod, diags := testModuleFromDir("testdata/valid-modules/override-output-type") + + assertNoDiagnostics(t, diags) + + if mod == nil { + t.Fatalf("module is nil") + } + + for name, want := range cases { + t.Run(fmt.Sprintf("output %s", name), func(t *testing.T) { + got, exists := mod.Outputs[name] + if !exists { + t.Fatalf("output %q not found", name) + } + + if !got.ConstraintType.Equals(want.constraintType) { + t.Errorf("wrong result for constraint type\ngot: %#v\nwant: %#v", got.ConstraintType, want.constraintType) + } + + if got.TypeSet != want.typeSet { + t.Errorf("wrong result for type set\ngot: %t want: %t", got.TypeSet, want.typeSet) + } + + if got.TypeDefaults != want.typeDefaults { + t.Errorf("wrong result for type defaults\ngot: %#v want: %#v", got.TypeDefaults, want.typeDefaults) + } + }) + } +} + func TestModuleOverrideResourceFQNs(t *testing.T) { mod, diags := testModuleFromDir("testdata/valid-modules/override-resource-provider") assertNoDiagnostics(t, diags) diff --git a/internal/configs/named_values.go b/internal/configs/named_values.go index 4cbb3e9e989d..789a4ed5cc16 100644 --- a/internal/configs/named_values.go +++ b/internal/configs/named_values.go @@ -388,12 +388,24 @@ type Output struct { Ephemeral bool Deprecated string + // ConstraintType is a type constraint which the result is guaranteed + // to conform to when used in the calling module. + ConstraintType cty.Type + // TypeDefaults describes any optional attribute defaults that should be + // applied to the Expr result before type conversion. + TypeDefaults *typeexpr.Defaults + Preconditions []*CheckRule DescriptionSet bool SensitiveSet bool EphemeralSet bool DeprecatedSet bool + // TypeSet is true if there was an explicit "type" argument in the + // configuration block. This is mainly to allow distinguish explicitly + // setting vs. just using the default type constraint when processing + // override files. + TypeSet bool DeclRange hcl.Range DeprecatedRange hcl.Range @@ -434,6 +446,19 @@ func decodeOutputBlock(block *hcl.Block, override bool) (*Output, hcl.Diagnostic o.Expr = attr.Expr } + if attr, exists := content.Attributes["type"]; exists { + ty, defaults, moreDiags := typeexpr.TypeConstraintWithDefaults(attr.Expr) + diags = append(diags, moreDiags...) + o.ConstraintType = ty + o.TypeDefaults = defaults + o.TypeSet = true + } + if o.ConstraintType == cty.NilType { + // If no constraint is given then the type will be inferred + // automatically from the value. + o.ConstraintType = cty.DynamicPseudoType + } + if attr, exists := content.Attributes["sensitive"]; exists { valDiags := gohcl.DecodeExpression(attr.Expr, nil, &o.Sensitive) diags = append(diags, valDiags...) @@ -587,6 +612,9 @@ var outputBlockSchema = &hcl.BodySchema{ { Name: "deprecated", }, + { + Name: "type", + }, }, Blocks: []hcl.BlockHeaderSchema{ {Type: "precondition"}, diff --git a/internal/configs/testdata/valid-files/output-type-constraint.tf b/internal/configs/testdata/valid-files/output-type-constraint.tf new file mode 100644 index 000000000000..13be13befd56 --- /dev/null +++ b/internal/configs/testdata/valid-files/output-type-constraint.tf @@ -0,0 +1,11 @@ +output "string" { + type = string + value = "Hello" +} + +output "object" { + type = object({ + name = optional(string, "Bart"), + }) + value = {} +} diff --git a/internal/configs/testdata/valid-modules/override-output-type/a_override.tf b/internal/configs/testdata/valid-modules/override-output-type/a_override.tf new file mode 100644 index 000000000000..ca9bf7f07075 --- /dev/null +++ b/internal/configs/testdata/valid-modules/override-output-type/a_override.tf @@ -0,0 +1,7 @@ +output "fully_overridden" { + type = number +} + +output "type_added_by_override" { + type = list(string) +} diff --git a/internal/configs/testdata/valid-modules/override-output-type/primary.tf b/internal/configs/testdata/valid-modules/override-output-type/primary.tf new file mode 100644 index 000000000000..0fff30800dc4 --- /dev/null +++ b/internal/configs/testdata/valid-modules/override-output-type/primary.tf @@ -0,0 +1,13 @@ +output "fully_overridden" { + value = "hello" + type = string +} + +output "no_override" { + value = "hello" + type = string +} + +output "type_added_by_override" { + value = "hello" +} diff --git a/internal/terraform/context_apply2_test.go b/internal/terraform/context_apply2_test.go index d271331946d0..2d534ea878a0 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/evaluate.go b/internal/terraform/evaluate.go index 4c61753711cf..ee1374427a8e 100644 --- a/internal/terraform/evaluate.go +++ b/internal/terraform/evaluate.go @@ -420,21 +420,66 @@ func (d *evaluationStateData) GetModule(addr addrs.ModuleCall, rng tfdiags.Sourc } outputConfigs := moduleConfig.Module.Outputs - // We don't do instance expansion during validation, and so we need to - // return an unknown value. Technically we should always return - // cty.DynamicVal here because the final value during plan will always - // be an object or tuple type with unpredictable attributes/elements, - // but because we never actually carry values forward from validation to - // planning we lie a little here and return unknown list and map types, - // just to give us more opportunities to catch author mistakes during - // validation. - // - // This means that in practice any expression that refers to a module - // call must be written to be valid for either a collection type or - // structural type of similar kind, so that it can be considered as - // valid during both the validate and plan walks. - if d.Operation == walkValidate { - // In case of non-expanded module calls we return a known object with unknonwn values + // typeDefined tracks if a module has defined any output type at all. We can + // use this as a flag to abandon some subtly incorrect legacy behavior. + // Start false because any TypeSet will flip the flag to true + typeDefined := false + + // noDynamicTypes indicates that the module fully defines all output types, + // and they themselves contain no dynamic types. This allows us to create + // more precise unknowns for outputs, and use lists and maps when + // applicable. + // Start true because any dynamic type will flip the flag to false. + noDynamicTypes := true + + for _, out := range outputConfigs { + typeDefined = typeDefined || out.TypeSet + noDynamicTypes = noDynamicTypes && !out.ConstraintType.HasDynamicTypes() + } + + if d.Operation == walkValidate && typeDefined { + atys := make(map[string]cty.Type, len(outputConfigs)) + as := make(map[string]cty.Value, len(outputConfigs)) + for name, c := range outputConfigs { + // atys is used to create the module object type for expanded modules + atys[name] = c.ConstraintType + // the unknown val can be used when we return a single module + // instance with unknown outputs + val := cty.UnknownVal(c.ConstraintType) + + if c.DeprecatedSet { + val = val.Mark(marks.NewDeprecation(c.Deprecated, absAddr.Output(name).ConfigOutputValue().ForDisplay())) + } + as[name] = val + } + instTy := cty.Object(atys) + + switch { + case callConfig.Count != nil && noDynamicTypes: + return cty.UnknownVal(cty.List(instTy)), diags + case callConfig.ForEach != nil && noDynamicTypes: + return cty.UnknownVal(cty.Map(instTy)), diags + case callConfig.Count != nil || callConfig.ForEach != nil: + return cty.DynamicVal, diags + default: + val := cty.ObjectVal(as) + return val, diags + } + } else if d.Operation == walkValidate { + // the legacy behavior here is slightly wrong, but we're going to + // preserve it for now when modules don't define any typed output. The + // fact that we are returning a list or map is incorrect when the types + // are unknown, because the known values we get later are going to be + // tuples and objects. This usually doesn't present a problem, but it is + // possible to write complex expressions where it can only pass during + // one of validation or planning because the types will cause a mismatch + // in the other case. + // This means that in practice any expression that refers to a module + // call must be written to be valid for either a collection type or + // structural type of similar kind, so that it can be considered as + // valid during both the validate and plan walks. + + // In case of non-expanded module calls we return a known object with unknown values // In case of an expanded module call we return unknown list/map // This means deprecation can only for non-expanded modules be detected during validate // since we don't want false positives. The plan walk will give definitive warnings. @@ -487,16 +532,15 @@ func (d *evaluationStateData) GetModule(addr addrs.ModuleCall, rng tfdiags.Sourc for name, cfg := range outputConfigs { outputAddr := moduleInstAddr.OutputValue(name) - // Although we do typically expect the graph dependencies to - // ensure that values get registered before they are needed, - // we track depedencies with specific output values where - // possible, instead of with entire module calls, and so - // in this specific case it's valid for some of this call's - // output values to not be known yet, with the graph builder - // being responsible for making sure that no expression - // in the configuration can actually observe that. + // Although we do typically expect the graph dependencies to ensure + // that values get registered before they are needed, we track + // dependencies with specific output values where possible, instead + // of with entire module calls, and so in this specific case it's + // valid for some of this call's output values to not be known yet, + // with the graph builder being responsible for making sure that no + // expression in the configuration can actually observe that. if !namedVals.HasOutputValue(outputAddr) { - attrs[name] = cty.DynamicVal + attrs[name] = cty.UnknownVal(cfg.ConstraintType) continue } outputVal := namedVals.GetOutputValue(outputAddr) @@ -535,7 +579,11 @@ func (d *evaluationStateData) GetModule(addr addrs.ModuleCall, rng tfdiags.Sourc elems = append(elems, instVal) diags = diags.Append(moreDiags) } - return cty.TupleVal(elems), diags + if noDynamicTypes { + return cty.ListVal(elems), diags + } else { + return cty.TupleVal(elems), diags + } case addrs.StringKeyType: attrs := make(map[string]cty.Value, len(instKeys)) @@ -544,7 +592,11 @@ func (d *evaluationStateData) GetModule(addr addrs.ModuleCall, rng tfdiags.Sourc attrs[string(instKey.(addrs.StringKey))] = instVal diags = diags.Append(moreDiags) } - return cty.ObjectVal(attrs), diags + if noDynamicTypes { + return cty.MapVal(attrs), diags + } else { + return cty.ObjectVal(attrs), diags + } default: diags = diags.Append(&hcl.Diagnostic{ @@ -602,7 +654,7 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc // TODO: When deferred actions are more stable and robust in stacks, it // would be nice to rework this function to rely on the ResourceInstanceKeys // result for _all_ of its work, rather than continuing to duplicate a bunch - // of the logic we've tried to encapsulate over ther already. + // of the logic we've tried to encapsulate over there already. if d.Operation == walkPlan || d.Operation == walkApply { if !d.Evaluator.Instances.ResourceInstanceExpanded(addr.Absolute(moduleAddr)) { // Then we've asked for a resource that hasn't been evaluated yet. @@ -795,14 +847,6 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc } case walkImport: - // Import does not yet plan resource changes, so new resources from - // config are not going to be found here. Once walkImport fully - // plans resources, this case should not longer be needed. - // In the single instance case, we can return a typed unknown value - // for the instance to better satisfy other expressions using the - // value. This of course will not help if statically known - // attributes are expected to be known elsewhere, but reduces the - // number of problematic configs for now. // Unlike in plan and apply above we can't be sure the count or // for_each instances are empty, so we return a DynamicVal. We // don't really have a good value to return otherwise -- empty diff --git a/internal/terraform/evaluate_test.go b/internal/terraform/evaluate_test.go index 0f590dbf71a9..e2853fa70bf5 100644 --- a/internal/terraform/evaluate_test.go +++ b/internal/terraform/evaluate_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/davecgh/go-spew/spew" + "github.com/hashicorp/hcl/v2/hcltest" "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/internal/addrs" @@ -568,7 +569,7 @@ func TestEvaluatorGetResource_changes(t *testing.T) { } func TestEvaluatorGetModule(t *testing.T) { - evaluator := evaluatorForModule(states.NewState().SyncWrapper(), plans.NewChanges().SyncWrapper()) + evaluator := evaluatorForModule(states.NewState().SyncWrapper(), plans.NewChanges().SyncWrapper(), nil, nil) evaluator.Instances.SetModuleSingle(addrs.RootModuleInstance, addrs.ModuleCall{Name: "mod"}) evaluator.NamedValues.SetOutputValue( addrs.OutputValue{Name: "out"}.Absolute(addrs.ModuleInstance{addrs.ModuleInstanceStep{Name: "mod"}}), @@ -593,7 +594,304 @@ func TestEvaluatorGetModule(t *testing.T) { } } -func evaluatorForModule(stateSync *states.SyncState, changesSync *plans.ChangesSync) *Evaluator { +func TestEvaluatorGetModule_validateTypedOutputs(t *testing.T) { + tests := map[string]struct { + configureModuleCall func(*configs.ModuleCall) + want cty.Value + }{ + "single": { + want: cty.ObjectVal(map[string]cty.Value{ + "out": cty.UnknownVal(cty.String), + }), + }, + "count": { + configureModuleCall: func(call *configs.ModuleCall) { + call.Count = hcltest.MockExprLiteral(cty.NumberIntVal(1)) + }, + want: cty.UnknownVal(cty.List(cty.Object(map[string]cty.Type{ + "out": cty.String, + }))), + }, + "for_each": { + configureModuleCall: func(call *configs.ModuleCall) { + call.ForEach = hcltest.MockExprLiteral(cty.MapVal(map[string]cty.Value{ + "a": cty.StringVal("a"), + })) + }, + want: cty.UnknownVal(cty.Map(cty.Object(map[string]cty.Type{ + "out": cty.String, + }))), + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + evaluator := evaluatorForModule(states.NewState().SyncWrapper(), plans.NewChanges().SyncWrapper(), func(out *configs.Output) { + out.ConstraintType = cty.String + out.TypeSet = true + }, test.configureModuleCall) + + data := &evaluationStateData{ + evaluationData: &evaluationData{ + Evaluator: evaluator, + }, + Operation: walkValidate, + } + scope := evaluator.Scope(data, nil, nil, lang.ExternalFuncs{}) + + got, diags := scope.Data.GetModule(addrs.ModuleCall{ + Name: "mod", + }, tfdiags.SourceRange{}) + + if len(diags) != 0 { + t.Errorf("unexpected diagnostics %s", spew.Sdump(diags)) + } + if !got.RawEquals(test.want) { + t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, test.want) + } + }) + } +} + +func TestEvaluatorGetModule_validateTypedOutputsWithDynamicTypes(t *testing.T) { + tests := map[string]struct { + configureModuleCall func(*configs.ModuleCall) + }{ + "count": { + configureModuleCall: func(call *configs.ModuleCall) { + call.Count = hcltest.MockExprLiteral(cty.NumberIntVal(1)) + }, + }, + "for_each": { + configureModuleCall: func(call *configs.ModuleCall) { + call.ForEach = hcltest.MockExprLiteral(cty.MapVal(map[string]cty.Value{ + "a": cty.StringVal("a"), + })) + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + evaluator := evaluatorForModule(states.NewState().SyncWrapper(), plans.NewChanges().SyncWrapper(), func(out *configs.Output) { + out.ConstraintType = cty.DynamicPseudoType + out.TypeSet = true + }, test.configureModuleCall) + + data := &evaluationStateData{ + evaluationData: &evaluationData{ + Evaluator: evaluator, + }, + Operation: walkValidate, + } + scope := evaluator.Scope(data, nil, nil, lang.ExternalFuncs{}) + + got, diags := scope.Data.GetModule(addrs.ModuleCall{ + Name: "mod", + }, tfdiags.SourceRange{}) + + if len(diags) != 0 { + t.Errorf("unexpected diagnostics %s", spew.Sdump(diags)) + } + if !got.RawEquals(cty.DynamicVal) { + t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, cty.DynamicVal) + } + }) + } +} + +func TestEvaluatorGetModule_planTypedOutputs(t *testing.T) { + tests := map[string]struct { + setupInstances func(*instances.Expander) + setupOutputs func(*namedvals.State) + want cty.Value + }{ + "count": { + setupInstances: func(expander *instances.Expander) { + expander.SetModuleCount(addrs.RootModuleInstance, addrs.ModuleCall{Name: "mod"}, 2) + }, + setupOutputs: func(namedValues *namedvals.State) { + namedValues.SetOutputValue( + addrs.OutputValue{Name: "out"}.Absolute(addrs.ModuleInstance{ + addrs.ModuleInstanceStep{Name: "mod", InstanceKey: addrs.IntKey(0)}, + }), + cty.StringVal("first").Mark(marks.Sensitive), + ) + namedValues.SetOutputValue( + addrs.OutputValue{Name: "out"}.Absolute(addrs.ModuleInstance{ + addrs.ModuleInstanceStep{Name: "mod", InstanceKey: addrs.IntKey(1)}, + }), + cty.StringVal("second").Mark(marks.Sensitive), + ) + }, + want: cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{"out": cty.StringVal("first").Mark(marks.Sensitive)}), + cty.ObjectVal(map[string]cty.Value{"out": cty.StringVal("second").Mark(marks.Sensitive)}), + }), + }, + "for_each": { + setupInstances: func(expander *instances.Expander) { + expander.SetModuleForEach(addrs.RootModuleInstance, addrs.ModuleCall{Name: "mod"}, map[string]cty.Value{ + "a": cty.StringVal("a"), + "b": cty.StringVal("b"), + }) + }, + setupOutputs: func(namedValues *namedvals.State) { + namedValues.SetOutputValue( + addrs.OutputValue{Name: "out"}.Absolute(addrs.ModuleInstance{ + addrs.ModuleInstanceStep{Name: "mod", InstanceKey: addrs.StringKey("a")}, + }), + cty.StringVal("first").Mark(marks.Sensitive), + ) + namedValues.SetOutputValue( + addrs.OutputValue{Name: "out"}.Absolute(addrs.ModuleInstance{ + addrs.ModuleInstanceStep{Name: "mod", InstanceKey: addrs.StringKey("b")}, + }), + cty.StringVal("second").Mark(marks.Sensitive), + ) + }, + want: cty.MapVal(map[string]cty.Value{ + "a": cty.ObjectVal(map[string]cty.Value{"out": cty.StringVal("first").Mark(marks.Sensitive)}), + "b": cty.ObjectVal(map[string]cty.Value{"out": cty.StringVal("second").Mark(marks.Sensitive)}), + }), + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + evaluator := evaluatorForModule(states.NewState().SyncWrapper(), plans.NewChanges().SyncWrapper(), func(out *configs.Output) { + out.ConstraintType = cty.String + out.TypeSet = true + }, nil) + + test.setupInstances(evaluator.Instances) + test.setupOutputs(evaluator.NamedValues) + + data := &evaluationStateData{ + evaluationData: &evaluationData{ + Evaluator: evaluator, + }, + Operation: walkPlan, + } + scope := evaluator.Scope(data, nil, nil, lang.ExternalFuncs{}) + + got, diags := scope.Data.GetModule(addrs.ModuleCall{ + Name: "mod", + }, tfdiags.SourceRange{}) + + if len(diags) != 0 { + t.Errorf("unexpected diagnostics %s", spew.Sdump(diags)) + } + if !got.RawEquals(test.want) { + t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, test.want) + } + }) + } +} + +func TestEvaluatorGetModule_planUntypedOutputsRemainStructural(t *testing.T) { + tests := map[string]struct { + setupInstances func(*instances.Expander) + setupOutputs func(*namedvals.State) + want cty.Value + }{ + "count": { + setupInstances: func(expander *instances.Expander) { + expander.SetModuleCount(addrs.RootModuleInstance, addrs.ModuleCall{Name: "mod"}, 2) + }, + setupOutputs: func(namedValues *namedvals.State) { + namedValues.SetOutputValue( + addrs.OutputValue{Name: "out"}.Absolute(addrs.ModuleInstance{ + addrs.ModuleInstanceStep{Name: "mod", InstanceKey: addrs.IntKey(0)}, + }), + cty.StringVal("first").Mark(marks.Sensitive), + ) + namedValues.SetOutputValue( + addrs.OutputValue{Name: "out"}.Absolute(addrs.ModuleInstance{ + addrs.ModuleInstanceStep{Name: "mod", InstanceKey: addrs.IntKey(1)}, + }), + cty.StringVal("second").Mark(marks.Sensitive), + ) + }, + want: cty.TupleVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{"out": cty.StringVal("first").Mark(marks.Sensitive)}), + cty.ObjectVal(map[string]cty.Value{"out": cty.StringVal("second").Mark(marks.Sensitive)}), + }), + }, + "for_each": { + setupInstances: func(expander *instances.Expander) { + expander.SetModuleForEach(addrs.RootModuleInstance, addrs.ModuleCall{Name: "mod"}, map[string]cty.Value{ + "a": cty.StringVal("a"), + "b": cty.StringVal("b"), + }) + }, + setupOutputs: func(namedValues *namedvals.State) { + namedValues.SetOutputValue( + addrs.OutputValue{Name: "out"}.Absolute(addrs.ModuleInstance{ + addrs.ModuleInstanceStep{Name: "mod", InstanceKey: addrs.StringKey("a")}, + }), + cty.StringVal("first").Mark(marks.Sensitive), + ) + namedValues.SetOutputValue( + addrs.OutputValue{Name: "out"}.Absolute(addrs.ModuleInstance{ + addrs.ModuleInstanceStep{Name: "mod", InstanceKey: addrs.StringKey("b")}, + }), + cty.StringVal("second").Mark(marks.Sensitive), + ) + }, + want: cty.ObjectVal(map[string]cty.Value{ + "a": cty.ObjectVal(map[string]cty.Value{"out": cty.StringVal("first").Mark(marks.Sensitive)}), + "b": cty.ObjectVal(map[string]cty.Value{"out": cty.StringVal("second").Mark(marks.Sensitive)}), + }), + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + evaluator := evaluatorForModule(states.NewState().SyncWrapper(), plans.NewChanges().SyncWrapper(), nil, nil) + + test.setupInstances(evaluator.Instances) + test.setupOutputs(evaluator.NamedValues) + + data := &evaluationStateData{ + evaluationData: &evaluationData{ + Evaluator: evaluator, + }, + Operation: walkPlan, + } + scope := evaluator.Scope(data, nil, nil, lang.ExternalFuncs{}) + + got, diags := scope.Data.GetModule(addrs.ModuleCall{ + Name: "mod", + }, tfdiags.SourceRange{}) + + if len(diags) != 0 { + t.Errorf("unexpected diagnostics %s", spew.Sdump(diags)) + } + if !got.RawEquals(test.want) { + t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, test.want) + } + }) + } +} + +func evaluatorForModule(stateSync *states.SyncState, changesSync *plans.ChangesSync, configureOutput func(*configs.Output), configureModuleCall func(*configs.ModuleCall)) *Evaluator { + moduleCall := &configs.ModuleCall{ + Name: "mod", + } + if configureModuleCall != nil { + configureModuleCall(moduleCall) + } + + output := &configs.Output{ + Name: "out", + Sensitive: true, + ConstraintType: cty.DynamicPseudoType, + } + if configureOutput != nil { + configureOutput(output) + } + return &Evaluator{ Meta: &ContextMeta{ Env: "foo", @@ -601,9 +899,7 @@ func evaluatorForModule(stateSync *states.SyncState, changesSync *plans.ChangesS Config: &configs.Config{ Module: &configs.Module{ ModuleCalls: map[string]*configs.ModuleCall{ - "mod": { - Name: "mod", - }, + "mod": moduleCall, }, }, Children: map[string]*configs.Config{ @@ -611,10 +907,7 @@ func evaluatorForModule(stateSync *states.SyncState, changesSync *plans.ChangesS Path: addrs.Module{"module.mod"}, Module: &configs.Module{ Outputs: map[string]*configs.Output{ - "out": { - Name: "out", - Sensitive: true, - }, + "out": output, }, }, }, diff --git a/internal/terraform/node_output.go b/internal/terraform/node_output.go index eadd28168153..d9d32f49613d 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,51 @@ 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) + } + + refs, moreDiags := langrefs.ReferencesInExpr(addrs.ParseRef, expr) + diags = diags.Append(moreDiags) + + scope := ctx.EvaluationScope(nil, nil, EvalDataForNoInstanceKey) + var hclCtx *hcl.EvalContext + if scope != nil { + hclCtx, moreDiags = scope.EvalContext(refs) + } else { + // This shouldn't happen in real code, but it can unfortunately arise + // in unit tests due to incompletely-implemented mocks. :( + hclCtx = &hcl.EvalContext{} + } + diags = diags.Append(moreDiags) + + 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(), + Expression: expr, + EvalContext: hclCtx, + }) + 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 042ce190fc50..c86a4eeadff3 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 000000000000..604e31bc321b --- /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" + } +}