From 5f9b189fcf820fb2cfacdd97fd81dd9064cc3774 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Fri, 30 Nov 2018 14:51:52 -0500
Subject: [PATCH] catch conversion errors in PrepareProviderConfig

Errors were being ignore with the intention that they would be caught
later in validation, but it turns out we nee dto catch those earlier.

The legacy schemas also allowed providers to set and empty string for a
bool value, which we need to handle here, since it's not being handled
from user input like a normal config value.
---
 helper/plugin/grpc_provider.go      | 21 ++++++++++++++++++--
 helper/plugin/grpc_provider_test.go | 30 ++++++++++++++++++++++++++---
 2 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go
index 3e1e74520..3f65cf4fd 100644
--- a/helper/plugin/grpc_provider.go
+++ b/helper/plugin/grpc_provider.go
@@ -3,6 +3,7 @@ package plugin
 import (
 	"encoding/json"
 	"errors"
+	"fmt"
 	"regexp"
 	"sort"
 	"strconv"
@@ -94,7 +95,7 @@ func (s *GRPCProviderServer) PrepareProviderConfig(_ context.Context, req *proto
 
 	// lookup any required, top-level attributes that are Null, and see if we
 	// have a Default value available.
-	configVal, _ = cty.Transform(configVal, func(path cty.Path, val cty.Value) (cty.Value, error) {
+	configVal, err = cty.Transform(configVal, func(path cty.Path, val cty.Value) (cty.Value, error) {
 		// we're only looking for top-level attributes
 		if len(path) != 1 {
 			return val, nil
@@ -126,20 +127,36 @@ func (s *GRPCProviderServer) PrepareProviderConfig(_ context.Context, req *proto
 		// find a default value if it exists
 		def, err := attrSchema.DefaultValue()
 		if err != nil {
+			resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, fmt.Errorf("error getting default for %q: %s", getAttr.Name, err))
 			return val, err
 		}
 
 		// no default
 		if def == nil {
-			return val, err
+			return val, nil
 		}
 
 		// create a cty.Value and make sure it's the correct type
 		tmpVal := hcl2shim.HCL2ValueFromConfigValue(def)
+
+		// helper/schema used to allow setting "" to a bool
+		if val.Type() == cty.Bool && tmpVal.RawEquals(cty.StringVal("")) {
+			// return a warning about the conversion
+			resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, "provider set empty string as default value for bool "+getAttr.Name)
+			tmpVal = cty.False
+		}
+
 		val, err = ctyconvert.Convert(tmpVal, val.Type())
+		if err != nil {
+			resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, fmt.Errorf("error setting default for %q: %s", getAttr.Name, err))
+		}
 
 		return val, err
 	})
+	if err != nil {
+		// any error here was already added to the diagnostics
+		return resp, nil
+	}
 
 	configVal, err = block.CoerceValue(configVal)
 	if err != nil {
diff --git a/helper/plugin/grpc_provider_test.go b/helper/plugin/grpc_provider_test.go
index a9e65e818..8f807c51d 100644
--- a/helper/plugin/grpc_provider_test.go
+++ b/helper/plugin/grpc_provider_test.go
@@ -526,7 +526,7 @@ func TestPrepareProviderConfig(t *testing.T) {
 			Schema: map[string]*schema.Schema{
 				"foo": &schema.Schema{
 					Type:     schema.TypeString,
-					Required: true,
+					Optional: true,
 					Default:  true,
 				},
 			},
@@ -537,12 +537,28 @@ func TestPrepareProviderConfig(t *testing.T) {
 				"foo": cty.StringVal("true"),
 			}),
 		},
+		{
+			Name: "test incorrect default bool type",
+			Schema: map[string]*schema.Schema{
+				"foo": &schema.Schema{
+					Type:     schema.TypeBool,
+					Optional: true,
+					Default:  "",
+				},
+			},
+			ConfigVal: cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.NullVal(cty.Bool),
+			}),
+			ExpectConfig: cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.False,
+			}),
+		},
 		{
 			Name: "test deprecated default",
 			Schema: map[string]*schema.Schema{
 				"foo": &schema.Schema{
 					Type:     schema.TypeString,
-					Required: true,
+					Optional: true,
 					Default:  "do not use",
 					Removed:  "don't use this",
 				},
@@ -580,12 +596,20 @@ func TestPrepareProviderConfig(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			if tc.ExpectError == "" && len(resp.Diagnostics) > 0 {
+			if tc.ExpectError != "" && len(resp.Diagnostics) > 0 {
 				for _, d := range resp.Diagnostics {
 					if !strings.Contains(d.Summary, tc.ExpectError) {
 						t.Fatalf("Unexpected error: %s/%s", d.Summary, d.Detail)
 					}
 				}
+				return
+			}
+
+			// we should have no errors past this point
+			for _, d := range resp.Diagnostics {
+				if d.Severity == proto.Diagnostic_ERROR {
+					t.Fatal(resp.Diagnostics)
+				}
 			}
 
 			val, err := msgpack.Unmarshal(resp.PreparedConfig.Msgpack, block.ImpliedType())