From a32028aeed11b9ae5626f8a20b389c90e5648dd2 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 6 Oct 2020 17:14:53 -0400 Subject: [PATCH] evaluate vars and outputs during import Outputs were not being evaluated during import, because it was not added to the walk filter. Remove any unnecessary walk filters from all the Execute nodes. --- terraform/context_eval_test.go | 2 +- terraform/node_module_variable.go | 8 +- terraform/node_output.go | 94 +++++---- terraform/node_resource_destroy.go | 217 ++++++++++----------- terraform/node_resource_destroy_deposed.go | 71 ++++--- terraform/transform_reference.go | 2 +- 6 files changed, 191 insertions(+), 203 deletions(-) diff --git a/terraform/context_eval_test.go b/terraform/context_eval_test.go index 145ef1372b..8f7035d01b 100644 --- a/terraform/context_eval_test.go +++ b/terraform/context_eval_test.go @@ -38,7 +38,7 @@ func TestContextEval(t *testing.T) { }, { `module.child.result`, - cty.UnknownVal(cty.Number), + cty.NumberIntVal(6), false, }, } diff --git a/terraform/node_module_variable.go b/terraform/node_module_variable.go index b635685d1a..78ddb6e8f2 100644 --- a/terraform/node_module_variable.go +++ b/terraform/node_module_variable.go @@ -153,13 +153,13 @@ func (n *nodeModuleVariable) Execute(ctx EvalContext, op walkOperation) error { var err error switch op { - case walkPlan, walkApply, walkDestroy: - vals, err = n.EvalModuleCallArgument(ctx, false) + case walkValidate: + vals, err = n.EvalModuleCallArgument(ctx, true) if err != nil { return err } - case walkValidate: - vals, err = n.EvalModuleCallArgument(ctx, true) + default: + vals, err = n.EvalModuleCallArgument(ctx, false) if err != nil { return err } diff --git a/terraform/node_output.go b/terraform/node_output.go index ec4f482762..439c2a9cf8 100644 --- a/terraform/node_output.go +++ b/terraform/node_output.go @@ -199,61 +199,55 @@ func (n *NodeApplyableOutput) References() []*addrs.Reference { // GraphNodeExecutable func (n *NodeApplyableOutput) Execute(ctx EvalContext, op walkOperation) error { - switch op { - // Everything except walkImport - case walkEval, walkPlan, walkApply, walkValidate, walkDestroy, walkPlanDestroy: - // 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) - // We'll handle errors below, after we have loaded the module. + // 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) + // 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 - diags = diags.Append(validateDependsOn(ctx, n.Config.DependsOn)) + // Outputs don't have a separate mode for validation, so validate + // depends_on expressions here too + diags = diags.Append(validateDependsOn(ctx, n.Config.DependsOn)) - // Ensure that non-sensitive outputs don't include sensitive values - _, marks := val.UnmarkDeep() - _, hasSensitive := marks["sensitive"] - if !n.Config.Sensitive && hasSensitive { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Output refers to sensitive values", - Detail: "Expressions used in outputs can only refer to sensitive values if the sensitive attribute is true.", - Subject: n.Config.DeclRange.Ptr(), - }) - } + // Ensure that non-sensitive outputs don't include sensitive values + _, marks := val.UnmarkDeep() + _, hasSensitive := marks["sensitive"] + if !n.Config.Sensitive && hasSensitive { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Output refers to sensitive values", + Detail: "Expressions used in outputs can only refer to sensitive values if the sensitive attribute is true.", + Subject: n.Config.DeclRange.Ptr(), + }) + } - state := ctx.State() - if state == nil { - return nil - } - - changes := ctx.Changes() // may be nil, if we're not working on a changeset - - // handling the interpolation error - if diags.HasErrors() { - if flagWarnOutputErrors { - log.Printf("[ERROR] Output interpolation %q failed: %s", n.Addr, diags.Err()) - // if we're continuing, make sure the output is included, and - // marked as unknown. If the evaluator was able to find a type - // for the value in spite of the error then we'll use it. - n.setValue(state, changes, cty.UnknownVal(val.Type())) - return EvalEarlyExitError{} - } - return diags.Err() - } - n.setValue(state, changes, val) - - // If we were able to evaluate a new value, we can update that in the - // refreshed state as well. - if state = ctx.RefreshState(); state != nil && val.IsWhollyKnown() { - n.setValue(state, changes, val) - } - - return nil - default: + state := ctx.State() + if state == nil { return nil } + + changes := ctx.Changes() // may be nil, if we're not working on a changeset + + // handling the interpolation error + if diags.HasErrors() { + if flagWarnOutputErrors { + log.Printf("[ERROR] Output interpolation %q failed: %s", n.Addr, diags.Err()) + // if we're continuing, make sure the output is included, and + // marked as unknown. If the evaluator was able to find a type + // for the value in spite of the error then we'll use it. + n.setValue(state, changes, cty.UnknownVal(val.Type())) + return EvalEarlyExitError{} + } + return diags.Err() + } + n.setValue(state, changes, val) + + // If we were able to evaluate a new value, we can update that in the + // refreshed state as well. + if state = ctx.RefreshState(); state != nil && val.IsWhollyKnown() { + n.setValue(state, changes, val) + } + + return nil } // dag.GraphNodeDotter impl. diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index c54bc0cec9..872df3654a 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -136,133 +136,130 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) var state *states.ResourceInstanceObject var provisionerErr error - switch op { - case walkApply, walkDestroy: - provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) + provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) + if err != nil { + return err + } + + changeApply, err = n.readDiff(ctx, providerSchema) + if err != nil { + return err + } + + evalReduceDiff := &EvalReduceDiff{ + Addr: addr.Resource, + InChange: &changeApply, + Destroy: true, + OutChange: &changeApply, + } + _, err = evalReduceDiff.Eval(ctx) + if err != nil { + return err + } + + // EvalReduceDiff may have simplified our planned change + // into a NoOp if it does not require destroying. + if changeApply == nil || changeApply.Action == plans.NoOp { + return EvalEarlyExitError{} + } + + state, err = n.ReadResourceInstanceState(ctx, addr) + if err != nil { + return err + } + + // Exit early if the state object is null after reading the state + if state == nil || state.Value.IsNull() { + return EvalEarlyExitError{} + } + + evalApplyPre := &EvalApplyPre{ + Addr: addr.Resource, + State: &state, + Change: &changeApply, + } + _, err = evalApplyPre.Eval(ctx) + if err != nil { + return err + } + + // Run destroy provisioners if not tainted + if state != nil && state.Status != states.ObjectTainted { + evalApplyProvisioners := &EvalApplyProvisioners{ + Addr: addr.Resource, + State: &state, + ResourceConfig: n.Config, + Error: &provisionerErr, + When: configs.ProvisionerWhenDestroy, + } + _, err := evalApplyProvisioners.Eval(ctx) if err != nil { return err } - - changeApply, err = n.readDiff(ctx, providerSchema) - if err != nil { - return err - } - - evalReduceDiff := &EvalReduceDiff{ - Addr: addr.Resource, - InChange: &changeApply, - Destroy: true, - OutChange: &changeApply, - } - _, err = evalReduceDiff.Eval(ctx) - if err != nil { - return err - } - - // EvalReduceDiff may have simplified our planned change - // into a NoOp if it does not require destroying. - if changeApply == nil || changeApply.Action == plans.NoOp { - return EvalEarlyExitError{} - } - - state, err = n.ReadResourceInstanceState(ctx, addr) - if err != nil { - return err - } - - // Exit early if the state object is null after reading the state - if state == nil || state.Value.IsNull() { - return EvalEarlyExitError{} - } - - evalApplyPre := &EvalApplyPre{ - Addr: addr.Resource, - State: &state, - Change: &changeApply, - } - _, err = evalApplyPre.Eval(ctx) - if err != nil { - return err - } - - // Run destroy provisioners if not tainted - if state != nil && state.Status != states.ObjectTainted { - evalApplyProvisioners := &EvalApplyProvisioners{ - Addr: addr.Resource, - State: &state, - ResourceConfig: n.Config, - Error: &provisionerErr, - When: configs.ProvisionerWhenDestroy, + if provisionerErr != nil { + // If we have a provisioning error, then we just call + // the post-apply hook now. + evalApplyPost := &EvalApplyPost{ + Addr: addr.Resource, + State: &state, + Error: &provisionerErr, } - _, err := evalApplyProvisioners.Eval(ctx) + _, err = evalApplyPost.Eval(ctx) if err != nil { return err } - if provisionerErr != nil { - // If we have a provisioning error, then we just call - // the post-apply hook now. - evalApplyPost := &EvalApplyPost{ - Addr: addr.Resource, - State: &state, - Error: &provisionerErr, - } - _, err = evalApplyPost.Eval(ctx) - if err != nil { - return err - } - } } + } - // Managed resources need to be destroyed, while data sources - // are only removed from state. - if addr.Resource.Resource.Mode == addrs.ManagedResourceMode { - evalApply := &EvalApply{ - Addr: addr.Resource, - Config: nil, // No configuration because we are destroying - State: &state, - Change: &changeApply, - Provider: &provider, - ProviderAddr: n.ResolvedProvider, - ProviderMetas: n.ProviderMetas, - ProviderSchema: &providerSchema, - Output: &state, - Error: &provisionerErr, - } - _, err = evalApply.Eval(ctx) - if err != nil { - return err - } - - evalWriteState := &EvalWriteState{ - Addr: addr.Resource, - ProviderAddr: n.ResolvedProvider, - ProviderSchema: &providerSchema, - State: &state, - } - _, err = evalWriteState.Eval(ctx) - if err != nil { - return err - } - } else { - log.Printf("[TRACE] NodeDestroyResourceInstance: removing state object for %s", n.Addr) - state := ctx.State() - state.SetResourceInstanceCurrent(n.Addr, nil, n.ResolvedProvider) + // Managed resources need to be destroyed, while data sources + // are only removed from state. + if addr.Resource.Resource.Mode == addrs.ManagedResourceMode { + evalApply := &EvalApply{ + Addr: addr.Resource, + Config: nil, // No configuration because we are destroying + State: &state, + Change: &changeApply, + Provider: &provider, + ProviderAddr: n.ResolvedProvider, + ProviderMetas: n.ProviderMetas, + ProviderSchema: &providerSchema, + Output: &state, + Error: &provisionerErr, } - - evalApplyPost := &EvalApplyPost{ - Addr: addr.Resource, - State: &state, - Error: &provisionerErr, - } - _, err = evalApplyPost.Eval(ctx) + _, err = evalApply.Eval(ctx) if err != nil { return err } - err = UpdateStateHook(ctx) + evalWriteState := &EvalWriteState{ + Addr: addr.Resource, + ProviderAddr: n.ResolvedProvider, + ProviderSchema: &providerSchema, + State: &state, + } + _, err = evalWriteState.Eval(ctx) if err != nil { return err } + } else { + log.Printf("[TRACE] NodeDestroyResourceInstance: removing state object for %s", n.Addr) + state := ctx.State() + state.SetResourceInstanceCurrent(n.Addr, nil, n.ResolvedProvider) + } + + evalApplyPost := &EvalApplyPost{ + Addr: addr.Resource, + State: &state, + Error: &provisionerErr, + } + _, err = evalApplyPost.Eval(ctx) + if err != nil { + return err + } + + err = UpdateStateHook(ctx) + if err != nil { + return err } return nil diff --git a/terraform/node_resource_destroy_deposed.go b/terraform/node_resource_destroy_deposed.go index 6d5af049be..a77cdf4761 100644 --- a/terraform/node_resource_destroy_deposed.go +++ b/terraform/node_resource_destroy_deposed.go @@ -76,44 +76,41 @@ func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walk var change *plans.ResourceInstanceChange var state *states.ResourceInstanceObject - switch op { - case walkPlan, walkPlanDestroy: - - readStateDeposed := &EvalReadStateDeposed{ - Addr: addr.Resource, - Output: &state, - Key: n.DeposedKey, - Provider: &provider, - ProviderSchema: &providerSchema, - } - _, err = readStateDeposed.Eval(ctx) - if err != nil { - return err - } - - diffDestroy := &EvalDiffDestroy{ - Addr: addr.Resource, - ProviderAddr: n.ResolvedProvider, - DeposedKey: n.DeposedKey, - State: &state, - Output: &change, - } - _, err = diffDestroy.Eval(ctx) - if err != nil { - return err - } - - writeDiff := &EvalWriteDiff{ - Addr: addr.Resource, - DeposedKey: n.DeposedKey, - ProviderSchema: &providerSchema, - Change: &change, - } - _, err = writeDiff.Eval(ctx) - if err != nil { - return err - } + readStateDeposed := &EvalReadStateDeposed{ + Addr: addr.Resource, + Output: &state, + Key: n.DeposedKey, + Provider: &provider, + ProviderSchema: &providerSchema, } + _, err = readStateDeposed.Eval(ctx) + if err != nil { + return err + } + + diffDestroy := &EvalDiffDestroy{ + Addr: addr.Resource, + ProviderAddr: n.ResolvedProvider, + DeposedKey: n.DeposedKey, + State: &state, + Output: &change, + } + _, err = diffDestroy.Eval(ctx) + if err != nil { + return err + } + + writeDiff := &EvalWriteDiff{ + Addr: addr.Resource, + DeposedKey: n.DeposedKey, + ProviderSchema: &providerSchema, + Change: &change, + } + _, err = writeDiff.Eval(ctx) + if err != nil { + return err + } + return nil } diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index b813226f5e..0b7d0e5663 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -301,7 +301,7 @@ func (m ReferenceMap) References(v dag.Vertex) []dag.Vertex { case addrs.ModuleCallInstance: subject = ri.Call default: - log.Printf("[WARN] ReferenceTransformer: reference not found: %q", subject) + log.Printf("[INFO] ReferenceTransformer: reference not found: %q", subject) continue } key = m.referenceMapKey(v, subject)