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

Make tctl <resource> ls command outputs consistent #9519

Merged
merged 10 commits into from
Mar 15, 2022

Conversation

lxea
Copy link
Contributor

@lxea lxea commented Dec 21, 2021

This makes tctl commands consistent between different outputs as described in #9379. This adds a kube ls command.

@lxea lxea force-pushed the lxea/tctl-ls-consistency branch from b8b2a1c to 6a95208 Compare December 22, 2021 15:20
Copy link
Contributor Author

@lxea lxea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to get Teleport version and host, GetWindowsDesktopServices must be used however I'm not sure how to associate a windows desktop with a windows desktop service. Any ideas @russjones ?

@russjones
Copy link
Contributor

@lxea @zmb3 should be able to help you out on the Windows Desktop questions.

@zmb3
Copy link
Collaborator

zmb3 commented Dec 22, 2021

@lxea there currently isn't an association between a Windows Desktop and the Windows Desktop Service (or services) that can connect to it.

We'll be adding this soon (feel free to follow #9277 for progress).

If there's anything in particular you need in the implementation to make this work easier let me know, but the plan is basically do make it work exactly like database access.

tool/tctl/common/collection.go Outdated Show resolved Hide resolved
tool/tctl/common/collection.go Outdated Show resolved Hide resolved
tool/tctl/common/desktop_command.go Outdated Show resolved Hide resolved
@russjones
Copy link
Contributor

@lxea Can we use a similar approach to what we did with tsh where we size the output to the size of the terminal? I think that will let us show more labels whenever we can.

@lxea lxea force-pushed the lxea/tctl-ls-consistency branch 3 times, most recently from 3cca378 to 0091b8f Compare February 18, 2022 17:35
@lxea lxea marked this pull request as ready for review February 18, 2022 17:37
@lxea lxea force-pushed the lxea/tctl-ls-consistency branch from 0091b8f to a9a08a6 Compare February 18, 2022 17:38
@github-actions github-actions bot added desktop-access tctl tctl - Teleport admin tool tsh tsh - Teleport's command line tool for logging into nodes running Teleport. labels Feb 18, 2022
@lxea lxea requested a review from zmb3 February 18, 2022 17:40
tool/tctl/common/collection.go Outdated Show resolved Hide resolved
tool/tctl/common/collection.go Outdated Show resolved Hide resolved
tool/tctl/common/collection.go Outdated Show resolved Hide resolved
@lxea lxea force-pushed the lxea/tctl-ls-consistency branch 5 times, most recently from 5a39773 to 1fd9a09 Compare February 28, 2022 09:52
@r0mant
Copy link
Collaborator

r0mant commented Mar 1, 2022

@Tener @zmb3 Since you guys looked at this before, mind re-reviewing?

tool/tctl/common/collection.go Outdated Show resolved Hide resolved
tool/tctl/common/desktop_command.go Outdated Show resolved Hide resolved
tool/tctl/main.go Show resolved Hide resolved
tool/tctl/common/kube_command.go Outdated Show resolved Hide resolved
tool/tctl/common/desktop_command.go Outdated Show resolved Hide resolved
tool/tctl/common/desktop_command.go Outdated Show resolved Hide resolved
@Tener
Copy link
Contributor

Tener commented Mar 4, 2022

@r0mant @lxea sorry for the delay. I have reviewed everything now, including the code I didn't look at close enough on the first pass. Sorry for pointing out stuff I could have found already in the past but missed...

@lxea lxea force-pushed the lxea/tctl-ls-consistency branch from 1fd9a09 to 84c7bce Compare March 8, 2022 09:29
Copy link
Contributor

@Tener Tener left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Tener
Copy link
Contributor

Tener commented Mar 8, 2022

@r0mant @zmb3 I've approved, but scripts require your approval too.

@lxea lxea force-pushed the lxea/tctl-ls-consistency branch from 84c7bce to d2d93ad Compare March 8, 2022 10:52
@lxea lxea force-pushed the lxea/tctl-ls-consistency branch from d2d93ad to 59f704b Compare March 8, 2022 14:03
@r0mant r0mant self-requested a review March 8, 2022 17:05
Copy link
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have a couple suggestions.

api/types/app.go Outdated Show resolved Hide resolved
tool/tctl/common/collection.go Outdated Show resolved Hide resolved
@lxea lxea force-pushed the lxea/tctl-ls-consistency branch from 59f704b to 53a5a4d Compare March 9, 2022 10:18
@lxea lxea requested a review from r0mant March 10, 2022 08:14
@lxea lxea force-pushed the lxea/tctl-ls-consistency branch from 53a5a4d to d89756d Compare March 14, 2022 11:34
Copy link
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving but please address suggestions I left before merging.

api/types/constants.go Outdated Show resolved Hide resolved
lib/asciitable/table.go Outdated Show resolved Hide resolved
lib/asciitable/table.go Show resolved Hide resolved
tool/tctl/common/collection.go Outdated Show resolved Hide resolved
tool/tctl/common/collection.go Outdated Show resolved Hide resolved
tool/tctl/common/node_command.go Outdated Show resolved Hide resolved
tool/tctl/common/node_command.go Outdated Show resolved Hide resolved
tool/tctl/common/node_command.go Outdated Show resolved Hide resolved
@lxea lxea force-pushed the lxea/tctl-ls-consistency branch 2 times, most recently from 88fc84d to bf9a99b Compare March 15, 2022 12:51
@lxea lxea enabled auto-merge (rebase) March 15, 2022 12:52
Alex McGrath added 10 commits March 15, 2022 12:59
New format:
```
Host UUID                                 Public Address Labels                    Version
---- ------------------------------------ -------------- ------------------------- -------------
corn 7b3ca8d8-710c-4305-aa29-e73628ac572c 127.0.0.1:3022 env=example,hostname=corn 8.0.0-alpha.1
```

