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

tsh db connect <rds-primary-endpoint> throws matches multiple databases error #31286

Closed
greedy52 opened this issue Aug 31, 2023 · 9 comments · Fixed by #31689
Closed

tsh db connect <rds-primary-endpoint> throws matches multiple databases error #31286

greedy52 opened this issue Aug 31, 2023 · 9 comments · Fixed by #31689
Assignees
Labels
bug database-access Database access related issues and PRs regression test-plan-problem Issues which have been surfaced by running the manual release test plan tsh tsh - Teleport's command line tool for logging into nodes running Teleport. ux

Comments

@greedy52
Copy link
Contributor

greedy52 commented Aug 31, 2023

Expected behavior:
tsh db connect <rds-primary-endpoint> should connect to db

Current behavior:

$ tsh db logout
$ tsh db ls --search steve-mysql
Name                   Description                                      Allowed Users Labels                                                                                                                                                            Connect 
---------------------- ------------------------------------------------ ------------- ----------------------------------------------------------------------------------------------------------------------------------------------------------------- ------- 
steve-mysql-custom-one Aurora cluster in ca-central-1 (custom endpoint) [*]           Env=dev,Name=steve-mysql,Owner=STeve,account-id=123123123123,endpoint-type=custom,engine-version=8.0.mysql_aurora.3.02.2,engine=aurora-mysql,region=ca-central-1          
steve-mysql-custom-two Aurora cluster in ca-central-1 (custom endpoint) [*]           Env=dev,Name=steve-mysql,Owner=STeve,account-id=123123123123,endpoint-type=custom,engine-version=8.0.mysql_aurora.3.02.2,engine=aurora-mysql,region=ca-central-1
steve-mysql            Aurora cluster in ca-central-1                   [*]           Env=dev,Name=steve-mysql,Owner=STeve,account-id=123123123123,endpoint-type=primary,engine-version=8.0.mysql_aurora.3.02.2,engine=aurora-mysql,region=ca-central-1
steve-mysql-reader     Aurora cluster in ca-central-1 (reader endpoint) [*]           Env=dev,Name=steve-mysql,Owner=STeve,account-id=123123123123,endpoint-type=reader,engine-version=8.0.mysql_aurora.3.02.2,engine=aurora-mysql,region=ca-central-1

$ tsh db connect --db-user alice --db-name test steve-mysql
ERROR: database "steve-mysql" matches multiple databases:

Name                                                        Description                                      Protocol Type URI                                                                     Allowed Users Labels
                                                      Connect 
----------------------------------------------------------- ------------------------------------------------ -------- ---- ----------------------------------------------------------------------- ------------- ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
----------------------------------------------------- ------- 
steve-mysql-custom-one-rds-aurora-ca-central-1-123123123123 Aurora cluster in ca-central-1 (custom endpoint) mysql    rds  one.cluster-custom-aabbccddeeff.ca-central-1.rds.amazonaws.com:3306     (unknown)     Env=dev,Name=steve-mysql,Owner=STeve,account-id=123123123123,endpoint-type=custom,engine-version=8.0.mysql_aurora.3.02.2,engine=aurora-mysql,region=ca-central-1,teleport.dev/cloud=AWS,teleport.dev/origin=cloud,tel
eport.internal/discovered-name=steve-mysql-custom-one         
steve-mysql-custom-two-rds-aurora-ca-central-1-123123123123 Aurora cluster in ca-central-1 (custom endpoint) mysql    rds  two.cluster-custom-aabbccddeeff.ca-central-1.rds.amazonaws.com:3306     (unknown)     Env=dev,Name=steve-mysql,Owner=STeve,account-id=123123123123,endpoint-type=custom,engine-version=8.0.mysql_aurora.3.02.2,engine=aurora-mysql,region=ca-central-1,teleport.dev/cloud=AWS,teleport.dev/origin=cloud,tel
eport.internal/discovered-name=steve-mysql-custom-two         
steve-mysql-rds-aurora-ca-central-1-123123123123            Aurora cluster in ca-central-1                   mysql    rds  steve-mysql.cluster-aabbccddeeff.ca-central-1.rds.amazonaws.com:3306    (unknown)     Env=dev,Name=steve-mysql,Owner=STeve,account-id=123123123123,endpoint-type=primary,engine-version=8.0.mysql_aurora.3.02.2,engine=aurora-mysql,region=ca-central-1,teleport.dev/cloud=AWS,teleport.dev/origin=cloud,te
leport.internal/discovered-name=steve-mysql                   
steve-mysql-reader-rds-aurora-ca-central-1-123123123123     Aurora cluster in ca-central-1 (reader endpoint) mysql    rds  steve-mysql.cluster-ro-aabbccddeeff.ca-central-1.rds.amazonaws.com:3306 (unknown)     Env=dev,Name=steve-mysql,Owner=STeve,account-id=123123123123,endpoint-type=reader,engine-version=8.0.mysql_aurora.3.02.2,engine=aurora-mysql,region=ca-central-1,teleport.dev/cloud=AWS,teleport.dev/origin=cloud,tel
eport.internal/discovered-name=steve-mysql-reader

Hint: use 'tsh db ls -v' or 'tsh db ls --format=[json|yaml]' to list all databases with full details.
Hint: try selecting the database with a more specific name (ex: tsh db connect steve-mysql-custom-one-rds-aurora-ca-central-1-123123123123).
Hint: try selecting the database with additional --labels or --query predicate.

Bug details:

  • Teleport version: "server_version": "14.0.0-alpha.2",

