Merge pull request #28457 from hashicorp/jbardin/provisioner-null-checks

additional null checks in provisioners
hashicorp · Apr 20, 2021 · d15f739 · d15f739
2 parents fabdf0b + 7f571b5
commit d15f739
Show file tree

Hide file tree

Showing 4 changed files with 105 additions and 4 deletions.
diff --git a/builtin/provisioners/local-exec/resource_provisioner.go b/builtin/provisioners/local-exec/resource_provisioner.go
@@ -82,8 +82,10 @@ func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceReques
 
 	if !envVal.IsNull() {
 		for k, v := range envVal.AsValueMap() {
-			entry := fmt.Sprintf("%s=%s", k, v.AsString())
-			env = append(env, entry)
+			if !v.IsNull() {
+				entry := fmt.Sprintf("%s=%s", k, v.AsString())
+				env = append(env, entry)
+			}
 		}
 	}
 
@@ -93,7 +95,9 @@ func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceReques
 	var cmdargs []string
 	if !intrVal.IsNull() && intrVal.LengthInt() > 0 {
 		for _, v := range intrVal.AsValueSlice() {
-			cmdargs = append(cmdargs, v.AsString())
+			if !v.IsNull() {
+				cmdargs = append(cmdargs, v.AsString())
+			}
 		}
 	} else {
 		if runtime.GOOS == "windows" {

diff --git a/builtin/provisioners/local-exec/resource_provisioner_test.go b/builtin/provisioners/local-exec/resource_provisioner_test.go
@@ -1,6 +1,7 @@
 package localexec
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"strings"
@@ -204,3 +205,48 @@ func TestResourceProvisioner_StopClose(t *testing.T) {
 	p.Stop()
 	p.Close()
 }
+
+func TestResourceProvisioner_nullsInOptionals(t *testing.T) {
+	output := cli.NewMockUi()
+	p := New()
+	schema := p.GetSchema().Provisioner
+
+	for i, cfg := range []cty.Value{
+		cty.ObjectVal(map[string]cty.Value{
+			"command": cty.StringVal("echo OK"),
+			"environment": cty.MapVal(map[string]cty.Value{
+				"FOO": cty.NullVal(cty.String),
+			}),
+		}),
+		cty.ObjectVal(map[string]cty.Value{
+			"command":     cty.StringVal("echo OK"),
+			"environment": cty.NullVal(cty.Map(cty.String)),
+		}),
+		cty.ObjectVal(map[string]cty.Value{
+			"command":     cty.StringVal("echo OK"),
+			"interpreter": cty.ListVal([]cty.Value{cty.NullVal(cty.String)}),
+		}),
+		cty.ObjectVal(map[string]cty.Value{
+			"command":     cty.StringVal("echo OK"),
+			"interpreter": cty.NullVal(cty.List(cty.String)),
+		}),
+		cty.ObjectVal(map[string]cty.Value{
+			"command":     cty.StringVal("echo OK"),
+			"working_dir": cty.NullVal(cty.String),
+		}),
+	} {
+		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+
+			cfg, err := schema.CoerceValue(cfg)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			// verifying there are no panics
+			p.ProvisionResource(provisioners.ProvisionResourceRequest{
+				Config:   cfg,
+				UIOutput: output,
+			})
+		})
+	}
+}
diff --git a/builtin/provisioners/remote-exec/resource_provisioner.go b/builtin/provisioners/remote-exec/resource_provisioner.go
@@ -128,6 +128,10 @@ func (p *provisioner) Close() error {
 func generateScripts(inline cty.Value) ([]string, error) {
 	var lines []string
 	for _, l := range inline.AsValueSlice() {
+		if l.IsNull() {
+			return nil, errors.New("invalid null string in 'scripts'")
+		}
+
 		s := l.AsString()
 		if s == "" {
 			return nil, errors.New("invalid empty string in 'scripts'")
@@ -169,11 +173,14 @@ func collectScripts(v cty.Value) ([]io.ReadCloser, error) {
 
 	if scriptList := v.GetAttr("scripts"); !scriptList.IsNull() {
 		for _, script := range scriptList.AsValueSlice() {
+			if script.IsNull() {
+				return nil, errors.New("invalid null string in 'script'")
+			}
 			s := script.AsString()
 			if s == "" {
 				return nil, errors.New("invalid empty string in 'script'")
 			}
-			scripts = append(scripts, script.AsString())
+			scripts = append(scripts, s)
 		}
 	}
 

diff --git a/builtin/provisioners/remote-exec/resource_provisioner_test.go b/builtin/provisioners/remote-exec/resource_provisioner_test.go
@@ -3,6 +3,7 @@ package remoteexec
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"io"
 	"log"
 	"testing"
@@ -274,3 +275,46 @@ func TestResourceProvisioner_connectionRequired(t *testing.T) {
 		t.Fatalf("expected 'missing connection' error: got %q", got)
 	}
 }
+
+func TestResourceProvisioner_nullsInOptionals(t *testing.T) {
+	output := cli.NewMockUi()
+	p := New()
+	schema := p.GetSchema().Provisioner
+
+	for i, cfg := range []cty.Value{
+		cty.ObjectVal(map[string]cty.Value{
+			"script": cty.StringVal("echo"),
+			"inline": cty.NullVal(cty.List(cty.String)),
+		}),
+		cty.ObjectVal(map[string]cty.Value{
+			"inline": cty.ListVal([]cty.Value{
+				cty.NullVal(cty.String),
+			}),
+		}),
+		cty.ObjectVal(map[string]cty.Value{
+			"script": cty.NullVal(cty.String),
+		}),
+		cty.ObjectVal(map[string]cty.Value{
+			"scripts": cty.NullVal(cty.List(cty.String)),
+		}),
+		cty.ObjectVal(map[string]cty.Value{
+			"scripts": cty.ListVal([]cty.Value{
+				cty.NullVal(cty.String),
+			}),
+		}),
+	} {
+		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+
+			cfg, err := schema.CoerceValue(cfg)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			// verifying there are no panics
+			p.ProvisionResource(provisioners.ProvisionResourceRequest{
+				Config:   cfg,
+				UIOutput: output,
+			})
+		})
+	}
+}