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

Add database roles to tsh db ls -v #35582

Merged
merged 6 commits into from Jan 2, 2024
Merged

Conversation

greedy52
Copy link
Contributor

@greedy52 greedy52 commented Dec 8, 2023

Pre-req to

changelog: Added "Database Roles" column to tsh db ls -v

Changes:

  • Added "Database Roles" column to tsh db ls -v
  • Added database_roles to tsh db ls -f json/yaml
  • Fixed an issue tsh db ls -R returns missing "Expires" column

Did some minor refactoring. Might be easier to review each commit separately.

Usage example:

$ tsh db ls -v --search mongo-auto
Name                   Description Protocol Type        URI             Allowed Users              Database Roles                                  Labels                                                   Connect 
---------------------- ----------- -------- ----------- --------------- -------------------------- ----------------------------------------------- -------------------------------------------------------- ------- 
self-hosted-mongo-auto             mongodb  self-hosted localhost:27017 [STeve] (Auto-provisioned) [my-custom-role@test read@test2 readWrite@test] EnableMongoAutoUser=true,teleport.dev/origin=config-file     

$ tsh db ls --search mongo-auto -f json | jq .[].database_roles
[
  "my-custom-role@test",
  "read@test2",
  "readWrite@test"
]

@greedy52 greedy52 added tsh tsh - Teleport's command line tool for logging into nodes running Teleport. database-access Database access related issues and PRs backport/branch/v14 labels Dec 8, 2023
@greedy52 greedy52 self-assigned this Dec 8, 2023
@greedy52 greedy52 marked this pull request as ready for review December 8, 2023 22:24
@greedy52 greedy52 requested a review from Tener December 8, 2023 22:25
@GavinFrazar
Copy link
Contributor

GavinFrazar commented Dec 8, 2023

how about an option -C,--columns=column,... to include columns? Usually easier to think about what columns you want than what columns you don't want imo.

Definitely a good idea to allow column filtering though! My first thought on seeing the PR title was how cluttered tsh db ls output is getting lol

@Tener
Copy link
Contributor

Tener commented Dec 11, 2023

It would be nice to support comma-separated list in -E, too.

tool/tsh/common/tsh.go Outdated Show resolved Hide resolved
@zmb3
Copy link
Collaborator

zmb3 commented Dec 11, 2023

tsh db ls is not the only place where we are using columnar output in ways that become unreadable.

This is another one of the things I'd love to get in a CLI style guide (see #12811).

I think we should do some form of an output format that can apply to anything.

Kubectl supports a few too many options (IMO), but there's surely some inspiration we can take from them:

@greedy52
Copy link
Contributor Author

greedy52 commented Dec 11, 2023

tsh db ls is not the only place where we are using columnar output in ways that become unreadable.

This is another one of the things I'd love to get in a CLI style guide (see #12811).

I think we should do some form of an output format that can apply to anything.

Kubectl supports a few too many options (IMO), but there's surely some inspiration we can take from them:

I will remove the --exclude-column from this PR for now. I'd love to join the discussion on these for the CLI style guide.

Docker also uses Go template:
https://docs.docker.com/config/formatting/

$ docker image list --format "table {{.ID}}\t{{.Repository}}\t{{.Tag}}\t{{.Size}}"
$ docker ps --format "table {{.Names}}\t{{.Ports}}"

@Tener
Copy link
Contributor

Tener commented Dec 12, 2023

Kubectl supports a few too many options (IMO), but there's surely some inspiration we can take from them:

I don't think we need this level of flexibility, but some customisation would be nice. Perhaps we can add generic config to asciitable.Table, read and populated by tsh and tctl?

"tsh db ls":
     columns: Proxy Name Protocol Description Type AllowedUsers     

Each call to asciitable.MakeTable would take an additional argument, "context", which would be the name of the operation that displays the table, tsh db ls in the example above.

@greedy52 greedy52 changed the title Add database roles to tsh db ls Add database roles to tsh db ls -v Jan 2, 2024
@greedy52 greedy52 force-pushed the STeve/35566_tsh_db_ls_db_roles branch from 52e549b to 801e36e Compare January 2, 2024 19:46
@greedy52 greedy52 added this pull request to the merge queue Jan 2, 2024
Merged via the queue into master with commit 3d2e9ec Jan 2, 2024
33 checks passed
@greedy52 greedy52 deleted the STeve/35566_tsh_db_ls_db_roles branch January 2, 2024 20:22
@public-teleport-github-review-bot

@greedy52 See the table below for backport results.

Branch Result
branch/v14 Failed

greedy52 added a commit that referenced this pull request Jan 3, 2024
* refactor and add --exclude-column

* add Database Roles column

* add database_roles to json,yaml

* remove --exclude-column and refactor databaseTableRow

* change slices import
github-merge-queue bot pushed a commit that referenced this pull request Jan 4, 2024
* Add database roles to `tsh db ls -v` (#35582)

* refactor and add --exclude-column

* add Database Roles column

* add database_roles to json,yaml

* remove --exclude-column and refactor databaseTableRow

* change slices import

* update copyright header
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/branch/v14 database-access Database access related issues and PRs size/md 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.

None yet

5 participants