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.

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)