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

tctl resource selection ux #30081

Merged
merged 5 commits into from
Aug 10, 2023
Merged

Conversation

GavinFrazar
Copy link
Contributor

@GavinFrazar GavinFrazar commented Aug 4, 2023

This PR allows users to select resources in tctl [get | edit | rm] for resources of kind db, db_server, kube_cluster, and kube_server using a prefix of the resource name.

Part of the implementation for RFD 129.

Majority of changes were in adding test coverage for these commands.

UX

If the name given by the user matches any resource's name exactly, other resources that match the prefix are ignored, to avoid annoying UX, for example:

## assume there are databases `foo`, `foo-bar`
## this command should select only database `foo`, because it matches exactly, and ignore `foo-bar`.
$ tctl rm db/foo

Additionally, since tctl rm is a destructive action, I decided to make it an error if the given name matches multiple resources at once, which is consistent with how we already allow tctl get <kind> and disallow tctl rm <kind>:

## assume there are databases: `foo`, `foo-bar`
$ tctl rm db/f
ERROR: db/f matches multiple databases as a name prefix:
foo
foo-bar

Use either a full resource name or an unambiguous prefix, for example:
$ tctl rm db/foo-bar

For tctl get and tctl edit, a prefix is allowed to match more than one resource:

## assume there are databases: `foo`, `foo-bar`
$ tctl get db/f
...prints both databases...

Related issue:

@smallinsky smallinsky requested a review from tigrato August 8, 2023 09:05
@GavinFrazar GavinFrazar enabled auto-merge August 9, 2023 00:27
@GavinFrazar GavinFrazar added this pull request to the merge queue Aug 9, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to Branch Protection failures Aug 9, 2023
You're not authorized to push to this branch. Visit "About protected branches" for more information.
@GavinFrazar GavinFrazar added this pull request to the merge queue Aug 9, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 9, 2023
@GavinFrazar GavinFrazar added this pull request to the merge queue Aug 10, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 10, 2023
@GavinFrazar GavinFrazar enabled auto-merge August 10, 2023 05:43
@GavinFrazar GavinFrazar added this pull request to the merge queue Aug 10, 2023
Merged via the queue into master with commit b1982ea Aug 10, 2023
@GavinFrazar GavinFrazar deleted the gavinfrazar/tctl-resource-selection-ux branch August 10, 2023 06:17
@public-teleport-github-review-bot

@GavinFrazar See the table below for backport results.

Branch Result
branch/v13 Failed

GavinFrazar added a commit that referenced this pull request Sep 16, 2023
backports #30081 to branch/v13.

* prefix matching for tctl get discovery resources:
  * kube_cluster
  * kube_server
  * db
  * db_server
  * skip 500ms wait for 0 databases in tests
GavinFrazar added a commit that referenced this pull request Sep 18, 2023
backports #30081 to branch/v13.

* prefix matching for tctl get discovery resources:
  * kube_cluster
  * kube_server
  * db
  * db_server
  * skip 500ms wait for 0 databases in tests
GavinFrazar added a commit that referenced this pull request Sep 18, 2023
backports #30081 to branch/v13.

* prefix matching for tctl get discovery resources:
  * kube_cluster
  * kube_server
  * db
  * db_server
  * skip 500ms wait for 0 databases in tests
GavinFrazar added a commit that referenced this pull request Sep 18, 2023
* [v13] show discovered name in non-verbose request search

backports #30196 to branch/v13.

* factor out discovered name func from tsh and tctl into tool/common

* [v13] tctl resource selection ux (#32086)

backports #30081 to branch/v13.

* prefix matching for tctl get discovery resources:
  * kube_cluster
  * kube_server
  * db
  * db_server
  * skip 500ms wait for 0 databases in tests
github-merge-queue bot pushed a commit that referenced this pull request Sep 19, 2023
* [v13] show discovered name in tsh kube ls

backports #30149 to branch/v13.

* update tsh kube ls tests
* fix SiteName godoc typo

* [v13] `tsh request search` displays discovered resource name (#32085)

* [v13] show discovered name in non-verbose request search

backports #30196 to branch/v13.

* factor out discovered name func from tsh and tctl into tool/common

* [v13] tctl resource selection ux (#32086)

backports #30081 to branch/v13.

* prefix matching for tctl get discovery resources:
  * kube_cluster
  * kube_server
  * db
  * db_server
  * skip 500ms wait for 0 databases in tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants