tektoncd / pipeline

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

Support using default value if parameter is empty #3758

Closed jmcshane closed 3 years ago

jmcshane commented 3 years ago

Feature request

Allow parameter defaulting to override empty value

Use case

In our pipelineruns, we are passing calculated values from the repository into a variety of tasks from the tekton catalog. It would be helpful if these pipelines could allow setting via a parameter of their own, but if the parameter is empty, pick up the task param default value.

Understandably, there is an important differentiation here between a valid empty value and one you want to override. A field such as defaultIfEmpty on a task or pipeline param would be incredibly helpful. Then, you can always send those values as empty via API calls or cel expressions (in triggers), but still have the desired task or pipeline behavior when the value is an empty string.

jmcshane commented 3 years ago

Here's a very simple use case, imagine I have a pipeline for running go lint, go test, and go build on a repository. Each of those tasks in the catalog can take a flags parameter. In the pipeline, I specify lint-flags, test-flags, and build-flags as the argument. I pass each of these flags to the associated taskRef. If these flags are unset, I want to pick up the value of the default in the clustertask.

I'm pretty sure I'm capable of writing this PR, but want to make sure it jives with the maintainer team.

ghost commented 3 years ago

If I'm understanding right the scenario is something like this:

kind: Task
metadata:
  name: go-build
spec:
  params:
  - name: build-flags
    default: "-a -p 8"
---
kind: Pipeline
  metadata:
    name: lint-test-build
spec:
  taskRef:
    name: go-build
  params:
  - name: build-flags
---
kind: PipelineRun
spec:
  pipelineRef:
    name: lint-test-build
  params:
  - name: build-flags
    value: "{{ repo-build-flags }}" # Computed by outside process. Might be an empty string.

In the case that the repo-build-flags computed value is an empty string you'd still like to utilize the Task's default value?

One approach that would work today would be to add an intermediary Task in your Pipeline that applies default values when a param is empty. Here's what that hypothetical Pipeline could look like:

kind: Pipeline
  metadata:
    name: lint-test-build
spec:
  taskRef:
    name: go-build
  params:
  - name: build-flags
  tasks:
  - name: apply-build-flag-defaults
    taskRef:
      name: defaulter # emits result named "out" with value of `in` or `default-value` if `in` is empty
    params:
    - name: in
      value: "$(params.build-flags)"
    - name: default-value
      value: "-a -p 8"
  - name: build
    taskRef:
      name: go-build
    params:
    - name: build-flags
      value: "$(tasks.apply-build-flag-defaults.results.out)"

Here's what the defaulter Task could look like:

kind: Task
metadata:
  name: defaulter
spec:
  params:
  - name: in
    description: "The value to be checked for emptiness."
  - name: default-value
    description: "The value to return in place of empty string."
  - name: sentinel
    description: "If in param matches sentinel it will be replaced with default-value."
    default: ""
  results:
  - name: out
    description: "Either the value of the in param or default-value if in equals sentinel."
  steps:
  - image: alpine:latest
    script: |
      if [ "$(params.in)" = "$(params.sentinel)" ]; then
        echo -n "$(params.default-value)" > "$(results.out.path)"
      else
        echo -n "$(params.in)" > "$(results.out.path)"
      fi

The Pipeline can decide both what the default-value should be as well as which specific value should trigger the defaulting behaviour. There are definitely downsides to this - the Task's default might be duplicated in the Pipeline, the extra Pod created to process the defaulting behaviour - but I think it's achievable with today's feature set.

The question then becomes whether Tekton Pipelines should introduce this as a feature of its API. My gut reaction is: probably not. As much as possible we try to only add features when there is literally no other way for an existing feature to solve the use case. I think there'd be a risk of immediate feature scope increase - for example, the set of "replaceable values" might need to be defined by the user - not only empty strings but also strings which are entirely whitespace, or match a specific regex, etc etc. We'd be back to suggesting users provide their own PipelineTasks to handle the processing. That's just my immediate reaction though. Happy to discuss further.

jmcshane commented 3 years ago

Thanks for the thoughtful reply @sbwsg. While I'm preferential to a single flag on the param rather than a cluster task to set these defaults, I see the value in avoiding feature creep. I'll close this issue for now and try out your suggested approach.