Workaround:
tsh db connect --db-user alice --db-name test steve-mysql-rds-aurora-ca-central-1-123123123123

Feels like a regression but it could also be "as designed" from the renaming feature.

Besides tsh db connect throws an error, the table dumped from the error is not very readable when there are lots of tags and the row running over to next line.

Test Plan: #31122

@greedy52 greedy52 added bug ux test-plan-problem Issues which have been surfaced by running the manual release test plan tsh tsh - Teleport's command line tool for logging into nodes running Teleport. regression database-access Database access related issues and PRs labels Aug 31, 2023
@GavinFrazar GavinFrazar self-assigned this Aug 31, 2023
@GavinFrazar
Copy link
Contributor

@smallinsky technically, this is by design. steve-mysql is the shortened "original name" of the db, but the full name is longer, so steve-mysql is just a prefix of the name and a prefix of the others as well. However, the intent of the prefix name thing is to not frustrate users by making them type the full thing out. I could change behavior such that if the name given fully matches the original name (which is added as a label to the db), then it will choose that one instead of matching prefixes. Thoughts?

@GavinFrazar
Copy link
Contributor

As for the line wrapping... that's terminal emulator specific I think - what do you guys think about making it print just the ambiguous names, without all the extra info?

@smallinsky
Copy link
Contributor

@GavinFrazar

Why we can't just check if a provided database name has exact match with with one of the shortened database "original name" ?

For backward compatibility perspective the tsh db connect test steve-mysql call should work exactly in the same way like before to make sure that all user script will not need any alignment.

ever, the intent of the prefix name thing is to not frustrate users by making them type the full thing out.

In this case a user can just type a tsh db connect test steve-my to to trigger prefix bahavior but when a customer explicitly provided a database name that matches with "origianl db name" they highly likely want to connect to use this database.

@greedy52
Copy link
Contributor Author

greedy52 commented Sep 1, 2023

I could change behavior such that if the name given fully matches the original name (which is added as a label to the db), then it will choose that one instead of matching prefixes.
Why we can't just check if a provided database name has exact match with with one of the shortened database "original name" ?

👍, when exactly one of the databases has the exact short/original name.
The logic is definitely getting complicated =p.

As for the line wrapping... that's terminal emulator specific I think - what do you guys think about making it print just the ambiguous names, without all the extra info?

I think that works. Or use verbose = false (aka asciitable.MakeTableWithTruncatedColumn).

@GavinFrazar
Copy link
Contributor

GavinFrazar commented Sep 1, 2023

suppose you have databases:

name discovered name label
mysql n/a
mysql-rds-aurora-us-west-1-123456789012 mysql
mysql-reader-rds-aurora-us-west-1-123456789012 mysql-reader

And then tsh db ls:

$ tsh db ls
Name         Description                               Allowed Users Labels Connect 
------------ ----------------------------------------- ------------- ------ ------- 
mysql        some user created db                      [* alice bob]                
mysql        Aurora cluster in us-west-1               [* alice bob]                
mysql-reader Aurora cluster in us-west-1 (reader en... [* alice bob]        

We have two options for how to handle this scenario:

  1. make it include the "discovered name" exact match in the logic, then tsh db connect mysql is ambiguous, and the only way to select the first db is to use --labels or --query, since its full name is ambiguous.
  2. make it prefer an exact match of a full name over an exact match of a discovered name label, then tsh db connect mysql will choose the first db, but the second db can only be selected with either more specific name or labels/query.

I think we should go with option 2 here - actual db name exact match > db discovered name label exact match.

@greedy52
Copy link
Contributor Author

greedy52 commented Sep 5, 2023

This is tough. I personally prefer option 2, especially for scripting purposes. However, one problem with option 2 is the average user is not educated on using the full name to connect the "discovered" mysql.

@smallinsky
Copy link
Contributor

smallinsky commented Sep 8, 2023

I think that option 2 is more UX friendly.

Also can we detect in tsh db ls that db name overlaps with other db discovered name label and in that case the discovered name can be automatically extended to mysql-rds-aurora-us-west-1-123456789012 so the result can look like:

$ tsh db ls
Name         Description                               Allowed Users Labels Connect 
------------ ----------------------------------------- ------------- ------ ------- 
mysql           some user created db                      [* alice bob]                
mysql-rds-...   Aurora cluster in us-west-1               [* alice bob]                
mysql-reader    Aurora cluster in us-west-1 (reader en... [* alice bob] 

We can also consider always printing discovered name label but this doesn't make sense since we have the -v option that does that.

@greedy52
Copy link
Contributor Author

greedy52 commented Sep 8, 2023

Also can we detect in tsh db ls that db name overlaps with other db discovered name label and in that case the discovered name can be automatically extended

Alternatively, since there is a breaking change log on this already, maybe tsh db ls can always show the long name to match web UI and Teleport Connect. That way "discovered name label" is only an internal thing. Maybe tsh db ls can also hint that now tsh db connect can take a prefix so there is no need to type the whole thing.

@GavinFrazar
Copy link
Contributor

I like the idea to make tsh db ls hint about using prefix names, and just display the full name of the database (with non-verbose it will truncate the column width and uses ellipses, i.e. mysql-rds-us-west-1-... or something).

The --verbose option makes columns never truncate, so it's not quite the same thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug database-access Database access related issues and PRs regression test-plan-problem Issues which have been surfaced by running the manual release test plan tsh tsh - Teleport's command line tool for logging into nodes running Teleport. ux
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants