stacks: ensure input values for components don't change between plan and apply (#35489)

* stacks: ensure input values for components don't change between plan and apply

* skip ephemeral values

internal/stacks/stackplan/component.go

@@ -65,6 +65,11 @@ type Component struct {
 	// be destroyed.
 	Dependents collections.Set[stackaddrs.AbsComponent]
 
+	// PlannedInputValues and PlannedInputValueMarks are the values that
+	// Terraform has planned to use for input variables in this component.
+	PlannedInputValues     map[addrs.InputVariable]plans.DynamicValue
+	PlannedInputValueMarks map[addrs.InputVariable][]cty.PathValueMarks
+
 	PlannedOutputValues map[addrs.OutputValue]cty.Value
 
 	PlannedChecks *states.CheckResults
@@ -119,6 +124,17 @@ func (c *Component) ForModulesRuntime() (*plans.Plan, error) {
 		}
 	}
 
+	variableValues := make(map[string]plans.DynamicValue, len(c.PlannedInputValues))
+	variableMarks := make(map[string][]cty.PathValueMarks, len(c.PlannedInputValueMarks))
+	for k, v := range c.PlannedInputValues {
+		variableValues[k.Name] = v
+	}
+	plan.VariableValues = variableValues
+	for k, v := range c.PlannedInputValueMarks {
+		variableMarks[k.Name] = v
+	}
+	plan.VariableMarks = variableMarks
+
 	plan.PriorState = priorState
 	plan.PrevRunState = priorState.DeepCopy() // This is just here to complete the data structure; we don't really do anything with it
 

internal/stacks/stackplan/from_proto.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/collections"
+	"github.com/hashicorp/terraform/internal/lang/marks"
 	"github.com/hashicorp/terraform/internal/plans"
 	"github.com/hashicorp/terraform/internal/plans/planfile"
 	"github.com/hashicorp/terraform/internal/plans/planproto"
@@ -153,6 +154,26 @@ func (l *Loader) AddRaw(rawMsg *anypb.Any) error {
 			return fmt.Errorf("decoding mode for %s: %w", addr, err)
 		}
 
