start to refactor EvalReadData

Remove extra fields, remove the depends_on logic from
NodePlannableResourceInstnace, and start breaking up the massive Eval
method.
This commit is contained in:
James Bardin 2020-05-06 18:04:09 -04:00
parent be20a7941d
commit 4a92b7888f
4 changed files with 149 additions and 199 deletions

View File

@ -34,66 +34,60 @@ type EvalReadData struct {
// in this planned change. // in this planned change.
Planned **plans.ResourceInstanceChange Planned **plans.ResourceInstanceChange
// ForcePlanRead, if true, overrides the usual behavior of immediately // State is the current state for the data source, and is updated once the
// reading from the data source where possible, instead forcing us to // new state has need read.
// _always_ generate a plan. This is used during the plan walk, since we // While data source are read-only, we need to start with the prior state
// mustn't actually apply anything there. (The resulting state doesn't // to determine if we have a change or not. If we needed to read a new
// get persisted) // value, but it still matches the previous state, then we can record a
ForcePlanRead *bool // NoNop change. If the states don't match then we record a Read change so
// that the new value is applied to the state.
State **states.ResourceInstanceObject
// The result from this EvalNode has a few different possibilities // The result from this EvalNode has a few different possibilities
// depending on the input: // depending on the input:
// - If Planned is nil then we assume we're aiming to _produce_ the plan, // - If Planned is nil then we assume we're aiming to either read the
// and so the following two outcomes are possible: // resource or produce a plan, and so the following two outcomes are
// - OutputChange.Action is plans.NoOp and OutputState is the complete // possible:
// result of reading from the data source. This is the easy path. // - OutputChange.Action is plans.NoOp and the
// - OutputChange.Action is plans.Read and OutputState is a planned // result of reading from the data source is stored in state. This is
// the easy path, and only happens during refresh.
// - OutputChange.Action is plans.Read and State is a planned
// object placeholder (states.ObjectPlanned). In this case, the // object placeholder (states.ObjectPlanned). In this case, the
// returned change must be recorded in the overral changeset and // returned change must be recorded in the overall changeset and this
// eventually passed to another instance of this struct during the // resource will be read during apply.
// apply walk. // - OutputChange.Action is plans.Update, in which case the change
// contains the complete state of this resource, and only needs to be
// stored into the final state during apply.
// - If Planned is non-nil then we assume we're aiming to complete a // - If Planned is non-nil then we assume we're aiming to complete a
// planned read from an earlier plan walk. In this case the only possible // planned read from an earlier plan walk, from one of the options above.
// non-error outcome is to set Output.Action (if non-nil) to a plans.NoOp OutputChange **plans.ResourceInstanceChange
// change and put the complete resulting state in OutputState, ready to
// be saved in the overall state and used for expression evaluation. // dependsOn stores the list of transitive resource addresses that any
// // configuration depends_on references may resolve to. This is used to
// FIXME: these fields are a mess. OutputValue is getting the config passed // determine if there are any changes that will force this data sources to
// in, OutputState is passed in as well, and OuputValue is replaced with // be deferred to apply.
// the state value which goes in OutputState. dependsOn []addrs.ConfigResource
OutputChange **plans.ResourceInstanceChange
OutputValue *cty.Value // refresh indicates this is being called from a refresh node, and we can't
OutputConfigValue *cty.Value // resolve any depends_on dependencies.
OutputState **states.ResourceInstanceObject refresh bool
} }
func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) {
state := *n.OutputState
absAddr := n.Addr.Absolute(ctx.Path()) absAddr := n.Addr.Absolute(ctx.Path())
var diags tfdiags.Diagnostics
var configVal cty.Value
var planned *plans.ResourceInstanceChange var planned *plans.ResourceInstanceChange
if n.Planned != nil { if n.Planned != nil {
planned = *n.Planned planned = *n.Planned
} }
forcePlanRead := false
if n.ForcePlanRead != nil {
forcePlanRead = *n.ForcePlanRead
}
if n.ProviderSchema == nil || *n.ProviderSchema == nil { if n.ProviderSchema == nil || *n.ProviderSchema == nil {
return nil, fmt.Errorf("provider schema not available for %s", n.Addr) return nil, fmt.Errorf("provider schema not available for %s", n.Addr)
} }
var diags tfdiags.Diagnostics
var configVal cty.Value
// TODO: Do we need to handle Delete changes here? EvalReadDataDiff and
// EvalReadDataApply did, but it seems like we should handle that via a
// separate mechanism since it boils down to just deleting the object from
// the state... and we do that on every plan anyway, forcing the data
// resource to re-read.
config := *n.Config config := *n.Config
provider := *n.Provider provider := *n.Provider
providerSchema := *n.ProviderSchema providerSchema := *n.ProviderSchema
@ -103,19 +97,13 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) {
return nil, fmt.Errorf("provider %q does not support data source %q", n.ProviderAddr.Provider.String(), n.Addr.Resource.Type) return nil, fmt.Errorf("provider %q does not support data source %q", n.ProviderAddr.Provider.String(), n.Addr.Resource.Type)
} }
// While data source are read-only, and don't necessarily use the prior
// state, we record it here and use it to determine if we have a change or
// not. If we needed to read a new 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.
objTy := schema.ImpliedType() objTy := schema.ImpliedType()
priorVal := cty.NullVal(objTy) priorVal := cty.NullVal(objTy)
if state != nil { if n.State != nil && *n.State != nil {
priorVal = state.Value priorVal = (*n.State).Value
} }
forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx) forEach, _ := evaluateForEachExpression(config.ForEach, ctx)
keyData := EvalDataForInstanceKey(n.Addr.Key, forEach) keyData := EvalDataForInstanceKey(n.Addr.Key, forEach)
var configDiags tfdiags.Diagnostics var configDiags tfdiags.Diagnostics
@ -125,50 +113,26 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) {
return nil, diags.Err() return nil, diags.Err()
} }
metaConfigVal := cty.NullVal(cty.DynamicPseudoType) metaConfigVal, metaDiags := n.providerMetas(ctx)
if n.ProviderMetas != nil { diags = diags.Append(metaDiags)
if m, ok := n.ProviderMetas[n.ProviderAddr.Provider]; ok && m != nil { if diags.HasErrors() {
// if the provider doesn't support this feature, throw an error return nil, diags.Err()
if (*n.ProviderSchema).ProviderMeta == nil {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: fmt.Sprintf("Provider %s doesn't support provider_meta", n.ProviderAddr.Provider.String()),
Detail: fmt.Sprintf("The resource %s belongs to a provider that doesn't support provider_meta blocks", n.Addr),
Subject: &m.ProviderRange,
})
} else {
var configDiags tfdiags.Diagnostics
metaConfigVal, _, configDiags = ctx.EvaluateBlock(m.Config, (*n.ProviderSchema).ProviderMeta, nil, EvalDataForNoInstanceKey)
diags = diags.Append(configDiags)
if configDiags.HasErrors() {
return nil, diags.Err()
}
}
}
} }
proposedNewVal := objchange.PlannedDataResourceObject(schema, configVal)
configKnown := configVal.IsWhollyKnown() configKnown := configVal.IsWhollyKnown()
// If our configuration contains any unknown values then we must defer the // If our configuration contains any unknown values, or we depend on any
// read to the apply phase by producing a "Read" change for this resource, // unknown values then we must defer the read to the apply phase by
// and a placeholder value for it in the state. // producing a "Read" change for this resource, and a placeholder value for
if forcePlanRead || !configKnown { // it in the state.
// If the configuration is still unknown when we're applying a planned if n.forcePlanRead(ctx) || !configKnown {
// change then that indicates a bug in Terraform, since we should have
// everything resolved by now.
if planned != nil {
return nil, fmt.Errorf(
"configuration for %s still contains unknown values during apply (this is a bug in Terraform; please report it!)",
absAddr,
)
}
if configKnown { if configKnown {
log.Printf("[TRACE] EvalReadData: %s configuration is fully known, but we're forcing a read plan to be created", absAddr) log.Printf("[TRACE] EvalReadData: %s configuration is fully known, but we're forcing a read plan to be created", absAddr)
} else { } else {
log.Printf("[TRACE] EvalReadData: %s configuration not fully known yet, so deferring to apply phase", absAddr) log.Printf("[TRACE] EvalReadData: %s configuration not fully known yet, so deferring to apply phase", absAddr)
} }
proposedNewVal := objchange.PlannedDataResourceObject(schema, configVal)
err := ctx.Hook(func(h Hook) (HookAction, error) { err := ctx.Hook(func(h Hook) (HookAction, error) {
return h.PreDiff(absAddr, states.CurrentGen, priorVal, proposedNewVal) return h.PreDiff(absAddr, states.CurrentGen, priorVal, proposedNewVal)
}) })
@ -196,63 +160,47 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) {
if n.OutputChange != nil { if n.OutputChange != nil {
*n.OutputChange = change *n.OutputChange = change
} }
if n.OutputValue != nil { if n.State != nil {
*n.OutputValue = change.After *n.State = &states.ResourceInstanceObject{
}
if n.OutputConfigValue != nil {
*n.OutputConfigValue = configVal
}
if n.OutputState != nil {
state := &states.ResourceInstanceObject{
Value: cty.NullVal(objTy), Value: cty.NullVal(objTy),
Status: states.ObjectPlanned, Status: states.ObjectPlanned,
} }
*n.OutputState = state
} }
return nil, diags.ErrWithWarnings() return nil, diags.ErrWithWarnings()
} }
if planned != nil { if planned != nil && !(planned.Action == plans.Read || planned.Action == plans.Update) {
if !(planned.Action == plans.Read || planned.Action == plans.Update) { // If any other action gets in here then that's always a bug; this
// If any other action gets in here then that's always a bug; this // EvalNode only deals with reading.
// EvalNode only deals with reading. return nil, fmt.Errorf(
return nil, fmt.Errorf( "invalid action %s for %s: only Read or Update is supported (this is a bug in Terraform; please report it!)",
"invalid action %s for %s: only Read or Update is supported (this is a bug in Terraform; please report it!)", planned.Action, absAddr,
planned.Action, absAddr, )
) }
// we have a change and it is complete, which means we read the data
// source during plan and only need to store it in state.
if planned != nil && planned.Action == plans.Update {
outputState := &states.ResourceInstanceObject{
Value: planned.After,
Status: states.ObjectReady,
} }
// we have a change and it is complete, which means we read the data err := ctx.Hook(func(h Hook) (HookAction, error) {
// source during plan. return h.PostApply(absAddr, states.CurrentGen, planned.After, nil)
if planned.Action == plans.Update { })
state = &states.ResourceInstanceObject{ if err != nil {
Value: planned.After, return nil, err
Status: states.ObjectReady,
}
err := ctx.Hook(func(h Hook) (HookAction, error) {
return h.PostRefresh(absAddr, states.CurrentGen, planned.Before, planned.After)
})
if err != nil {
return nil, err
}
if n.OutputChange != nil {
*n.OutputChange = planned
}
if n.OutputValue != nil {
*n.OutputValue = planned.After
}
if n.OutputConfigValue != nil {
*n.OutputConfigValue = configVal
}
if n.OutputState != nil {
*n.OutputState = state
}
return nil, diags.ErrWithWarnings()
} }
if n.OutputChange != nil {
*n.OutputChange = planned
}
if n.State != nil {
*n.State = outputState
}
return nil, diags.ErrWithWarnings()
} }
var change *plans.ResourceInstanceChange var change *plans.ResourceInstanceChange
@ -265,7 +213,7 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) {
}, },
) )
if validateResp.Diagnostics.HasErrors() { if validateResp.Diagnostics.HasErrors() {
return nil, validateResp.Diagnostics.InConfigBody(n.Config.Config).Err() return nil, validateResp.Diagnostics.InConfigBody(config.Config).Err()
} }
// If we get down here then our configuration is complete and we're read // If we get down here then our configuration is complete and we're read
@ -275,7 +223,7 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) {
err := ctx.Hook(func(h Hook) (HookAction, error) { err := ctx.Hook(func(h Hook) (HookAction, error) {
// We don't have a state yet, so we'll just give the hook an // We don't have a state yet, so we'll just give the hook an
// empty one to work with. // empty one to work with.
return h.PreRefresh(absAddr, states.CurrentGen, cty.NullVal(cty.DynamicPseudoType)) return h.PreRefresh(absAddr, states.CurrentGen, priorVal)
}) })
if err != nil { if err != nil {
return nil, err return nil, err
@ -286,7 +234,7 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) {
Config: configVal, Config: configVal,
ProviderMeta: metaConfigVal, ProviderMeta: metaConfigVal,
}) })
diags = diags.Append(resp.Diagnostics.InConfigBody(n.Config.Config)) diags = diags.Append(resp.Diagnostics.InConfigBody(config.Config))
if diags.HasErrors() { if diags.HasErrors() {
return nil, diags.Err() return nil, diags.Err()
} }
@ -343,7 +291,8 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) {
action := plans.NoOp action := plans.NoOp
if !newVal.IsNull() && newVal.IsKnown() && newVal.Equals(priorVal).False() { if !newVal.IsNull() && newVal.IsKnown() && newVal.Equals(priorVal).False() {
// FIXME: for now we are abusing Update to mean "apply this new value" // since a data source is read-only, update here only means that we
// need to update the state.
action = plans.Update action = plans.Update
} }
@ -358,13 +307,13 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) {
}, },
} }
state = &states.ResourceInstanceObject{ outputState := &states.ResourceInstanceObject{
Value: change.After, Value: newVal,
Status: states.ObjectReady, // because we completed the read from the provider Status: states.ObjectReady, // because we completed the read from the provider
} }
err = ctx.Hook(func(h Hook) (HookAction, error) { err = ctx.Hook(func(h Hook) (HookAction, error) {
return h.PostRefresh(absAddr, states.CurrentGen, change.Before, newVal) return h.PostRefresh(absAddr, states.CurrentGen, priorVal, newVal)
}) })
if err != nil { if err != nil {
return nil, err return nil, err
@ -373,15 +322,56 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) {
if n.OutputChange != nil { if n.OutputChange != nil {
*n.OutputChange = change *n.OutputChange = change
} }
if n.OutputValue != nil { if n.State != nil {
*n.OutputValue = change.After *n.State = outputState
}
if n.OutputConfigValue != nil {
*n.OutputConfigValue = configVal
}
if n.OutputState != nil {
*n.OutputState = state
} }
return nil, diags.ErrWithWarnings() return nil, diags.ErrWithWarnings()
} }
func (n *EvalReadData) providerMetas(ctx EvalContext) (cty.Value, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics
metaConfigVal := cty.NullVal(cty.DynamicPseudoType)
if n.ProviderMetas != nil {
if m, ok := n.ProviderMetas[n.ProviderAddr.Provider]; ok && m != nil {
// if the provider doesn't support this feature, throw an error
if (*n.ProviderSchema).ProviderMeta == nil {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: fmt.Sprintf("Provider %s doesn't support provider_meta", n.ProviderAddr.Provider.String()),
Detail: fmt.Sprintf("The resource %s belongs to a provider that doesn't support provider_meta blocks", n.Addr),
Subject: &m.ProviderRange,
})
} else {
var configDiags tfdiags.Diagnostics
metaConfigVal, _, configDiags = ctx.EvaluateBlock(m.Config, (*n.ProviderSchema).ProviderMeta, nil, EvalDataForNoInstanceKey)
diags = diags.Append(configDiags)
}
}
}
return metaConfigVal, diags
}
// ForcePlanRead, if true, overrides the usual behavior of immediately
// reading from the data source where possible, instead forcing us to
// _always_ generate a plan. This is used during the plan walk, since we
// mustn't actually apply anything there. (The resulting state doesn't
// get persisted)
func (n *EvalReadData) forcePlanRead(ctx EvalContext) bool {
if n.refresh && len(n.Config.DependsOn) > 0 {
return true
}
// Check and see if any depends_on dependencies have
// changes, since they won't show up as changes in the
// configuration.
changes := ctx.Changes()
for _, d := range n.dependsOn {
for _, change := range changes.GetConfigResourceChanges(d) {
if change != nil && change.Action != plans.NoOp {
return true
}
}
}
return false
}

View File

@ -7,7 +7,6 @@ import (
"github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/providers"
"github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/states"
"github.com/hashicorp/terraform/tfdiags" "github.com/hashicorp/terraform/tfdiags"
"github.com/zclconf/go-cty/cty"
) )
type nodeExpandRefreshableDataResource struct { type nodeExpandRefreshableDataResource struct {
@ -195,7 +194,6 @@ func (n *NodeRefreshableDataResourceInstance) EvalTree() EvalNode {
var providerSchema *ProviderSchema var providerSchema *ProviderSchema
var change *plans.ResourceInstanceChange var change *plans.ResourceInstanceChange
var state *states.ResourceInstanceObject var state *states.ResourceInstanceObject
var configVal cty.Value
return &EvalSequence{ return &EvalSequence{
Nodes: []EvalNode{ Nodes: []EvalNode{
@ -216,15 +214,15 @@ func (n *NodeRefreshableDataResourceInstance) EvalTree() EvalNode {
// generate an incomplete planned object if the configuration // generate an incomplete planned object if the configuration
// includes values that won't be known until apply. // includes values that won't be known until apply.
&EvalReadData{ &EvalReadData{
Addr: addr.Resource, Addr: addr.Resource,
Config: n.Config, Config: n.Config,
Provider: &provider, Provider: &provider,
ProviderAddr: n.ResolvedProvider, ProviderAddr: n.ResolvedProvider,
ProviderMetas: n.ProviderMetas, ProviderMetas: n.ProviderMetas,
ProviderSchema: &providerSchema, ProviderSchema: &providerSchema,
OutputChange: &change, OutputChange: &change,
OutputConfigValue: &configVal, State: &state,
OutputState: &state, refresh: true,
}, },
&EvalIf{ &EvalIf{

View File

@ -3,8 +3,6 @@ package terraform
import ( import (
"fmt" "fmt"
"github.com/zclconf/go-cty/cty"
"github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/addrs"
"github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/configs"
"github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/plans"
@ -182,7 +180,7 @@ func (n *NodeApplyableResourceInstance) evalTreeDataResource(addr addrs.AbsResou
ProviderAddr: n.ResolvedProvider, ProviderAddr: n.ResolvedProvider,
ProviderMetas: n.ProviderMetas, ProviderMetas: n.ProviderMetas,
ProviderSchema: &providerSchema, ProviderSchema: &providerSchema,
OutputState: &state, State: &state,
}, },
&EvalWriteState{ &EvalWriteState{
@ -215,7 +213,6 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe
var err error var err error
var createNew bool var createNew bool
var createBeforeDestroyEnabled bool var createBeforeDestroyEnabled bool
var configVal cty.Value
var deposedKey states.DeposedKey var deposedKey states.DeposedKey
return &EvalSequence{ return &EvalSequence{
@ -293,7 +290,6 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe
State: &state, State: &state,
PreviousDiff: &diff, PreviousDiff: &diff,
OutputChange: &diffApply, OutputChange: &diffApply,
OutputValue: &configVal,
OutputState: &state, OutputState: &state,
}, },

View File

@ -8,7 +8,6 @@ import (
"github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/states"
"github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/addrs"
"github.com/zclconf/go-cty/cty"
) )
// NodePlannableResourceInstance represents a _single_ resource // NodePlannableResourceInstance represents a _single_ resource
@ -51,9 +50,6 @@ func (n *NodePlannableResourceInstance) evalTreeDataResource(addr addrs.AbsResou
var providerSchema *ProviderSchema var providerSchema *ProviderSchema
var change *plans.ResourceInstanceChange var change *plans.ResourceInstanceChange
var state *states.ResourceInstanceObject var state *states.ResourceInstanceObject
var configVal cty.Value
forcePlanRead := new(bool)
return &EvalSequence{ return &EvalSequence{
Nodes: []EvalNode{ Nodes: []EvalNode{
@ -71,34 +67,6 @@ func (n *NodePlannableResourceInstance) evalTreeDataResource(addr addrs.AbsResou
Output: &state, Output: &state,
}, },
// If we already have a non-planned state then we already dealt
// with this during the refresh walk and so we have nothing to do
// here.
&EvalIf{
If: func(ctx EvalContext) (bool, error) {
depChanges := false
// Check and see if any depends_on dependencies have
// changes, since they won't show up as changes in the
// configuration.
changes := ctx.Changes()
depChanges = func() bool {
for _, d := range n.dependsOn {
for _, change := range changes.GetConfigResourceChanges(d) {
if change != nil && change.Action != plans.NoOp {
return true
}
}
}
return false
}()
*forcePlanRead = depChanges
return true, nil
},
Then: EvalNoop{},
},
&EvalValidateSelfRef{ &EvalValidateSelfRef{
Addr: addr.Resource, Addr: addr.Resource,
Config: config.Config, Config: config.Config,
@ -112,10 +80,9 @@ func (n *NodePlannableResourceInstance) evalTreeDataResource(addr addrs.AbsResou
ProviderAddr: n.ResolvedProvider, ProviderAddr: n.ResolvedProvider,
ProviderMetas: n.ProviderMetas, ProviderMetas: n.ProviderMetas,
ProviderSchema: &providerSchema, ProviderSchema: &providerSchema,
ForcePlanRead: forcePlanRead,
OutputChange: &change, OutputChange: &change,
OutputValue: &configVal, State: &state,
OutputState: &state, dependsOn: n.dependsOn,
}, },
&EvalWriteState{ &EvalWriteState{
@ -153,8 +120,7 @@ func (n *NodePlannableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe
Addr: addr.Resource, Addr: addr.Resource,
Provider: &provider, Provider: &provider,
ProviderSchema: &providerSchema, ProviderSchema: &providerSchema,
Output: &state,
Output: &state,
}, },
&EvalValidateSelfRef{ &EvalValidateSelfRef{