tektoncd / pipeline

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

Reusing Params substitution in StepActions does not always work #7935

Open haroonc opened 2 months ago

haroonc commented 2 months ago

Expected Behavior

Assuming StepAction has two Params, param1 and param2, when a Param2 uses value of $(params.param1) as default value, we expect it to be substituted with the value of param1 provided by the user.

Actual Behavior

TaskRun fails with an error:

* failed to create task run pod "simple-task-run": non-existent variable in "$(params.param1)": steps[0].env[VAR2]. Maybe invalid TaskSpec

Steps to Reproduce the Problem

Apply following StepAction and TaskRun

apiVersion: tekton.dev/v1alpha1
kind: StepAction
metadata:
  name: simple-step-action
  labels:
    app.kubernetes.io/version: "0.1.0"
  annotations:
    tekton.dev/pipelines.minVersion: "0.41.0"
    tekton.dev/categories: Build Tools
    tekton.dev/tags: maven
    tekton.dev/displayName: "simple-step-action"
    tekton.dev/platforms: "linux/amd64"
spec:
  params:
    - name: param1
      type: string
      description: >-
        This param is required
    - name: param2
      description: >-
        This param uses value of `param1` as default value
      type: string
      default: $(params.param1)

  image: debian
  env:
    - name: "VAR1"
      value: "$(params.param1)"
    - name: "VAR2"
      value: "$(params.param2)"
  script: |
    #!/bin/sh
    set -eu

    echo "Value of VAR1 is correctly substituted to [${VAR1}]"

    #Value of VAR2 is going to be "$( params.param2 )"
    echo "If param2 is not defined value of VAR2 is incorrect [${VAR2}]"

---
apiVersion: tekton.dev/v1
kind: TaskRun
metadata:
  name: simple-task-run
spec:
  taskSpec:
    steps:
      - name: param-substitution-test
        ref:
          name: simple-step-action
        params:
          - name: param1
            value: "Value for Param 1"
      - name: param-substitution-test-2
        ref:
          name: simple-step-action
        params:
          - name: param1
            value: "Value for Param 1"
          - name: param2
            value: "Value for Param 2"

Additional Info

Client version: 0.32.0
Pipeline version: v0.59.0
chitrangpatel commented 2 months ago

I think this is a general issue in param substitutions. This is true for even Tasks. My guess is that we do not perform substitution of params into other params at the same level. e.g. param substitutions of param a into defaults of param b in the TaskSpec. I'm not sure why though 🤔.

I think the ability to say that the default of param b is the value of param a is quite useful and we should be doing that.

WDYT @tektoncd/core-maintainers ?

vdemeester commented 2 months ago

My guess is that we do not perform substitution of params into other params at the same level. e.g. param substitutions of param a into defaults of param b in the TaskSpec. I'm not sure why though 🤔.

Yes, we do not perform "multi-pass" during substitution (aka a param substitution giving another param to substitute). I think there is another issue in the tracker about this as well.