Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add regexp.replace support in role templates #7152

Merged
merged 9 commits into from
Jun 10, 2021

Conversation

nklaassen
Copy link
Contributor

@nklaassen nklaassen commented Jun 3, 2021

Closes #3374

This PR adds support for {{regexp.replace(namespace.variable, expression, replacement)}} syntax in role templates.

See the updated tests for some examples.

@nklaassen nklaassen marked this pull request as ready for review June 3, 2021 02:08
@nklaassen nklaassen requested review from smallinsky and tcsc June 3, 2021 02:08
@russjones
Copy link
Contributor

@stevenGravy Can you verify this solves the customers issue, I think they are using regexp.replace in Kubernetes Groups in their role?

Copy link
Contributor

@klizhentas klizhentas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nklaassen looks good UX-wise, but need docs in this PR please

@nklaassen
Copy link
Contributor Author

@nklaassen looks good UX-wise, but need docs in this PR please

@klizhentas sure, https://goteleport.com/docs/access-controls/guides/role-templates/#interpolation-rules seems like a good place to add an example, I'll do that.

@klizhentas @russjones what should the behaviour be when the expression does not match at all? As-is it will just pass the value through unchanged, but I'm realizing that may not be what we want.

I'm thinking of a case like {{regexp.replace(external.email, "(.*)@example1.com", "$1")}}, but the input (external.email) is alice@example2.com. In this case should it be an error and return nothing, or pass alice@example2.com through unchanged?

@nklaassen
Copy link
Contributor Author

In the latest commit, all values which do not match the regular expression at all are filtered out

@nklaassen nklaassen requested a review from inertial-frame as a code owner June 7, 2021 23:23
@nklaassen nklaassen requested a review from inertial-frame June 8, 2021 18:00
@russjones
Copy link
Contributor

@nklaassen Once this is merged, please backport to branch/v6.2.

func (r regexpReplaceTransformer) transform(in string) (string, error) {
// filter out inputs which to not match the regexp at all
if !r.re.MatchString(in) {
return "", nil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this return in unmodified?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm filtering out inputs which do not match the regex, that seems to be more useful and the behaviour people actually want. Empty strings are removed from the output set in Interpolate.

@klizhentas
Copy link
Contributor

@nklaassen filtering out non matching values is a good default for both allow and deny expressions

@nklaassen nklaassen enabled auto-merge (squash) June 10, 2021 02:49
@nklaassen nklaassen merged commit 92137cd into master Jun 10, 2021
@nklaassen nklaassen deleted the nklaassen/regexp-replace branch June 10, 2021 02:55
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend variable interpolation syntax
6 participants