From f08bf76ef2e5d11ba21127ae3c1db3ce6dbe3a20 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Mon, 16 Oct 2017 16:14:55 -0400
Subject: [PATCH] only cache Input "answers", always call Input

While merging the cached Input configs in the correct order prevents
overwriting existing config values, it doesn't prevent an earlier
provider from inserting unwanted values into later provider
configurations.

Diff the key-values returned by Input with the pre-input config, and
store only the "answers" that were added during the Input call.

Always call Input, even if we already have some values, since a
previously cached config may not be complete.
---
 terraform/context_input_test.go | 20 +++++++++++++++-----
 terraform/eval_provider.go      | 32 ++++++++++----------------------
 terraform/eval_provider_test.go | 15 +++++++--------
 3 files changed, 32 insertions(+), 35 deletions(-)

diff --git a/terraform/context_input_test.go b/terraform/context_input_test.go
index 750db918a5..8c20998551 100644
--- a/terraform/context_input_test.go
+++ b/terraform/context_input_test.go
@@ -1,6 +1,7 @@
 package terraform
 
 import (
+	"errors"
 	"reflect"
 	"strings"
 	"sync"
@@ -211,16 +212,25 @@ func TestContext2Input_providerOnce(t *testing.T) {
 	count := 0
 	p.InputFn = func(i UIInput, c *ResourceConfig) (*ResourceConfig, error) {
 		count++
-		return nil, nil
+		_, set := c.Config["from_input"]
+
+		if count == 1 {
+			if set {
+				return nil, errors.New("from_input should not be set")
+			}
+			c.Config["from_input"] = "x"
+		}
+
+		if count > 1 && !set {
+			return nil, errors.New("from_input should be set")
+		}
+
+		return c, nil
 	}
 
 	if err := ctx.Input(InputModeStd); err != nil {
 		t.Fatalf("err: %s", err)
 	}
-
-	if count != 1 {
-		t.Fatalf("should only be called once: %d", count)
-	}
 }
 
 func TestContext2Input_providerId(t *testing.T) {
diff --git a/terraform/eval_provider.go b/terraform/eval_provider.go
index e42f1c0dc3..328d72b5ea 100644
--- a/terraform/eval_provider.go
+++ b/terraform/eval_provider.go
@@ -99,12 +99,8 @@ type EvalInputProvider struct {
 }
 
 func (n *EvalInputProvider) Eval(ctx EvalContext) (interface{}, error) {
-	// If we already configured this provider, then don't do this again
-	if v := ctx.ProviderInput(n.Name); v != nil {
-		return nil, nil
-	}
-
 	rc := *n.Config
+	orig := rc.DeepCopy()
 
 	// Wrap the input into a namespace
 	input := &PrefixUIInput{
@@ -121,27 +117,19 @@ func (n *EvalInputProvider) Eval(ctx EvalContext) (interface{}, error) {
 			"Error configuring %s: %s", n.Name, err)
 	}
 
-	// Set the input that we received so that child modules don't attempt
-	// to ask for input again.
+	// We only store values that have changed through Input.
+	// The goal is to cache cache input responses, not to provide a complete
+	// config for other providers.
+	confMap := make(map[string]interface{})
 	if config != nil && len(config.Config) > 0 {
-		// This repository of provider input results on the context doesn't
-		// retain config.ComputedKeys, so we need to filter those out here
-		// in order that later users of this data won't try to use the unknown
-		// value placeholder as if it were a literal value. This map is just
-		// of known values we've been able to complete so far; dynamic stuff
-		// will be merged in by EvalBuildProviderConfig on subsequent
-		// (post-input) walks.
-		confMap := config.Config
-		if config.ComputedKeys != nil {
-			for _, key := range config.ComputedKeys {
-				delete(confMap, key)
+		// any values that weren't in the original ResourcConfig will be cached
+		for k, v := range config.Config {
+			if _, ok := orig.Config[k]; !ok {
+				confMap[k] = v
 			}
 		}
-
-		ctx.SetProviderInput(n.Name, confMap)
-	} else {
-		ctx.SetProviderInput(n.Name, map[string]interface{}{})
 	}
+	ctx.SetProviderInput(n.Name, confMap)
 
 	return nil, nil
 }
diff --git a/terraform/eval_provider_test.go b/terraform/eval_provider_test.go
index 000dc51287..cca02a76b4 100644
--- a/terraform/eval_provider_test.go
+++ b/terraform/eval_provider_test.go
@@ -137,9 +137,7 @@ func TestEvalInputProvider(t *testing.T) {
 			}
 
 			rawConfig, err := config.NewRawConfig(map[string]interface{}{
-				"set_in_config": "input",
-				"set_by_input":  "input",
-				"computed":      "fake_computed",
+				"set_by_input": "input",
 			})
 			if err != nil {
 				return nil, err
@@ -152,7 +150,8 @@ func TestEvalInputProvider(t *testing.T) {
 	}
 	ctx := &MockEvalContext{ProviderProvider: provider}
 	rawConfig, err := config.NewRawConfig(map[string]interface{}{
-		"mock_config": "mock",
+		"mock_config":   "mock",
+		"set_in_config": "input",
 	})
 	if err != nil {
 		t.Fatalf("NewRawConfig failed: %s", err)
@@ -182,12 +181,12 @@ func TestEvalInputProvider(t *testing.T) {
 	}
 
 	inputCfg := ctx.SetProviderInputConfig
+
+	// we should only have the value that was set during Input
 	want := map[string]interface{}{
-		"set_in_config": "input",
-		"set_by_input":  "input",
-		// "computed" is omitted because it value isn't known at input time
+		"set_by_input": "input",
 	}
 	if !reflect.DeepEqual(inputCfg, want) {
-		t.Errorf("got incorrect input config %#v; want %#v", inputCfg, want)
+		t.Errorf("got incorrect input config:\n%#v\nwant:\n%#v", inputCfg, want)
 	}
 }