Type Constraints for Output Blocks

.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"

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.

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)

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"},

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 = {}
+}

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)
+}

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"
+}

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)
+		}
+	}
+}

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