Merge pull request #19521 from hashicorp/jbardin/prepare-provider-config

catch conversion errors in PrepareProviderConfig
hashicorp · Nov 30, 2018 · 3dacdba · 3dacdba
2 parents 12572e9 + 5f9b189
commit 3dacdba
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 5 deletions.
diff --git 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
@@ -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())