Add `and` and `or` conditional operators

[johanfylling]

trafficstars

Conditional behavior can already be expressed in Rego through how statements are organized into complete rules. Adding conditional operators like and/&& and or/|| both allows for more condensed code, but also improves the authoring experience for developers experienced with other languages that facilitate this feature.

• operands
  • undefined operands does not abort evaluation, but effectively equivalent to the boolean false value (?)
  • operands must be boolean (true/false) or undefined, or (?)
  • presence of operand value is equivalent to the boolean true value (?)
    • excluding false
    • 1 && 2 == true (?)
• result value type
  • boolean
    • 1 < 2 && 3 > 2 == true
• and keyword
  • &&, or
  • and
• or keyword
  • ||, or
  • or
• conditional operator expressions can be "chained" into a larger expression
  • 1 > 2 || 1 == 1 && 1 < 2
  • and has higher precedence than or
    • Parenthesis (()) has higher precedence than and
• and operator is equivalent to sequential expressions in rule body, but result is a boolean value (?)

package example

# And operator
allow if input.role == “guest” && input.action == “read”

# Or operator
allow if input.role == “admin” || isParent(user, child)

allow if {
  input.action == "edit"
  input.role == “admin” || isParent(user, child) # inlined, helper function not required
  ...
}

This issue could possibly be split in two, one for each operator.

[anderseknert]

Thanks for creating this!

IMHO, or should not be limited to boolean results, but rather return the first non-false/undefined value found. If not, we'll still end up with a bunch of OR conditions that aren't possible to inline, and this would just be one more you'd have to remember from a list that's already quite long.

name := concat(" ", [input.first_name, input.last_name]) || "unknown"

There was a previous issue about this (suggesting //, IIRC) somewhere, but I can't find it now. I think having two different operators for expressing the same thing but with slightly different behavior would be a mistake, and my vote would be for one flexible or, which could be used either as you described or as in my example above.

As for and, I'd place that in the "wait and see" category. Besides providing some symmetry with or, I'm not sure it helps solve any problems users actually have, or at least not that I've seen reported.

Obviously all just my five cents :)

[johanfylling]

There was a previous issue about this (suggesting //, IIRC) somewhere, but I can't find it now.

https://github.com/open-policy-agent/opa/issues/2345 discusses introducing an alternative (//) operator.

A possible issue with the or operator returning non-boolean values is that it might surprise users. E.g.

has_phone_number := input.phone_numbers.work || input.phone_numbers.work

If the first defined non-false value is returned, has_phone_number will not be true/false as many users may expect, but instead <some-number>/undefined.

We can of course define the or operator however we want, but there is benefits to not contradicting common expectations.

---

As for and, I'd place that in the "wait and see" category.

Agreed. The and operator is less important, as this is already a natural part of body evaluation. The argument for this operator would be to meet user expectations; if we provide an or operator, many would expect to be able to compose the equivalent expressions containing and, and a mix thereof.

[anderseknert]

Tbh, the only expectation I could read out of that snippet was has_phone_number, but that's only because you named the rule that way. I can't recall seeing OR used that way in other languages, and I'd flag it immediately in a code review, lol.

We've waited a decade to introduce this, so I think it's fine that we do it our way :) Slightly modified, your example does raise the issue of how to deal with something like this though:

user := {
    "has_phone_number": is_number(input.phone_numbers.work) || false,
     # ...
}

That OTOH does seem like something you might want to use or for, but wouldn't work if we only return a non-falsey value 🤔 I guess false has higher precedence than undefined, and so this could work in that way. But if chained, we wouldn't want to stop at false, i.e.

value := undefined || false || true

Must of course return true.

[srenatus]

has_phone_number := input.phone_numbers.work || input.phone_numbers.work

If the first defined non-false value is returned, has_phone_number will not be true/false as many users may expect, but instead /undefined.

If you want a boolean, you'd do

- has_phone_number := input.phone_numbers.work || input.phone_numbers.work
+ has_phone_number if input.phone_numbers.work || input.phone_numbers.work

[anderseknert]

Hmm, assuming the or is in the rule body, that’d still give you undefined on missing phone number, would it not?

[srenatus]

Yeah in my mind that's as good as a false. 😎

[anderseknert]

Lol, sure, but that'd equivalent to:

has_phone_number if input.phone_numbers.work

and the || makes no difference in that case, is all I'm saying :) Regal rule coming in 3... 2...

[srenatus]

Sorry, I wasn't paying close attention, it seems. I thought this was about having a fallback of sorts.

Unrelated, I think some thought needs to go into the interplay of not and or.

the_question := to_be or not to_be

deny if not is_service or not is_authenticated

If we find better examples, maybe. These don't quite convince even me that there's something to think about 😅

As for and, I'd place that in the "wait and see" category.

Agreed. The and operator is less important, as this is already a natural part of body evaluation. The argument for this operator would be to meet user expectations; if we provide an or operator, many would expect to be able to compose the equivalent expressions containing and, and a mix thereof

I think and doesn't work well, or as intuitively, if it is not for booleans only? An operator called and should be commutative, so p and q should have the same result as q and p.

value := is_admin and has_claims

has_claims := ["foo"]

is_admin	has_claims	value
true	undefined	false (undefined?)
true	true	true
false	...	false
undefined	...	false

This way, it should be the same as for

value := has_claims and is_admin

If p and q for a non-undefined/false value would resolve to the value of q, then p and q wouldn't be the same as q and p.

The same is true for or. Now, that operator is just so commonly used in JS as a non-cummutative OR, where

the || operator actually returns the value of one of the specified operands, so if this operator is used with non-Boolean values, it will return a non-Boolean value.

