From 5f52aba3aeefd92ebeb9256aad1744d1d3e9ed3f Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Wed, 10 Apr 2019 09:34:39 -0400
Subject: [PATCH] Remove unknown value strings from apply diffs

The synthetic config value used to create the Apply diff should contain
no unknown config values. Any remaining UnknownConfigValues were due to
that being used as a placeholder for values yet to be computed, and
these should be marked NewComputed in the diff.
---
 helper/schema/shims.go      | 25 ++++++++++++++++
 helper/schema/shims_test.go | 60 +++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)

diff --git a/helper/schema/shims.go b/helper/schema/shims.go
index 4099d82d4b..e66fc09d45 100644
--- a/helper/schema/shims.go
+++ b/helper/schema/shims.go
@@ -6,6 +6,7 @@ import (
 	"github.com/zclconf/go-cty/cty"
 	ctyjson "github.com/zclconf/go-cty/cty/json"
 
+	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/configs/configschema"
 	"github.com/hashicorp/terraform/terraform"
 )
@@ -31,6 +32,8 @@ func diffFromValues(prior, planned cty.Value, res *Resource, cust CustomizeDiffF
 	configSchema := res.CoreConfigSchema()
 
 	cfg := terraform.NewResourceConfigShimmed(planned, configSchema)
+	removeConfigUnknowns(cfg.Config)
+	removeConfigUnknowns(cfg.Raw)
 
 	diff, err := schemaMap(res.Schema).Diff(instanceState, cfg, cust, nil, false)
 	if err != nil {
@@ -40,6 +43,28 @@ func diffFromValues(prior, planned cty.Value, res *Resource, cust CustomizeDiffF
 	return diff, err
 }
 
+// During apply the only unknown values are those which are to be computed by
+// the resource itself. These may have been marked as unknown config values, and
+// need to be removed to prevent the UnknownVariableValue from appearing the diff.
+func removeConfigUnknowns(cfg map[string]interface{}) {
+	for k, v := range cfg {
+		switch v := v.(type) {
+		case string:
+			if v == config.UnknownVariableValue {
+				cfg[k] = ""
+			}
+		case []interface{}:
+			for _, i := range v {
+				if m, ok := i.(map[string]interface{}); ok {
+					removeConfigUnknowns(m)
+				}
+			}
+		case map[string]interface{}:
+			removeConfigUnknowns(v)
+		}
+	}
+}
+
 // ApplyDiff takes a cty.Value state and applies a terraform.InstanceDiff to
 // get a new cty.Value state. This is used to convert the diff returned from
 // the legacy provider Diff method to the state required for the new
diff --git a/helper/schema/shims_test.go b/helper/schema/shims_test.go
index e3e9df745c..7c523f42b0 100644
--- a/helper/schema/shims_test.go
+++ b/helper/schema/shims_test.go
@@ -3590,6 +3590,14 @@ func TestShimSchemaMap_Diff(t *testing.T) {
 				}
 			}
 
+			// there would be no unknown config variables during apply, so
+			// return early here.
+			for _, v := range tc.ConfigVariables {
+				if s, ok := v.Value.(string); ok && s == config.UnknownVariableValue {
+					return
+				}
+			}
+
 			// our diff function can't set DestroyTainted, but match the
 			// expected value here for the test fixtures
 			if tainted {
@@ -3610,3 +3618,55 @@ func TestShimSchemaMap_Diff(t *testing.T) {
 func resourceSchemaToBlock(s map[string]*Schema) *configschema.Block {
 	return (&Resource{Schema: s}).CoreConfigSchema()
 }
+
+func TestRemoveConfigUnknowns(t *testing.T) {
+	cfg := map[string]interface{}{
+		"id": "74D93920-ED26-11E3-AC10-0800200C9A66",
+		"route_rules": []interface{}{
+			map[string]interface{}{
+				"cidr_block":        "74D93920-ED26-11E3-AC10-0800200C9A66",
+				"destination":       "0.0.0.0/0",
+				"destination_type":  "CIDR_BLOCK",
+				"network_entity_id": "1",
+			},
+			map[string]interface{}{
+				"cidr_block":       "74D93920-ED26-11E3-AC10-0800200C9A66",
+				"destination":      "0.0.0.0/0",
+				"destination_type": "CIDR_BLOCK",
+				"sub_block": []interface{}{
+					map[string]interface{}{
+						"computed": "74D93920-ED26-11E3-AC10-0800200C9A66",
+					},
+				},
+			},
+		},
+	}
+
+	expect := map[string]interface{}{
+		"id": "",
+		"route_rules": []interface{}{
+			map[string]interface{}{
+				"cidr_block":        "",
+				"destination":       "0.0.0.0/0",
+				"destination_type":  "CIDR_BLOCK",
+				"network_entity_id": "1",
+			},
+			map[string]interface{}{
+				"cidr_block":       "",
+				"destination":      "0.0.0.0/0",
+				"destination_type": "CIDR_BLOCK",
+				"sub_block": []interface{}{
+					map[string]interface{}{
+						"computed": "",
+					},
+				},
+			},
+		},
+	}
+
+	removeConfigUnknowns(cfg)
+
+	if !reflect.DeepEqual(cfg, expect) {
+		t.Fatalf("\nexpected: %#v\ngot: %#v", expect, cfg)
+	}
+}