Pipeline: Directly access Pipeline Params from taskSpec

[pritidesai]

With the PR #1554, we added support to specify taskSpec along with pipelineSpec so that we can embed task and pipeline specifications instead of referencing them. But, there is one caveat that it leads to when it comes to accessing pipeline params since its modeled based on the way task and pipeline are referenced. Ideally, we would like to directly access pipeline parameters directly instead of defining a param under taskSpec and then use that param. For example, task param MESSAGE is bound to pipeline param MESSAGE before referring to it inside the task script as $(inputs.params.MESSAGE):

apiVersion: tekton.dev/v1alpha1
kind: PipelineRun
metadata:
  name: pipelinerun-with-taskspec-to-echo-message
spec:
  pipelineSpec:
    params:
      - name: MESSAGE
        description: "Message, default is Hello World!"
        type: string
        default: "Hello World!"
    tasks:
      - name: echo-message
        taskSpec:
          inputs:
            params:
              - name: MESSAGE
                type: string
                default: "Hello World!"
          steps:
            - name: echo
              image: ubuntu
              script: |
                #!/usr/bin/env bash
                echo "$(inputs.params.MESSAGE)"
        params:
          - name: MESSAGE
            value: $(params.MESSAGE)
  params:
    - name: MESSAGE
      value: "Good Morning!"

Can we simplify this approach and instead of declaring task parameters, allow task authors to directly use pipeline params?

apiVersion: tekton.dev/v1alpha1
kind: PipelineRun
metadata:
  name: pipelinerun-with-taskspec-to-echo-message
spec:
  pipelineSpec:
    params:
      - name: MESSAGE
        description: "Message, default is Hello World!"
        type: string
        default: "Hello World!"
    tasks:
      - name: echo-message
        taskSpec:
          steps:
            - name: echo
              image: ubuntu
              script: |
                #!/usr/bin/env bash
                echo "$(params.MESSAGE)"
  params:
    - name: MESSAGE
      value: "Good Morning!"

[dibyom]

Interesting idea...possibly related to https://github.com/tektoncd/pipeline/issues/1185 ?
My $0.02 - while I definitely like the idea of being more consistent param usage, IMO we should do it across the board instead of just in one place. Otherwise something like this can become confusing -- say you start off with task specs embedded in your pipeline. Later you want to extract it out into its own task. Now you'd have to change all instance of params to input.params. The same thing applies if you want to take an existing task and embed it in your pipeline (say for small modifications/debugging/whatever)

[bobcatfish]

I agree with @dibyom - @pritidesai are you able to share more details around the use case you need this for?

imo embedding inside of Runs should only happen when either:

1. we're talking about getting up and running quickly (e.g. getting a new user to run a "hello world" and see it work right away
2. the Run is completely generated by a tool on top of it and the user isn't exposed to it at all

Embedding the specs reduces reuse, and like @dibyom is saying, the transition to wanting to extract the specs could be a bit rough

[pritidesai]

Hey @bobcatfish and @dibyom, this was emerged as part of the PR discussion #1554

https://github.com/tektoncd/pipeline/pull/1554#discussion_r354818856

[afrittoli]

IMO if I decide to embed it's because it's something that I don't plan on reusing, or perhaps like @bobcatfish said something small or completely generated; in either case I feel that being forced to use parameters in tasks makes the yaml more verbose, harder to read and ultimately it gives up part of the advantage of embedding.

[pritidesai]

yup agree with @bobcatfish, @dibyom, and @afrittoli, YAML becomes more verbose and harder to follow as the number of parameters grows and will be ideal to support for auto generated YAML but I guess generating a simplified YAML with different usage of params might create more confusion 😕

[bobcatfish]

I guess if it's being embedded anyway we might as well let ppl use the pipeline params :(

[tekton-robot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

[tekton-robot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
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.

/lifecycle stale

Send feedback to tektoncd/plumbing.

[tekton-robot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

[tekton-robot]

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[bobcatfish]

The request here makes sense so I'm inclined to reopen, however it seems like no one has needed it since this was originally requested?