search

tektoncd / pipeline

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

Feature: expression support in $() #2812

Closed bitsofinfo closed 2 years ago

bitsofinfo commented 4 years ago

Would be great to be able to do something like this in a Task (or anywhere $() variable syntax is supported)

$(inputs.params.payload.decodeb64().parseJSON().key1)

CEL is supported in triggers CEL interceptors, would be great if just globally available in all of tekton.

bobcatfish commented 4 years ago

@bitsofinfo could you give some examples of the kinds of things you'd like to use CEL for? Before we add CEL support fullstop we'd want to make sure it's the best solution to meet your needs.

note we are also exploring CEL and JSONPath for conditions: https://github.com/tektoncd/pipeline/issues/2127

If you need CEL mostly for expression evaluation, we've talked before about adding JSONPath support, do you know if JSONPath could meet your needs or do you need functionality provided by CEL that isn't provided by JSONPath?

bobcatfish commented 4 years ago

( @ImJasonH maybe another use case for the CEL custom task? :D)

bitsofinfo commented 4 years ago

@bobcatfish here is an example:

$(inputs.params.payload.decodeb64().parseJSON().key1)

Others? well I don't have specifics beyond mine above. But I can just imagine how much i'd use such a capability to do simple string mutations without having to echo the params into shells for further mutation, it gets awkward. Triggers uses CEL and the functions provided by it for string manipulation works for me. Would be consistent to use that, but if JSONPath provides the same + additional functionality + more functions... then use that. I really would just like more capability for expressions, it means I have to write less custom code to do things with these params and other variables.

dibyom commented 4 years ago

/kind feature

tekton-robot commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@tekton-robot: Closing this issue.

