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
There was a problem hiding this comment.
i will manually backport these to all release branches
| 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.
There was a problem hiding this comment.
fixed this in the latest
public-teleport-github-review-bot
bot
removed the request for review
from rudream
nklaassen
force-pushed
the
nklaassen/rotate-ux
branch
from
bcf9189 to
d0f46ee
Compare
|
@nklaassen See the table below for backport results.
|
Backport #49171 to branch/v17 changelog: Added an interactive mode for tctl auth rotate
This is a partial backport of #49171 that just adds a couple plural resource shortcuts that were missing.
Backport #49171 to branch/v17 changelog: Added an interactive mode for tctl auth rotate
This is a partial backport of #49171 that just adds a couple plural resource shortcuts that were missing.
| case types.SPIFFECA: | ||
| // TODO(strideynet): populate any known manual steps during SPIFFE CA rotation. | ||
| fallthrough | ||
| case types.OktaCA: | ||
| // TODO(smallinsky): populate any known manual steps during Okta CA rotation. | ||
| fallthrough |
There was a problem hiding this comment.
oh btw @strideynet @smallinsky I meant to tag you guys before merging this, but I wasn't sure if there will be any manual steps required during a rotation for the SPIFFE or Okta CAs that the user has to do, if there are we can add them here
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