diff --git a/internal/stacks/stackplan/component.go b/internal/stacks/stackplan/component.go index 025f0ef922..0801304a4d 100644 --- a/internal/stacks/stackplan/component.go +++ b/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 diff --git a/internal/stacks/stackplan/from_proto.go b/internal/stacks/stackplan/from_proto.go index 1f95deac42..9a3b7bf222 100644 --- a/internal/stacks/stackplan/from_proto.go +++ b/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](), diff --git a/internal/stacks/stackruntime/apply_test.go b/internal/stacks/stackruntime/apply_test.go index 0a2e5915eb..659717ba95 100644 --- a/internal/stacks/stackruntime/apply_test.go +++ b/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 diff --git a/internal/stacks/stackruntime/internal/stackeval/component_instance.go b/internal/stacks/stackruntime/internal/stackeval/component_instance.go index 03cd0822ed..17e3eccd09 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component_instance.go +++ b/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