Skip to content

use active db cert principals when available#31188

Merged
GavinFrazar merged 3 commits intomasterfrom
gavinfrazar/reuse-db-principals-from-cert-on-tsh-db-connect
Aug 30, 2023
Merged

use active db cert principals when available#31188
GavinFrazar merged 3 commits intomasterfrom
gavinfrazar/reuse-db-principals-from-cert-on-tsh-db-connect

Conversation

@GavinFrazar
Copy link
Copy Markdown
Contributor

@GavinFrazar GavinFrazar commented Aug 30, 2023

fixes #31147 and #30901

@GavinFrazar GavinFrazar requested a review from greedy52 August 30, 2023 01:59
@github-actions github-actions Bot added size/sm tsh tsh - Teleport's command line tool for logging into nodes running Teleport. labels Aug 30, 2023
@github-actions github-actions Bot requested review from gzdunek and ravicious August 30, 2023 02:00
@GavinFrazar GavinFrazar requested review from smallinsky and removed request for gzdunek and ravicious August 30, 2023 02:00
@GavinFrazar
Copy link
Copy Markdown
Contributor Author

GavinFrazar commented Aug 30, 2023

@greedy52 @smallinsky I think I need to refactor this code to be less complicated, although it's hard when we have sparse test coverage for tsh db connect and lots of backwards compatibility to maintain. So to keep it simple to review and merge for the test plan, I've just added the code needed to fix the regression and tested manually.

I think it would be worthwhile to add a db cmd override for testing tsh db connect scenarios comprehensively in CI without the db cli binaries available, so I'll work on a separate PR for that as well.

That tsh db connect ux change keeps coming back to haunt me 😅 I actually tested this exact scenario before, but because I had setup only one allowed db user/name for my user, tsh db connect mydb was choosing those by default and made it seem like the prior behavior was still working 🤦

Comment thread tool/tsh/common/db.go
Comment thread tool/tsh/common/db.go
@GavinFrazar GavinFrazar enabled auto-merge August 30, 2023 17:24
@GavinFrazar GavinFrazar added this pull request to the merge queue Aug 30, 2023
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 30, 2023
@GavinFrazar GavinFrazar added this pull request to the merge queue Aug 30, 2023
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 30, 2023
@GavinFrazar GavinFrazar added this pull request to the merge queue Aug 30, 2023
Merged via the queue into master with commit 5e2eea2 Aug 30, 2023
@GavinFrazar GavinFrazar deleted the gavinfrazar/reuse-db-principals-from-cert-on-tsh-db-connect branch August 30, 2023 19:35
@public-teleport-github-review-bot
Copy link
Copy Markdown

@GavinFrazar See the table below for backport results.

Branch Result
branch/v13 Failed
branch/v14 Create PR

GavinFrazar added a commit that referenced this pull request Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-for-v14 size/sm 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 db connect ignores db names and users set in tsh db login

3 participants