From 3ac6e575f13a486aaac1d9d2e0ac3f80c0705bdb Mon Sep 17 00:00:00 2001
From: Martin Atkins <mart@degeneration.co.uk>
Date: Tue, 29 May 2018 11:47:49 -0700
Subject: [PATCH] core: EvalSequence must continue when only warnings are
 returned

To avoid a massively-disruptive change to how EvalNode works, we're now
"smuggling" warnings through the error return value for these, but this
depends on all of the Eval machinery correctly handling this special case
and continuing evaluation when only warnings are returned.

Previous changes missed EvalSequence as a place where execution halts on
error. Now it will accumulate diagnostics itself, aborting if any of
them are error diagnostics, and then wrap its own result up in an error
to be returned by the main Eval function, which already treats non-fatal
errors as a special case, though now produces an explicit log message
about that situation to make it easier to spot in trace logs.

This also includes a more detailed warning message for the warning about
provider input being disabled. While this warning should be removed before
we release anyway, having this additional detail is helpful in debugging
tests where it's being returned.
---
 terraform/eval.go               | 11 ++++++++---
 terraform/eval_provider.go      | 10 +++++++++-
 terraform/eval_sequence.go      | 14 ++++++++++++--
 terraform/graph_walk_context.go |  1 +
 4 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/terraform/eval.go b/terraform/eval.go
index d9a756b6e4..48ed3533ac 100644
--- a/terraform/eval.go
+++ b/terraform/eval.go
@@ -2,6 +2,8 @@ package terraform
 
 import (
 	"log"
+
+	"github.com/hashicorp/terraform/tfdiags"
 )
 
 // EvalNode is the interface that must be implemented by graph nodes to
@@ -54,9 +56,12 @@ func EvalRaw(n EvalNode, ctx EvalContext) (interface{}, error) {
 	log.Printf("[TRACE] %s: eval: %T", path, n)
 	output, err := n.Eval(ctx)
 	if err != nil {
-		if _, ok := err.(EvalEarlyExitError); ok {
-			log.Printf("[TRACE] %s: eval: %T, err: %s", path, n, err)
-		} else {
+		switch err.(type) {
+		case EvalEarlyExitError:
+			log.Printf("[TRACE] %s: eval: %T, early exit err: %s", path, n, err)
+		case tfdiags.NonFatalError:
+			log.Printf("[WARN] %s: eval: %T, non-fatal err: %s", path, n, err)
+		default:
 			log.Printf("[ERROR] %s: eval: %T, err: %s", path, n, err)
 		}
 	}
diff --git a/terraform/eval_provider.go b/terraform/eval_provider.go
index 2e509f250c..88ac69c597 100644
--- a/terraform/eval_provider.go
+++ b/terraform/eval_provider.go
@@ -145,6 +145,14 @@ func (n *EvalInputProvider) Eval(ctx EvalContext) (interface{}, error) {
 	// provider's configuration schema to automatically infer what we need
 	// to prompt for.
 	var diags tfdiags.Diagnostics
-	diags = diags.Append(tfdiags.SimpleWarning(fmt.Sprintf("%s: provider input is temporarily disabled", n.Addr)))
+	diag := &hcl.Diagnostic{
+		Severity: hcl.DiagWarning,
+		Summary:  "Provider input is temporarily disabled",
+		Detail:   fmt.Sprintf("Skipped gathering input for %s because the input step is currently disabled pending a change to the provider API.", n.Addr),
+	}
+	if n.Config != nil {
+		diag.Subject = n.Config.DeclRange.Ptr()
+	}
+	diags = diags.Append(diag)
 	return nil, diags.ErrWithWarnings()
 }
diff --git a/terraform/eval_sequence.go b/terraform/eval_sequence.go
index 82d81782af..ebc351babc 100644
--- a/terraform/eval_sequence.go
+++ b/terraform/eval_sequence.go
@@ -1,22 +1,32 @@
 package terraform
 
+import (
+	"github.com/hashicorp/terraform/tfdiags"
+)
+
 // EvalSequence is an EvalNode that evaluates in sequence.
 type EvalSequence struct {
 	Nodes []EvalNode
 }
 
 func (n *EvalSequence) Eval(ctx EvalContext) (interface{}, error) {
+	var diags tfdiags.Diagnostics
+
 	for _, n := range n.Nodes {
 		if n == nil {
 			continue
 		}
 
 		if _, err := EvalRaw(n, ctx); err != nil {
-			return nil, err
+			diags = diags.Append(err)
+			if diags.HasErrors() {
+				// Halt if we get some errors, but warnings are okay.
+				break
+			}
 		}
 	}
 
-	return nil, nil
+	return nil, diags.ErrWithWarnings()
 }
 
 // EvalNodeFilterable impl.
diff --git a/terraform/graph_walk_context.go b/terraform/graph_walk_context.go
index 5492d4f845..454202f5c1 100644
--- a/terraform/graph_walk_context.go
+++ b/terraform/graph_walk_context.go
@@ -125,6 +125,7 @@ func (w *ContextGraphWalker) ExitEvalTree(v dag.Vertex, output interface{}, err
 	// non-fatal list, rather than returning it directly, so that the graph
 	// walk can continue.
 	if nferr, ok := err.(tfdiags.NonFatalError); ok {
+		log.Printf("[WARN] %s: %s", dag.VertexName(v), nferr)
 		w.NonFatalDiagnostics = w.NonFatalDiagnostics.Append(nferr.Diagnostics)
 		return nil
 	}