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

Add support for splitting strings to CEL expressions. #411

Merged
merged 1 commit into from
Feb 6, 2020

Conversation

bigkevmcd
Copy link
Member

Changes

This adds an additional CEL function for splitting strings on a separator.

e.g. body.ref.split('/') would split the ref on '/' and return a list of strings.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

See the contribution guide for more details.

Release Notes

This adds a CEL expression function for splitting strings.

e.g. to extract the tag or branch from a hook event in an overlay:

            overlays:
            - key: intercepted.branch_name
              expression: "body.ref.split('/')[2]"

@tekton-robot tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 4, 2020
@bigkevmcd
Copy link
Member Author

This could easily be flipped to become split(body.ref, '/') instead of body.ref.split('/')

@dibyom
Copy link
Member

dibyom commented Feb 5, 2020

This could easily be flipped to become split(body.ref, '/') instead of body.ref.split('/')

Yeah, I think I like split(body.ref, '/') slightly better since it also matches the pattern from truncate

This adds an additional CEL function for splitting strings on a separator.

e.g. split(body.ref, '/') would split the ref on '/' and return a list of strings.
@bitsofinfo
Copy link
Contributor

bitsofinfo commented Feb 6, 2020

really, really need this function to be able to mutate values and capture them as new fields (overlay) in the body. Please merge at least into the master so I can start playing with it. thanks!

@bobcatfish
Copy link
Collaborator

bobcatfish commented Feb 6, 2020

Thanks for adding this, complete with tests cases and docs @bigkevmcd !!

This seems fine but I think it would be worth a larger conversation about how many of these custom functions we want to support.

Shameless plug for Rego, looks like it comes with a number of string manipulation functions built in: https://www.openpolicyagent.org/docs/latest/policy-reference/#strings

Related: @wlynch feature request for string splitting google/cel-go#306

/approve
/lgtm

image

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 6, 2020
@tekton-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bobcatfish

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 6, 2020
@bitsofinfo
Copy link
Contributor

@bobcatfish I agree, would be great if some bridge could be developed to just expose functions available in other libraries that would not have to be directly maintained by tekton. i.e. like how helm exposes all the go template functions provided by sprig (http://masterminds.github.io/sprig/) greatly enhancing the capabilities.

if another lib like you suggested could be added to expose much more CEL string manipulation functions that would be awesome!

@tekton-robot tekton-robot merged commit 46c7c2e into tektoncd:master Feb 6, 2020
@bigkevmcd
Copy link
Member Author

@bobcatfish yes, I'd like to deprecate match in favour of canonical as there's less ambiguity (there's a matches function in CEL already).

Other than that, I can't see many more items being added, splitting strings, decoding base64 and trimming strings are the things that I wanna do in my pipelines, but clearly, there might be other useful things, we're dealing with broadly the same data across different environments.

I currently have no plans for further functions, but it's fairly easy to add them... :-)

@dibyom
Copy link
Member

dibyom commented Feb 6, 2020

Agreed. I think we need base64 (for things like pub/sub events that are base64 encoded); trimming strings (tags, commits, refs). The header one is interesting...mainly due to how go represents header values as a array of strings. Other than these 3, I don't see any new ones in the short term!

@bitsofinfo
Copy link
Contributor

is there a nightly release I can point to that has this? or i'll have to build it all and deploy locally?

@TristonianJones
Copy link
Contributor

Hey everyone, awesome to see CEL being embraced in the wild. I am totally open to reviewing PRs for google/cel-go#306 if that would help accelerate your project.

@bitsofinfo
Copy link
Contributor

is this not possible?

apiVersion: tekton.dev/v1alpha1
kind: TriggerBinding
metadata:
  name: deploy
spec:
  params:

    - name: appName
      value: "$(split(body.action_values[0],'_')[1])"

@bigkevmcd
Copy link
Member Author

is this not possible?

No, see #367

But, you can use the CEL interceptor, without actually intercepting, and use the overlay function there.

Something like this should work:

apiVersion: tekton.dev/v1alpha1
kind: EventListener
metadata:
  name: listener-interceptor
spec:
  triggers:
    - name: demo-trigger
      interceptors:
        - cel:
            overlays:
            - key: intercepted.appName
              expression: "split(body.action_values[0],'_')[1]"
      bindings:
        - name: deploy
      template:
        name: deploy-template

Then you could use the value like this:

apiVersion: tekton.dev/v1alpha1
kind: TriggerBinding
metadata:
  name: deploy
spec:
  params:
    - name: appName
      value: $(body.intercepted.appName)

@TristonianJones
Copy link
Contributor

BTW, I created google/cel-go#316, so if your favorite string function isn't there, just let me know and I'll see if I can get it in.

@bitsofinfo
Copy link
Contributor

@bigkevmcd yep thats what i ended up doing, thanks!

@bitsofinfo
Copy link
Contributor

bitsofinfo commented Mar 5, 2020

@TristonianJones would be great to have a replace function. or... can anyone point me to an example of how to essentially do a string replace w/ CEL? I've looked but can't think of anything.

i need to take a value and replace . with - so I can use that value in meta-data names in a pipelinerun.

looks like it will be in go-cel eventually w/ google/cel-go#316

@bigkevmcd bigkevmcd deleted the string-separation branch March 10, 2020 10:28
savitaashture pushed a commit to savitaashture/triggers that referenced this pull request Apr 1, 2021
Remove patch changes related to disabling EL to run as https
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants