diff --git a/terraform/eval_read_data.go b/terraform/eval_read_data.go index 0d852b6f1..6219f695b 100644 --- a/terraform/eval_read_data.go +++ b/terraform/eval_read_data.go @@ -34,66 +34,60 @@ type EvalReadData struct { // in this planned change. Planned **plans.ResourceInstanceChange - // 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) - ForcePlanRead *bool + // State is the current state for the data source, and is updated once the + // new state has need read. + // While data source are read-only, we need to start with the prior state + // 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. + State **states.ResourceInstanceObject // The result from this EvalNode has a few different possibilities // depending on the input: - // - If Planned is nil then we assume we're aiming to _produce_ the plan, - // and so the following two outcomes are possible: - // - OutputChange.Action is plans.NoOp and OutputState is the complete - // result of reading from the data source. This is the easy path. - // - OutputChange.Action is plans.Read and OutputState is a planned + // - If Planned is nil then we assume we're aiming to either read the + // resource or produce a plan, and so the following two outcomes are + // possible: + // - OutputChange.Action is plans.NoOp and the + // 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 - // returned change must be recorded in the overral changeset and - // eventually passed to another instance of this struct during the - // apply walk. + // returned change must be recorded in the overall changeset and this + // resource will be read during apply. + // - 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 - // planned read from an earlier plan walk. In this case the only possible - // non-error outcome is to set Output.Action (if non-nil) to a plans.NoOp - // change and put the complete resulting state in OutputState, ready to - // be saved in the overall state and used for expression evaluation. - // - // FIXME: these fields are a mess. OutputValue is getting the config passed - // in, OutputState is passed in as well, and OuputValue is replaced with - // the state value which goes in OutputState. - OutputChange **plans.ResourceInstanceChange - OutputValue *cty.Value - OutputConfigValue *cty.Value - OutputState **states.ResourceInstanceObject + // planned read from an earlier plan walk, from one of the options above. + OutputChange **plans.ResourceInstanceChange + + // dependsOn stores the list of transitive resource addresses that any + // configuration depends_on references may resolve to. This is used to + // determine if there are any changes that will force this data sources to + // be deferred to apply. + dependsOn []addrs.ConfigResource + + // refresh indicates this is being called from a refresh node, and we can't + // resolve any depends_on dependencies. + refresh bool } func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { - state := *n.OutputState absAddr := n.Addr.Absolute(ctx.Path()) + var diags tfdiags.Diagnostics + var configVal cty.Value + var planned *plans.ResourceInstanceChange if n.Planned != nil { planned = *n.Planned } - forcePlanRead := false - if n.ForcePlanRead != nil { - forcePlanRead = *n.ForcePlanRead - } - if n.ProviderSchema == nil || *n.ProviderSchema == nil { 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 provider := *n.Provider 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) } - // 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() priorVal := cty.NullVal(objTy) - if state != nil { - priorVal = state.Value + if n.State != nil && *n.State != nil { + priorVal = (*n.State).Value } - forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx) + forEach, _ := evaluateForEachExpression(config.ForEach, ctx) keyData := EvalDataForInstanceKey(n.Addr.Key, forEach) var configDiags tfdiags.Diagnostics @@ -125,50 +113,26 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { return nil, diags.Err() } - 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) - if configDiags.HasErrors() { - return nil, diags.Err() - } - } - } + metaConfigVal, metaDiags := n.providerMetas(ctx) + diags = diags.Append(metaDiags) + if diags.HasErrors() { + return nil, diags.Err() } - proposedNewVal := objchange.PlannedDataResourceObject(schema, configVal) - configKnown := configVal.IsWhollyKnown() - // If our configuration contains any unknown values then we must defer the - // read to the apply phase by producing a "Read" change for this resource, - // and a placeholder value for it in the state. - if forcePlanRead || !configKnown { - // If the configuration is still unknown when we're applying a planned - // 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 our configuration contains any unknown values, or we depend on any + // unknown values then we must defer the read to the apply phase by + // producing a "Read" change for this resource, and a placeholder value for + // it in the state. + if n.forcePlanRead(ctx) || !configKnown { if configKnown { log.Printf("[TRACE] EvalReadData: %s configuration is fully known, but we're forcing a read plan to be created", absAddr) } else { 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) { return h.PreDiff(absAddr, states.CurrentGen, priorVal, proposedNewVal) }) @@ -196,63 +160,47 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { if n.OutputChange != nil { *n.OutputChange = change } - if n.OutputValue != nil { - *n.OutputValue = change.After - } - if n.OutputConfigValue != nil { - *n.OutputConfigValue = configVal - } - - if n.OutputState != nil { - state := &states.ResourceInstanceObject{ + if n.State != nil { + *n.State = &states.ResourceInstanceObject{ Value: cty.NullVal(objTy), Status: states.ObjectPlanned, } - *n.OutputState = state } return nil, diags.ErrWithWarnings() } - if planned != nil { - if !(planned.Action == plans.Read || planned.Action == plans.Update) { - // If any other action gets in here then that's always a bug; this - // EvalNode only deals with reading. - return nil, fmt.Errorf( - "invalid action %s for %s: only Read or Update is supported (this is a bug in Terraform; please report it!)", - planned.Action, absAddr, - ) + if planned != nil && !(planned.Action == plans.Read || planned.Action == plans.Update) { + // If any other action gets in here then that's always a bug; this + // EvalNode only deals with reading. + return nil, fmt.Errorf( + "invalid action %s for %s: only Read or Update is supported (this is a bug in Terraform; please report it!)", + 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 - // source during plan. - if planned.Action == plans.Update { - state = &states.ResourceInstanceObject{ - Value: planned.After, - 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() + err := ctx.Hook(func(h Hook) (HookAction, error) { + return h.PostApply(absAddr, states.CurrentGen, planned.After, nil) + }) + if err != nil { + return nil, err } + + if n.OutputChange != nil { + *n.OutputChange = planned + } + if n.State != nil { + *n.State = outputState + } + return nil, diags.ErrWithWarnings() } var change *plans.ResourceInstanceChange @@ -265,7 +213,7 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { }, ) 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 @@ -275,7 +223,7 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { err := ctx.Hook(func(h Hook) (HookAction, error) { // We don't have a state yet, so we'll just give the hook an // 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 { return nil, err @@ -286,7 +234,7 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { Config: configVal, ProviderMeta: metaConfigVal, }) - diags = diags.Append(resp.Diagnostics.InConfigBody(n.Config.Config)) + diags = diags.Append(resp.Diagnostics.InConfigBody(config.Config)) if diags.HasErrors() { return nil, diags.Err() } @@ -343,7 +291,8 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { action := plans.NoOp 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 } @@ -358,13 +307,13 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { }, } - state = &states.ResourceInstanceObject{ - Value: change.After, + outputState := &states.ResourceInstanceObject{ + Value: newVal, Status: states.ObjectReady, // because we completed the read from the provider } 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 { return nil, err @@ -373,15 +322,56 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { if n.OutputChange != nil { *n.OutputChange = change } - if n.OutputValue != nil { - *n.OutputValue = change.After - } - if n.OutputConfigValue != nil { - *n.OutputConfigValue = configVal - } - if n.OutputState != nil { - *n.OutputState = state + if n.State != nil { + *n.State = outputState } 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 +} diff --git a/terraform/node_data_refresh.go b/terraform/node_data_refresh.go index 57b3a91b1..a3e37bf3e 100644 --- a/terraform/node_data_refresh.go +++ b/terraform/node_data_refresh.go @@ -7,7 +7,6 @@ import ( "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/tfdiags" - "github.com/zclconf/go-cty/cty" ) type nodeExpandRefreshableDataResource struct { @@ -195,7 +194,6 @@ func (n *NodeRefreshableDataResourceInstance) EvalTree() EvalNode { var providerSchema *ProviderSchema var change *plans.ResourceInstanceChange var state *states.ResourceInstanceObject - var configVal cty.Value return &EvalSequence{ Nodes: []EvalNode{ @@ -216,15 +214,15 @@ func (n *NodeRefreshableDataResourceInstance) EvalTree() EvalNode { // generate an incomplete planned object if the configuration // includes values that won't be known until apply. &EvalReadData{ - Addr: addr.Resource, - Config: n.Config, - Provider: &provider, - ProviderAddr: n.ResolvedProvider, - ProviderMetas: n.ProviderMetas, - ProviderSchema: &providerSchema, - OutputChange: &change, - OutputConfigValue: &configVal, - OutputState: &state, + Addr: addr.Resource, + Config: n.Config, + Provider: &provider, + ProviderAddr: n.ResolvedProvider, + ProviderMetas: n.ProviderMetas, + ProviderSchema: &providerSchema, + OutputChange: &change, + State: &state, + refresh: true, }, &EvalIf{ diff --git a/terraform/node_resource_apply_instance.go b/terraform/node_resource_apply_instance.go index 2a55f23a7..9c2dfbb27 100644 --- a/terraform/node_resource_apply_instance.go +++ b/terraform/node_resource_apply_instance.go @@ -3,8 +3,6 @@ package terraform import ( "fmt" - "github.com/zclconf/go-cty/cty" - "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/plans" @@ -182,7 +180,7 @@ func (n *NodeApplyableResourceInstance) evalTreeDataResource(addr addrs.AbsResou ProviderAddr: n.ResolvedProvider, ProviderMetas: n.ProviderMetas, ProviderSchema: &providerSchema, - OutputState: &state, + State: &state, }, &EvalWriteState{ @@ -215,7 +213,6 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe var err error var createNew bool var createBeforeDestroyEnabled bool - var configVal cty.Value var deposedKey states.DeposedKey return &EvalSequence{ @@ -293,7 +290,6 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe State: &state, PreviousDiff: &diff, OutputChange: &diffApply, - OutputValue: &configVal, OutputState: &state, }, diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index 7102ded3c..c12de1eec 100644 --- a/terraform/node_resource_plan_instance.go +++ b/terraform/node_resource_plan_instance.go @@ -8,7 +8,6 @@ import ( "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/addrs" - "github.com/zclconf/go-cty/cty" ) // NodePlannableResourceInstance represents a _single_ resource @@ -51,9 +50,6 @@ func (n *NodePlannableResourceInstance) evalTreeDataResource(addr addrs.AbsResou var providerSchema *ProviderSchema var change *plans.ResourceInstanceChange var state *states.ResourceInstanceObject - var configVal cty.Value - - forcePlanRead := new(bool) return &EvalSequence{ Nodes: []EvalNode{ @@ -71,34 +67,6 @@ func (n *NodePlannableResourceInstance) evalTreeDataResource(addr addrs.AbsResou 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{ Addr: addr.Resource, Config: config.Config, @@ -112,10 +80,9 @@ func (n *NodePlannableResourceInstance) evalTreeDataResource(addr addrs.AbsResou ProviderAddr: n.ResolvedProvider, ProviderMetas: n.ProviderMetas, ProviderSchema: &providerSchema, - ForcePlanRead: forcePlanRead, OutputChange: &change, - OutputValue: &configVal, - OutputState: &state, + State: &state, + dependsOn: n.dependsOn, }, &EvalWriteState{ @@ -153,8 +120,7 @@ func (n *NodePlannableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe Addr: addr.Resource, Provider: &provider, ProviderSchema: &providerSchema, - - Output: &state, + Output: &state, }, &EvalValidateSelfRef{