Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: expression support in $() #2812

Closed
bitsofinfo opened this issue Jun 11, 2020 · 36 comments
Closed

Feature: expression support in $() #2812

bitsofinfo opened this issue Jun 11, 2020 · 36 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@bitsofinfo
Copy link

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
Copy link
Collaborator

@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: #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
Copy link
Collaborator

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

@bitsofinfo
Copy link
Author

@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
Copy link
Member

dibyom commented Jun 12, 2020

/kind feature

@tekton-robot tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 12, 2020
@tekton-robot
Copy link
Collaborator

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
Copy link
Collaborator

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 added the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Aug 14, 2020
@tekton-robot
Copy link
Collaborator

@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.

@vdemeester
Copy link
Member

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

@tekton-robot tekton-robot reopened this Aug 17, 2020
@tekton-robot
Copy link
Collaborator

@vdemeester: Reopened this issue.

In response to this:

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

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.

@tekton-robot tekton-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Aug 17, 2020
@bobcatfish
Copy link
Collaborator

@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
Copy link
Author

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
Copy link
Collaborator

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, tektoncd/community#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
Copy link
Collaborator

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 tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 22, 2020
@bitsofinfo
Copy link
Author

/remove-lifecycle stale

@tekton-robot tekton-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 22, 2020
@bitsofinfo
Copy link
Author

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

@bobcatfish
Copy link
Collaborator

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 bitsofinfo changed the title Feature: CEL expression support in $() Feature: expression support in $() Nov 23, 2020
@bitsofinfo
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Author

how is the triggers project currently handling this?

@bobcatfish
Copy link
Collaborator

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
Copy link
Collaborator

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 tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 22, 2021
@afrittoli
Copy link
Member

/remove-lifecycle stale

@tekton-robot tekton-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 3, 2021
@afrittoli
Copy link
Member

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 #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
Copy link
Collaborator

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 tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 1, 2021
@bitsofinfo
Copy link
Author

/remove-lifecycle stale

@tekton-robot tekton-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 2, 2021
@tekton-robot
Copy link
Collaborator

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 tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 15, 2021
@bitsofinfo
Copy link
Author

/remove-lifecycle stale

@tekton-robot tekton-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 18, 2021
@tekton-robot
Copy link
Collaborator

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 tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 16, 2022
@bitsofinfo
Copy link
Author

/remove-lifecycle stale

@tekton-robot tekton-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 18, 2022
@tekton-robot
Copy link
Collaborator

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 tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 18, 2022
@tekton-robot
Copy link
Collaborator

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 tekton-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 18, 2022
@tekton-robot
Copy link
Collaborator

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
Copy link
Collaborator

@tekton-robot: Closing this issue.

In response to this:

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.

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.

@bitsofinfo
Copy link
Author

/reopen still valid

@bremme
Copy link

bremme commented Jan 13, 2023

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests

8 participants