Skip to content

Instantly share code, notes, and snippets.

@imjasonh
Last active October 28, 2021 06:06
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 1 You must be signed in to fork a gist
  • Save imjasonh/7e351dc84bbd7474215bf8df64b48a22 to your computer and use it in GitHub Desktop.
Save imjasonh/7e351dc84bbd7474215bf8df64b48a22 to your computer and use it in GitHub Desktop.

Much of this proposal is cribbed from proposals @ahmetb has previously made, ideas are not my own.

Expected Behavior

Users have an easy cruft-free way to express a multi-statement script in the body of a Task step, without having to understand advanced topics about containers, like what an entrypoint or a command is.

Users should only need to have passing familiarity with a shell environment to be successful using Tekton.

Actual Behavior

A user who wants to use shell-isms (|, >, $(...), ${VAR:-default}) has to understand how to set the step's command to a shell, pass -c as an arg, and even then has to stay on YAML's good side to stay successful:

steps:
- image: ubuntu
  command: bash
  args:
  - -c
  - |
    echo $(cat ${FILE:/tmp/file}) >> out.txt)
    cat out.txt

(this example is contrived, sure, but it's complicated by all the YAML garbage around it)

Proposed Solution

Instead, steps could let users define these script strings directly in the API, to be invoked by a default shell. Compare:

steps:
- name: ubuntu
  script:
  - echo $(cat ${FILE:/tmp/file}) >> out.txt
  - cat out.txt

I hear what you're saying. But @ImJasonH, the Kubernetes Container type doesn't include a field called script. You are correct. Which is why I propose changing our API from

type TaskSpec struct {
  Steps []corev1.Container `json:"steps"`
  ... 
}

to:

type Step struct {
  corev1.Container
  Script []string `json:"script"`
}

type TaskSpec struct {
  Steps []Step `json:"steps"`
}

This doesn't change how YAML is structured, only how objects are described in Go code. It'll be a pretty annoying mechanical change to update internal code and existing API callers, but I happen to think it's worth it.

Validation

If a step specifies a script value it cannot also specify args or command.

Implementation

When a step specifies script, the controller will translate that to a Container passed to a Pod more or less the same as it does today.

A step specified as:

steps:
- name: golang:1.11
  script:
  - go get ./...
  - cd cmd/app
  - go build -o myapp .
  - tar czvf app.tar.gz myapp

will be equivalent to a step specified as:

steps:
- name: golang:1.11
  command: /bin/bash
  args:
  - -c
  - |
    set -euo pipefail
    eval "$(set -x; go get ./...)"
    eval "$(set -x; cd cmd/app)"
    eval "$(set -x; go build -o .)"
    eval "$(set -x; tar czvf app.tar.gz ./app)"

Lingering problems/questions

This syntactic sugar does not solve some other notable usability issues, such as cd only doing anything within the context of a single step -- subsequent steps are back to /workspace until they cd somewhere else or set workingdir.

How this syntax works with input subtitutions, e.g., ${inputs.params.flags} might also lead to confusion among users, which we should design for.

Furthermore, what if the container image doesn't include a shell, or includes some exotic non-Bash shell we don't want to support? Should we allow users to override /bin/bash in that case? Can/should we detect it?

@ahmetb
Copy link

ahmetb commented Apr 8, 2019

echo $(cat ${FILE:/tmp/file}) >> out.txt)
cat out.txt

ideally there should be

  • set -e; at the beginning
  • && at the end of the first line so the script doesn't continue to execute when first line fails :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment