From 23c378a87679ac69de060763a00172d5cd7c36e7 Mon Sep 17 00:00:00 2001 From: "Pierre N." Date: Thu, 12 Nov 2026 07:37:44 +0700 Subject: [PATCH] k8s: keep credential plugins off the terminal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- docs/cluster-setup.md | 54 +++++++ internal/k8s/client.go | 49 ++++++- internal/k8s/credplugin.go | 238 ++++++++++++++++++++++++++++++ internal/k8s/credplugin_test.go | 253 ++++++++++++++++++++++++++++++++ internal/ui/connect.go | 40 ++++- internal/ui/credplugin_test.go | 66 +++++++++ internal/ui/model.go | 7 +- internal/ui/nocluster.go | 28 +++- internal/ui/plugins.go | 2 +- main.go | 25 +++- main_test.go | 15 ++ 11 files changed, 761 insertions(+), 16 deletions(-) create mode 100644 internal/k8s/credplugin.go create mode 100644 internal/k8s/credplugin_test.go create mode 100644 internal/ui/credplugin_test.go diff --git a/docs/cluster-setup.md b/docs/cluster-setup.md index 1c49617..664b780 100644 --- a/docs/cluster-setup.md +++ b/docs/cluster-setup.md @@ -147,6 +147,60 @@ A `401`/`403` is **not** treated as "no cluster": that is a live cluster refusing your credentials, a different problem with a different fix, and k10s connects anyway so you can see whatever you are allowed to see. +A third failure gets its own wording, because its fix is different again: + +3. **The credential plugin failed.** See the next section. + +## Credential plugins (tsh, aws, gke-gcloud-auth-plugin, kubelogin) + +Many kubeconfigs do not hold a token. The user entry names a program +(`exec:`) that fetches one, and client-go runs it before the first request: +`tsh kube credentials` for Teleport, `aws eks get-token`, `kubelogin` for +AKS, `gke-gcloud-auth-plugin`. + +Under `kubectl` that program shares the terminal, so it can print "tap your +security key" and read an MFA code. Under a TUI it cannot: k10s owns the +keyboard, a prompt would swallow keystrokes meant for k10s, and whatever the +plugin printed would land on top of the frame. That is where k9s hangs +([derailed/k9s#2048](https://github.com/derailed/k9s/issues/2048)), with +`kill -9` as the way out. + +k10s does two things instead: + +- **The plugin never gets the terminal.** It runs with stdin marked + unavailable — what `kubectl exec -i` does — so a plugin that needs to + prompt fails at once rather than waiting for input nobody can type. One + that declares `interactiveMode: Always` is refused before it runs. +- **What it said is what you see.** The plugin's stderr goes to k10s, not + the screen, and its last line is the reason under **No cluster**: + + ``` + credential plugin tsh: Having problems with relogin, please use 'tsh login/tsh kube login' manually + ``` + +The fix is in another shell: `tsh login` or `tsh kube login `, +`aws sso login`, `az login` — whatever the message names — then `r`. +Credentials the plugin caches (tsh keeps certificates under `~/.tsh`) are +picked up on the retry. + +A plugin that takes a while (minting a certificate, calling STS) does not +cost you the connection: its run time is spent inside the first request's +timeout, which client-go cannot interrupt, so k10s retries that one request +once with the credentials now cached. + +**Teleport.** Two setups do not fit the exec plugin at all, and `tsh` says +so: per-session MFA, whose certificates last about a minute, and a proxy +behind an L7 load balancer, which refuses direct Kubernetes clients. Both +are what `tsh proxy kube` is for. It keeps the session in its own terminal +and writes a kubeconfig into `$KUBECONFIG`: + +```bash +tsh proxy kube --exec # opens a shell with KUBECONFIG set +k10s # inside that shell +``` + +`:ctx` then lists only what that kubeconfig holds. + ## Related - [backends.md](backends.md) — the live backend, the demo backend, and what diff --git a/internal/k8s/client.go b/internal/k8s/client.go index ec6e35f..12c2f99 100644 --- a/internal/k8s/client.go +++ b/internal/k8s/client.go @@ -6,6 +6,7 @@ package k8s import ( "fmt" "os" + "path/filepath" "time" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -96,6 +97,9 @@ func New(path, context string) (*Client, error) { } restCfg.QPS = 50 restCfg.Burst = 100 + // A plugin that prompts would be prompting a terminal the TUI owns. See + // credplugin.go. + nonInteractive(restCfg.ExecProvider) // Before any client is built from it, so every one of them, and the // discovery copy below, carries the guard. if readOnly.Load() { @@ -116,7 +120,7 @@ func New(path, context string) (*Client, error) { // itself: that config also backs the informers' long-lived list-watch // connections, which a blanket per-request timeout would kill. discoCfg := *restCfg - discoCfg.Timeout = 10 * time.Second + discoCfg.Timeout = discoveryTimeout disco, err := discovery.NewDiscoveryClientForConfig(&discoCfg) if err != nil { return nil, fmt.Errorf("discovery client: %w", err) @@ -146,12 +150,49 @@ func New(path, context string) (*Client, error) { CurrentContext: cur, Server: restCfg.Host, } - if v, err := disco.ServerVersion(); err == nil { + c.probe(disco) + return c, nil +} + +// discoveryTimeout bounds the one-shot discovery GETs. A variable so a test +// can make it short. +var discoveryTimeout = 10 * time.Second + +// probe asks the API server its version: the one request New makes, and so +// the only evidence that there is a cluster at the other end at all (see +// Reachable). +// +// A credential plugin runs inside that request's timeout. client-go runs it +// before sending and cannot interrupt it, so a plugin that takes its time — +// tsh minting a certificate, aws calling STS — spends the budget, and the +// request itself then fails with "context deadline exceeded" even though +// the plugin succeeded. Its credentials are cached by then, so one more try +// is the network alone. Without it the user sees No cluster and presses r, +// which does the same thing by hand (gravitational/teleport#49622 is k9s +// doing exactly this). +// +// When the plugin is what failed, the error says so, in the plugin's own +// words where it had any: that is a different problem from the API server +// not answering, with a different fix. +func (c *Client) probe(disco discovery.DiscoveryInterface) { + plugin := c.RestConfig.ExecProvider + started := time.Now() + v, err := disco.ServerVersion() + if err != nil && plugin != nil && requestTimedOut(err) { + v, err = disco.ServerVersion() + } + switch { + case err == nil: c.Version = v.GitVersion - } else { + case plugin != nil && credentialPluginFailed(err): + c.versionErr = &CredentialPluginError{ + Plugin: filepath.Base(plugin.Command), + Said: tap.said(started), + Err: err, + } + default: c.versionErr = err } - return c, nil } // Reachable reports whether there is actually a cluster behind this diff --git a/internal/k8s/credplugin.go b/internal/k8s/credplugin.go new file mode 100644 index 0000000..5a37624 --- /dev/null +++ b/internal/k8s/credplugin.go @@ -0,0 +1,238 @@ +package k8s + +import ( + "bufio" + "context" + "errors" + "net" + "os" + "path/filepath" + "strings" + "sync" + "time" + + "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" +) + +// Credential plugins and the terminal. +// +// A kubeconfig user is often not a token but a program — tsh (Teleport), +// aws, gke-gcloud-auth-plugin, kubelogin — that client-go runs to get one. +// Under kubectl that program shares the terminal: it can print "tap your +// security key" and read an MFA code from stdin. Under a TUI it cannot. +// Bubble Tea owns stdin, so a prompt swallows keystrokes meant for k10s, and +// whatever the plugin prints lands on top of the frame. That is the k9s +// failure (derailed/k9s#2048): a session that looks hung, with kill -9 as +// the only way out. +// +// Two rules follow, and this file is both of them. +// +// 1. The plugin never gets the terminal while the TUI is up. New marks +// stdin unavailable — what kubectl does when stdin is spoken for, as in +// `kubectl exec -i` — so client-go runs the plugin without it and the +// plugin fails fast instead of waiting for input nobody can type. +// Logging in is done outside k10s, in a shell, then `r`. +// +// 2. What the plugin says is kept, not painted over. client-go hands every +// plugin the os.Stderr of the moment the first client is built, so +// CaptureStderr swaps a pipe in before that, and the last line a plugin +// wrote becomes the error the user reads: "Having problems with relogin, +// please use 'tsh login'" instead of "exit code 1". + +// stdinUnavailableMessage is the reason client-go gives when a plugin that +// declares `interactiveMode: Always` is refused before it runs. +const stdinUnavailableMessage = "k10s owns the terminal, so the credential plugin cannot prompt; " + + "log in with it in another shell, then press r" + +// nonInteractive keeps an exec plugin off the terminal (rule 1 above). +// Nothing else about the plugin changes, so its own declaration still +// decides between running without stdin (Never, IfAvailable) and refusing +// to run at all (Always). +func nonInteractive(ep *clientcmdapi.ExecConfig) { + if ep == nil { + return + } + ep.StdinUnavailable = true + ep.StdinUnavailableMessage = stdinUnavailableMessage +} + +// stderrTap is os.Stderr rerouted through a pipe k10s reads (rule 2). It +// remembers only the last non-empty line and when it arrived: plugins say +// one useful thing, at the end, and a message from a different attempt +// must not be pinned on this one. +type stderrTap struct { + mu sync.Mutex + terminal *os.File // what os.Stderr was before the swap + pipe *os.File // the write end now in os.Stderr + last string + at time.Time +} + +var ( + captureOnce sync.Once + tap = &stderrTap{} +) + +// start swaps the pipe in and drains it for as long as the write end lives. +func (t *stderrTap) start() error { + r, w, err := os.Pipe() + if err != nil { + return err + } + t.terminal, t.pipe = os.Stderr, w + os.Stderr = w + go func() { + sc := bufio.NewScanner(r) + sc.Buffer(make([]byte, 0, 64*1024), 1<<20) + for sc.Scan() { + line := strings.TrimSpace(sc.Text()) + if line == "" { + continue + } + t.mu.Lock() + t.last, t.at = line, time.Now() + t.mu.Unlock() + } + // A line too long for the buffer ends the scan; nothing to do about + // it but stop reading, and nothing is lost that mattered. + _ = sc.Err() + _ = r.Close() + }() + return nil +} + +// said returns the last line written at or after since, "" if none. The +// pipe is read on another goroutine, so a line the plugin wrote just before +// exiting may still be in flight when its error arrives; said waits a +// moment for it rather than reporting silence that is not. +func (t *stderrTap) said(since time.Time) string { + deadline := time.Now().Add(200 * time.Millisecond) + for { + t.mu.Lock() + line, at := t.last, t.at + t.mu.Unlock() + if !at.Before(since) { + // The prefix is the plugin's; the error carries its own. + line = strings.TrimPrefix(line, "ERROR: ") + return strings.TrimPrefix(line, "error: ") + } + if time.Now().After(deadline) { + return "" + } + time.Sleep(5 * time.Millisecond) + } +} + +// CaptureStderr routes os.Stderr through k10s for the rest of the process +// and returns what it was, for the few things still allowed to write to the +// terminal: the editor, a foreground plugin, main's own last words. Call it +// once, before the first client is built — client-go copies os.Stderr for +// the plugin at that point and never looks again. If the pipe cannot be +// made nothing is captured and the terminal stays the plugin's stderr, as +// it always was. +func CaptureStderr() (terminal *os.File) { + captureOnce.Do(func() { + if err := tap.start(); err != nil { + tap.terminal = os.Stderr + } + }) + return tap.terminal +} + +// ReleaseStderr puts the terminal back as os.Stderr. For after the TUI has +// exited: anything said then belongs on the screen, not in the pipe. +func ReleaseStderr() { + if tap.terminal != nil { + os.Stderr = tap.terminal + } +} + +// TerminalStderr is the terminal's stderr whether or not CaptureStderr has +// run: the right destination for a process that takes the terminal over. +func TerminalStderr() *os.File { + if tap.terminal != nil { + return tap.terminal + } + return os.Stderr +} + +// CredentialPlugin names the exec credential plugin a kubeconfig context +// signs in with — "tsh", "aws", "kubelogin" — or "" when the context uses +// none. It reads kubeconfig only, so it can be asked before connecting, +// which is when the UI wants to say who it is waiting on. +func CredentialPlugin(path, context string) string { + rules := clientcmd.NewDefaultClientConfigLoadingRules() + if path != "" { + rules.ExplicitPath = path + } + raw, err := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(rules, &clientcmd.ConfigOverrides{}).RawConfig() + if err != nil { + return "" + } + return credentialPlugin(raw, context) +} + +func credentialPlugin(raw clientcmdapi.Config, context string) string { + if context == "" { + context = raw.CurrentContext + } + ctx := raw.Contexts[context] + if ctx == nil { + return "" + } + auth := raw.AuthInfos[ctx.AuthInfo] + if auth == nil || auth.Exec == nil { + return "" + } + return filepath.Base(auth.Exec.Command) +} + +// CredentialPluginError is the context's exec credential plugin failing, +// which is a different situation from the API server not answering: the +// cluster may be fine, and the fix is to log in again, outside k10s. +type CredentialPluginError struct { + Plugin string // the command's base name: tsh, aws, kubelogin + Said string // the last line it wrote to stderr, "" if it was silent + Err error // client-go's own error, for errors.Is and errors.As +} + +func (e *CredentialPluginError) Error() string { + what := e.Said + if what == "" { + what = pluginClause(e.Err) + } + return "credential plugin " + e.Plugin + ": " + what +} + +func (e *CredentialPluginError) Unwrap() error { return e.Err } + +// credentialPluginFailed reports whether err is the plugin failing rather +// than the request it was fetching credentials for. client-go formats the +// plugin's error into the request's with %v, so the chain is cut there and +// the text is all there is to go on. +func credentialPluginFailed(err error) bool { + return err != nil && strings.Contains(err.Error(), "getting credentials: ") +} + +// pluginClause cuts client-go's error down to the part about the plugin: +// `Get "https://…/version": getting credentials: exec: executable tsh +// failed with exit code 1` → `exec: executable tsh failed with exit code 1`. +func pluginClause(err error) string { + msg := err.Error() + if i := strings.LastIndex(msg, "getting credentials: "); i >= 0 { + msg = msg[i+len("getting credentials: "):] + } + return msg +} + +// requestTimedOut reports whether err is a request's own budget running +// out: rest.Config.Timeout fires through the request context, and the +// transport says so in a net.Error. +func requestTimedOut(err error) bool { + if errors.Is(err, context.DeadlineExceeded) { + return true + } + var ne net.Error + return errors.As(err, &ne) && ne.Timeout() +} diff --git a/internal/k8s/credplugin_test.go b/internal/k8s/credplugin_test.go new file mode 100644 index 0000000..0e79987 --- /dev/null +++ b/internal/k8s/credplugin_test.go @@ -0,0 +1,253 @@ +package k8s + +import ( + "errors" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "runtime" + "strings" + "sync/atomic" + "testing" + "time" + + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" +) + +// execCredential is what a plugin prints to hand client-go a bearer token. +const execCredential = `{"apiVersion":"client.authentication.k8s.io/v1beta1","kind":"ExecCredential","status":{"token":"plugin-token"}}` + +// writeExecPlugin writes a shell script that stands in for an exec +// credential plugin and returns its path. name is the base name — "tsh" — +// because that is what k10s calls the plugin in its errors. +func writeExecPlugin(t *testing.T, name, body string) string { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("the fake plugin is a shell script") + } + path := filepath.Join(t.TempDir(), name) + if err := os.WriteFile(path, []byte("#!/bin/sh\n"+body+"\n"), 0o700); err != nil { + t.Fatal(err) + } + return path +} + +// writeExecKubeconfig writes a kubeconfig whose single context signs in +// through the exec plugin at command, and points $KUBECONFIG at it. +func writeExecKubeconfig(t *testing.T, name, server, ca, command, interactiveMode string) { + t.Helper() + path := filepath.Join(t.TempDir(), "exec.yaml") + body := fmt.Sprintf(`apiVersion: v1 +kind: Config +clusters: +- name: %[1]s + cluster: + server: %[2]s + certificate-authority-data: %[3]s +users: +- name: %[1]s + user: + exec: + apiVersion: client.authentication.k8s.io/v1beta1 + command: %[4]s + args: [kube, credentials] + interactiveMode: %[5]s +contexts: +- name: %[1]s + context: + cluster: %[1]s + user: %[1]s +current-context: %[1]s +`, name, server, ca, command, interactiveMode) + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("KUBECONFIG", path) +} + +// captureStderrForTest does what main's k8s.CaptureStderr does, scoped to +// one test: the plugin's stderr goes through a tap, and the terminal comes +// back afterwards. +func captureStderrForTest(t *testing.T) { + t.Helper() + prev := tap + tt := &stderrTap{} + if err := tt.start(); err != nil { + t.Fatal(err) + } + tap = tt + t.Cleanup(func() { + os.Stderr = tt.terminal + _ = tt.pipe.Close() + tap = prev + }) +} + +// The plugin is run, its token is used, and it is run without the terminal: +// the TUI owns stdin, so client-go is told it is unavailable, as kubectl +// does when stdin is spoken for. +func TestExecPluginRunsWithoutTheTerminal(t *testing.T) { + srv, ca, tokens := tlsAPIServer(t) + plugin := writeExecPlugin(t, "tsh", "printf '%s' '"+execCredential+"'") + writeExecKubeconfig(t, "tp", srv.URL, ca, plugin, "IfAvailable") + + c, err := New("", "") + if err != nil { + t.Fatalf("New: %v", err) + } + if err := c.Reachable(); err != nil { + t.Fatalf("Reachable: %v", err) + } + if got := tokens(); len(got) == 0 || got[len(got)-1] != "plugin-token" { + t.Errorf("bearer tokens the API server saw = %q, want the plugin's", got) + } + ep := c.RestConfig.ExecProvider + if ep == nil || !ep.StdinUnavailable || ep.StdinUnavailableMessage == "" { + t.Errorf("ExecProvider = %+v, want stdin marked unavailable with a reason", ep) + } + if p := CredentialPlugin("", ""); p != "tsh" { + t.Errorf("CredentialPlugin = %q, want tsh", p) + } +} + +// A plugin that declares it must have a terminal (interactiveMode: Always) +// is refused before it runs, and the refusal is reported as the plugin's +// failure rather than the cluster's. +func TestExecPluginThatInsistsOnATerminalIsRefusedNotRun(t *testing.T) { + srv, ca, _ := tlsAPIServer(t) + mark := filepath.Join(t.TempDir(), "ran") + plugin := writeExecPlugin(t, "tsh", "touch '"+mark+"'; printf '%s' '"+execCredential+"'") + writeExecKubeconfig(t, "tp", srv.URL, ca, plugin, "Always") + + c, err := New("", "") + if err != nil { + t.Fatalf("New: %v", err) + } + err = c.Reachable() + var pe *CredentialPluginError + if !errors.As(err, &pe) { + t.Fatalf("Reachable = %v, want a CredentialPluginError", err) + } + if pe.Plugin != "tsh" || !strings.Contains(err.Error(), "interactive") { + t.Errorf("error = %q, want it to name tsh and the interactive-mode refusal", err) + } + if _, statErr := os.Stat(mark); statErr == nil { + t.Error("the plugin ran; it should have been refused before that") + } +} + +// What the plugin wrote to stderr is the error the user reads, not the exit +// code: "Having problems with relogin…" is actionable, "exit code 1" is not. +func TestExecPluginsLastWordsReachTheUser(t *testing.T) { + captureStderrForTest(t) + srv, ca, _ := tlsAPIServer(t) + const said = "Having problems with relogin, please use 'tsh login/tsh kube login' manually" + plugin := writeExecPlugin(t, "tsh", `echo "ERROR: `+said+`" >&2; exit 1`) + writeExecKubeconfig(t, "tp", srv.URL, ca, plugin, "IfAvailable") + + c, err := New("", "") + if err != nil { + t.Fatalf("New: %v", err) + } + err = c.Reachable() + if err == nil { + t.Fatal("Reachable = nil, want the plugin's failure") + } + if want := "credential plugin tsh: " + said; err.Error() != want { + t.Errorf("error = %q\n want %q", err, want) + } + // Still client-go's error underneath, for anything that asks. + if !strings.Contains(errors.Unwrap(err).Error(), "exit code 1") { + t.Errorf("Unwrap = %v, want client-go's exec error", errors.Unwrap(err)) + } +} + +// A silent plugin still gets client-go's clause, cut down to the part about +// the plugin. +func TestCredentialPluginErrorWithoutLastWords(t *testing.T) { + err := &CredentialPluginError{ + Plugin: "aws", + Err: errors.New(`Get "https://x/version": getting credentials: exec: executable aws failed with exit code 255`), + } + if want := "credential plugin aws: exec: executable aws failed with exit code 255"; err.Error() != want { + t.Errorf("Error = %q, want %q", err, want) + } +} + +// The plugin's running time is spent inside the probe's timeout, and +// client-go cannot interrupt it. A slow plugin therefore used to turn into +// "No cluster: context deadline exceeded" even though it succeeded; the +// retry, with its credentials now cached, is the network alone. +func TestSlowExecPluginDoesNotCountAgainstTheServer(t *testing.T) { + prev := discoveryTimeout + discoveryTimeout = 300 * time.Millisecond + t.Cleanup(func() { discoveryTimeout = prev }) + + srv, ca, tokens := tlsAPIServer(t) + plugin := writeExecPlugin(t, "tsh", "sleep 1; printf '%s' '"+execCredential+"'") + writeExecKubeconfig(t, "tp", srv.URL, ca, plugin, "IfAvailable") + + c, err := New("", "") + if err != nil { + t.Fatalf("New: %v", err) + } + if err := c.Reachable(); err != nil { + t.Fatalf("Reachable = %v, want the retry to have reached the server", err) + } + if c.Version == "" { + t.Error("Version empty after a successful probe") + } + if got := tokens(); len(got) == 0 || got[len(got)-1] != "plugin-token" { + t.Errorf("bearer tokens the API server saw = %q, want the plugin's", got) + } +} + +// Without a plugin in the way a timeout is what it says: the server did not +// answer, and asking again would only double the wait. +func TestProbeTimeoutWithoutAPluginIsNotRetried(t *testing.T) { + prev := discoveryTimeout + discoveryTimeout = 200 * time.Millisecond + t.Cleanup(func() { discoveryTimeout = prev }) + + var hits atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + time.Sleep(600 * time.Millisecond) + })) + t.Cleanup(srv.Close) + t.Setenv("KUBECONFIG", writeKubeconfig(t, t.TempDir(), "slow", srv.URL)) + + c, err := New("", "") + if err != nil { + t.Fatalf("New: %v", err) + } + err = c.Reachable() + if err == nil || !requestTimedOut(err) { + t.Fatalf("Reachable = %v, want a timeout", err) + } + if n := hits.Load(); n != 1 { + t.Errorf("server hit %d times, want 1: no plugin, so no retry", n) + } +} + +func TestCredentialPluginNamesTheContextsPlugin(t *testing.T) { + raw := clientcmdapi.Config{ + CurrentContext: "tp", + Contexts: map[string]*clientcmdapi.Context{ + "tp": {AuthInfo: "tp"}, + "kind": {AuthInfo: "kind"}, + "lost": {AuthInfo: "nobody"}, + }, + AuthInfos: map[string]*clientcmdapi.AuthInfo{ + "tp": {Exec: &clientcmdapi.ExecConfig{Command: "/usr/local/bin/tsh"}}, + "kind": {Token: "t"}, + }, + } + for ctx, want := range map[string]string{"": "tsh", "tp": "tsh", "kind": "", "lost": "", "absent": ""} { + if got := credentialPlugin(raw, ctx); got != want { + t.Errorf("credentialPlugin(%q) = %q, want %q", ctx, got, want) + } + } +} diff --git a/internal/ui/connect.go b/internal/ui/connect.go index 5df0b04..160c851 100644 --- a/internal/ui/connect.go +++ b/internal/ui/connect.go @@ -2,6 +2,8 @@ package ui import ( "errors" + "io" + "os/exec" tea "github.com/charmbracelet/bubbletea" @@ -40,6 +42,16 @@ type Startup struct { // open a session into it, is hidden and refused. internal/k8s enforces // the same rule on the wire; this is the part the user sees. ReadOnly bool + // CredentialPlugin names the exec plugin a context signs in with ("tsh", + // "aws"), or "" — read from kubeconfig, no request made. A plugin + // minting a certificate is the slow part of connecting and the part the + // user can do something about, so the connecting toast names it. + CredentialPlugin func(context string) string + // TerminalStderr is where a process that takes the terminal over (the + // editor, a foreground plugin) writes its errors. While k10s runs, + // os.Stderr is a pipe that collects what credential plugins say (see + // k8s.CaptureStderr), and Bubble Tea would hand the process that pipe. + TerminalStderr io.Writer } // NewStartup builds a model that is already renderable and connects in the @@ -48,6 +60,8 @@ func NewStartup(s Startup) *Model { m := New(&pendingSource{kinds: s.Kinds, contexts: s.Contexts, ctx: s.Context}) m.connect = s.Connect m.readOnly = s.ReadOnly + m.credPlugin = s.CredentialPlugin + m.termStderr = s.TerminalStderr // A demo context is not something kubeconfig can hand back, so it has to // be asked for by name; anything else stays "" — kubeconfig's // current-context is the only cluster k10s opens on by itself. @@ -60,10 +74,32 @@ func NewStartup(s Startup) *Model { m.kubeCtxs = append([]string(nil), s.Contexts...) m.connecting = true m.connName = s.Context - m.toast = m.withThemeWarning("connecting…") + m.toast = m.connectingToast("connecting…", s.Context) return m } +// connectingToast is the status line while Connect runs. When the context +// signs in through a credential plugin, that is what the wait is for, and +// naming it ("… connecting · credentials from tsh") tells the user which +// tool to go and log in with if the wait never ends. +func (m *Model) connectingToast(base, name string) string { + if m.credPlugin != nil { + if p := m.credPlugin(name); p != "" { + base += " · credentials from " + p + } + } + return m.withThemeWarning(base) +} + +// toTerminal points a process that is about to take the terminal over at +// the terminal's own stderr; see Startup.TerminalStderr. +func (m *Model) toTerminal(c *exec.Cmd) *exec.Cmd { + if m.termStderr != nil && c.Stderr == nil { + c.Stderr = m.termStderr + } + return c +} + // connectCmd runs Connect off the event loop. Each attempt carries a // generation so a slow first attempt landing after the user has picked // another context can be discarded instead of overwriting it. @@ -80,7 +116,7 @@ func (m *Model) connectCmd(name string) tea.Cmd { if name == "" { m.connName = m.src.ClusterInfo().Context } - m.toast = m.withThemeWarning("… connecting") + m.toast = m.connectingToast("… connecting", name) return func() tea.Msg { src, warn := fn(name) return srcConnectedMsg{gen: gen, src: src, warn: warn} diff --git a/internal/ui/credplugin_test.go b/internal/ui/credplugin_test.go new file mode 100644 index 0000000..5c88958 --- /dev/null +++ b/internal/ui/credplugin_test.go @@ -0,0 +1,66 @@ +package ui + +import ( + "bytes" + "os/exec" + "strings" + "testing" + + tea "github.com/charmbracelet/bubbletea" + + "github.com/p10node/k10s/internal/domain" + "github.com/p10node/k10s/internal/mock" +) + +// While a context signs in through a credential plugin, the connecting +// toast names it: that is what the wait is for, and the tool to log in with +// if it never ends. +func TestConnectingToastNamesTheCredentialPlugin(t *testing.T) { + t.Setenv("K10S_CONFIG", t.TempDir()+"/config.yaml") + plugins := map[string]string{"teleport-prod": "tsh", "kind-kind": ""} + m := NewStartup(Startup{ + Kinds: mock.New("").Kinds(), + Contexts: []string{"kind-kind", "teleport-prod"}, + Context: "teleport-prod", + Connect: func(string) (domain.Source, string) { return mock.New(""), "" }, + CredentialPlugin: func(ctx string) string { return plugins[ctx] }, + }) + m.Update(tea.WindowSizeMsg{Width: 140, Height: 44}) + if !strings.Contains(m.toast, "credentials from tsh") { + t.Errorf("startup toast = %q, want it to name tsh", m.toast) + } + + m.connectCmd("kind-kind") + if strings.Contains(m.toast, "credentials from") { + t.Errorf("toast for a token context = %q, names a plugin it does not use", m.toast) + } + m.connectCmd("teleport-prod") + if !strings.Contains(m.toast, "credentials from tsh") { + t.Errorf("toast = %q, want it to name tsh", m.toast) + } +} + +// Processes that take the terminal over write errors to the terminal, not +// to the pipe os.Stderr has become; a stderr the caller chose is kept. +func TestProcessesThatTakeTheTerminalGetItsStderr(t *testing.T) { + var terminal bytes.Buffer + m := New(mock.New("")) + m.termStderr = &terminal + + c := m.toTerminal(exec.Command("true")) + if c.Stderr != &terminal { + t.Errorf("Stderr = %v, want the terminal", c.Stderr) + } + + var own bytes.Buffer + c = exec.Command("true") + c.Stderr = &own + if m.toTerminal(c).Stderr != &own { + t.Error("a stderr the caller set was replaced") + } + + m.termStderr = nil + if c := m.toTerminal(exec.Command("true")); c.Stderr != nil { + t.Errorf("with no terminal known, Stderr = %v, want nil (Bubble Tea's default)", c.Stderr) + } +} diff --git a/internal/ui/model.go b/internal/ui/model.go index ed46b2f..57e0bd0 100644 --- a/internal/ui/model.go +++ b/internal/ui/model.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "io" "os" "slices" "strconv" @@ -226,6 +227,10 @@ type Model struct { connecting bool connGen int connName string + // credPlugin and termStderr are Startup.CredentialPlugin and + // Startup.TerminalStderr; see connect.go. + credPlugin func(context string) string + termStderr io.Writer // readOnly is --readonly. Actions that change the cluster or open a // session into it are hidden and refused, and so are dangerous plugins @@ -1148,7 +1153,7 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return editExitMsg{source: msg.source, original: msg.original, session: msg.session, kind: msg.kind, ns: msg.ns, name: msg.name, path: msg.path, err: err} } } - return m, tea.ExecProcess(c, func(err error) tea.Msg { + return m, tea.ExecProcess(m.toTerminal(c), func(err error) tea.Msg { return editExitMsg{source: msg.source, original: msg.original, session: msg.session, kind: msg.kind, ns: msg.ns, name: msg.name, path: msg.path, err: err} }) diff --git a/internal/ui/nocluster.go b/internal/ui/nocluster.go index 515de23..f3c619e 100644 --- a/internal/ui/nocluster.go +++ b/internal/ui/nocluster.go @@ -100,7 +100,17 @@ func (m *Model) noClusterLines(inner int) []string { "", } if m.offlineWhy != "" { - out = append(out, line(th.Err, m.offlineWhy), "") + // Wrapped, not truncated: a credential plugin's reason ("Having + // problems with relogin, please use 'tsh login/tsh kube login' + // manually") carries the fix in its second half. + why := wrapLine(m.offlineWhy, inner-len(pad)) + if len(why) > 3 { + why = why[:3] + } + for _, w := range why { + out = append(out, line(th.Err, w)) + } + out = append(out, "") } out = append(out, line(th.Subtle, "k10s reads the same kubeconfig kubectl does — $KUBECONFIG,"), @@ -201,6 +211,22 @@ whatever that context can reach. No kubeconfig, no cluster, nothing to show. If the last one fails, k10s cannot connect either — that is the thing to fix, and it is not a k10s problem. + 5 · CREDENTIAL PLUGINS (tsh, aws, gke-gcloud-auth-plugin, kubelogin) + + Many kubeconfigs name a program that fetches the token instead of + holding one. kubectl lets that program use the terminal, to ask for + an MFA code; k10s cannot — the TUI owns the keyboard — so it runs the + plugin without one, and the plugin fails fast instead of waiting. + The panel then shows the plugin's own last line, e.g. + + credential plugin tsh: Having problems with relogin, please use + 'tsh login/tsh kube login' manually + + Log in with that tool in another shell (tsh login, aws sso login, + az login…), then press r. Teleport with per-session MFA, or behind + an L7 load balancer, wants its local proxy instead: + tsh proxy kube --exec then run k10s in that shell + IN k10s r retry the connection diff --git a/internal/ui/plugins.go b/internal/ui/plugins.go index 5301061..de63d19 100644 --- a/internal/ui/plugins.go +++ b/internal/ui/plugins.go @@ -136,7 +136,7 @@ func (m *Model) runPlugin(item plugin.Named, vars plugin.Vars) tea.Cmd { } m.toast = "⟩ plugin " + label - return tea.ExecProcess(cmd, func(err error) tea.Msg { + return tea.ExecProcess(m.toTerminal(cmd), func(err error) tea.Msg { msg := foregroundPluginResult(item, label, err) msg.session = session return msg diff --git a/main.go b/main.go index dfddbc4..526538c 100644 --- a/main.go +++ b/main.go @@ -97,6 +97,12 @@ func main() { } k8s.SilenceLogging() + // Credential plugins (tsh, aws, kubelogin) write to whatever os.Stderr + // is when the first client is built. From here on that is a pipe k10s + // reads, so a plugin's complaint reaches the No cluster panel instead of + // the raw terminal under the TUI; the terminal itself goes only to + // processes that are meant to have it. + terminal := k8s.CaptureStderr() // Before the first connection: only clients built after this carry the // guard. if readOnly { @@ -120,16 +126,21 @@ func main() { } m := ui.NewStartup(ui.Startup{ - Kinds: k8s.Kinds(), - Contexts: ctxNames, - Context: curCtx, - Connect: newSource, - ReadOnly: readOnly, + Kinds: k8s.Kinds(), + Contexts: ctxNames, + Context: curCtx, + Connect: newSource, + ReadOnly: readOnly, + CredentialPlugin: func(ctx string) string { return k8s.CredentialPlugin("", ctx) }, + TerminalStderr: terminal, }) p := tea.NewProgram(m, tea.WithAltScreen(), tea.WithMouseCellMotion()) - if _, err := p.Run(); err != nil { - fmt.Fprintln(os.Stderr, "k10s:", err) + _, runErr := p.Run() + // The TUI is gone; whatever is said from here belongs on the screen. + k8s.ReleaseStderr() + if runErr != nil { + fmt.Fprintln(os.Stderr, "k10s:", runErr) os.Exit(1) } diff --git a/main_test.go b/main_test.go index 6de1532..2606616 100644 --- a/main_test.go +++ b/main_test.go @@ -11,6 +11,7 @@ import ( "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" + "github.com/p10node/k10s/internal/k8s" "github.com/p10node/k10s/internal/update" ) @@ -94,3 +95,17 @@ func TestSelfUpdateStepsAsideForAPackageManager(t *testing.T) { t.Errorf("selfUpdate error = %q, want %q", err, want) } } + +// A credential plugin's own words survive the trim: they are the actionable +// part, and nothing in them is advice aimed at another tool. +func TestNoClusterReasonKeepsACredentialPluginsWords(t *testing.T) { + err := &k8s.CredentialPluginError{ + Plugin: "tsh", + Said: "Having problems with relogin, please use 'tsh login/tsh kube login' manually", + Err: errors.New("getting credentials: exec: executable tsh failed with exit code 1"), + } + want := "credential plugin tsh: Having problems with relogin, please use 'tsh login/tsh kube login' manually" + if got := noClusterReason(err); got != want { + t.Errorf("noClusterReason = %q\n want %q", got, want) + } +}