Allow script mode to accept scripts that do not start with a shebang.

This prefixes scripts that do not start with a shebang with a default value of "/bin/sh".
This matches the default entrypoint/shell for docker containers, and should help
prevent an easy-to-make mistake.

Author: dlorenc

docs/tasks.md

@@ -156,10 +156,11 @@ If this field is present, the step cannot specify `command`.
 When specified, a `script` gets invoked as if it were the contents of a file in
 the container. Any `args` are passed to the script file.
 
-Scripts should start with a
-[shebang](https://en.wikipedia.org/wiki/Shebang_(Unix)) line to declare what
-tool should be used to interpret the script. That tool must then also be
-available within the step's container.
+Scripts that do not start with a shebang
+[shebang](https://en.wikipedia.org/wiki/Shebang_(Unix)) line will use a default
+value of `#!/bin/sh`, although users can override this by starting their script
+with a shebang to declare what tool should be used to interpret the script.
+That tool must then also be available within the step's container.
 
 This allows you to execute a Bash script, if the image includes `bash`:
 

examples/taskruns/step-script.yaml

@@ -10,6 +10,10 @@ spec:
         default: param-value
 
     steps:
+    - name: noshebang
+      image: ubuntu
+      script: |
+        echo "no shebang"
     - name: bash
       image: ubuntu
       env:

pkg/apis/pipeline/v1alpha1/task_validation.go

@@ -138,12 +138,6 @@ func validateSteps(steps []Step) *apis.FieldError {
 					Paths:   []string{"script"},
 				}
 			}
-			if !strings.HasPrefix(strings.TrimSpace(s.Script), "#!") {
-				return &apis.FieldError{
-					Message: "script must start with a shebang (#!)",
-					Paths:   []string{"script"},
-				}
-			}
 		}
 
 		if s.Name == "" {

pkg/apis/pipeline/v1alpha1/task_validation_test.go

@@ -631,20 +631,6 @@ func TestTaskSpecValidateError(t *testing.T) {
 			Message: `multiple volumes with same name "workspace"`,
 			Paths:   []string{"volumes.name"},
 		},
-	}, {
-		name: "step with script without shebang",
-		fields: fields{
-			Steps: []v1alpha1.Step{{
-				Container: corev1.Container{
-					Image: "my-image",
-				},
-				Script: "does not begin with shebang",
-			}},
-		},
-		expectedError: apis.FieldError{
-			Message: "script must start with a shebang (#!)",
-			Paths:   []string{"steps.script"},
-		},
 	}, {
 		name: "step with script and command",
 		fields: fields{

pkg/pod/pod_test.go

@@ -435,7 +435,7 @@ func TestMakePod(t *testing.T) {
 					Name:  "one",
 					Image: "image",
 				},
-				Script: "echo hello from step one",
+				Script: "#!/bin/sh\necho hello from step one",
 			}, {
 				Container: corev1.Container{
 					Name:         "two",
@@ -462,6 +462,7 @@ print("Hello from Python")`,
 				Args: []string{"-c", `tmpfile="/tekton/scripts/script-0-mz4c7"
 touch ${tmpfile} && chmod +x ${tmpfile}
 cat > ${tmpfile} << 'script-heredoc-randomly-generated-mssqb'
+#!/bin/sh
 echo hello from step one
 script-heredoc-randomly-generated-mssqb
 tmpfile="/tekton/scripts/script-1-78c5n"

pkg/pod/script.go

@@ -19,6 +19,7 @@ package pod
 import (
 	"fmt"
 	"path/filepath"
+	"strings"
 
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
 	"github.com/tektoncd/pipeline/pkg/names"
@@ -28,6 +29,7 @@ import (
 const (
 	scriptsVolumeName = "scripts"
 	scriptsDir        = "/tekton/scripts"
+	defaultShebang    = "#!/bin/sh\n"
 )
 
 var (
@@ -68,6 +70,25 @@ func convertScripts(shellImage string, steps []v1alpha1.Step) (*corev1.Container
 			continue
 		}
 
+		// Check for a shebang, and add a default if it's not set.
+		// The shebang must be the first non-empty line.
+		lines := strings.Split(s.Script, "\n")
+		hasShebang := false
+		for _, l := range lines {
+			if strings.TrimSpace(l) == "" {
+				continue
+			}
+			// This is the first non-empty line.
+			if strings.HasPrefix(l, "#!") {
+				hasShebang = true
+			}
+			break
+		}
+		script := s.Script
+		if !hasShebang {
+			script = defaultShebang + s.Script
+		}
+
 		// At least one step uses a script, so we should return a
 		// non-nil init container.
 		placeScripts = true
@@ -88,7 +109,7 @@ touch ${tmpfile} && chmod +x ${tmpfile}
 cat > ${tmpfile} << '%s'
 %s
 %s
-`, tmpFile, heredoc, s.Script, heredoc)
+`, tmpFile, heredoc, script, heredoc)
 
 		// Set the command to execute the correct script in the mounted
 		// volume.

pkg/pod/script_test.go

@@ -56,13 +56,23 @@ func TestConvertScripts(t *testing.T) {
 	}}
 
 	gotInit, got := convertScripts(images.ShellImage, []v1alpha1.Step{{
-		Script:    "script-1",
+		Script: `#!/bin/sh
+script-1`,
 		Container: corev1.Container{Image: "step-1"},
 	}, {
 		// No script to convert here.
 		Container: corev1.Container{Image: "step-2"},
 	}, {
-		Script: "script-3",
+		Script: `
+#!/bin/sh
+script-3`,
+		Container: corev1.Container{
+			Image:        "step-3",
+			VolumeMounts: preExistingVolumeMounts,
+			Args:         []string{"my", "args"},
+		},
+	}, {
+		Script: `no-shebang`,
 		Container: corev1.Container{
 			Image:        "step-3",
 			VolumeMounts: preExistingVolumeMounts,
@@ -77,13 +87,22 @@ func TestConvertScripts(t *testing.T) {
 		Args: []string{"-c", `tmpfile="/tekton/scripts/script-0-mz4c7"
 touch ${tmpfile} && chmod +x ${tmpfile}
 cat > ${tmpfile} << 'script-heredoc-randomly-generated-mssqb'
+#!/bin/sh
 script-1
 script-heredoc-randomly-generated-mssqb
 tmpfile="/tekton/scripts/script-2-78c5n"
 touch ${tmpfile} && chmod +x ${tmpfile}
 cat > ${tmpfile} << 'script-heredoc-randomly-generated-6nl7g'
+
+#!/bin/sh
 script-3
 script-heredoc-randomly-generated-6nl7g
+tmpfile="/tekton/scripts/script-3-j2tds"
+touch ${tmpfile} && chmod +x ${tmpfile}
+cat > ${tmpfile} << 'script-heredoc-randomly-generated-vr6ds'
+#!/bin/sh
+no-shebang
+script-heredoc-randomly-generated-vr6ds
 `},
 		VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount},
 	}
@@ -98,6 +117,15 @@ script-heredoc-randomly-generated-6nl7g
 		Command:      []string{"/tekton/scripts/script-2-78c5n"},
 		Args:         []string{"my", "args"},
 		VolumeMounts: append(preExistingVolumeMounts, scriptsVolumeMount),
+	}, {
+		Image:   "step-3",
+		Command: []string{"/tekton/scripts/script-3-j2tds"},
+		Args:    []string{"my", "args"},
+		VolumeMounts: []corev1.VolumeMount{
+			{Name: "pre-existing-volume-mount", MountPath: "/mount/path"},
+			{Name: "another-one", MountPath: "/another/one"},
+			{Name: "scripts", MountPath: "/tekton/scripts"},
+		},
 	}}
 	if d := cmp.Diff(wantInit, gotInit); d != "" {
 		t.Errorf("Init Container Diff (-want, +got): %s", d)