So if we go with "or resolves to the truthy value and doesn't require booleans", we'd follow suit with JS, which is well-known, but we'd also lose commutativity. 🤔 I think when we discussed //, I liked it because it doesn't evoke intuitions as much as an operator called or. || would also be closer to the JS side, and less logical.

[srenatus]

We've waited a decade to introduce this, so I think it's fine that we do it our way :) Slightly modified, your example does raise the issue of how to deal with something like this though:

user := {
    "has_phone_number": is_number(input.phone_numbers.work) || false,
   # ...
}

This is going to be quite tricky to implement. Just like not is_number(input.phone_numbers.work) breaks intuition for many users because it's compiled to

local0 = input.phone_numbers.work
not is_number(local0)

Extra work would need to go into this to have

is_number(input.phone_numbers.work) || false

evaluate to false if input has no phone_numbers key. And I think it perhaps wouldn't be for the better? How will you explain that || works like this and not is_number(input.phone_numbers.work) doesn't? (I remember long discussions about changing the semantics in such a way that "undefined" would be "passed as function argument"...) Perhaps we'd need to "fix" not, too.

[anderseknert]

Great points, Stephan. I guess we can't change how not works in existing policies but import future.keywords.not (or import rego.v2 or whatever) for how we'd want not to work could be an option. It would be a shame if we had to limit what future Rego looked like based on current constraints, and things we wished we'd done better. At the end of the day — no one asked for more keywords,. Many have asked for Rego to be easier to write and read. IMO. Whatever we come up with, let's make sure that's what we deliver.

[johanfylling]

If you want a boolean, you'd do has_phone_number if input.phone_numbers.work || input.phone_numbers.work

For sure, that's how you'd do it for a rule 👍. Wouldn't work inside a body, though. And some users might expect has_phone_number to always be assigned a true or false value.

My concern is that || might have too much baggage, which would maybe confuse authors and reviewers into making the wrong assumptions.

Another place where non-bool or output would maybe not behave as most expect:

p if {
  f(input.user, input.phone_numbers.work || input.phone_numbers.home)
}

f(user, true) if {
  # do something involving phones
}

f(user, false) if {
  # do something not involving phones
}

[srenatus]

Another place where non-bool or output would maybe not behave as most expect:

p if {
  f(input.user, input.phone_numbers.work || input.phone_numbers.home)
}

f(user, true) if {
  # do something involving phones
}

f(user, false) if {
  # do something not involving phones
}

That's true, but you could argue that there's a way to make it boolean, if your function requires a boolean parameter, like

p if {
  f(input.user, "work" in object.keys(input.phone_numbers) || "home" in object.keys(input.phone_numbers))
}

Another thing to consider is "top-level expression" usage in rule bodies:

allow if a or b

Of course you could have two allow bodies, but I think there are valid use cases for this, when there are longer lists of expressions, and you want to avoid a helper rule. 🤔

[johanfylling]

That's true, but you could argue that there's a way to make it boolean

Absolutely 👍 . I'm not arguing against the usefulness of an alternative operator; it seems like the more versatile option compared to a strictly boolean or. My concern is user expectations, and that maybe using the || infix for something that's in all other ways actually an alternative op might lead to more wrong assumptions and harder to find bugs.

Since we're considering not implementing and, using e.g. a // infix would maybe also not automatically make users expect && to be available, which they'll probably do with ||.

[srenatus]

It's a two-sided coin though. I think many people just like to find what they expect to find 😅 So going with a JS-y version has some appeal, tbh.

[anderseknert]

My concern is user expectations, and that maybe using the || infix for something that's in all other ways actually an alternative op

Isn’t that how or works in JS, Python, Ruby, Clojure… basically any dynamically typed language? Obviously with some local variations.

If you mean that the “double pipe” syntax sets some other expectation, yeah, could be! (I think only JS uses that of the ones mentioned above). That would IMHO be an argument for a different syntax, not different behavior.

[johanfylling]

Yes, as long as we can avoid possible confusion for users I think it doesn't matter much if we implement something akin to the alternative operator but call it or 😄. It's the choice of syntax that matters in the end. Personally I'd prefer something different than ||; like //, or even or might have less baggage..

[anderseknert]

I too would prefer that we called it or. It's the most readable option, stylistically "matches" not, and while expectations may vary to some degree, its behavior would likely be obvious when found in expressions.

The question then is if/how we could pull that off, considering it's already the name of a built-in function: https://github.com/open-policy-agent/opa/blob/main/v1/ast/builtins.go#L687

Since it's an operator with an infix form (|), perhaps there's some clever way to claim the name by renaming it, as the name is "internal" anyway? opa fmt also rewrites or to |, so few policies should use or directly today. Still, not entirely trivial 😅

Or perhaps we could do something similar to what we did with contains, which is either a keyword or a built-in function, determined by where it's found?

This isn't valid Rego today, so there's probably no conflict in that regard 🤔

rule if {
    foo or bar
}

[johanfylling]

Maybe else could work too? Semantically, it'd be pretty close to how rules are processed.

It reads pretty ok if used in an assignment or a function call (maybe a bit awkward).

rule if {
    baz := foo else bar
    f(baz else box)
}

Less so in other expressions.

rule if {
    foo else bar
}

[srenatus]

😬 Nonono please let's not consider "else". It's used already for rules, and pairs nicely (more or less) with if/else there...

[stale[bot]]

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days. Although currently inactive, the issue could still be considered and actively worked on in the future. More details about the use-case this issue attempts to address, the value provided by completing it or possible solutions to resolve it would help to prioritize the issue.