stackeval: The naming of promises

If a stack configuration has a dependency cycle we'll tend to detect it
by the promises failing to resolve due to a self-reference. Describing
that situation to end-users has been historically hard in existing
Terraform's dag-based implementation, and there are some similar
challenges for this promise-based design too.

To deal with it while minimizing runtime overhead in the happy path we'll
teach the various objects to retroactively report names for the promises
they've already instantiated and then only when we encounter the
ErrSelfDependent error will we walk the tree of objects to find
user-friendly names for all of the promises that have contributed to the
results so far.

internal/stacks/stackaddrs/component.go

@@ -30,6 +30,13 @@ type ComponentInstance struct {
 func (ComponentInstance) inStackConfigSigil()   {}
 func (ComponentInstance) inStackInstanceSigil() {}
 
+func (c ComponentInstance) String() string {
+	if c.Key == nil {
+		return c.Component.String()
+	}
+	return c.Component.String() + c.Key.String()
+}
+
 // ConfigComponentInstance places a [ComponentInstance] in the context of a
 // particular [Stack].
 type ConfigComponentInstance = InStackConfig[ComponentInstance]

internal/stacks/stackaddrs/in_stack.go

@@ -4,12 +4,14 @@ package stackaddrs
 // sense to consider as belonging statically to a [Stack].
 type StackItemConfig interface {
 	inStackConfigSigil()
+	String() string
 }
 
 // StackItemDynamic is a type set containing all of the address types that make
 // sense to consider as belonging dynamically to a [StackInstance].
 type StackItemDynamic interface {
 	inStackInstanceSigil()
+	String() string
 }
 
 // InStackConfig is the generic form of addresses representing configuration
@@ -27,6 +29,13 @@ func Config[T StackItemConfig](stackAddr Stack, relAddr T) InStackConfig[T] {
 	}
 }
 
+func (ist InStackConfig[T]) String() string {
+	if ist.Stack.IsRoot() {
+		return ist.Item.String()
+	}
+	return ist.Stack.String() + "." + ist.Item.String()
+}
+
 // InStackInstance is the generic form of addresses representing dynamic
 // instances of objects that exist within an instance of a stack.
 type InStackInstance[T StackItemDynamic] struct {
@@ -41,6 +50,13 @@ func Absolute[T StackItemDynamic](stackAddr StackInstance, relAddr T) InStackIns
 	}
 }
 
+func (ist InStackInstance[T]) String() string {
+	if ist.Stack.IsRoot() {
+		return ist.Item.String()
+	}
+	return ist.Stack.String() + "." + ist.Item.String()
+}
+
 // ConfigForAbs returns the "in stack config" equivalent of the given
 // "in stack instance" (absolute) address by just discarding any
 // instance keys from the stack instance steps.

internal/stacks/stackaddrs/provider_config.go

@@ -1,6 +1,8 @@
 package stackaddrs
 
 import (
+	"fmt"
+
 	"github.com/hashicorp/terraform/internal/addrs"
 )
 
