Skip to content

Correctly filter out system roles#40545

Merged
gzdunek merged 4 commits into
masterfrom
gzdunek/filter-out-system-roles
Apr 22, 2024
Merged

Correctly filter out system roles#40545
gzdunek merged 4 commits into
masterfrom
gzdunek/filter-out-system-roles

Conversation

@gzdunek
Copy link
Copy Markdown
Contributor

@gzdunek gzdunek commented Apr 15, 2024

The web UI's listRoles handler has a logic that filters out system roles.
That worked fine in the past, but now we paginate the results, and filtering pages can result in inconsistent number of results on them.

@gzdunek gzdunek added backport/branch/v14 no-changelog Indicates that a PR does not require a changelog entry labels Apr 15, 2024
@gzdunek gzdunek requested a review from fspmarshall April 15, 2024 07:37
@github-actions github-actions Bot requested review from Tener and ryanclark April 15, 2024 07:37
Copy link
Copy Markdown
Contributor

@marcoandredinis marcoandredinis left a comment

Choose a reason for hiding this comment

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

Can we please add some tests?

Comment thread api/types/role.go Outdated
@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from fspmarshall April 15, 2024 16:21
@gzdunek gzdunek added this pull request to the merge queue Apr 22, 2024
Merged via the queue into master with commit 22a3389 Apr 22, 2024
@gzdunek gzdunek deleted the gzdunek/filter-out-system-roles branch April 22, 2024 10:22
@public-teleport-github-review-bot
Copy link
Copy Markdown

@gzdunek See the table below for backport results.

Branch Result
branch/v14 Failed
branch/v15 Failed

@public-teleport-github-review-bot
Copy link
Copy Markdown

@gzdunek See the table below for backport results.

Branch Result
branch/v14 Failed
branch/v15 Failed

gzdunek added a commit that referenced this pull request Apr 22, 2024
* Correctly filter out system roles

* Move `SkipSystemRoles` check above `SearchKeywords` check

* Add tests

(cherry picked from commit 22a3389)
gzdunek added a commit that referenced this pull request Apr 22, 2024
* Correctly filter out system roles

* Move `SkipSystemRoles` check above `SearchKeywords` check

* Add tests

(cherry picked from commit 22a3389)
github-merge-queue Bot pushed a commit that referenced this pull request Apr 22, 2024
* Correctly filter out system roles

* Move `SkipSystemRoles` check above `SearchKeywords` check

* Add tests

(cherry picked from commit 22a3389)
github-merge-queue Bot pushed a commit that referenced this pull request Apr 22, 2024
* Correctly filter out system roles

* Move `SkipSystemRoles` check above `SearchKeywords` check

* Add tests

(cherry picked from commit 22a3389)
@gzdunek gzdunek mentioned this pull request Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants