KEP-2876: Core library proposal for CEL

[jpbetz]

• One-line PR description: Propose what core library of functions we will make available to CEL for Beta

Proposed additions are:

• isSorted for lists with comparable elements. This is useful for ensuring that a list is kept in-order.
• sum, min and max functions for lists of summable/comparable elements. This is the
  core set of aggregate functions for lists, with CEL they can also be used on scalars by using defining list literals
  inline , e.g. [self.val1, self.val2].max()
• indexOf / lastIndexOf for lists (overloading the existing string functions), this can be useful for
• validating partial order (i.e. the tasks of a workflow)

Also:

• Explain mechanism to make post-alpha library updates (which will need to be done with great care given that they will make CRDs incompatible with older Kubernetes versions)
• Update KEP to reflect that numerical comparisons will be fully supported in Beta
• Update KEP to reflect that int-or-string will be typesafe in Beta

• Issue link:
  CRD Validation Expression Language #2876

• Other comments:

/sig api-machinery

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jpbetz

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:

• keps/sig-api-machinery/OWNERS [jpbetz]

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

[jpbetz]

Thanks for the review @liggitt. Feedback addressed. This is now down to a fairly short list of proposed changes.

[deads2k]

I recall from xpath that you could add functions with a namespace associated with them, namespace:doThing( as I recall. This allowed versioning of functions and using it here would allow us to know whether an expression could be enforced by the server during creation of the CRD. Is that a thing in CEL?

[jpbetz]

No, unfortunately. There is no namespacing. All functions are declared in a global scope. Functions are uniquely identified by the combination of name + arguments, so multiple (overloaded) functions of the same name may coexist and the compiler will bind the correct one based on arguments. (The 1st argument can be the receiver, so foo(x) and x.foo() are equivalent callsites).

cc @TristonianJones for any insights on how to handle versioning/function library evolution.

[liggitt]

lgtm. looks like toc needs updating, and over to @deads2k

[deads2k]

I'd like to explore the limits on this. Is it possible to take a URL (scheme://host:) and parse out the host if we allow

1. dns names
2. ipv4
3. ipv6
4. ipv4 mapped in ipv6

I'm interested in how difficult this is to do with existing functions and if it is very difficult, would we really hold this line? I see it messed up hand-written parsing code very frequently.

[jpbetz]

I think it's intractable with what we have in CEL now due to the lack of regex Find.

Options:

1. Add regex find and findAll - Very unlikely that people will get the regex exactly right
2. Add URL parsing functions. This should probably be: string -> URL conversion + getScheme, getUserinfo, getHost, getPath, getQuery, getFragment

I'm currently in favor of adding both. We made an explicit decision to not add regex replace (shouldn't really be needed for validation), but I hadn't thought carefully about match vs. find.

[deads2k]

Another one that stands out is the proper parsing of image pull specs. I bet you could get very close with clever regexes and splits, but would avoid providing a built-in? Grammar here for reference: https://github.com/distribution/distribution/blob/v2.7.1/reference/reference.go#L4-L24

[jpbetz]

Looks like we use that in k8s already.

I'm a bit concerned with keeping this stable since the format is fairly involved.

Looks like the absolute minimum would be:

• string -> image type conversion
• getName
• hasTag/getTag
• hasDigest/getDigest

But the name has a host and a port, and the digest has an algorithm, and the tag can be a semver and someone will want to compare those.

[deads2k]

/lgtm

[liggitt]

Suggested change

	We will not add functions to do things that can already be reasonably accomplished with existing functions Improving ease-of-use
	We will not add functions to do things that can already be reasonably accomplished with existing functions. Improving ease-of-use

[liggitt]

I assume url(string) errors and prevents further evaluation if the parse fails?

clarify whether getHost includes the port or not (whether it is URL#Hostname() or URL.Host)

user info has further embedded structure: username or username:<password>. are we expecting callers to split that apart themselves? is this intending to return URL.UserInfo.String()? if so, does that return escaped or unescaped versions of those fields?

in general, need to specify whether escaped or unescaped versions of the pieces are returned

[liggitt]

lgtm as well, a couple follow-up clarifications to address at your convenience