Skip to content

k8s: keep credential plugins off the terminal - #41

Merged
junior merged 2 commits into
mainfrom
exec-plugin-noninteractive
Nov 12, 2026
Merged

junior merged 2 commits into
mainfrom
exec-plugin-noninteractive

Conversation

@pierreneter

Copy link
Copy Markdown
Member

Problem

A kubeconfig user is often an exec credential plugin — tsh kube credentials
(Teleport), aws eks get-token, kubelogin, gke-gcloud-auth-plugin. client-go
runs it before the first request, and with interactiveMode: IfAvailable (the
default, and what tsh writes) it hands the plugin os.Stdin and os.Stderr
whenever stdin is a TTY.

Under a TUI that is always true, and always wrong:

Change

One rule: the TUI owns the terminal, the plugin never gets it.

  • StdinUnavailable = true on ExecProvider — what kubectl exec -i does
    when stdin is spoken for. IfAvailable plugins run without stdin and fail
    fast; Always plugins are refused before they run, with a k10s reason.
  • k8s.CaptureStderr swaps os.Stderr for a pipe before the first client
    is built (client-go copies os.Stderr at that point and never looks again).
    The last line a plugin wrote becomes k8s.CredentialPluginError, the
    No cluster reason, and the chain still unwraps to client-go's error.
    $EDITOR and foreground plugins get the terminal's real stderr via
    Startup.TerminalStderr; k8s.ReleaseStderr restores it after Run.
  • Probe retried once when it timed out with a plugin in play. Credentials
    are cached by then, so the retry is the network alone. No change without a
    plugin (test asserts a single hit).
  • Connecting toast names the plugin (… connecting · credentials from tsh),
    the No cluster reason wraps instead of truncating, /setup and
    docs/cluster-setup.md get a "Credential plugins" section with the Teleport
    caveats (tsh proxy kube for per-session MFA / L7 LB).

Not in this PR

  • A separate "auth expired" state mid-session (watch_health) that keeps cached rows.
  • An l action that runs the context's exec command interactively through
    tea.ExecProcess, then retries.
  • tsh-specific hints parsed from the exec args.

Tests

  • internal/k8s/credplugin_test.go: fake plugin as a shell script against the
    existing TLS test API server — runs without the terminal and its token is
    used; Always is refused and the plugin does not run; the plugin's last
    stderr line is the error verbatim; a 1s plugin under a 300ms probe succeeds
    via the retry (fails with the retry removed); a slow server without a plugin
    is still hit once; CredentialPlugin() naming.
  • internal/ui/credplugin_test.go: toast names the plugin; toTerminal
    keeps a caller-set stderr and does nothing without a terminal.
  • main_test.go: noClusterReason keeps the plugin's words intact.
  • just check and go test -race ./internal/k8s/ green.

Unverified

Whether tsh's WebAuthn tap works with stdin unavailable (touch needs no
stdin, but tsh may bail early). Needs a real per-session-MFA cluster. If it
does not, the documented path is tsh proxy kube <cluster> --exec.

@pierreneter
pierreneter requested a review from junior as a code owner November 12, 2026 00:44
A kubeconfig user that is an exec plugin (tsh, aws, kubelogin) could
prompt for MFA or a relogin while the TUI owned stdin: keystrokes went to
the plugin, its stderr was painted over the frame, and the session looked
hung. That is the k9s failure in derailed/k9s#2048.

- Mark stdin unavailable on the ExecProvider, as kubectl does when stdin
  is spoken for. The plugin runs without a terminal and fails fast; one
  that declares interactiveMode: Always is refused before it runs.
- Capture os.Stderr through a pipe before the first client is built, so
  the plugin's last line is the No cluster reason ("credential plugin
  tsh: Having problems with relogin, please use 'tsh login…'") instead
  of "exit code 1". The editor and foreground plugins still get the
  terminal's stderr.
- Retry the version probe once when it timed out with a plugin in play:
  the plugin's run time is spent inside the request's timeout, which
  client-go cannot interrupt, so a slow plugin turned into "No cluster:
  context deadline exceeded" even though it succeeded.
- Name the plugin in the connecting toast, wrap the No cluster reason
  instead of truncating it, and document the behaviour in /setup and
  docs/cluster-setup.md.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@pierreneter
pierreneter force-pushed the exec-plugin-noninteractive branch from 3d9783b to 23c378a Compare November 12, 2026 00:45
@junior

junior commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator

Looks good, and the model is right: the TUI owns the terminal, StdinUnavailable is how kubectl says so too, and the capture lasts only while Run does, so the subcommands, the pod shell and the editor are untouched. The gate on main with #39 and this merged in is clean: gofmt, vet and go test -short on Go 1.27.4 (with -race for ., internal/k8s and internal/ui) and on Go 1.26.11.

Three things, none blocking:

  1. A panic inside the TUI loses its stack trace. Bubble Tea 1.3.10 prints the panic value with fmt.Printf, but the stack with debug.PrintStack(), which writes to os.Stderr: it goes into the pipe and only its last line is kept. Keeping the tail of what the pipe read (the last 64 KiB, say) and writing it to the terminal when Run returns an error would keep crash reports useful. I can send that as a follow-up.
  2. A cluster that does not answer now takes twice as long to show No cluster when its context uses a plugin, since every timeout with a plugin is retried: an EKS context with the VPN down waits about 20s instead of 10s. Fine if slow plugins are the common case; just noting it.
  3. CredentialPlugin loads the whole kubeconfig on the UI goroutine at every connect, for the toast. With a KUBECONFIG of many files that could show as a short pause; the name could come from the config main already loads for the context list.

@junior
junior merged commit e6ac73b into main Nov 12, 2026
3 checks passed
junior added a commit that referenced this pull request Nov 14, 2026
Since #41, os.Stderr is a pipe while the TUI runs, so that a credential
plugin cannot write over the screen. When the TUI panics, Bubble Tea
prints the panic's value to stdout and then its stack trace with
debug.PrintStack, which writes to os.Stderr: the trace went into the
pipe, which kept only its last line, so a crash report said what
happened but not where.

The pipe's reader now also keeps the last 64 KiB it read, as written.
When Run returns tea.ErrProgramPanic, main closes the pipe, gives the
reader up to a second to finish (a credential plugin that is still
running holds the pipe open too), and writes those bytes to the
terminal ahead of its own error line. This covers both panics Bubble
Tea recovers: one on its event loop, and one in a command's goroutine.

Signed-off-by: Adao Oliveira Jr <junior@users.noreply.github.com>
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.

2 participants