tektoncd / pipeline

A cloud-native Pipeline resource.
https://tekton.dev
Apache License 2.0
8.43k stars 1.77k forks source link

Allow to escape a variable substitution expression #4931

Open mgoltzsche opened 2 years ago

mgoltzsche commented 2 years ago

Feature request

Currently, using Tekton v0.36.0, it is not possible to escape a variable substitution expression. However it should be possible to do so explicitly for a particular variable substitution.

Related issue: https://github.com/tektoncd/pipeline/issues/3718

cc @abayer

Use case

Being able to escape Tekton's expressions is particularly important when building a DSL on top of Tekton that is authored by users that don't deal with Kubernetes and Tekton at all. Those users might use Tekton expressions accidentally or intentionally although they're not supposed to do so because Tekton's syntax would leak into the DSL/abstraction syntax (Tekton would be an implementation detail which should not be user-facing and that could be replaced at any time). When users accidentally use a Tekton expression within the abstraction the problem gets worse since the Tekton webhook rejects the resource, not even providing understandable feedback to the user.

To illustrate the problem, let's imagine a fictional DSL that supports a GitHub Actions-like disambiguated expression syntax (${{...}}) that gets translated into Tekton expressions ($(...)). Using such a DSL, users should also be able to specify shell scripts that may contain $(...) expressions that might clash with Tekton's syntax. The following example DSL snippet refers to a parameter someparam using the DSL's syntax and calls a shell script params.processor:

task:
  params:
    someparam:
      type: string
  steps:
  - image: fakeimage:1.2.3
    script: |
      echo ${{params.someparam}}
      echo $(params.processor)

The corresponding generated Tekton Task would look as follows:

apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: some-task
spec:
  params:
  - name: someparam
    type: string
  steps:
  - name: some-step
    image: fakeimage:1.2.3
    script: |
      echo $(params.someparam)
      echo $(params.processor)
    env:
    - name: PARAM_someparam
      value: $(params.someparam)

In this case the name of the shell script clashes with Tekton's expression syntax. Therefore, when attempting to apply the Tekton Task, it gets rejected by the Tekton webhook with the following error:

$ kubectl apply -f task.yaml
Error from server (BadRequest): error when creating "task.yaml": admission webhook "validation.webhook.pipeline.tekton.dev" denied the request: validation failed: non-existent variable in "echo $(params.someparam)\necho $(params.processor)\n": spec.steps[0].script

This problem applies to all Tekton resources, not just Tasks.

Proposed solution

To solve this edge case reliably, Tekton could support $ as escape character so that $(params.processor) can be passed through as plain text when it is escaped using $$(params.processor) (which model transformation implementations could do automatically for users). Ideally there should be a public function within the Tekton code base to escape Tekton expressions correspondingly.

mgoltzsche commented 2 years ago

As a workaround to escape the Tekton expression I tried to define a dollar parameter within the Tekton Task and give it a default value of $ and then substitute occurrences of $ within user-defined plain text template fragments/scripts with $(params.dollar), making the generated Tekton Task and a corresponding PipelineRun look as follows:

---
apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: some-task
spec:
  params:
  - name: someparam
    type: string
    default: somevalue
  - name: dollar
    type: string
    default: $
  steps:
  - name: some-step
    image: fakeimage:1.2.3
    script: |
      echo $(params.someparam)
      echo $(params.dollar)(params.processor)
    env:
    - name: PARAM_someparam
      value: $(params.someparam)
---
apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
  name: some-pipelinerun
spec:
  pipelineSpec:
    tasks:
    - name: some-task
      taskRef:
        name: some-task

While the Tekton webhook accepts the Task this way, the PipelineRun fails with a similar error message the webhook returned in the previously described attempt: failed to create task run pod "some-pipelinerun-some-task": non-existent variable in "echo somevalue\necho $(params.processor)\n": spec.steps[0].script. Maybe missing or invalid Task default/some-task. Apparently this is because Tekton performs the variable substitution twice or rather also substitutes variables within the output of its variable substitution, making it impossible to escape such a substitution :cry:!

Using such a workaround would be okay-ish if it would work but Tekton shouldn't perform the variable substitution twice - doing so can cause all kinds of unexpected errors and potentially even security vulnerabilities allowing expression injection attacks also in regular Tekton workflows when Tekton expressions within user-defined parameter values are evaluated (those values might be passed through from external systems/webhooks, e.g. with a git commit message).

tekton-robot commented 2 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale with a justification. Stale issues rot after an additional 30d of inactivity and eventually close. If this issue is safe to close now please do so with /close with a justification. If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

abayer commented 2 years ago

/lifecycle frozen

This is still relevant - our escaping + substitution interaction is fragile.

gdlxn-ibm commented 1 year ago

I found workaround that works for me. What I did is to use a dollar shell variable to escape the Tekton parameter:

---
apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: some-task
spec:
  params:
  - name: someparam
    type: string
    default: somevalue
  steps:
  - name: some-step
    image: fakeimage:1.2.3
    script: |
      dollar="$"
      echo $(params.someparam)
      echo ${dollar}(params.processor)
    env:
    - name: PARAM_someparam
      value: $(params.someparam)
---
apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
  name: some-pipelinerun
spec:
  pipelineSpec:
    tasks:
    - name: some-task
      taskRef:
        name: some-task
jandusek4 commented 4 months ago

I found workaround that works for me. What I did is to use a dollar shell variable to escape the Tekton parameter:

---
apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: some-task
spec:
  params:
  - name: someparam
    type: string
    default: somevalue
  steps:
  - name: some-step
    image: fakeimage:1.2.3
    script: |
      dollar="$"
      echo $(params.someparam)
      echo ${dollar}(params.processor)
    env:
    - name: PARAM_someparam
      value: $(params.someparam)
---
apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
  name: some-pipelinerun
spec:
  pipelineSpec:
    tasks:
    - name: some-task
      taskRef:
        name: some-task

After many hours of searching, this one finally allowed to me properly handle the issue with $() tekton interpretation which actually should be left alone as string only - lifesaver.