stacks: don't validate input variables during the apply phase (#35561)

docs/re-reformat-templatestring-funct
Liam Cervante 2 years ago committed by GitHub
parent 1d770d605e
commit bb73fb1d69
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -1707,132 +1707,6 @@ func TestApplyWithStateManipulation(t *testing.T) {
}
}
func TestApplyWithChangedComponentValues(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)
applyChanges, applyDiags := collectApplyOutput(applyChangesCh, diagsCh)
if len(applyDiags) != 1 {
t.Fatalf("expected exactly one diagnostic, got %s", applyDiags.ErrWithWarnings())
}
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)
}
wantChanges := []stackstate.AppliedChange{
&stackstate.AppliedChangeComponentInstance{
ComponentAddr: mustAbsComponent("component.self"),
ComponentInstanceAddr: mustAbsComponentInstance("component.self"),
OutputValues: make(map[addrs.OutputValue]cty.Value),
},
// no resources should have been created because the input variable was
// invalid.
}
sort.SliceStable(applyChanges, func(i, j int) bool {
return appliedChangeSortKey(applyChanges[i]) < appliedChangeSortKey(applyChanges[j])
})
if diff := cmp.Diff(wantChanges, applyChanges, ctydebug.CmpOptions, cmpCollectionsSet); diff != "" {
t.Errorf("wrong changes\n%s", diff)
}
}
func TestApplyWithChangedInputValues(t *testing.T) {
ctx := context.Background()
cfg := loadMainBundleConfigForTest(t, filepath.Join("with-single-input", "valid"))
@ -1927,7 +1801,7 @@ func TestApplyWithChangedInputValues(t *testing.T) {
go Apply(ctx, &applyReq, &applyResp)
applyChanges, applyDiags := collectApplyOutput(applyChangesCh, diagsCh)
if len(applyDiags) != 2 {
if len(applyDiags) != 1 {
t.Fatalf("expected exactly two diagnostics, got %s", applyDiags.ErrWithWarnings())
}
@ -1937,10 +1811,7 @@ func TestApplyWithChangedInputValues(t *testing.T) {
tfdiags.Error,
"Inconsistent value for input variable during apply",
"The value for non-ephemeral input variable \"input\" was set to a different value during apply than was set during plan. Only ephemeral input variables can change between the plan and apply phases."),
expectDiagnostic(
tfdiags.Error,
"Planned input variable value changed",
"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."))
)
wantChanges := []stackstate.AppliedChange{
&stackstate.AppliedChangeComponentInstance{

@ -17,7 +17,6 @@ import (
"github.com/hashicorp/terraform/internal/instances"
"github.com/hashicorp/terraform/internal/lang/marks"
"github.com/hashicorp/terraform/internal/plans"
"github.com/hashicorp/terraform/internal/plans/objchange"
"github.com/hashicorp/terraform/internal/promising"
"github.com/hashicorp/terraform/internal/providers"
"github.com/hashicorp/terraform/internal/stacks/stackaddrs"
@ -91,7 +90,20 @@ func (c *ComponentInstance) CheckInputVariableValues(ctx context.Context, phase
// We actually checked the errors statically already, so we only care about
// the value here.
return EvalComponentInputVariables(ctx, varDecls, wantTy, defs, decl, phase, c)
val, diags := EvalComponentInputVariables(ctx, varDecls, wantTy, defs, decl, phase, c)
if phase == ApplyPhase {
if !val.IsWhollyKnown() {
// We can't apply a configuration that has unknown values in it.
// This means an error has occured somewhere else, while gathering
// the input variables. We return a nil value here, whatever caused
// the error should have raised an error diagnostic separately.
return cty.NilVal, diags
}
// Note, that unknown values during the planning phase are totally fine.
}
return val, diags
}
// inputValuesForModulesRuntime adapts the result of
@ -104,17 +116,12 @@ func (c *ComponentInstance) CheckInputVariableValues(ctx context.Context, phase
//
// 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
func (c *ComponentInstance) inputValuesForModulesRuntime(ctx context.Context, phase EvalPhase) terraform.InputValues {
valsObj := c.InputVariableValues(ctx, phase)
if valsObj == cty.NilVal {
return nil, diags
return nil
}
// 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
@ -124,7 +131,7 @@ func (c *ComponentInstance) inputValuesForModulesRuntime(ctx context.Context, pr
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, diags
return nil
}
wantAttrs := wantTy.AttributeTypes()
ret := make(terraform.InputValues, len(wantAttrs))
@ -139,71 +146,8 @@ func (c *ComponentInstance) inputValuesForModulesRuntime(ctx context.Context, pr
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
}
// We're unmarking the value here so that we can compare it to the
// planned value. We're only checking for equality from here on out,
// so we don't need to worry about the marks.
applyValue, _ := v.UnmarkDeep()
if errs := objchange.AssertValueCompatible(plannedValue, applyValue); len(errs) > 0 {
// Then the value has changed between the planning and apply
// phases. This is a bug in Terraform. We don't want to expose
// the actual values here as they could contain sensitive
// information. This should be a rare error message, so we
// don't need to be too verbose.
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, diags
return ret
}
// Providers evaluates the "providers" argument from the component
@ -615,8 +559,7 @@ func (c *ComponentInstance) CheckModuleTreePlan(ctx context.Context) (*plans.Pla
}
stackPlanOpts := c.main.PlanningOpts()
inputValues, inputValueDiags := c.inputValuesForModulesRuntime(ctx, nil, PlanPhase)
diags = diags.Append(inputValueDiags)
inputValues := c.inputValuesForModulesRuntime(ctx, PlanPhase)
if inputValues == nil || diags.HasErrors() {
return nil, diags
}
@ -868,9 +811,8 @@ 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, inputValueDiags := c.inputValuesForModulesRuntime(ctx, plan.VariableValues, ApplyPhase)
diags = diags.Append(inputValueDiags)
if inputValues == nil || inputValueDiags.HasErrors() {
inputValues := c.inputValuesForModulesRuntime(ctx, ApplyPhase)
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

Loading…
Cancel
Save