With `--yaml`
```yaml
kind: node
metadata:
  expires: "2021-12-16T15:26:44.887862347Z"
  id: 1639667804888460055
  labels:
    env: example
  name: 7b3ca8d8-710c-4305-aa29-e73628ac572c
spec:
  addr: 127.0.0.1:3022
  cmd_labels:
    hostname:
      command:
      - hostname
      period: 1m0s
      result: corn
  hostname: corn
  public_addr: corn.lan:8443
  rotation:
    current_id: ""
    last_rotated: "0001-01-01T00:00:00Z"
    schedule:
      standby: "0001-01-01T00:00:00Z"
      update_clients: "0001-01-01T00:00:00Z"
      update_servers: "0001-01-01T00:00:00Z"
    started: "0001-01-01T00:00:00Z"
  version: 8.0.0-alpha.1
version: v2
```
```
Host Name        Public Address       URI                 Labels                          Version
---- ----------- -------------------- ------------------- ------------------------------- -------------
corn example-app example-app.corn.lan http://0.0.0.0:8000 teleport.dev/origin=config-file 8.0.0-alpha.1
```
```
Host Name     Protocol URI          Labels                          Version
---- -------- -------- ------------ ------------------------------- -------------
corn postgres postgres 0.0.0.0:5432 teleport.dev/origin=config-file 8.0.0-alpha.1
```
```
Cluster     Labels Version
----------- ------ -------------
minikube           8.0.0-alpha.1
honkcluster        8.0.3
```

```yaml
kind: kube_service
metadata:
  expires: "2021-12-16T16:07:14.898611765Z"
  id: 1639670234899200604
  name: 7b3ca8d8-710c-4305-aa29-e73628ac572c
spec:
  addr: 127.0.0.1:3027
  hostname: ""
  kube_clusters:
  - name: minikube
  rotation:
    current_id: ""
    last_rotated: "0001-01-01T00:00:00Z"
    schedule:
      standby: "0001-01-01T00:00:00Z"
      update_clients: "0001-01-01T00:00:00Z"
      update_servers: "0001-01-01T00:00:00Z"
    started: "0001-01-01T00:00:00Z"
  version: 8.0.0-alpha.1
version: v2
---
kind: kube_service
metadata:
  expires: "2021-12-16T16:10:13.068855399Z"
  id: 1639670413069866294
  name: 9153a5a9-e85e-4972-8c50-6fa8923282b2
spec:
  addr: remote.kube.proxy.teleport.cluster.local
  hostname: ""
  kube_clusters:
  - name: honkcluster
  rotation:
    current_id: ""
    last_rotated: "0001-01-01T00:00:00Z"
    schedule:
      standby: "0001-01-01T00:00:00Z"
      update_clients: "0001-01-01T00:00:00Z"
      update_servers: "0001-01-01T00:00:00Z"
    started: "0001-01-01T00:00:00Z"
  version: 8.0.3
version: v2
```yaml
```
Host Public Address       AD Domain   Labels               Version
---- -------------------- ----------- -------------------- ---------
corn 192.168.122.144:3389 example.com teleport..3 (9       9.0.0-dev
corn 192.168.122.51:3389  example.com teleport.rd Evle.com 9.0.0-dev
```

```yaml
kind: windows_desktop
metadata:
  expires: "2022-02-18T16:12:52.422659238Z"
  id: 1645200172423989197
  labels:
    teleport.dev/computer_name: WIN-LA2V0OD7SK0
    teleport.dev/dns_host_name: WIN-LA2V0OD7SK0.example.com
    teleport.dev/is_domain_controller: "true"
    teleport.dev/origin: dynamic
    teleport.dev/os: Windows Server 2012 R2 Standard Evaluation
    teleport.dev/os_version: 6.3 (9600)
    teleport.dev/windows_domain: example.com
  name: WIN-LA2V0OD7SK0-example-com
spec:
  addr: 192.168.122.51:3389
  domain: example.com
  host_id: 2c807641-92ae-4c70-88fe-b93e7b0aa179
version: v3
```
@lxea lxea force-pushed the lxea/tctl-ls-consistency branch from bf9a99b to 1023e3f Compare March 15, 2022 12:59
@lxea lxea merged commit 40200e8 into master Mar 15, 2022
@lxea lxea deleted the lxea/tctl-ls-consistency branch March 15, 2022 13:22
@TwiN
Copy link

TwiN commented Apr 13, 2023

While MakeTableWithTruncatedColumn does make the outputs consistent, it also broke automations that extracted the name from ls commands 😔

@Tener
Copy link
Contributor

Tener commented Apr 14, 2023

@TwiN

While MakeTableWithTruncatedColumn does make the outputs consistent, it also broke automations that extracted the name from ls commands 😔

--format=json switch is there for automation purposes. For example, if you want to extract database names, you can do:

tctl db ls --format=json | jq '.[].metadata.name'

A great number of other commands support --format=json, and the output format rarely changes. If there is a command which doesn't support that flag feel free to raise a feature request and tell us about your use case.

@TwiN
Copy link

TwiN commented Apr 14, 2023

@Tener Good to know! I must've missed the --format flag!
Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
desktop-access tctl tctl - Teleport admin tool 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.

6 participants