bugfix: fixed template validation panic in case of incorrect ChangeScript configuration

nomad/structs/structs.go

@@ -45,6 +45,7 @@ import (
 	"github.com/miekg/dns"
 	"github.com/mitchellh/copystructure"
 	"golang.org/x/crypto/blake2b"
+	"golang.org/x/exp/slices"
 )
 
 var (
@@ -7644,20 +7645,6 @@ var (
 	TemplateChangeModeInvalidError = errors.New("Invalid change mode. Must be one of the following: noop, signal, script, restart")
 )
 
-// ChangeScript holds the configuration for the script that is executed if
-// change mode is set to script
-type ChangeScript struct {
-	// Command is the full path to the script
-	Command string
-	// Args is a slice of arguments passed to the script
-	Args []string
-	// Timeout is the amount of seconds we wait for the script to finish
-	Timeout time.Duration
-	// FailOnError indicates whether a task should fail in case script execution
-	// fails or log script failure and don't interrupt the task
-	FailOnError bool
-}
-
 // Template represents a template configuration to be rendered for a given task
 type Template struct {
 	// SourcePath is the path to the template to be rendered
@@ -7735,9 +7722,8 @@ func (t *Template) Copy() *Template {
 	nt := new(Template)
 	*nt = *t
 
-	if t.Wait != nil {
-		nt.Wait = t.Wait.Copy()
-	}
+	nt.ChangeScript = t.ChangeScript.Copy()
+	nt.Wait = t.Wait.Copy()
 
 	return nt
 }
@@ -7780,8 +7766,12 @@ func (t *Template) Validate() error {
 			_ = multierror.Append(&mErr, fmt.Errorf("cannot use signals with env var templates"))
 		}
 	case TemplateChangeModeScript:
-		if t.ChangeScript.Command == "" {
-			_ = multierror.Append(&mErr, fmt.Errorf("must specify script path value when change mode is script"))
+		if t.ChangeScript == nil {
+			_ = multierror.Append(&mErr, fmt.Errorf("must specify change script configuration value when change mode is script"))
+		}
+
+		if err = t.ChangeScript.Validate(); err != nil {
+			_ = multierror.Append(&mErr, err)
 		}
 	default:
 		_ = multierror.Append(&mErr, TemplateChangeModeInvalidError)
@@ -7822,6 +7812,47 @@ func (t *Template) DiffID() string {
 	return t.DestPath
 }
 
+// ChangeScript holds the configuration for the script that is executed if
+// change mode is set to script
+type ChangeScript struct {
+	// Command is the full path to the script
+	Command string
+	// Args is a slice of arguments passed to the script
+	Args []string
+	// Timeout is the amount of seconds we wait for the script to finish
+	Timeout time.Duration
+	// FailOnError indicates whether a task should fail in case script execution
+	// fails or log script failure and don't interrupt the task
+	FailOnError bool
+}
+
+func (cs *ChangeScript) Copy() *ChangeScript {
+	if cs == nil {
+		return nil
+	}
+
+	ncs := new(ChangeScript)
+	*ncs = *cs
+
+	// args is a slice!
+	ncs.Args = slices.Clone(cs.Args)
+
+	return ncs
+}
+
+// Validate makes sure all the required fields of ChangeScript are present
+func (cs *ChangeScript) Validate() error {
+	if cs == nil {
+		return nil
+	}
+
+	if cs.Command == "" {
+		return fmt.Errorf("must specify script path value when change mode is script")
+	}
+
+	return nil
+}
+
 // WaitConfig is the Min/Max duration used by the Consul Template Watcher. Consul
 // Template relies on pointer based business logic. This struct uses pointers so
 // that we tell the different between zero values and unset values.
@@ -7839,11 +7870,11 @@ func (wc *WaitConfig) Copy() *WaitConfig {
 	nwc := new(WaitConfig)
 
 	if wc.Min != nil {
-		nwc.Min = &*wc.Min
+		nwc.Min = wc.Min
 	}
 
 	if wc.Max != nil {
-		nwc.Max = &*wc.Max
+		nwc.Max = wc.Max
 	}
 
 	return nwc

nomad/structs/structs_test.go

@@ -2586,16 +2586,19 @@ func TestTemplate_Copy(t *testing.T) {
 		SourcePath:   "/local/file.txt",
 		DestPath:     "/local/dest.txt",
 		EmbeddedTmpl: "tpl",
-		ChangeMode:   TemplateChangeModeSignal,
-		ChangeSignal: "SIGHUP",
-		Splay:        10 * time.Second,
-		Perms:        "777",
-		Uid:          pointer.Of(1000),
-		Gid:          pointer.Of(2000),
-		LeftDelim:    "[[",
-		RightDelim:   "]]",
-		Envvars:      true,
-		VaultGrace:   time.Minute,
+		ChangeMode:   TemplateChangeModeScript,
+		ChangeScript: &ChangeScript{
+			Command: "/bin/foo",
+			Args:    []string{"--force", "--debug"},
+		},
+		Splay:      10 * time.Second,
+		Perms:      "777",
+		Uid:        pointer.Of(1000),
+		Gid:        pointer.Of(2000),
+		LeftDelim:  "[[",
+		RightDelim: "]]",
+		Envvars:    true,
+		VaultGrace: time.Minute,
 		Wait: &WaitConfig{
 			Min: pointer.Of(time.Second),
 			Max: pointer.Of(time.Minute),
@@ -2606,8 +2609,9 @@ func TestTemplate_Copy(t *testing.T) {
 	t1.SourcePath = "/local/file2.txt"
 	t1.DestPath = "/local/dest2.txt"
 	t1.EmbeddedTmpl = "tpl2"
-	t1.ChangeMode = TemplateChangeModeRestart
-	t1.ChangeSignal = ""
+	t1.ChangeMode = TemplateChangeModeSignal
+	t1.ChangeScript.Command = "/bin/foobar"
+	t1.ChangeScript.Args = []string{"--forces", "--debugs"}
 	t1.Splay = 5 * time.Second
 	t1.Perms = "700"
 	t1.Uid = pointer.Of(5000)
@@ -2623,7 +2627,8 @@ func TestTemplate_Copy(t *testing.T) {
 	require.NotEqual(t, t1.DestPath, t2.DestPath)
 	require.NotEqual(t, t1.EmbeddedTmpl, t2.EmbeddedTmpl)
 	require.NotEqual(t, t1.ChangeMode, t2.ChangeMode)
-	require.NotEqual(t, t1.ChangeSignal, t2.ChangeSignal)
+	require.NotEqual(t, t1.ChangeScript.Command, t2.ChangeScript.Command)
+	require.NotEqual(t, t1.ChangeScript.Args, t2.ChangeScript.Args)
 	require.NotEqual(t, t1.Splay, t2.Splay)
 	require.NotEqual(t, t1.Perms, t2.Perms)
 	require.NotEqual(t, t1.Uid, t2.Uid)
@@ -2760,6 +2765,33 @@ func TestTemplate_Validate(t *testing.T) {
 			},
 			Fail: false,
 		},
+		{
+			Tmpl: &Template{
+				SourcePath:   "foo",
+				DestPath:     "local/foo",
+				ChangeMode:   "script",
+				ChangeScript: nil,
+			},
+			Fail: true,
+		},
+		{
+			Tmpl: &Template{
+				SourcePath:   "foo",
+				DestPath:     "local/foo",
+				ChangeMode:   "script",
+				ChangeScript: &ChangeScript{Command: ""},
+			},
+			Fail: true,
+		},
+		{
+			Tmpl: &Template{
+				SourcePath:   "foo",
+				DestPath:     "local/foo",
+				ChangeMode:   "script",
+				ChangeScript: &ChangeScript{Command: "/bin/foo"},
+			},
+			Fail: false,
+		},
 	}
 
 	for i, c := range cases {