In response to [this](https://github.com/tektoncd/pipeline/issues/2812#issuecomment-674244097): >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](https://github.com/tektoncd/plumbing). Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
vdemeester commented 4 years ago

/remove-lifecycle rotten /remove-lifecycle stale /reopen

tekton-robot commented 4 years ago

@vdemeester: Reopened this issue.

In response to [this](https://github.com/tektoncd/pipeline/issues/2812#issuecomment-674763361): >/remove-lifecycle rotten >/remove-lifecycle stale >/reopen Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
bobcatfish commented 4 years ago

@jerop has been planning to make an issue about having CEL custom task support (in the context of the conditions work she has been doing), maybe it makes sense to close this one in favor of that issue once it exists?

bitsofinfo commented 4 years ago

if the new issue covers the feature I'm describing of making CEL expressions available anywhere in tekton where $() syntax is already supported sure, otherwise please leave it open.

bobcatfish commented 4 years ago

The new issue would be about supporting CEL with custom tasks, so it wouldn't quite do what you're describing.

About adding CEL support in $(), we're working on some API design principles, https://github.com/tektoncd/community/pull/171 and adding CEL support directly into our API conflicts with a few of them. Specifically it expands our API surface to include all of CEL and doesn't make it possible for folks to use other expression languages if they desire.

Using something like custom tasks to implement this means that supporting CEL can be optional and also that folks can use other custom tasks to express any expression language they want.

tekton-robot commented 3 years ago

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.

bitsofinfo commented 3 years ago

/remove-lifecycle stale

bitsofinfo commented 3 years ago

CEL, JSONPath, whichever, whatever. Being able to do expressions anywhere $() would still be a great addition

bobcatfish commented 3 years ago

Hey @bitsofinfo - we've been working on a set of "design principles" over in the community repo - some of the principles around flexibility apply here: https://github.com/tektoncd/community/blob/master/design-principles.md#flexibility

When a specific choice (tool, resource, language, etc) has to be made at the Task or Pipeline levels, users should be able to extend it to add their own choices.

Even in triggers when we have support for CEL we try to make sure we can support other languages and tools as well - so I don't think it's likely we'd want to add CEL support directly in the $() syntax (unless we revisit those design principles).

I could see maybe jsonpath support? (also touched on in #3255 and #1393) But that would give you quite a bit less than CEL would. But if your use cases are mostly around indexing into maps and arrays, vs. being needing a full expression language, maybe we can pursue that instead?

We also have #3149 which would allow for CEL support via custom tasks (i.e. no extra pod needed, values provided via results).

bitsofinfo commented 3 years ago

I've removed CEL from the subject. The particular implementation I guess doesn't matter. But being able to do something like this, whatever the impl, would be great. 

$(inputs.params.payload.decodeb64().parseJSON().key1)
bobcatfish commented 3 years ago

The decodeb64() and parseJSON() bits are interesting, that'd definitely push more toward an expression language, I'm guessing jsonpath doesn't have support for anything like that :(

I'm not sure how we'd be able to a provide functionality like that while: a) remaining agnostic and letting ppl use the lang of their choice b) keeping Tasks and Pipelines conformant (e.g. can use a Task in the catalog on any Tekton based system)

The only way I could imagine that working is maybe if you could provide something to your pipelines controller, like an interceptor, that knows how to parse the expression? But at that point that seems potentially more complicated than using a Custom Task to evaluate the expression.

bitsofinfo commented 3 years ago

how is the triggers project currently handling this?

bobcatfish commented 3 years ago

In triggers this is being handled with "interceptors" which run as separate services, e.g. the CEL interceptor which is currently built into the eventlistener but won't be in the future

(I'm hoping that one day our CEL custom task controller and the triggers cel interceptor can be the same thing, i.e. run the same code)

tekton-robot commented 3 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.

afrittoli commented 3 years ago

/remove-lifecycle stale

afrittoli commented 3 years ago

This is still an open issue.

I agree with @bobcatfish that we should keep API changes to a minimum, so perhaps it would help us take a step back on this a look at concrete use cases that these would solve, so that we see if there is any alternative solution.

I also feel like this is related to https://github.com/tektoncd/pipeline/issues/1393.

If we support something like $(inputs.params.payload.decodeb64().parseJSON().key1), the content of key1 would be marshalled back into JSON? What if it's a list, would it be passed as a parameter of type list?

One slightly related issue that I'm facing is when trying to use a task loop with a dynamically generated list of inputs. Example: I want to run a CI job that does a container build on all images touched by a PR. A task can produce such list and store it in a result, but the custom task won't be able to read it because it expects an input parameter of type list, not string.

tekton-robot commented 3 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.

bitsofinfo commented 3 years ago

/remove-lifecycle stale

tekton-robot commented 3 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.

bitsofinfo commented 3 years ago

/remove-lifecycle stale

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.

bitsofinfo commented 2 years ago

/remove-lifecycle stale

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.

tekton-robot commented 2 years ago

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten with a justification. Rotten issues close after an additional 30d of inactivity. 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 rotten

Send feedback to tektoncd/plumbing.

tekton-robot commented 2 years ago

Rotten issues close after 30d of inactivity. Reopen the issue with /reopen with a justification. Mark the issue as fresh with /remove-lifecycle rotten with a justification. If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

tekton-robot commented 2 years ago

@tekton-robot: Closing this issue.

In response to [this](https://github.com/tektoncd/pipeline/issues/2812#issuecomment-1159223185): >Rotten issues close after 30d of inactivity. >Reopen the issue with `/reopen` with a justification. >Mark the issue as fresh with `/remove-lifecycle rotten` with a justification. >If this issue should be exempted, mark the issue as frozen with `/lifecycle frozen` with a justification. > >/close > >Send feedback to [tektoncd/plumbing](https://github.com/tektoncd/plumbing). Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
bitsofinfo commented 2 years ago

/reopen still valid

bremme commented 1 year ago

I was looking to use expressions inline to modify some parts of my task. This sound like a solution that could work for me. Just commenting to keep the issue alive.

TristonianJones commented 1 year ago

@bitsofinfo @bremme Is there any support you could use from the CEL team to make this happen?