pipeline icon indicating copy to clipboard operation
pipeline copied to clipboard

More flexible way to provide value for object keys

Open chuangw6 opened this issue 3 years ago • 4 comments

Feature request

When users provide object param value in taskSpec.Params.default (or taskrunSpec.Params.Value), it might be beneficial if it's allowed that users can provide values for only a subset of the keys instead of having to provide values for all keys which is now enforced by the validation webhook (or reconciler for taskrun level).

For example, the following task & taskrun definition are invalid in the current implementation because validation webhook will complain about missing key - commit in taskSpec.Params.default, and reconciler will complain about missing key - url in taskrunSpec.Params.Value . But it would be great if the examples are treated as valid to allow that only url is provided in default, and commit is provided from taskrun.

apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: object-task
spec:
  params:
    - name: giturl
      type: object
      properties:
        url:
          type: string
        commit:
          type: string
      default:
        url: "abc.com"
  steps:
    ...
---
apiVersion: tekton.dev/v1beta1
kind: TaskRun
metadata:
  generateName: object-taskrun
spec:
  params:
    - name: gitrepo
      value:
        commit: "sha123"
  taskRef:
    name: object-task

Use case

Say we want to have 10 taskruns to run the above taskrun, and we want all 10 taskruns use same value for the key url, but use different value for the commit key. If it's allowed that only the shared value for url is provided in the default, and each taskrun only provides the value for the key commit, there will be no redundant lines to give value for commit from taskrun level.

chuangw6 avatar Aug 03 '22 20:08 chuangw6

The idea was sparked from a conversation I had with @chitrangpatel.

If anyone has any comments/feedback/questions about this, please add them here. Thanks

chuangw6 avatar Aug 03 '22 20:08 chuangw6

cc @wlynch @vdemeester @chitrangpatel @dibyom @jerop @lbernick @ywluogg @Yongxuanzhang

chuangw6 avatar Aug 03 '22 20:08 chuangw6

sgtm! I think what you described matches what I would expect the behavior to be.

It might be helpful to add a simple TaskRun spec that doesn't work today, but perhaps should?

wlynch avatar Aug 03 '22 20:08 wlynch

sgtm! I think what you described matches what I would expect the behavior to be.

Yeah, in the initial implementation, we just think the object param as whole i.e. if default/taskrun wants to provide value for it, they must provide value for all the keys. But just thinking it might be beneficial if they just provide a subset of keys as long as they cover all the keys together.

It might be helpful to add a simple TaskRun spec that doesn't work today, but perhaps should?

SG! Added!

chuangw6 avatar Aug 03 '22 20:08 chuangw6