From a26446931b310f131a48d468c39038e917afb9b7 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Tue, 16 Jun 2020 12:40:48 -0400
Subject: [PATCH] validate depends_on for outputs

If depends_on is allowed for outputs, we should validate that the
expressions are valid. Since outputs are always evaluated, and
validation is just done by this evaluation, we can check the
depends_on validation during evaluation too.
---
 terraform/eval_output.go      | 19 +++++++++++--------
 terraform/eval_output_test.go |  4 +++-
 terraform/node_output.go      |  5 ++---
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/terraform/eval_output.go b/terraform/eval_output.go
index f1a195f6d..4fea60f11 100644
--- a/terraform/eval_output.go
+++ b/terraform/eval_output.go
@@ -4,10 +4,10 @@ import (
 	"fmt"
 	"log"
 
-	"github.com/hashicorp/hcl/v2"
 	"github.com/zclconf/go-cty/cty"
 
 	"github.com/hashicorp/terraform/addrs"
+	"github.com/hashicorp/terraform/configs"
 	"github.com/hashicorp/terraform/plans"
 	"github.com/hashicorp/terraform/states"
 )
@@ -32,9 +32,8 @@ func (n *EvalDeleteOutput) Eval(ctx EvalContext) (interface{}, error) {
 // EvalWriteOutput is an EvalNode implementation that writes the output
 // for the given name to the current state.
 type EvalWriteOutput struct {
-	Addr      addrs.OutputValue
-	Sensitive bool
-	Expr      hcl.Expression
+	Addr   addrs.OutputValue
+	Config *configs.Output
 	// ContinueOnErr allows interpolation to fail during Input
 	ContinueOnErr bool
 }
@@ -45,9 +44,13 @@ func (n *EvalWriteOutput) Eval(ctx EvalContext) (interface{}, error) {
 
 	// This has to run before we have a state lock, since evaluation also
 	// reads the state
-	val, diags := ctx.EvaluateExpr(n.Expr, cty.DynamicPseudoType, nil)
+	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))
+
 	state := ctx.State()
 	if state == nil {
 		return nil, nil
@@ -80,7 +83,7 @@ func (n *EvalWriteOutput) setValue(addr addrs.AbsOutputValue, state *states.Sync
 		// changeset below, if we have one on this graph walk.
 		log.Printf("[TRACE] EvalWriteOutput: Saving value for %s in state", addr)
 		stateVal := cty.UnknownAsNull(val)
-		state.SetOutputValue(addr, stateVal, n.Sensitive)
+		state.SetOutputValue(addr, stateVal, n.Config.Sensitive)
 	} else {
 		log.Printf("[TRACE] EvalWriteOutput: Removing %s from state (it is now null)", addr)
 		state.RemoveOutputValue(addr)
@@ -100,7 +103,7 @@ func (n *EvalWriteOutput) setValue(addr addrs.AbsOutputValue, state *states.Sync
 		if !val.IsNull() {
 			change = &plans.OutputChange{
 				Addr:      addr,
-				Sensitive: n.Sensitive,
+				Sensitive: n.Config.Sensitive,
 				Change: plans.Change{
 					Action: plans.Create,
 					Before: cty.NullVal(cty.DynamicPseudoType),
@@ -110,7 +113,7 @@ func (n *EvalWriteOutput) setValue(addr addrs.AbsOutputValue, state *states.Sync
 		} else {
 			change = &plans.OutputChange{
 				Addr:      addr,
-				Sensitive: n.Sensitive,
+				Sensitive: n.Config.Sensitive,
 				Change: plans.Change{
 					// This is just a weird placeholder delete action since
 					// we don't have an actual prior value to indicate.
diff --git a/terraform/eval_output_test.go b/terraform/eval_output_test.go
index 7d21bbd2e..1a2e738c2 100644
--- a/terraform/eval_output_test.go
+++ b/terraform/eval_output_test.go
@@ -3,6 +3,7 @@ package terraform
 import (
 	"testing"
 
+	"github.com/hashicorp/terraform/configs"
 	"github.com/hashicorp/terraform/states"
 
 	"github.com/hashicorp/terraform/addrs"
@@ -44,7 +45,8 @@ func TestEvalWriteMapOutput(t *testing.T) {
 
 	for _, tc := range cases {
 		evalNode := &EvalWriteOutput{
-			Addr: addrs.OutputValue{Name: tc.name},
+			Config: &configs.Output{},
+			Addr:   addrs.OutputValue{Name: tc.name},
 		}
 		ctx.EvaluateExprResult = tc.val
 		t.Run(tc.name, func(t *testing.T) {
diff --git a/terraform/node_output.go b/terraform/node_output.go
index 1039c3ec4..6e303e9ba 100644
--- a/terraform/node_output.go
+++ b/terraform/node_output.go
@@ -228,9 +228,8 @@ func (n *NodeApplyableOutput) EvalTree() EvalNode {
 			&EvalOpFilter{
 				Ops: []walkOperation{walkEval, walkRefresh, walkPlan, walkApply, walkValidate, walkDestroy, walkPlanDestroy},
 				Node: &EvalWriteOutput{
-					Addr:      n.Addr.OutputValue,
-					Sensitive: n.Config.Sensitive,
-					Expr:      n.Config.Expr,
+					Addr:   n.Addr.OutputValue,
+					Config: n.Config,
 				},
 			},
 		},