RFD 78: Role Variables

[nklaassen]

Issue: https://github.com/gravitational/teleport/issues/11042

Note to reviewers: I will be substantially redesigning this, no need to review for now, most things will change

[Tener]

Let's discuss either role-local variables or traits transformations next week.

My 2c: we could use CEL (https://github.com/google/cel-spec, https://github.com/google/cel-go) for matching and transforming parts. This gives us a stable base to build upon without reinventing the wheel but still allowing for extensibility. I know we have our eyes on https://github.com/gravitational/predicate, but CEL appears to be more feature-rich: macros and object construction look particularly interesting.

[nklaassen]

@klizhentas Thanks for your review.

If we will put the variables definitions in the role spec, I agree it makes sense to make them role-local only.

I like the trait transformations idea, I had thought of embedding the variables in the cert as a trait but didn't consider they could be defined separately from the role, I think your suggestion could offer pretty good semantics and would make the variables reusable across roles and potentially trusted clusters (something to consider at least.)

I will consider these some more, and I have sent a calendar invite to discuss this with you and @zmb3

[klizhentas]

@Tener CEL looks interesting. However, we are already using predicate in Teleport elsewhere - all roles, OIDC connectors, search expressions. For that reason sticking with predicate is more reasonable choice for now. We could re-evaluate choosing a different language, but we should consider refactoring the entire Teleport code-base then.

[nklaassen]

@klizhentas I have updated this RFD to focus on trait transforms now instead of the old role variables design. I will be OOO for the next ~week, if you get a chance to review while I'm gone I will respond when I get back.

[nklaassen]

@klizhentas I like the idea of working with sets over lists.

My current design with filter and the override map was mostly guided by an attempt to avoid multi-statement predicate expressions with mutable state. But I see the value in having the single expression which can do everything and be quite extensible. I will do some experimenting with it and give it some more thought.

[klizhentas]

@nklaassen I have successfully modeled the maps in another tool. Here is what I got:

• Single map expression

Each login rule only contains one expression that evaluates to a map. This way it is much easier to model, test and evaluate it. The expression can be as simple as specifying static map values:

{
  'fruits': []{'apple', 'strawberry', 'banana'},
  'veggies': []{'potato'},
  'mushrooms': []{} # empty set 
  'other': []{'', 'a', 'a'} # duplicates and empty will be removed
}

• To create a map with only certain arguments selected from other sources, users can refer to existing variables and other maps:

{
   'groups': external['groups'], # copy and flatten the entire set from groups 
   'logins': []{external.email}, # copy just one value
}

• To modify the variables and add values, users can use the if functional expression:

{
            'our-fruits': If(
                external['fruits'].contains('banana'),
                external['fruits'].add('blueberry'),
                external['fruits']
            )
        }

You can also spot set methods - contains and add used above and if statement.

• And finally, users can override and remove keys from the external traits or traits specified in previous rule to chain rules using remove_keys and overwrite methods of maps:

external.remove_keys('logins', 'groups').overwrite({
  'logins': []{external.email}
}

prev.traits.overwrite({
  'logins': []{external.email}
}

This adds the following syntax to the predicate language

• Dictionary with set-like behavior.

dict.add_value(key, value) # add single value to set it does not exists
dict['key'].contains(value) # checks if value exists and returns true if it does
dict['key'].add(value) # add value to set corresponding to key if it not exists
dict.overwrite(another_dict)
dict.remove_keys(key1, key2)
if(cond, true-case, false-case) # functional-style if else

[russjones]

@nklaassen How hard would it be to add support for lower and upper? We've had a lot of requests for that functionality, see https://github.com/gravitational/teleport/issues/5267.

[nklaassen]

@nklaassen How hard would it be to add support for lower and upper? We've had a lot of requests for that functionality, see #5267.

It's not hard at all with the predicate language. @klizhentas would this be difficult to model in Z3?

[klizhentas]

@nklaassen I don't think this will be hard to model lower and upper.

[klizhentas]

@nklaassen I'm looking at this RFD now with a fresh eye, and functional format:

map(
    key(...

Looks a bit too alien for what users are used to now. Can we support simplified yaml format

traits:
    key: [val]

that will be a shortcut to:

traits: |
     map(key, set("val"))

cc @hugoShaka