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: print kubernetes info in profile status #4348

Merged
merged 1 commit into from
Oct 14, 2020
Merged

Conversation

awly
Copy link
Contributor

@awly awly commented Sep 18, 2020

Print when k8s support is detected, and if so what users/groups are
used.

Fixes #3447
Updates #3952

@webvictim
Copy link
Contributor

I tested this locally and it works OK - it got the correct output for 2/3 clusters I had listed in tsh status. There was a Vagrant cluster which reported no Kubernetes enabled (which is correct) and a demo cluster in GKE which reported that Kubernetes was enabled and gave the correct list of kubernetes_groups.

  1. There was one discrepancy - the output for my local Teleport cluster (which runs Teleport Enterprise and has a license with Kubernetes enabled, but doesn't actually have the functionality switched on in the proxy_service section of the config) reports Kubernetes: enabled but with no users/groups listed. I'm not sure why RouteToCluster is being set without Kubernetes being enabled?

teleport version: Teleport Enterprise v4.3.0git:v4.3.0-0-g73b68f4 go1.13.2

proxy_service section of the config:

proxy_service:
  enabled: yes
  ssh_public_addr: teleport.cluster:3023
  public_addr: teleport.cluster:3080
  tunnel_listen_addr: 0.0.0.0:3080
  tunnel_public_addr: teleport.cluster:3080
  https_key_file: /etc/letsencrypt/live/teleport.cluster/privkey.pem
  https_cert_file: /etc/letsencrypt/live/teleport.cluster/fullchain.pem

tsh status output:

> Profile URL:       https://teleport.cluster:3080
  Logged in as:      webvictim
  Cluster:           teleport.cluster
  Roles:             admin*
  Logins:            gus, test
  Kubernetes:        enabled
  Valid until:       2020-09-21 21:20:49 -0300 ADT [valid for 12h0m0s]
  Extensions:        permit-agent-forwarding, permit-port-forwarding, permit-pty

This probably isn't a show-stopper, but it might confuse people as to why we're reporting Kubernetes being enabled in their tsh output when they're not explicitly using it.

  1. I'm wondering whether our changing the indenting/spacing of the tsh output might potentially break some automation that people have built around that? Hopefully things like cut/awk shouldn't break (and it's not good practice to parse tsh output using scripts anyway) but I figured it was worth flagging. I do know a number of customers have built automation around their usage of tsh, but I'm not sure what we can do as a mitigation.

@awly
Copy link
Contributor Author

awly commented Sep 21, 2020

@webvictim I noticed the first issue too, when I re-used a Teleport cluster name:

  • create cluster A with k8s enabled - RouteToCluster is set and tsh status says enabled
  • create a new cluster A with k8s disabled - RouteToCluster is set and tsh status still says enabled
    Did you re-use that cluster name by chance? There could be something cached in the profile to cause this.

Regarding the second issue - good to flag it, but it seems like parsing the output in a way that depends on the exact number of spaces is doomed to fail. My hope is that the number of users parsing status output is tiny, and most of them doing it with awk or other column-oriented tools (that handle extra whitespace fine).
I think it's very unlikely to break anyone, @benarent WDYT?

@benarent
Copy link
Contributor

I don't know of anyone using it.. but that likely means there is. I'll defer to @russjones, I think if we do it it should at least be in a minor release with notes.

I'm also leaning to put this into teleport status -d first, and we can come back and change the default behavior of tsh after 5.0.

@russjones
Copy link
Contributor

My view is that tsh is for humans, especially if you don't use a formatting flag, so I am not against changing the formatting in this case.

// The TLS cert may have k8s users and groups even when proxy isn't
// configured to talk to a k8s cluster.
// RouteToCluster is only set when k8s support is enabled though.
KubeEnabled: tlsID.RouteToCluster != "",
Copy link
Contributor

@russjones russjones Sep 30, 2020

Choose a reason for hiding this comment

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

I don't think this is not a reliable indicator that Kubernetes support is enabled. If you are using the -J flag for SSH RouteToCluster will be set. What about something like the following:

var kubeEnabled bool
if len(tlsID.KubernetesUsers) > 0 || len(tlsID.KubernetesGroups) > 0 {
   kubeEnabled = true
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

KubernetesUsers is set to user login in the default admin role, so that's not a reliable indicator either.
I just realized that #4427 will make k8s support explicitly visible in the cert, so I'll wait for that to merge and use the new TLS extension

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good to me.

@awly awly force-pushed the andrew/tsh-status-kube branch 2 times, most recently from a5b81cb to 7a54cb3 Compare October 1, 2020 20:39
@awly awly force-pushed the andrew/tsh-status-kube branch 2 times, most recently from d8a5229 to 36a8f66 Compare October 8, 2020 21:18
Print when k8s support is detected, and if so what users/groups are
used.
@awly
Copy link
Contributor Author

awly commented Oct 8, 2020

PTAL
Updated to use the new TLS cert extension (from #4427) to figure out when k8s support is enabled.
Also added server-side defaulting for k8s cluster name, so that tsh login without --kube-cluster still prints the right output and cluster name.

@awly awly merged commit fda813e into master Oct 14, 2020
@awly awly deleted the andrew/tsh-status-kube branch October 14, 2020 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

List Kubernetes groups and context status in tsh status
5 participants