diff --git a/child/child.go b/child/child.go index bd83e817a..ee31ba948 100644 --- a/child/child.go +++ b/child/child.go @@ -119,11 +119,14 @@ type NewInput struct { Splay time.Duration // Setsid flag, if set to true will create the child processes with their own - // session ID and Process group ID. The default value of false will cause the - // child processes to have their own PGID but they will have the same SID as - // that of their parent + // session ID and Process group ID. Note this overrides Setpgid below. Setsid bool + // Setpgid flag, if set to true will create the child processes with their + // own Process group ID. If set the child processes to have their own PGID + // but they will have the same SID as that of their parent + Setpgid bool + // an optional logger that can be used for messages pertinent to the child process Logger *log.Logger } @@ -157,7 +160,7 @@ func New(i *NewInput) (*Child, error) { killTimeout: i.KillTimeout, splay: i.Splay, stopCh: make(chan struct{}, 1), - setpgid: !i.Setsid, + setpgid: i.Setpgid && !i.Setsid, setsid: i.Setsid, logger: i.Logger, } diff --git a/child/child_test.go b/child/child_test.go index 5547228d6..699f062dc 100644 --- a/child/child_test.go +++ b/child/child_test.go @@ -370,6 +370,7 @@ func TestStop_noWaitForSplay(t *testing.T) { c.splay = 100 * time.Second c.reloadSignal = nil c.killSignal = syscall.SIGUSR1 + c.setpgid = true out := gatedio.NewByteBuffer() c.stdout = out diff --git a/child/command-prep.go b/child/command-prep.go index 10068971e..9108e4f37 100644 --- a/child/command-prep.go +++ b/child/command-prep.go @@ -8,7 +8,7 @@ import ( "strings" ) -func CommandPrep(command []string) ([]string, error) { +func CommandPrep(command []string) ([]string, bool, error) { switch { case len(command) == 1 && len(strings.Fields(command[0])) > 1: // command is []string{"command using arguments or shell features"} @@ -24,15 +24,15 @@ func CommandPrep(command []string) ([]string, error) { } } if shell == "" { - return []string{}, exec.ErrNotFound + return []string{}, false, exec.ErrNotFound } cmd := []string{shell, "-c", command[0]} - return cmd, nil + return cmd, true, nil case len(command) >= 1 && len(strings.TrimSpace(command[0])) > 0: // command is already good ([]string{"foo"}, []string{"foo", "bar"}, ..) - return command, nil + return command, false, nil default: // command is []string{} or []string{""} - return []string{}, exec.ErrNotFound + return []string{}, false, exec.ErrNotFound } } diff --git a/child/command-prep_test.go b/child/command-prep_test.go index 1455ced8e..47300d126 100644 --- a/child/command-prep_test.go +++ b/child/command-prep_test.go @@ -9,28 +9,33 @@ import ( func Test_CommandPrep(t *testing.T) { type cmd []string cases := []struct { - n string - in cmd - out cmd - err error + n string + in cmd + out cmd + subsh bool + err error }{ - {n: "0", in: cmd{}, out: cmd{}, err: exec.ErrNotFound}, + {n: "empty", in: cmd{}, out: cmd{}, err: exec.ErrNotFound}, {n: "''", in: cmd{""}, out: cmd{}, err: exec.ErrNotFound}, {n: "' '", in: cmd{" "}, out: cmd{}, err: exec.ErrNotFound}, {n: "'f'", in: cmd{"foo"}, out: cmd{"foo"}, err: nil}, - {n: "'f b'", in: cmd{"foo bar"}, out: cmd{"sh", "-c", "foo bar"}, err: nil}, + {n: "'f b'", in: cmd{"foo bar"}, subsh: true, out: cmd{"sh", "-c", "foo bar"}, err: nil}, {n: "'f','b'", in: cmd{"foo", "bar"}, out: cmd{"foo", "bar"}, err: nil}, {n: "'f','b','z'", in: cmd{"foo", "bar", "zed"}, out: cmd{"foo", "bar", "zed"}, err: nil}, } for _, tc := range cases { t.Run(tc.n, func(t *testing.T) { - out, err := CommandPrep(tc.in) + out, subsh, err := CommandPrep(tc.in) if !reflect.DeepEqual(cmd(out), tc.out) { - t.Errorf("bad prepCommand output. wanted: %#v, got %#v", tc.out, out) + t.Errorf("bad commandPrep command output;"+ + "wanted: %#v, got %#v", tc.out, out) } if err != tc.err { t.Errorf("bad prepCommand error. wanted: %v, got %v", tc.err, err) } + if tc.subsh != subsh { + t.Errorf("incorrectly marked as using subshell") + } }) } } diff --git a/child/command-prep_windows.go b/child/command-prep_windows.go index f352a0c63..ea5c7267b 100644 --- a/child/command-prep_windows.go +++ b/child/command-prep_windows.go @@ -8,16 +8,16 @@ import ( "strings" ) -func CommandPrep(command []string) ([]string, error) { +func CommandPrep(command []string) ([]string, bool, error) { switch { case len(command) == 1 && len(strings.Fields(command[0])) == 1: // command is []string{"foo"} - return []string{command[0]}, nil + return []string{command[0]}, false, nil case len(command) > 1: // command is []string{"foo", "bar"} - return command, nil + return command, false, nil default: // command is []string{}, []string{""}, []string{"foo bar"} - return []string{}, exec.ErrNotFound + return []string{}, false, exec.ErrNotFound } } diff --git a/manager/runner.go b/manager/runner.go index 6141f11f9..6ed40e4e1 100644 --- a/manager/runner.go +++ b/manager/runner.go @@ -1199,7 +1199,7 @@ type spawnChildInput struct { // spawnChild spawns a child process with the given inputs and returns the // resulting child. func spawnChild(i *spawnChildInput) (*child.Child, error) { - args, err := child.CommandPrep(i.Command) + args, subshell, err := child.CommandPrep(i.Command) if err != nil { return nil, errors.Wrap(err, "failed parsing command") } @@ -1215,6 +1215,7 @@ func spawnChild(i *spawnChildInput) (*child.Child, error) { KillSignal: i.KillSignal, KillTimeout: i.KillTimeout, Splay: i.Splay, + Setpgid: subshell, // setpgid for subshells to propagate signals }) if err != nil { return nil, errors.Wrap(err, "error creating child") diff --git a/manager/runner_test.go b/manager/runner_test.go index 5ddee3c9c..0fb162d29 100644 --- a/manager/runner_test.go +++ b/manager/runner_test.go @@ -1114,7 +1114,7 @@ func TestRunner_command(t *testing.T) { os.Setenv("FOO", "bar") parseTest := func(tc testCase) { - out, err := child.CommandPrep(tc.input) + out, _, err := child.CommandPrep(tc.input) mismatchErr := "bad command parse\n got: '%#v'\nwanted: '%#v'" switch { case err != nil && len(tc.input) > 0 && tc.input[0] != "": @@ -1128,7 +1128,7 @@ func TestRunner_command(t *testing.T) { runTest := func(tc testCase) { stdout := new(bytes.Buffer) stderr := new(bytes.Buffer) - args, err := child.CommandPrep(tc.input) + args, _, err := child.CommandPrep(tc.input) switch { case err == exec.ErrNotFound && len(args) == 0: return // expected @@ -1265,7 +1265,7 @@ func TestRunner_commandPath(t *testing.T) { PATH := os.Getenv("PATH") defer os.Setenv("PATH", PATH) os.Setenv("PATH", "") - cmd, err := child.CommandPrep([]string{"echo hi"}) + cmd, _, err := child.CommandPrep([]string{"echo hi"}) if err != nil && err != exec.ErrNotFound { t.Fatal(err) }