+		inputVals := make(map[addrs.InputVariable]plans.DynamicValue)
+		inputValMarks := make(map[addrs.InputVariable][]cty.PathValueMarks)
+		for name, rawVal := range msg.PlannedInputValues {
+			val := addrs.InputVariable{
+				Name: name,
+			}
+			inputVals[val] = rawVal.Value.Msgpack
+			inputValMarks[val] = make([]cty.PathValueMarks, len(rawVal.SensitivePaths))
+			for _, path := range rawVal.SensitivePaths {
+				path, err := planfile.PathFromProto(path)
+				if err != nil {
+					return fmt.Errorf("decoding sensitive path %q for %s: %w", val, addr, err)
+				}
+				inputValMarks[val] = append(inputValMarks[val], cty.PathValueMarks{
+					Path:  path,
+					Marks: cty.NewValueMarks(marks.Sensitive),
+				})
+			}
+		}
+
 		outputVals := make(map[addrs.OutputValue]cty.Value)
 		for name, rawVal := range msg.PlannedOutputValues {
 			v, err := tfstackdata1.DynamicValueFromTFStackData1(rawVal, cty.DynamicPseudoType)
@@ -169,14 +190,16 @@ func (l *Loader) AddRaw(rawMsg *anypb.Any) error {
 
 		if !l.ret.Components.HasKey(addr) {
 			l.ret.Components.Put(addr, &Component{
-				PlannedAction:       plannedAction,
-				Mode:                mode,
-				PlanApplyable:       msg.PlanApplyable,
-				PlanComplete:        msg.PlanComplete,
-				Dependencies:        dependencies,
-				Dependents:          collections.NewSet[stackaddrs.AbsComponent](),
-				PlannedOutputValues: outputVals,
-				PlannedChecks:       checkResults,
+				PlannedAction:          plannedAction,
+				Mode:                   mode,
+				PlanApplyable:          msg.PlanApplyable,
+				PlanComplete:           msg.PlanComplete,
+				Dependencies:           dependencies,
+				Dependents:             collections.NewSet[stackaddrs.AbsComponent](),
+				PlannedInputValues:     inputVals,
+				PlannedInputValueMarks: inputValMarks,
+				PlannedOutputValues:    outputVals,
+				PlannedChecks:          checkResults,
 
 				ResourceInstancePlanned:         addrs.MakeMap[addrs.AbsResourceInstanceObject, *plans.ResourceInstanceChangeSrc](),
 				ResourceInstancePriorState:      addrs.MakeMap[addrs.AbsResourceInstanceObject, *states.ResourceInstanceObjectSrc](),

internal/stacks/stackruntime/apply_test.go

@@ -1707,6 +1707,117 @@ func TestApplyWithStateManipulation(t *testing.T) {
 	}
 }
 
+func TestApplyWithChangedValues(t *testing.T) {
+	ctx := context.Background()
+	cfg := loadMainBundleConfigForTest(t, filepath.Join("with-single-input", "valid"))
+
+	fakePlanTimestamp, err := time.Parse(time.RFC3339, "1991-08-25T20:57:08Z")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	changesCh := make(chan stackplan.PlannedChange)
+	diagsCh := make(chan tfdiags.Diagnostic)
+	lock := depsfile.NewLocks()
+	lock.SetProvider(
+		addrs.NewDefaultProvider("testing"),
+		providerreqs.MustParseVersion("0.0.0"),
+		providerreqs.MustParseVersionConstraints("=0.0.0"),
+		providerreqs.PreferredHashes([]providerreqs.Hash{}),
+	)
+	req := PlanRequest{
+		Config: cfg,
+		ProviderFactories: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("testing"): func() (providers.Interface, error) {
+				return stacks_testing_provider.NewProvider(), nil
+			},
+		},
+		DependencyLocks: *lock,
+
+		ForcePlanTimestamp: &fakePlanTimestamp,
+
+		InputValues: map[stackaddrs.InputVariable]ExternalInputValue{
+			stackaddrs.InputVariable{Name: "input"}: {
+				Value: cty.StringVal("hello"),
+			},
+		},
+	}
+	resp := PlanResponse{
+		PlannedChanges: changesCh,
+		Diagnostics:    diagsCh,
+	}
+
+	go Plan(ctx, &req, &resp)
+	planChanges, diags := collectPlanOutput(changesCh, diagsCh)
+	if len(diags) > 0 {
+		t.Fatalf("expected no diagnostics, got %s", diags.ErrWithWarnings())
+	}
+
+	planLoader := stackplan.NewLoader()
+	for _, change := range planChanges {
+		proto, err := change.PlannedChangeProto()
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		for _, rawMsg := range proto.Raw {
+			err = planLoader.AddRaw(rawMsg)
+			if err != nil {
+				t.Fatal(err)
+			}
+		}
+	}
+	plan, err := planLoader.Plan()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Now deliberately edit the plan so that it'll produce different outputs.
+	plan.RootInputValues[stackaddrs.InputVariable{Name: "input"}] = cty.StringVal("world")
+
+	applyReq := ApplyRequest{
+		Config: cfg,
+		Plan:   plan,
+		ProviderFactories: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("testing"): func() (providers.Interface, error) {
+				return stacks_testing_provider.NewProvider(), nil
+			},
+		},
+		DependencyLocks: *lock,
+	}
+
+	applyChangesCh := make(chan stackstate.AppliedChange)
+	diagsCh = make(chan tfdiags.Diagnostic)
+
+	applyResp := ApplyResponse{
+		AppliedChanges: applyChangesCh,
+		Diagnostics:    diagsCh,
+	}
+
+	go Apply(ctx, &applyReq, &applyResp)
+	_, applyDiags := collectApplyOutput(applyChangesCh, diagsCh)
+	if len(applyDiags) != 1 {
+		t.Fatalf("expected exactly one diagnostic, got %s", applyDiags.ErrWithWarnings())
+	}
+
+	// We don't care about the changes here, just that we got the diagnostic
+	// we expected when we messed the plan up earlier.
+
+	gotSeverity, wantSeverity := applyDiags[0].Severity(), tfdiags.Error
+	gotSummary, wantSummary := applyDiags[0].Description().Summary, "Planned input variable value changed"
+	gotDetail, wantDetail := applyDiags[0].Description().Detail, "The planned value for input variable \"input\" has changed between the planning and apply phases for component.self. This is a bug in Terraform - please report it."
+
+	if gotSeverity != wantSeverity {
+		t.Errorf("expected severity %q, got %q", wantSeverity, gotSeverity)
+	}
+	if gotSummary != wantSummary {
+		t.Errorf("expected summary %q, got %q", wantSummary, gotSummary)
+	}
+	if gotDetail != wantDetail {
+		t.Errorf("expected detail %q, got %q", wantDetail, gotDetail)
+	}
+}
+
 func collectApplyOutput(changesCh <-chan stackstate.AppliedChange, diagsCh <-chan tfdiags.Diagnostic) ([]stackstate.AppliedChange, tfdiags.Diagnostics) {
 	var changes []stackstate.AppliedChange
 	var diags tfdiags.Diagnostics

internal/stacks/stackruntime/internal/stackeval/component_instance.go

@@ -96,12 +96,24 @@ func (c *ComponentInstance) CheckInputVariableValues(ctx context.Context, phase
 // inputValuesForModulesRuntime adapts the result of
 // [ComponentInstance.InputVariableValues] to the representation that the
 // main Terraform modules runtime expects.
-func (c *ComponentInstance) inputValuesForModulesRuntime(ctx context.Context, phase EvalPhase) terraform.InputValues {
+//
+// The second argument (expectedValues) is the value that the apply operation
+// expects to see for the input variables, which is typically the input
+// values from the plan.
+//
+// During the planning phase, the expectedValues should be nil, as they will
+// only be checked during the apply phase.
+func (c *ComponentInstance) inputValuesForModulesRuntime(ctx context.Context, previousValues map[string]plans.DynamicValue, phase EvalPhase) (terraform.InputValues, tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
+
 	valsObj := c.InputVariableValues(ctx, phase)
 	if valsObj == cty.NilVal {
-		return nil
+		return nil, diags
 	}
 
+	// module is the configuration for the root module of this component.
+	module := c.call.Config(ctx).ModuleTree(ctx).Module
+
 	// valsObj might be an unknown value during the planning phase, in which
 	// case we'll return an InputValues with all of the expected variables
 	// defined as unknown values of their expected type constraints. To
@@ -111,7 +123,7 @@ func (c *ComponentInstance) inputValuesForModulesRuntime(ctx context.Context, ph
 	if wantTy == cty.NilType {
 		// The configuration is too invalid for us to know what type we're
 		// expecting, so we'll just bail.
-		return nil
+		return nil, diags
 	}
 	wantAttrs := wantTy.AttributeTypes()
 	ret := make(terraform.InputValues, len(wantAttrs))
@@ -126,8 +138,70 @@ func (c *ComponentInstance) inputValuesForModulesRuntime(ctx context.Context, ph
 			Value:      v,
 			SourceType: terraform.ValueFromCaller,
 		}
+
+		// While we're here, we'll just add a diagnostic if the value has
+		// somehow changed between the planning and apply phases. All of these
+		// diagnostics acknowledge that the root cause here is a bug in
+		// Terraform.
+		if phase == ApplyPhase {
+			config := module.Variables[name]
+			if config.Ephemeral {
+				// Ephemeral variables are allowed to change between the plan
+				// and apply stages, so we won't bother checking them.
+				continue
+			}
+
+			raw, ok := previousValues[name]
+			if !ok {
+				// This shouldn't happen because we should have a value for
+				// every input variable that we have a value for in the plan.
+				diags = diags.Append(&hcl.Diagnostic{
+					Severity: hcl.DiagError,
+					Summary:  "Missing input variable value",
+					Detail: fmt.Sprintf(
+						"The input variable %q is required but was not set in the plan for %s. This is a bug in Terraform - please report it.",
+						name, c.Addr(),
+					),
+					Subject: c.call.Declaration(ctx).DeclRange.ToHCL().Ptr(),
+				})
+				continue
+			}
+
+			plannedValue, err := raw.Decode(cty.DynamicPseudoType)
+			if err != nil {
+				// Then something has gone wrong when decoding the value.
+				diags = diags.Append(&hcl.Diagnostic{
+					Severity: hcl.DiagError,
+					Summary:  "Invalid planned input variable value",
+					Detail:   fmt.Sprintf("Failed to decode the planned value for input variable %q: %s. This is a bug in Terraform - please report it.", name, err),
+					Subject:  c.call.Declaration(ctx).DeclRange.ToHCL().Ptr(),
+				})
+				continue
+			}
+
+			if equals, _ := plannedValue.Equals(v).Unmark(); !equals.IsKnown() {
+				// We unmark the value as we don't care about the actual value,
+				// only whether it was equal or not.
+				//
+				// An unknown equals value means that the value was unknown
+				// during the planning stage so we'll just accept the apply
+				// value and not raise any diagnostics.
+			} else if !equals.True() {
+				// Then the value has changed between the planning and apply
+				// phases. This is a bug in Terraform.
+				diags = diags.Append(&hcl.Diagnostic{
+					Severity: hcl.DiagError,
+					Summary:  "Planned input variable value changed",
+					Detail: fmt.Sprintf(
+						"The planned value for input variable %q has changed between the planning and apply phases for %s. This is a bug in Terraform - please report it.",
+						name, c.Addr(),
+					),
+					Subject: c.call.Declaration(ctx).DeclRange.ToHCL().Ptr(),
+				})
+			}
+		}
 	}
-	return ret
+	return ret, diags
 }
 
 // Providers evaluates the "providers" argument from the component
@@ -539,14 +613,9 @@ func (c *ComponentInstance) CheckModuleTreePlan(ctx context.Context) (*plans.Pla
 			}
 
 			stackPlanOpts := c.main.PlanningOpts()
-			inputValues := c.inputValuesForModulesRuntime(ctx, PlanPhase)
-			if inputValues == nil {
-				// inputValuesForModulesRuntime uses nil (as opposed to a
-				// non-nil zerolen map) to represent that the definition of
-				// the input variables was so invalid that we cannot do
-				// anything with it, in which case we'll just return early
-				// and assume the plan walk driver will find the diagnostics
-				// via another return path.
+			inputValues, inputValueDiags := c.inputValuesForModulesRuntime(ctx, nil, PlanPhase)
+			diags = diags.Append(inputValueDiags)
+			if inputValues == nil || diags.HasErrors() {
 				return nil, diags
 			}
 
@@ -797,8 +866,9 @@ func (c *ComponentInstance) ApplyModuleTreePlan(ctx context.Context, plan *plans
 	// shallow-copy it. This is NOT a deep copy, so don't modify anything
 	// that's reachable through any pointers without copying those first too.
 	modifiedPlan := *plan
-	inputValues := c.inputValuesForModulesRuntime(ctx, ApplyPhase)
-	if inputValues == nil {
+	inputValues, inputValueDiags := c.inputValuesForModulesRuntime(ctx, plan.VariableValues, ApplyPhase)
+	diags = diags.Append(inputValueDiags)
+	if inputValues == nil || inputValueDiags.HasErrors() {
 		// inputValuesForModulesRuntime uses nil (as opposed to a
 		// non-nil zerolen map) to represent that the definition of
 		// the input variables was so invalid that we cannot do