@@ -31,6 +33,10 @@ type ProviderConfig struct {
 func (ProviderConfig) inStackConfigSigil()   {}
 func (ProviderConfig) inStackInstanceSigil() {}
 
+func (c ProviderConfig) String() string {
+	return fmt.Sprintf("provider[%q].%s", c.Provider, c.Name)
+}
+
 // ConfigProviderConfig places a [ProviderConfig] in the context of a particular [Stack].
 type ConfigProviderConfig = InStackConfig[ProviderConfig]
 
@@ -48,6 +54,13 @@ type ProviderConfigInstance struct {
 func (ProviderConfigInstance) inStackConfigSigil()   {}
 func (ProviderConfigInstance) inStackInstanceSigil() {}
 
+func (c ProviderConfigInstance) String() string {
+	if c.Key == nil {
+		return c.ProviderConfig.String()
+	}
+	return c.ProviderConfig.String() + c.Key.String()
+}
+
 // ConfigProviderConfigInstance places a [ProviderConfigInstance] in the context of a particular [Stack].
 type ConfigProviderConfigInstance = InStackConfig[ProviderConfigInstance]
 

internal/stacks/stackaddrs/stack.go

@@ -1,6 +1,8 @@
 package stackaddrs
 
 import (
+	"strings"
+
 	"github.com/hashicorp/terraform/internal/addrs"
 )
 
@@ -40,6 +42,25 @@ func (s Stack) Child(name string) Stack {
 	return append(ret, StackStep{name})
 }
 
+func (s Stack) String() string {
+	if s.IsRoot() {
+		// Callers should typically not ask for the string representation of
+		// the main root stack, but we'll return a reasonable placeholder
+		// for situations like e.g. internal logs where we just fmt %s in an
+		// arbitrary stack address that is sometimes the main stack.
+		return "<main>"
+	}
+	var buf strings.Builder
+	for i, step := range s {
+		if i != 0 {
+			buf.WriteByte('.')
+		}
+		buf.WriteString("stack.")
+		buf.WriteString(step.Name)
+	}
+	return buf.String()
+}
+
 // StackInstance represents the address of an instance of a stack within
 // the tree of stacks.
 //
@@ -107,3 +128,25 @@ func (s StackInstance) ConfigAddr() Stack {
 	}
 	return ret
 }
+
+func (s StackInstance) String() string {
+	if s.IsRoot() {
+		// Callers should typically not ask for the string representation of
+		// the main root stack, but we'll return a reasonable placeholder
+		// for situations like e.g. internal logs where we just fmt %s in an
+		// arbitrary stack address that is sometimes the main stack.
+		return "<main>"
+	}
+	var buf strings.Builder
+	for i, step := range s {
+		if i != 0 {
+			buf.WriteByte('.')
+		}
+		buf.WriteString("stack.")
+		buf.WriteString(step.Name)
+		if step.Key != nil {
+			buf.WriteString(step.Key.String())
+		}
+	}
+	return buf.String()
+}

internal/stacks/stackruntime/internal/stackeval/diagnostics.go

@@ -1,8 +1,141 @@
 package stackeval
 
-import "github.com/hashicorp/terraform/internal/tfdiags"
+import (
+	"fmt"
+	"strings"
+
+	"github.com/hashicorp/terraform/internal/promising"
+	"github.com/hashicorp/terraform/internal/tfdiags"
+)
 
 type withDiagnostics[T any] struct {
 	Result      T
 	Diagnostics tfdiags.Diagnostics
 }
+
+// taskSelfDependencyDiagnostics transforms a [promising.ErrSelfDependent]
+// error into one or more error diagnostics suitable for returning to an
+// end user, after first trying to discover user-friendly names for each
+// of the promises involved using the .
+func taskSelfDependencyDiagnostics(err promising.ErrSelfDependent, root namedPromiseReporter) tfdiags.Diagnostics {
+
+	promiseNames := collectPromiseNames(root)
+	distinctPromises := make(map[promising.PromiseID]struct{})
+	for _, id := range err {
+		distinctPromises[id] = struct{}{}
+	}
+
+	var diags tfdiags.Diagnostics
+	switch len(distinctPromises) {
+	case 0:
+		// Should not get here; there can't be a promise cycle without any
+		// promises involved in it.
+		panic("promising.ErrSelfDependent without any promises")
+	case 1:
+		const diagSummary = "Object depends on itself"
+		var promiseID promising.PromiseID
+		for id := range distinctPromises {
+			promiseID = id
+		}
+		name, ok := promiseNames[promiseID]
+		if ok {
+			diags = diags.Append(tfdiags.Sourceless(
+				tfdiags.Error,
+				diagSummary,
+				fmt.Sprintf("The object %s depends on its own results, so there is no correct order of operations.", name),
+			))
+		} else {
+			// This is the worst case to report, since something depended on
+			// itself but we don't actually know its name. We can't really say
+			// anything useful here, so we'll treat this as a bug and then
+			// we can add whatever promise name was missing in order to fix
+			// that bug.
+			diags = diags.Append(tfdiags.Sourceless(
+				tfdiags.Error,
+				diagSummary,
+				"One of the objects in your configuration refers to its own results, but Terraform was not able to detect which one. The fact that Terraform cannot name the object is a bug; please report it!",
+			))
+		}
+	default:
+		// If we have more than one promise involved then it's non-deterministic
+		// which one we'll detect, since it depends on how the tasks get
+		// scheduled by the Go runtime. To return a deterministic-ish result
+		// anyway we'll arbitrarily descide to report whichever promise has
+		// the lexically-least name as defined by Go's own less than operator
+		// when applied to strings.
+		selectedIdx := 0
+		selectedName := promiseNames[err[0]]
+		for i, id := range err {
+			if selectedName == "" {
+				// If we don't have a name yet then we'll take whatever we get
+				selectedIdx = i
+				selectedName = promiseNames[id]
+				continue
+			}
+			candidateName := promiseNames[id]
+			if candidateName != "" && candidateName < selectedName {
+				selectedIdx = i
+				selectedName = candidateName
+			}
+		}
+		// Now we'll rotate the list of promise IDs so that the one we selected
+		// appears first.
+		ids := make([]promising.PromiseID, 0, len(err))
+		ids = append(ids, err[selectedIdx:]...)
+		ids = append(ids, err[:selectedIdx]...)
+		var nameList strings.Builder
+		for _, id := range ids {
+			name := promiseNames[id]
+			if name == "" {
+				// We should minimize the number of unnamed promises so that
+				// we can typically say at least something useful about what
+				// objects are involved.
+				name = "(...)"
+			}
+			fmt.Fprintf(&nameList, "\n  - %s", name)
+		}
+		diags = diags.Append(tfdiags.Sourceless(
+			tfdiags.Error,
+			"Objects depend on themselves",
+			fmt.Sprintf(
+				"The following objects in your configuration form a circular dependency chain through their references:%s\n\nTerraform uses references to decide a suitable order for visiting objects, so objects may not refer to their own results either directly or indirectly.",
+				nameList.String(),
+			),
+		))
+
+	}
+	return diags
+}
+
+// namedPromiseReporter is an interface implemented by the types in this
+// package that perform asynchronous work using the promises model implemented
+// by package promising, allowing discovery of user-friendly names for promises
+// involved in a particular operation.
+//
+// We handle this as an out-of-band action so we can avoid the overhead of
+// maintaining this metadata in the common case, and instead deal with it
+// retroactively only in the rare case that there's a self-dependency problem
+// that exhibits as a promise resolution error.
+type namedPromiseReporter interface {
+	// reportNamedPromises calls the given callback for each promise that
+	// the caller is responsible for, giving a user-friendly name for
+	// whatever data or action that promise was responsible for.
+	//
+	// reportNamedPromises should also delegate to the same method on any
+	// directly-nested objects that might themselves have promises, so that
+	// collectPromiseNames can walk the whole tree. This should be done only
+	// in situations where the original reciever's implementation is itself
+	// acting as the physical container for the child objects, and not just
+	// when an object is _logically_ nested within another object.
+	reportNamedPromises(func(id promising.PromiseID, name string))
+}
+
+func collectPromiseNames(r namedPromiseReporter) map[promising.PromiseID]string {
+	ret := make(map[promising.PromiseID]string)
+	r.reportNamedPromises(func(id promising.PromiseID, name string) {
+		if id != promising.NoPromise {
+			ret[id] = name
+		}
+	})
+	return ret
+}

internal/stacks/stackruntime/internal/stackeval/input_variable_config.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/ext/typeexpr"
+	"github.com/hashicorp/terraform/internal/promising"
 	"github.com/hashicorp/terraform/internal/stacks/stackaddrs"
 	"github.com/hashicorp/terraform/internal/stacks/stackconfig"
 	"github.com/hashicorp/terraform/internal/tfdiags"
@@ -23,6 +24,7 @@ type InputVariableConfig struct {
 
 var _ Validatable = (*InputVariableConfig)(nil)
 var _ Referenceable = (*InputVariableConfig)(nil)
+var _ namedPromiseReporter = (*InputVariableConfig)(nil)
 
 func newInputVariableConfig(main *Main, addr stackaddrs.ConfigInputVariable, config *stackconfig.InputVariable) *InputVariableConfig {
 	return &InputVariableConfig{
@@ -133,3 +135,8 @@ func (v *InputVariableConfig) Validate(ctx context.Context) tfdiags.Diagnostics
 	)
 	return diags
 }
+
+// reportNamedPromises implements namedPromiseReporter.
+func (s *InputVariableConfig) reportNamedPromises(cb func(id promising.PromiseID, name string)) {
+	// Nothing to report yet
+}

internal/stacks/stackruntime/internal/stackeval/main.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"sync"
 
+	"github.com/hashicorp/terraform/internal/promising"
 	"github.com/hashicorp/terraform/internal/stacks/stackaddrs"
 	"github.com/hashicorp/terraform/internal/stacks/stackconfig"
 )
@@ -29,6 +30,8 @@ type Main struct {
 	mainStackConfig *StackConfig
 }
 
+var _ namedPromiseReporter = (*Main)(nil)
+
 type mainPlanning struct {
 	opts PlanOpts
 }
@@ -84,3 +87,12 @@ func (m *Main) StackCallConfig(ctx context.Context, addr stackaddrs.ConfigStackC
 	}
 	return caller.StackCall(ctx, addr.Item)
 }
