Proper expression language (EL) for conditions

[alexsnaps]

Currently conditions for a limit have the current format:

  conditions:
    - "req.method == GET"

Where left-hand side (lhs) has to be the variable to look up and the right-hand side (rhs) the value it'll be tested against. Currently we only support the equality operator (==) and further more, only the format where it has a white space before and after the operator is actually supported (where more spaces will probably not have the result one would expect).

I started some work on parsing these condition using a proper scanner which turns the input &str (each condition) into tokens (identifier, operator & ... a String literal). But right now it still has to fallback to position to know which is the identifier (lhs) and which is the value (rhs). I think that's less optimal and possibly creates really surprising things to read, as e.g. get == GET would be a valid condition.

I'm proposing to have String literals just be… string literals as one would know them from different programing languages or... yaml for this matter (i.e. surrounded by quotation (" or ') marks). That would achieve a few things:

• White spaces don't matter anymore (or the lack thereof, see thread 'main' panicked at 'index out of bounds: the len is 1 but the index is 1', limitador/src/limit.rs:131:29 #94)
• Positioning doesn't matter, method == "GET" and "GET" == method are equivalent, as one would expect as == is commutative.
• Reading the condition is straightforward and less error prone

What would that EL look like tho?

• Should we use double (") or/and single (') quotes?
• Based on my reading, there can only ever be a String value in there, so we don't need to support Number literals, right?
• Use backslah (\) for escaping? Using both single & double quotes?

The downside is that would be a breaking change. Existing limits.yaml file would be "wrong". I already wanted to eagerly parse the conditions when parsing the limits.yaml file (unlike the lazy way of today). So we could give a good error message to users when the scanner would have a [TokenType::Identifier, TokenType::Operator, TokenType::Identifier] when it'd expect one (and probably the last one) to be a TokenType::Literal instead.

tl;dr

Have condition be in the format of:

  conditions:
    - "req.method == 'GET'"

todo

• Validate conditions when parsing the limit file see Validate complete environment on startup #100
• Back TryFrom<&str> for Condition by a proper Scanner impl.

[guicassolato]

Authorino's when conditions are represented as structured expressions always composed of 3 separate fields: selector, operator and value. One good thing of using structured fields is that, to an extend, they can be validated at the level of the YAML spec (OAS, in the case of a CRD), rather then only parsed/compiled by the component at runtime.

Authorino's selectors are JSON paths that not only represent a value (fetched from the Authorization JSON) but also accept functions (“string modifiers”).

[alexsnaps]

Looking at where this comes from today, and if we wanted to move away from the conditions requiring an evaluation, the "natural" way of expressing would be like:

{
  key: "req.method",
  exact_match: "GET"
}

[alexsnaps]

Not quite sure what people's preference is here, but I'll come with mine…
Since Limitador doesn't dependent on any way on the k8s in any way, I'd stick to what is there:

• plain strings that are being evaluated: var == "value"
• support string types for now (as this is really what's going on underneath): i.e. it'd be GET == "1"
• support the "old" (i.e. current) behavior for some time, we need to come up with a deprecation strategy, and probably link to some content on the website on how to migrate your current config
• add support for != see Support more operations in conditions #18

There are a few reason for this, but mostly this is the most straight forward change, but mostly addresses the type system clearly… unlike splitting the condition in 3 distinct parts… how is the type of the value expressed? And I know there is only string for now, but I don't want to close any doors here at this stage. Finally we can always build more "k8s native" expressive things at the CRD levels… 

Trying to answer this, as this impacts how we make progress on wrapping up v1.0 of the server and more specifically:

• Back TryFrom<&str> for Condition by a proper Scanner impl. #107
• Validate complete environment on startup #100
• Support more operations in conditions #18