feat: interactive tctl auth rotate #49171
Conversation
|
I need to fix mfa prompts and logs, need prompts to potentially handle stdin and output to not mess with the bubbletea rendering edit: done |
|
Updated demo with MFA prompts https://goteleport.zoom.us/clips/share/A2F3MRZsZGYyU09NRVIxMlFCS1YxOHAwaVZRAQ |
lib/asciitable/table.go
Outdated
There was a problem hiding this comment.
Pull these changes into a separate PR so that they can be backported?
There was a problem hiding this comment.
I plan to backport all this to 17, but I'm default-against backporting anything other than bugfixes unless there's a compelling reason to do so
There was a problem hiding this comment.
Does this result in any measurable difference rendering a table? If so, that would improve the UX for folks using tsh ls on a large cluster.
There was a problem hiding this comment.
I don't know if it does, I made the change for dev ergonomics not performance, it's just nicer to write the table directly to a writer I already have than to get a buffer and copy it. Is table rendering a known problem on large clusters? I'd naively expect the network calls to dominate over the table rendering. For all I know buffering the writes in memory could actually be much faster than making a bunch of small writes to stdout. I just wouldn't backport something like this past the current release branch, I don't understand the value of backporting minor UX changes, if our users value stability on release branches and we value dev velocity it seems counterproductive
There was a problem hiding this comment.
I guess I am just used to having a very high bar for deciding what should be backported. I view every backport as a potential source of instability that we will ship out in a patch release with very little testing. Even if the change is trivial, the more changes we have the less obvious it is exactly what has changed in a given patch release and the more we (or our customers) have to look at if a patch happens to break something
lib/services/resource.go
Outdated
There was a problem hiding this comment.
Should these changes be backported?
There was a problem hiding this comment.
I plan to backport all this to 17, but I'm default-against backporting anything other than bugfixes unless there's a compelling reason to do so
There was a problem hiding this comment.
Wouldn't it be a UX improvement? Is that why you included them here?
There was a problem hiding this comment.
I included them here for selfish reasons, so I can use the same string in the text and in the example tctl get command. By default I wouldn't backport something like this but I can manually backport if you think it would be worthwhile
| targetPhase, "nodes", adaptServerGetter(func() ([]types.Server, error) { | ||
| return client.GetNodes(context.TODO(), apidefaults.Namespace) | ||
| })), |
There was a problem hiding this comment.
I think this will cause the waiting for nodes spinner to spin indefinitely if there are any agentless nodes present. We probably want to hit ListUnifiedResources with a filter here instead.
WriterTo.WriteTo is supposed to return an int64, but fmt.Fprintf returns a normal int, we never need this byte count anyway so I just removed it.
Co-authored-by: rosstimothy <[email protected]>
The PR adds an interactive mode for
tctl auth rotate. The best way to describe it is probably to try it yourself, or check the demo video https://goteleport.zoom.us/clips/share/A2F3MRZsZGYyU09NRVIxMlFCS1YxOHAwaVZRAQI am using https://github.com/charmbracelet/bubbletea to handle the tricky parts of writing an interactive TUI. I recommend checking out the README there. The control loop is based on implementing
bubbletea.Modelwhich has methodsInit(),Update(), andView(). We already have bubbletea as a dependency fortsh latency ssh.changelog: Added an interactive mode for
tctl auth rotate