+
+// reportNamedPromises implements namedPromiseReporter.
+func (m *Main) reportNamedPromises(cb func(id promising.PromiseID, name string)) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	if m.mainStackConfig != nil {
+		m.mainStackConfig.reportNamedPromises(cb)
+	}
+}

internal/stacks/stackruntime/internal/stackeval/stack_call_config.go

@@ -25,8 +25,9 @@ type StackCallConfig struct {
 	inputVariableValues promising.Once[withDiagnostics[map[stackaddrs.InputVariable]cty.Value]]
 }
 
-var _ Validatable = (*InputVariableConfig)(nil)
+var _ Validatable = (*StackCallConfig)(nil)
 var _ ExpressionScope = (*StackCallConfig)(nil)
+var _ namedPromiseReporter = (*StackCallConfig)(nil)
 
 func newStackCallConfig(main *Main, addr stackaddrs.ConfigStackCall, config *stackconfig.EmbeddedStack) *StackCallConfig {
 	return &StackCallConfig{
@@ -199,3 +200,8 @@ func (s *StackCallConfig) Validate(ctx context.Context) tfdiags.Diagnostics {
 	)
 	return diags
 }
+
+// reportNamedPromises implements namedPromiseReporter.
+func (s *StackCallConfig) reportNamedPromises(cb func(id promising.PromiseID, name string)) {
+	cb(s.inputVariableValues.PromiseID(), s.Addr().String()+" inputs")
+}

internal/stacks/stackruntime/internal/stackeval/stack_config.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/terraform/internal/instances"
+	"github.com/hashicorp/terraform/internal/promising"
 	"github.com/hashicorp/terraform/internal/stacks/stackaddrs"
 	"github.com/hashicorp/terraform/internal/stacks/stackconfig"
 	"github.com/hashicorp/terraform/internal/tfdiags"
@@ -34,6 +35,7 @@ type StackConfig struct {
 }
 
 var _ ExpressionScope = (*StackConfig)(nil)
+var _ namedPromiseReporter = (*StackConfig)(nil)
 
 func newStackConfig(main *Main, addr stackaddrs.Stack, config *stackconfig.ConfigNode) *StackConfig {
 	return &StackConfig{
@@ -179,3 +181,19 @@ func (s *StackConfig) resolveExpressionReference(ctx context.Context, ref stacka
 		return nil, diags
 	}
 }
+
+// reportNamedPromises implements namedPromiseReporter.
+func (s *StackConfig) reportNamedPromises(cb func(id promising.PromiseID, name string)) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	for _, child := range s.children {
+		child.reportNamedPromises(cb)
+	}
+	for _, child := range s.inputVariables {
+		child.reportNamedPromises(cb)
+	}
+	for _, child := range s.stackCalls {
+		child.reportNamedPromises(cb)
+	}
+}