From fcff723dc310fbdd8235484669ee263355c16bfc Mon Sep 17 00:00:00 2001 From: Dan Lorenc Date: Wed, 4 Dec 2019 16:02:16 -0600 Subject: [PATCH] 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 | 9 +++--- examples/taskruns/step-script.yaml | 4 +++ pkg/apis/pipeline/v1alpha1/task_validation.go | 6 ---- .../pipeline/v1alpha1/task_validation_test.go | 14 -------- pkg/pod/pod_test.go | 3 +- pkg/pod/script.go | 23 ++++++++++++- pkg/pod/script_test.go | 32 +++++++++++++++++-- 7 files changed, 63 insertions(+), 28 deletions(-) diff --git a/docs/tasks.md b/docs/tasks.md index 1fa0717285f..4d561b70fa5 100644 --- a/docs/tasks.md +++ b/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`: diff --git a/examples/taskruns/step-script.yaml b/examples/taskruns/step-script.yaml index 68e750fb03a..c5b173024fe 100644 --- a/examples/taskruns/step-script.yaml +++ b/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: diff --git a/pkg/apis/pipeline/v1alpha1/task_validation.go b/pkg/apis/pipeline/v1alpha1/task_validation.go index 2082212526c..0ced3df1d3b 100644 --- a/pkg/apis/pipeline/v1alpha1/task_validation.go +++ b/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 == "" { diff --git a/pkg/apis/pipeline/v1alpha1/task_validation_test.go b/pkg/apis/pipeline/v1alpha1/task_validation_test.go index daf2c152c5f..7b8b8447489 100644 --- a/pkg/apis/pipeline/v1alpha1/task_validation_test.go +++ b/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{ diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index de292cde6b9..649187b0216 100644 --- a/pkg/pod/pod_test.go +++ b/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" diff --git a/pkg/pod/script.go b/pkg/pod/script.go index bc052361672..6f7ead8edac 100644 --- a/pkg/pod/script.go +++ b/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. diff --git a/pkg/pod/script_test.go b/pkg/pod/script_test.go index c5d7fcb2e97..3c16e75b0a3 100644 --- a/pkg/pod/script_test.go +++ b/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)