Merge pull request #30658 from hashicorp/alisdair/preconditions-postconditions-refresh-only

core: Eval pre/postconditions in refresh-only mode
This commit is contained in:
Alisdair McDiarmid 2022-03-11 13:44:51 -05:00 committed by GitHub
commit 6cd0876596
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
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

@ -19,11 +19,12 @@ import (
// nodeExpandOutput is the placeholder for a non-root module output that has
// not yet had its module path expanded.
type nodeExpandOutput struct {
Addr addrs.OutputValue
Module addrs.Module
Config *configs.Output
Changes []*plans.OutputChangeSrc
Destroy bool
Addr addrs.OutputValue
Module addrs.Module
Config *configs.Output
Changes []*plans.OutputChangeSrc
Destroy bool
RefreshOnly bool
}
var (
@ -66,9 +67,10 @@ func (n *nodeExpandOutput) DynamicExpand(ctx EvalContext) (*Graph, error) {
}
o := &NodeApplyableOutput{
Addr: absAddr,
Config: n.Config,
Change: change,
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 {
@ -80,18 +84,20 @@ func (t *OutputTransformer) transform(g *Graph, c *configs.Config) error {
case c.Path.IsRoot():
node = &NodeApplyableOutput{
Addr: addr.Absolute(addrs.RootModuleInstance),
Config: o,
Change: rootChange,
Addr: addr.Absolute(addrs.RootModuleInstance),
Config: o,
Change: rootChange,
RefreshOnly: t.RefreshOnly,
}
default:
node = &nodeExpandOutput{
Addr: addr,
Module: c.Path,
Config: o,
Changes: changes,
Destroy: t.Destroy,
Addr: addr,
Module: c.Path,
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"
)
@ -110,19 +108,9 @@ func (diags Diagnostics) ToHCL() hcl.Diagnostics {
fromExpr := diag.FromExpr()
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))
Summary: desc.Summary,
Detail: desc.Detail,
Severity: severity.ToHCL(),
}
if source.Subject != nil {
hclDiag.Subject = source.Subject.ToHCL().Ptr()