Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix setting request reason for automatic ssh access requests. #43076

Merged
merged 2 commits into from
Jun 18, 2024

Conversation

AntonAM
Copy link
Contributor

@AntonAM AntonAM commented Jun 17, 2024

It seems that after adding automatic role access requests ( #39003 ) , request reason functionality got broken - in tsh codepath we never set reason for resource requests and only set reason for role requests if there's more than one role. This PR fixes that, so we again get request reason from the flag or prompt user for it.
I also added tsh logout/login in the tests, I think second test case didn't actually work because we had approved access request from the first one.

Changelog: Fix setting request reason for automatic ssh access requests.

Fixes #42634

@AntonAM AntonAM requested review from nklaassen and atburke June 17, 2024 03:43
@github-actions github-actions bot added size/sm tsh tsh - Teleport's command line tool for logging into nodes running Teleport. labels Jun 17, 2024
tool/tsh/common/tsh_test.go Outdated Show resolved Hide resolved
@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from nklaassen June 17, 2024 16:04
@AntonAM
Copy link
Contributor Author

AntonAM commented Jun 17, 2024

@zmb3 can I get excludeflake for this? TestSSHAccessRequest is pretty slow and I didn't find a simple way to make it faster without removing stuff (previous PR also required excludeflake).

@zmb3
Copy link
Collaborator

zmb3 commented Jun 17, 2024

/excludeflake TestSSHAccessRequest

@AntonAM AntonAM added this pull request to the merge queue Jun 17, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 17, 2024
@AntonAM AntonAM added this pull request to the merge queue Jun 17, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 17, 2024
@AntonAM AntonAM added this pull request to the merge queue Jun 17, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 17, 2024
@AntonAM AntonAM enabled auto-merge June 18, 2024 00:53
@AntonAM AntonAM added this pull request to the merge queue Jun 18, 2024
Merged via the queue into master with commit db7baed Jun 18, 2024
37 checks passed
@AntonAM AntonAM deleted the anton/fix-ssh-request-reason branch June 18, 2024 01:11
@public-teleport-github-review-bot

@AntonAM See the table below for backport results.

Branch Result
branch/v15 Failed
branch/v16 Create PR

AntonAM added a commit that referenced this pull request Jun 18, 2024
* Fix setting request reason for automatic ssh access requests.

* Use slices.ContainsFunc for test check.
github-merge-queue bot pushed a commit that referenced this pull request Jun 18, 2024
#43180)

* Fix setting request reason for automatic ssh access requests.

* Use slices.ContainsFunc for test check.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/branch/v15 backport/branch/v16 bug size/sm ssh tsh tsh - Teleport's command line tool for logging into nodes running Teleport.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"tsh ssh --request-reason" doesn't create access request with the provided reason.
4 participants