core: Eval pre/postconditions in refresh-only mode

Evaluate precondition and postcondition blocks in refresh-only mode, but
report any failures as warnings instead of errors. This ensures that any
deviation from the contract defined by condition blocks is reported as
early as possible, without preventing the completion of a state refresh
operation.

Prior to this commit, Terraform evaluated output preconditions and data
source pre/postconditions as normal in refresh-only mode, while managed
resource pre/postconditions were not evaluated at all. This omission
could lead to confusing partial condition errors, or failure to detect
undesired changes which would otherwise cause resources to become
invalid.

Reporting the failures as errors also meant that changes retrieved
during refresh could cause the refresh operation to fail. This is also
undesirable, as the primary purpose of the operation is to update local
state. Precondition/postcondition checks are still valuable here, but
should be informative rather than blocking.
This commit is contained in:
Alisdair McDiarmid 2022-03-09 16:04:01 -05:00
parent 2ee64dc7e0
commit a103c65140
10 changed files with 357 additions and 42 deletions

View File

@ -2283,6 +2283,34 @@ resource "test_resource" "a" {
}
})
t.Run("precondition fail refresh-only", func(t *testing.T) {
state := states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(mustResourceInstanceAddr("test_resource.a"), &states.ResourceInstanceObjectSrc{
AttrsJSON: []byte(`{"value":"boop","output":"blorp"}`),
Status: states.ObjectReady,
}, mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`))
})
_, diags := ctx.Plan(m, state, &PlanOpts{
Mode: plans.RefreshOnlyMode,
SetVariables: InputValues{
"boop": &InputValue{
Value: cty.StringVal("nope"),
SourceType: ValueFromCLIArg,
},
},
})
assertNoErrors(t, diags)
if len(diags) == 0 {
t.Fatalf("no diags, but should have warnings")
}
if got, want := diags.ErrWithWarnings().Error(), "Resource precondition failed: Wrong boop."; got != want {
t.Fatalf("wrong warning:\ngot: %s\nwant: %q", got, want)
}
if !p.ReadResourceCalled {
t.Errorf("Provider's ReadResource wasn't called; should've been")
}
})
t.Run("postcondition fail", func(t *testing.T) {
p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) (resp providers.PlanResourceChangeResponse) {
m := req.ProposedNewState.AsValueMap()
@ -2308,7 +2336,108 @@ resource "test_resource" "a" {
t.Fatalf("wrong error:\ngot: %s\nwant: %q", got, want)
}
if !p.PlanResourceChangeCalled {
t.Errorf("Provider's PlanResourceChangeCalled wasn't called; should've been")
t.Errorf("Provider's PlanResourceChange wasn't called; should've been")
}
})
t.Run("postcondition fail refresh-only", func(t *testing.T) {
state := states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(mustResourceInstanceAddr("test_resource.a"), &states.ResourceInstanceObjectSrc{
AttrsJSON: []byte(`{"value":"boop","output":"blorp"}`),
Status: states.ObjectReady,
}, mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`))
})
p.ReadResourceFn = func(req providers.ReadResourceRequest) (resp providers.ReadResourceResponse) {
newVal, err := cty.Transform(req.PriorState, func(path cty.Path, v cty.Value) (cty.Value, error) {
if len(path) == 1 && path[0] == (cty.GetAttrStep{Name: "output"}) {
return cty.StringVal(""), nil
}
return v, nil
})
if err != nil {
// shouldn't get here
t.Fatalf("ReadResourceFn transform failed")
return providers.ReadResourceResponse{}
}
return providers.ReadResourceResponse{
NewState: newVal,
}
}
_, diags := ctx.Plan(m, state, &PlanOpts{
Mode: plans.RefreshOnlyMode,
SetVariables: InputValues{
"boop": &InputValue{
Value: cty.StringVal("boop"),
SourceType: ValueFromCLIArg,
},
},
})
assertNoErrors(t, diags)
if len(diags) == 0 {
t.Fatalf("no diags, but should have warnings")
}
if got, want := diags.ErrWithWarnings().Error(), "Resource postcondition failed: Output must not be blank."; got != want {
t.Fatalf("wrong warning:\ngot: %s\nwant: %q", got, want)
}
if !p.ReadResourceCalled {
t.Errorf("Provider's ReadResource wasn't called; should've been")
}
if p.PlanResourceChangeCalled {
t.Errorf("Provider's PlanResourceChange was called; should'nt've been")
}
})
t.Run("precondition and postcondition fail refresh-only", func(t *testing.T) {
state := states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(mustResourceInstanceAddr("test_resource.a"), &states.ResourceInstanceObjectSrc{
AttrsJSON: []byte(`{"value":"boop","output":"blorp"}`),
Status: states.ObjectReady,
}, mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`))
})
p.ReadResourceFn = func(req providers.ReadResourceRequest) (resp providers.ReadResourceResponse) {
newVal, err := cty.Transform(req.PriorState, func(path cty.Path, v cty.Value) (cty.Value, error) {
if len(path) == 1 && path[0] == (cty.GetAttrStep{Name: "output"}) {
return cty.StringVal(""), nil
}
return v, nil
})
if err != nil {
// shouldn't get here
t.Fatalf("ReadResourceFn transform failed")
return providers.ReadResourceResponse{}
}
return providers.ReadResourceResponse{
NewState: newVal,
}
}
_, diags := ctx.Plan(m, state, &PlanOpts{
Mode: plans.RefreshOnlyMode,
SetVariables: InputValues{
"boop": &InputValue{
Value: cty.StringVal("nope"),
SourceType: ValueFromCLIArg,
},
},
})
assertNoErrors(t, diags)
if got, want := len(diags), 2; got != want {
t.Errorf("wrong number of warnings, got %d, want %d", got, want)
}
warnings := diags.ErrWithWarnings().Error()
wantWarnings := []string{
"Resource precondition failed: Wrong boop.",
"Resource postcondition failed: Output must not be blank.",
}
for _, want := range wantWarnings {
if !strings.Contains(warnings, want) {
t.Errorf("missing warning:\ngot: %s\nwant to contain: %q", warnings, want)
}
}
if !p.ReadResourceCalled {
t.Errorf("Provider's ReadResource wasn't called; should've been")
}
if p.PlanResourceChangeCalled {
t.Errorf("Provider's PlanResourceChange was called; should'nt've been")
}
})
}
@ -2432,6 +2561,39 @@ resource "test_resource" "a" {
}
})
t.Run("precondition fail refresh-only", func(t *testing.T) {
plan, diags := ctx.Plan(m, states.NewState(), &PlanOpts{
Mode: plans.RefreshOnlyMode,
SetVariables: InputValues{
"boop": &InputValue{
Value: cty.StringVal("nope"),
SourceType: ValueFromCLIArg,
},
},
})
assertNoErrors(t, diags)
if len(diags) == 0 {
t.Fatalf("no diags, but should have warnings")
}
if got, want := diags.ErrWithWarnings().Error(), "Resource precondition failed: Wrong boop."; got != want {
t.Fatalf("wrong warning:\ngot: %s\nwant: %q", got, want)
}
for _, res := range plan.Changes.Resources {
switch res.Addr.String() {
case "test_resource.a":
if res.Action != plans.Create {
t.Fatalf("unexpected %s change for %s", res.Action, res.Addr)
}
case "data.test_data_source.a":
if res.Action != plans.Read {
t.Fatalf("unexpected %s change for %s", res.Action, res.Addr)
}
default:
t.Fatalf("unexpected %s change for %s", res.Action, res.Addr)
}
}
})
t.Run("postcondition fail", func(t *testing.T) {
p.ReadDataSourceResponse = &providers.ReadDataSourceResponse{
State: cty.ObjectVal(map[string]cty.Value{
@ -2458,6 +2620,60 @@ resource "test_resource" "a" {
t.Errorf("Provider's ReadDataSource wasn't called; should've been")
}
})
t.Run("postcondition fail refresh-only", func(t *testing.T) {
p.ReadDataSourceResponse = &providers.ReadDataSourceResponse{
State: cty.ObjectVal(map[string]cty.Value{
"foo": cty.StringVal("boop"),
"results": cty.ListValEmpty(cty.String),
}),
}
_, diags := ctx.Plan(m, states.NewState(), &PlanOpts{
Mode: plans.RefreshOnlyMode,
SetVariables: InputValues{
"boop": &InputValue{
Value: cty.StringVal("boop"),
SourceType: ValueFromCLIArg,
},
},
})
assertNoErrors(t, diags)
if got, want := diags.ErrWithWarnings().Error(), "Resource postcondition failed: Results cannot be empty."; got != want {
t.Fatalf("wrong error:\ngot: %s\nwant: %q", got, want)
}
})
t.Run("precondition and postcondition fail refresh-only", func(t *testing.T) {
p.ReadDataSourceResponse = &providers.ReadDataSourceResponse{
State: cty.ObjectVal(map[string]cty.Value{
"foo": cty.StringVal("nope"),
"results": cty.ListValEmpty(cty.String),
}),
}
_, diags := ctx.Plan(m, states.NewState(), &PlanOpts{
Mode: plans.RefreshOnlyMode,
SetVariables: InputValues{
"boop": &InputValue{
Value: cty.StringVal("nope"),
SourceType: ValueFromCLIArg,
},
},
})
assertNoErrors(t, diags)
if got, want := len(diags), 2; got != want {
t.Errorf("wrong number of warnings, got %d, want %d", got, want)
}
warnings := diags.ErrWithWarnings().Error()
wantWarnings := []string{
"Resource precondition failed: Wrong boop.",
"Resource postcondition failed: Results cannot be empty.",
}
for _, want := range wantWarnings {
if !strings.Contains(warnings, want) {
t.Errorf("missing warning:\ngot: %s\nwant to contain: %q", warnings, want)
}
}
})
}
func TestContext2Plan_outputPrecondition(t *testing.T) {
@ -2530,6 +2746,36 @@ output "a" {
t.Fatalf("wrong error:\ngot: %s\nwant: %q", got, want)
}
})
t.Run("condition fail refresh-only", func(t *testing.T) {
plan, diags := ctx.Plan(m, states.NewState(), &PlanOpts{
Mode: plans.RefreshOnlyMode,
SetVariables: InputValues{
"boop": &InputValue{
Value: cty.StringVal("nope"),
SourceType: ValueFromCLIArg,
},
},
})
assertNoErrors(t, diags)
if len(diags) == 0 {
t.Fatalf("no diags, but should have warnings")
}
if got, want := diags.ErrWithWarnings().Error(), "Module output value precondition failed: Wrong boop."; got != want {
t.Errorf("wrong warning:\ngot: %s\nwant: %q", got, want)
}
addr := addrs.RootModuleInstance.OutputValue("a")
outputPlan := plan.Changes.OutputValue(addr)
if outputPlan == nil {
t.Fatalf("no plan for %s at all", addr)
}
if got, want := outputPlan.Addr, addr; !got.Equal(want) {
t.Errorf("wrong current address\ngot: %s\nwant: %s", got, want)
}
if got, want := outputPlan.Action, plans.Create; got != want {
t.Errorf("wrong planned action\ngot: %s\nwant: %s", got, want)
}
})
}
func TestContext2Plan_preconditionErrors(t *testing.T) {

View File

@ -48,12 +48,14 @@ func (c checkType) FailureSummary() string {
//
// If any of the rules do not pass, the returned diagnostics will contain
// errors. Otherwise, it will either be empty or contain only warnings.
func evalCheckRules(typ checkType, rules []*configs.CheckRule, ctx EvalContext, self addrs.Referenceable, keyData instances.RepetitionData) (diags tfdiags.Diagnostics) {
func evalCheckRules(typ checkType, rules []*configs.CheckRule, ctx EvalContext, self addrs.Referenceable, keyData instances.RepetitionData, diagSeverity tfdiags.Severity) (diags tfdiags.Diagnostics) {
if len(rules) == 0 {
// Nothing to do
return nil
}
severity := diagSeverity.ToHCL()
for _, rule := range rules {
const errInvalidCondition = "Invalid condition result"
var ruleDiags tfdiags.Diagnostics
@ -85,7 +87,7 @@ func evalCheckRules(typ checkType, rules []*configs.CheckRule, ctx EvalContext,
}
if result.IsNull() {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Severity: severity,
Summary: errInvalidCondition,
Detail: "Condition expression must return either true or false, not null.",
Subject: rule.Condition.Range().Ptr(),
@ -98,7 +100,7 @@ func evalCheckRules(typ checkType, rules []*configs.CheckRule, ctx EvalContext,
result, err = convert.Convert(result, cty.Bool)
if err != nil {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Severity: severity,
Summary: errInvalidCondition,
Detail: fmt.Sprintf("Invalid condition result value: %s.", tfdiags.FormatError(err)),
Subject: rule.Condition.Range().Ptr(),
@ -118,7 +120,7 @@ func evalCheckRules(typ checkType, rules []*configs.CheckRule, ctx EvalContext,
errorValue, err = convert.Convert(errorValue, cty.String)
if err != nil {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Severity: severity,
Summary: "Invalid error message",
Detail: fmt.Sprintf("Unsuitable value for error message: %s.", tfdiags.FormatError(err)),
Subject: rule.ErrorMessage.Range().Ptr(),
@ -133,7 +135,7 @@ func evalCheckRules(typ checkType, rules []*configs.CheckRule, ctx EvalContext,
errorMessage = "Failed to evaluate condition error message."
}
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Severity: severity,
Summary: typ.FailureSummary(),
Detail: errorMessage,
Subject: rule.Condition.Range().Ptr(),

View File

@ -99,7 +99,7 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer {
&RootVariableTransformer{Config: b.Config, RawValues: b.RootVariableValues},
&ModuleVariableTransformer{Config: b.Config},
&LocalTransformer{Config: b.Config},
&OutputTransformer{Config: b.Config},
&OutputTransformer{Config: b.Config, RefreshOnly: b.skipPlanChanges},
// Add orphan resources
&OrphanResourceInstanceTransformer{

View File

@ -24,6 +24,7 @@ type nodeExpandOutput struct {
Config *configs.Output
Changes []*plans.OutputChangeSrc
Destroy bool
RefreshOnly bool
}
var (
@ -69,6 +70,7 @@ func (n *nodeExpandOutput) DynamicExpand(ctx EvalContext) (*Graph, error) {
Addr: absAddr,
Config: n.Config,
Change: change,
RefreshOnly: n.RefreshOnly,
}
log.Printf("[TRACE] Expanding output: adding %s as %T", o.Addr.String(), o)
g.Add(o)
@ -157,6 +159,10 @@ type NodeApplyableOutput struct {
Config *configs.Output // Config is the output in the config
// If this is being evaluated during apply, we may have a change recorded already
Change *plans.OutputChangeSrc
// Refresh-only mode means that any failing output preconditions are
// reported as warnings rather than errors
RefreshOnly bool
}
var (
@ -270,10 +276,15 @@ func (n *NodeApplyableOutput) Execute(ctx EvalContext, op walkOperation) (diags
}
}
checkRuleSeverity := tfdiags.Error
if n.RefreshOnly {
checkRuleSeverity = tfdiags.Warning
}
checkDiags := evalCheckRules(
checkOutputPrecondition,
n.Config.Preconditions,
ctx, nil, EvalDataForNoInstanceKey,
checkRuleSeverity,
)
diags = diags.Append(checkDiags)
if diags.HasErrors() {
@ -285,7 +296,10 @@ func (n *NodeApplyableOutput) Execute(ctx EvalContext, op walkOperation) (diags
if !changeRecorded || !val.IsWhollyKnown() {
// This has to run before we have a state lock, since evaluation also
// reads the state
val, diags = ctx.EvaluateExpr(n.Config.Expr, cty.DynamicPseudoType, nil)
var evalDiags tfdiags.Diagnostics
val, evalDiags = ctx.EvaluateExpr(n.Config.Expr, cty.DynamicPseudoType, nil)
diags = diags.Append(evalDiags)
// We'll handle errors below, after we have loaded the module.
// Outputs don't have a separate mode for validation, so validate
// depends_on expressions here too

View File

@ -655,6 +655,7 @@ func (n *NodeAbstractResourceInstance) plan(
checkResourcePrecondition,
n.Config.Preconditions,
ctx, nil, keyData,
tfdiags.Error,
)
diags = diags.Append(checkDiags)
if diags.HasErrors() {
@ -1476,7 +1477,7 @@ func (n *NodeAbstractResourceInstance) providerMetas(ctx EvalContext) (cty.Value
// value, but it still matches the previous state, then we can record a NoNop
// change. If the states don't match then we record a Read change so that the
// new value is applied to the state.
func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, currentState *states.ResourceInstanceObject) (*plans.ResourceInstanceChange, *states.ResourceInstanceObject, instances.RepetitionData, tfdiags.Diagnostics) {
func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, currentState *states.ResourceInstanceObject, checkRuleSeverity tfdiags.Severity) (*plans.ResourceInstanceChange, *states.ResourceInstanceObject, instances.RepetitionData, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics
var keyData instances.RepetitionData
var configVal cty.Value
@ -1510,6 +1511,7 @@ func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, currentSt
checkResourcePrecondition,
n.Config.Preconditions,
ctx, nil, keyData,
checkRuleSeverity,
)
diags = diags.Append(checkDiags)
if diags.HasErrors() {
@ -1689,6 +1691,7 @@ func (n *NodeAbstractResourceInstance) applyDataSource(ctx EvalContext, planned
checkResourcePrecondition,
n.Config.Preconditions,
ctx, nil, keyData,
tfdiags.Error,
)
diags = diags.Append(checkDiags)
if diags.HasErrors() {

View File

@ -184,6 +184,7 @@ func (n *NodeApplyableResourceInstance) dataResourceExecute(ctx EvalContext) (di
n.Config.Postconditions,
ctx, n.ResourceInstanceAddr().Resource,
repeatData,
tfdiags.Error,
)
diags = diags.Append(checkDiags)
@ -361,6 +362,7 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext)
checkResourcePostcondition,
n.Config.Postconditions,
ctx, addr, repeatData,
tfdiags.Error,
)
diags = diags.Append(checkDiags)

View File

@ -95,7 +95,12 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) (di
return diags
}
change, state, repeatData, planDiags := n.planDataSource(ctx, state)
checkRuleSeverity := tfdiags.Error
if n.skipPlanChanges {
checkRuleSeverity = tfdiags.Warning
}
change, state, repeatData, planDiags := n.planDataSource(ctx, state, checkRuleSeverity)
diags = diags.Append(planDiags)
if diags.HasErrors() {
return diags
@ -122,6 +127,7 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) (di
checkResourcePostcondition,
n.Config.Postconditions,
ctx, addr.Resource, repeatData,
checkRuleSeverity,
)
diags = diags.Append(checkDiags)
@ -263,9 +269,28 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext)
checkResourcePostcondition,
n.Config.Postconditions,
ctx, addr.Resource, repeatData,
tfdiags.Error,
)
diags = diags.Append(checkDiags)
} else {
// In refresh-only mode we need to evaluate the for-each expression in
// order to supply the value to the pre- and post-condition check
// blocks. This has the unfortunate edge case of a refresh-only plan
// executing with a for-each map which has the same keys but different
// values, which could result in a post-condition check relying on that
// value being inaccurate. Unless we decide to store the value of the
// for-each expression in state, this is unavoidable.
forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx)
repeatData := EvalDataForInstanceKey(n.ResourceInstanceAddr().Resource.Key, forEach)
checkDiags := evalCheckRules(
checkResourcePrecondition,
n.Config.Preconditions,
ctx, nil, repeatData,
tfdiags.Warning,
)
diags = diags.Append(checkDiags)
// Even if we don't plan changes, we do still need to at least update
// the working state to reflect the refresh result. If not, then e.g.
// any output values refering to this will not react to the drift.
@ -275,6 +300,19 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext)
if diags.HasErrors() {
return diags
}
// Here we also evaluate post-conditions after updating the working
// state, because we want to check against the result of the refresh.
// Unlike in normal planning mode, these checks are still evaluated
// even if pre-conditions generated diagnostics, because we have no
// planned changes to block.
checkDiags = evalCheckRules(
checkResourcePostcondition,
n.Config.Postconditions,
ctx, addr.Resource, repeatData,
tfdiags.Warning,
)
diags = diags.Append(checkDiags)
}
return diags

View File

@ -19,9 +19,13 @@ type OutputTransformer struct {
Config *configs.Config
Changes *plans.Changes
// if this is a planed destroy, root outputs are still in the configuration
// If this is a planned destroy, root outputs are still in the configuration
// so we need to record that we wish to remove them
Destroy bool
// Refresh-only mode means that any failing output preconditions are
// reported as warnings rather than errors
RefreshOnly bool
}
func (t *OutputTransformer) Transform(g *Graph) error {
@ -83,6 +87,7 @@ func (t *OutputTransformer) transform(g *Graph, c *configs.Config) error {
Addr: addr.Absolute(addrs.RootModuleInstance),
Config: o,
Change: rootChange,
RefreshOnly: t.RefreshOnly,
}
default:
@ -92,6 +97,7 @@ func (t *OutputTransformer) transform(g *Graph, c *configs.Config) error {
Config: o,
Changes: changes,
Destroy: t.Destroy,
RefreshOnly: t.RefreshOnly,
}
}

View File

@ -1,6 +1,8 @@
package tfdiags
import (
"fmt"
"github.com/hashicorp/hcl/v2"
)
@ -24,6 +26,20 @@ const (
Warning Severity = 'W'
)
// ToHCL converts a Severity to the equivalent HCL diagnostic severity.
func (s Severity) ToHCL() hcl.DiagnosticSeverity {
switch s {
case Warning:
return hcl.DiagWarning
case Error:
return hcl.DiagError
default:
// The above should always be exhaustive for all of the valid
// Severity values in this package.
panic(fmt.Sprintf("unknown diagnostic severity %s", s))
}
}
type Description struct {
Address string
Summary string

View File

@ -1,8 +1,6 @@
package tfdiags
import (
"fmt"
"github.com/hashicorp/hcl/v2"
)
@ -112,17 +110,7 @@ func (diags Diagnostics) ToHCL() hcl.Diagnostics {
hclDiag := &hcl.Diagnostic{
Summary: desc.Summary,
Detail: desc.Detail,
}
switch severity {
case Warning:
hclDiag.Severity = hcl.DiagWarning
case Error:
hclDiag.Severity = hcl.DiagError
default:
// The above should always be exhaustive for all of the valid
// Severity values in this package.
panic(fmt.Sprintf("unknown diagnostic severity %s", severity))
Severity: severity.ToHCL(),
}
if source.Subject != nil {
hclDiag.Subject = source.Subject.ToHCL().Ptr()