From 20adb9d9b776903e2b93bc42aee6defdd3d80e04 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 28 Aug 2018 16:30:52 -0700 Subject: [PATCH] core: Evaluate resource references from plan where possible Our state representation is not able to preserve unknown values, so it's not suitable for retaining the transient incomplete values we produce during planning. Instead, we'll discard the unknown values when writing to state and have the expression evaluator prefer an object from the plan where possible. We still use the shape of the transient state to inform things like the resource's "each mode", so the plan only masks the object values themselves. --- states/instance_object.go | 13 ++++++- terraform/evaluate.go | 66 +++++++++++++++++++++++++++++++-- terraform/graph_walk_context.go | 1 + 3 files changed, 76 insertions(+), 4 deletions(-) diff --git a/states/instance_object.go b/states/instance_object.go index 65c74962d..484743051 100644 --- a/states/instance_object.go +++ b/states/instance_object.go @@ -68,7 +68,18 @@ const ( // so the caller must not mutate the receiver any further once once this // method is called. func (o *ResourceInstanceObject) Encode(ty cty.Type, schemaVersion uint64) (*ResourceInstanceObjectSrc, error) { - src, err := ctyjson.Marshal(o.Value, ty) + // Our state serialization can't represent unknown values, so we convert + // them to nulls here. This is lossy, but nobody should be writing unknown + // values here and expecting to get them out again later. + // + // We get unknown values here while we're building out a "planned state" + // during the plan phase, but the value stored in the plan takes precedence + // for expression evaluation. The apply step should never produce unknown + // values, but if it does it's the responsibility of the caller to detect + // and raise an error about that. + val := cty.UnknownAsNull(o.Value) + + src, err := ctyjson.Marshal(val, ty) if err != nil { return nil, err } diff --git a/terraform/evaluate.go b/terraform/evaluate.go index f1d3b016c..131d08ce0 100644 --- a/terraform/evaluate.go +++ b/terraform/evaluate.go @@ -16,6 +16,7 @@ import ( "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/configs/configschema" "github.com/hashicorp/terraform/lang" + "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/tfdiags" ) @@ -54,6 +55,10 @@ type Evaluator struct { // State is the current state, embedded in a wrapper that ensures that // it can be safely accessed and modified concurrently. State *states.SyncState + + // Changes is the set of proposed changes, embedded in a wrapper that + // ensures they can be safely accessed and modified concurrently. + Changes *plans.ChangesSync } // Scope creates an evaluation scope for the given module path and optional @@ -541,6 +546,23 @@ func (d *evaluationStateData) getResourceInstanceSingle(addr addrs.ResourceInsta return cty.UnknownVal(ty), diags } + // If there's a pending change for this instance in our plan, we'll prefer + // that. This is important because the state can't represent unknown values + // and so its data is inaccurate when changes are pending. + if change := d.Evaluator.Changes.GetResourceInstanceChange(addr.Absolute(d.ModulePath), states.CurrentGen); change != nil { + val, err := change.After.Decode(ty) + if err != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid resource instance data in plan", + Detail: fmt.Sprintf("Instance %s data could not be decoded from the plan: %s.", addr.Absolute(d.ModulePath), err), + Subject: &config.DeclRange, + }) + return cty.UnknownVal(ty), diags + } + return val, diags + } + ios, err := is.Current.Decode(ty) if err != nil { // This shouldn't happen, since by the time we get here @@ -554,7 +576,7 @@ func (d *evaluationStateData) getResourceInstanceSingle(addr addrs.ResourceInsta return cty.UnknownVal(ty), diags } - return ios.Value, nil + return ios.Value, diags } func (d *evaluationStateData) getResourceInstancesAll(addr addrs.Resource, rng tfdiags.SourceRange, config *configs.Resource, rs *states.Resource, providerAddr addrs.AbsProviderConfig) (cty.Value, tfdiags.Diagnostics) { @@ -593,6 +615,25 @@ func (d *evaluationStateData) getResourceInstancesAll(addr addrs.Resource, rng t key := addrs.IntKey(i) is, exists := rs.Instances[key] if exists { + instAddr := addr.Instance(key).Absolute(d.ModulePath) + + // Prefer pending value in plan if present. See getResourceInstanceSingle + // comment for the rationale. + if change := d.Evaluator.Changes.GetResourceInstanceChange(instAddr, states.CurrentGen); change != nil { + val, err := change.After.Decode(ty) + if err != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid resource instance data in plan", + Detail: fmt.Sprintf("Instance %s data could not be decoded from the plan: %s.", instAddr, err), + Subject: &config.DeclRange, + }) + continue + } + vals[i] = val + continue + } + ios, err := is.Current.Decode(ty) if err != nil { // This shouldn't happen, since by the time we get here @@ -600,7 +641,7 @@ func (d *evaluationStateData) getResourceInstancesAll(addr addrs.Resource, rng t diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid resource instance data in state", - Detail: fmt.Sprintf("Instance %s data could not be decoded from the state: %s.", addr.Instance(key).Absolute(d.ModulePath), err), + Detail: fmt.Sprintf("Instance %s data could not be decoded from the state: %s.", instAddr, err), Subject: &config.DeclRange, }) continue @@ -626,6 +667,25 @@ func (d *evaluationStateData) getResourceInstancesAll(addr addrs.Resource, rng t vals := make(map[string]cty.Value, len(rs.Instances)) for k, is := range rs.Instances { if sk, ok := k.(addrs.StringKey); ok { + instAddr := addr.Instance(k).Absolute(d.ModulePath) + + // Prefer pending value in plan if present. See getResourceInstanceSingle + // comment for the rationale. + if change := d.Evaluator.Changes.GetResourceInstanceChange(instAddr, states.CurrentGen); change != nil { + val, err := change.After.Decode(ty) + if err != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid resource instance data in plan", + Detail: fmt.Sprintf("Instance %s data could not be decoded from the plan: %s.", instAddr, err), + Subject: &config.DeclRange, + }) + continue + } + vals[string(sk)] = val + continue + } + ios, err := is.Current.Decode(ty) if err != nil { // This shouldn't happen, since by the time we get here @@ -633,7 +693,7 @@ func (d *evaluationStateData) getResourceInstancesAll(addr addrs.Resource, rng t diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid resource instance data in state", - Detail: fmt.Sprintf("Instance %s data could not be decoded from the state: %s.", addr.Instance(k).Absolute(d.ModulePath), err), + Detail: fmt.Sprintf("Instance %s data could not be decoded from the state: %s.", instAddr, err), Subject: &config.DeclRange, }) continue diff --git a/terraform/graph_walk_context.go b/terraform/graph_walk_context.go index 5e037ed8c..03c192a86 100644 --- a/terraform/graph_walk_context.go +++ b/terraform/graph_walk_context.go @@ -68,6 +68,7 @@ func (w *ContextGraphWalker) EnterPath(path addrs.ModuleInstance) EvalContext { Config: w.Context.config, Operation: w.Operation, State: w.State, + Changes: w.Changes, Schemas: w.Context.schemas, VariableValues: w.variableValues, VariableValuesLock: &w.variableValuesLock,