From f86a9b0e81d0742fac46baa66bf8f34b618969b0 Mon Sep 17 00:00:00 2001 From: Olof Mattsson <14146601+omattsson@users.noreply.github.com> Date: Fri, 30 Oct 2026 10:24:02 +0100 Subject: [PATCH 1/7] feat(stack): add stack port-forward (#178) Forward a local port to a service of a stack through the server WebSocket route GET /api/v1/stack-instances/:id/port-forward. - One WebSocket per accepted local connection, binary frames both ways, keepalive pings and a read deadline. - Header-only auth; no ?token= retry on stream routes. - Typed handshake errors with the server error codes; fatal errors stop the command, per-connection errors close one tunnel. - Close-code messages for 1001, 1003, 1008, 1009, 1011, 4001, 4002, 4502. - Local port rules: LOCAL 0 or 1025-65535, default port fallback, a clear error for an explicit port in use. - --insecure needs --allow-insecure-stream. - Refresh the vendored swagger.json; README, CLAUDE.md and agent guide. Co-Authored-By: Claude Opus 5.5 --- .github/copilot-instructions.md | 4 +- CLAUDE.md | 5 +- README.md | 13 + cli/cmd/addrinuse_other.go | 7 + cli/cmd/addrinuse_windows.go | 15 + cli/cmd/agent_guide.md | 3 + cli/cmd/stack_portforward.go | 450 ++++++++++++++++++++ cli/cmd/stack_portforward_test.go | 487 +++++++++++++++++++++ cli/pkg/client/portforward.go | 277 ++++++++++++ cli/pkg/client/portforward_test.go | 539 ++++++++++++++++++++++++ cli/pkg/client/websocket.go | 290 +++++++++++++ cli/test/contract/testdata/swagger.json | 151 ++++++- 12 files changed, 2236 insertions(+), 5 deletions(-) create mode 100644 cli/cmd/addrinuse_other.go create mode 100644 cli/cmd/addrinuse_windows.go create mode 100644 cli/cmd/stack_portforward.go create mode 100644 cli/cmd/stack_portforward_test.go create mode 100644 cli/pkg/client/portforward.go create mode 100644 cli/pkg/client/portforward_test.go diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index bf15316..b1fd502 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -9,7 +9,7 @@ stackctl is a Go CLI tool (Cobra) for managing Kubernetes stack deployments. It - All code lives under `cli/` — the Go module root (`cli/go.mod`). Run Go commands from `cli/`. - Go version: see `cli/go.mod` (1.26.10 or later). CI uses `go-version-file: cli/go.mod`. - Commands: `cli/cmd/` (one file per command group, subcommands registered in `init()`) -- HTTP client: `cli/pkg/client/client.go` (REST), `cli/pkg/client/auth.go` (session renewal), `cli/pkg/client/websocket.go` (deployment log streaming) +- HTTP client: `cli/pkg/client/client.go` (REST), `cli/pkg/client/auth.go` (session renewal), `cli/pkg/client/websocket.go` (deployment log streaming, stream dial, close codes, keepalive), `cli/pkg/client/portforward.go` (port-forward tunnel) - Types: `cli/pkg/types/types.go` (client-side structs matching API responses) - Output: `cli/pkg/output/output.go` (table, JSON, YAML, quiet formatters) - Config: `cli/pkg/config/config.go` (YAML via `gopkg.in/yaml.v3`, `~/.stackmanager/config.yaml`), `cli/pkg/config/token.go` (token files), `lock_unix.go`, `lock_windows.go` and `lock_other.go` (file locks) @@ -22,7 +22,7 @@ stackctl is a Go CLI tool (Cobra) for managing Kubernetes stack deployments. It | `config` | set, get, list, use-context, current-context, delete-context | config.go | | `login`/`logout`/`whoami` | top-level; supports password + OIDC loopback flow; `logout --all` ends every session of the user | login.go | | `auth` | register | login.go | -| `stack` | list, get, create, update, deploy, stop, clean, delete, status, logs, clone, extend, values, compare, history, rollback, history-values, watch, preview, pods, recent, follow, unfollow, action (list, run, log) | stack.go; preview/pods/recent: stack_inspect.go; follow/unfollow: stack_followers.go; action: stack_action.go; TTL and stop-time helpers: ttl.go | +| `stack` | list, get, create, update, deploy, stop, clean, delete, status, logs, clone, extend, values, compare, history, rollback, history-values, watch, preview, pods, recent, follow, unfollow, action (list, run, log), port-forward | stack.go; preview/pods/recent: stack_inspect.go; follow/unfollow: stack_followers.go; action: stack_action.go; port-forward: stack_portforward.go; TTL and stop-time helpers: ttl.go | | `template` | list, get, create, update, delete, clone, instantiate, quick-deploy, publish, unpublish, update-chart, add-chart, remove-chart, versions (list, get, diff) | template.go; add-chart/remove-chart: chart_changes.go | | `definition` | list, get, create, update, delete, export, import, update-chart, add-chart, remove-chart, check-upgrade, upgrade, link-template | definition.go; add-chart/remove-chart/check-upgrade/upgrade/link-template: chart_changes.go | | `override` | list, get, set, unset, delete, branch (list/get/set/delete), quota (get/set/delete) | override.go | diff --git a/CLAUDE.md b/CLAUDE.md index d2f84a1..c1173ae 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -32,6 +32,7 @@ cli/ # Go module root (github.com/omattsson/stackctl/cl stack_inspect.go # stack preview/pods/recent (deploy-preview, live pods and events, recent) stack_action.go # stack action list/run/log (custom actions of the hooks config; job log polling with backoff) stack_followers.go # stack follow/unfollow (in-app notifications; not the --follow log flag) + stack_portforward.go # stack port-forward (parsePortSpec, --address, listeners with port fallback, one tunnel per local connection); addrinuse_*.go: EADDRINUSE per OS ttl.go # --ttl/--stop-at/--until parsing, TTL limit check (GET /api/v1/ui-config, 30-day fallback), expiry text server.go # server info (ping, ui-config; public routes, no credentials) template.go # template list/get/create/update/clone/publish/unpublish/versions/instantiate/quick-deploy/delete @@ -62,7 +63,8 @@ cli/ # Go module root (github.com/omattsson/stackctl/cl client/ client.go # HTTP client wrapper (auth headers, base URL, error handling) auth.go # Session renewal (refresh token, 401 retry), login/refresh/logout calls - websocket.go # WebSocket client (watch, deploy log streaming) + websocket.go # WebSocket client (watch, deploy log streaming); stream dial without ?token= (dialWSOpts), handshake error codes, close codes, keepalive + portforward.go # DialPortForward and Tunnel (byte copy, half-close handling, keepalive) types/ types.go # Client-side structs matching API responses config/ @@ -192,6 +194,7 @@ All API calls go to `/api/v1/*`. Key route groups: - `/api/v1/audit-logs` — list/export; `--action` and `--entity-type` are exact matches (`deploy`, `stack_instance`, …; v0.7.0+, see `auditFilterValuesHelp`). Older servers: HTTP method + last route segment (`create` / `deploy`) - `/api/v1/users` — list/delete/disable/enable/password (admin). `PUT /:id/role` `{"role"}` (k8s-stack-manager v0.8.0+, `user set-role `, user by ID or username via `resolveUserID`): response `id`, `old_role`, `new_role`, `changed`; 403 own role / caller not admin, 409 SSO user / last admin, 400 invalid role; a plain 404 (no route) maps to the v0.8.0 upgrade hint (`setRoleError`) - `/ws` — the server closes a socket with code 1008 and reason `session revoked` or `token expired`. `Client.Watch` returns a `WatchStream`; `Err()` gives a `*WSClosedError` ("Connection closed by the server: . Log in again."). `stack watch` exits non-zero on it; for `token expired` it calls `Client.RenewSession` (forced renewal, false without refresh token) and reconnects, at most once per 30 s (`stackWatchMinReconnect`), then with `--id` GETs each pending instance (`watchCatchUp`) so a terminal status in the gap counts. Ctrl-C during renewal or dial exits 0. Another connection loss: `--id` with pending instances exits non-zero ("connection lost before all instances reached a terminal status"), else a stderr note and exit 0. `StreamDeploymentLogs` returns the same `WSClosedError`; `--follow` (`followLogsCtx`, op deploy/stop/clean/rollback, "" for `stack logs -f`) uses the same `renewAfterClose`; it reads the instance after the first connect (the server sets deploying/stopping/cleaning before the 202; not for `logs -f`) and after each reconnect (`StreamOptions.AfterSubscribe`). A normal close (code 1000, stream status `unknown`) reads the instance (`followEndError`): a final status counts, else it fails like a lost connection. When it cannot follow to the end it says that the operation continues on the server and points to `stack watch --id` / `stack status` (never "run again"); errors name the operation ("rollback failed (status error): ..."). Terminal statuses live in `client.IsTerminalStatus` (running, stopped, draft, error, partial, failed) and `client.IsFailedStatus` (error, partial, failed → exit 1) +- `GET /api/v1/stack-instances/:id/port-forward?service=&port=` — `stack port-forward` (omattsson/stackctl#178, k8s-stack-manager#722). WebSocket upgrade, auth only by header (`dialWSOpts` with `stream`: no `?token=` retry, a 401 gets one forced `RenewSession` and one more dial; no `Origin` header). Refusals are HTTP statuses with `{"error","code"}` (`WSHandshakeError.Code`, `StreamCode*`): `FeatureDisabled()` (404 `feature_disabled`), `NoRoute()` (404 without a code: older server), `UpgradeDropped()` (2xx, or 400 without a code or `upgrade_required`: a proxy), `Fatal()` (400, 401, 403, 404, 409 stop the command; 429, 5xx and network errors close one connection). After the upgrade: binary frames of at most 32 KiB both ways (server read limit 64 KiB), no half-close. `Client.Tunnel`: one reader and one writer goroutine, pings and the close through `WriteControl`; ping every 30 s, read deadline 75 s moved by data, ping or pong; a local EOF sends close 1000 and reads for at most 2 s; a server close 1000 does `CloseWrite` on the local socket. Close codes (`Close*` constants, `CloseCodeText`): 1001 shutdown, 1008 revoked (`StreamClosedError.Fatal`, unwraps to `WSClosedError`), 1011, 4001 time limit, 4002 idle, 4502 pod stream error. `--insecure` is refused unless `--allow-insecure-stream`. - `/api/v1/admin/orphaned-namespaces` — `orphaned list [--details]`; `DELETE /:ns?confirm=` is needed for a namespace without the label managed-by=k8s-stack-manager (409 without it; `orphaned delete --confirm`). `test/contract/path_contract_test.go` checks every `/api/v1` literal of `pkg/client` and `cmd` against the vendored swagger - `POST /api/v1/auth/logout-all` — `logout --all`: uses the session token of the context when a token file exists (also when an API key is set), else the API key; a 401 gets one forced `RenewSession` and one retry. On failure the command stops and keeps the token file; on success the normal logout follows. - `GET /api/v1/dashboard` — `dashboard`: covers all users; `failing_instances` has status error only, `expiring_soon` the next hour. For the role `user`, `clusters` has ID and name only (no health): the table hides clusters without `health_status`. diff --git a/README.md b/README.md index b3a6424..766b5e5 100644 --- a/README.md +++ b/README.md @@ -251,6 +251,11 @@ stackctl stack recent # your 5 most recently updated stacks stackctl stack preview my-app stackctl stack preview my-app --chart backend > backend-values.yaml +# Forward a local port to a service of the stack (runs until Ctrl-C) +stackctl stack port-forward my-app mysql 3306 # then: mysql -h 127.0.0.1 -P 3306 +stackctl stack port-forward my-app frontend 80 # local port 10080 +stackctl stack port-forward my-app mysql 0:3306 -q # any free port; stdout: 127.0.0.1: + # Custom actions of the hooks config (for example a database refresh) stackctl stack action list my-app stackctl stack action run my-app refresh-db --param market=dk --yes --wait @@ -298,6 +303,13 @@ stackctl stack rollback my-app --target-log `stack get` shows `Following` and `Followers` (k8s-stack-manager v0.8.0 or later). `stack preview -q` prints the names of the charts whose values differ from the last deploy. `stack preview` writes the values-drift warning to stderr, so the YAML on stdout stays valid. +`stack port-forward [LOCAL:]REMOTE` opens a local port that forwards to a service of the stack through the server (`GET /api/v1/stack-instances/:id/port-forward`). You need no kubeconfig. REMOTE is the service port. LOCAL must be 0 (any free port) or from 1025 to 65535. Without LOCAL, the local port is REMOTE when REMOTE is above 1024, else REMOTE + 10000; when that port is in use, stackctl tries the next 20 ports, then any free port. An explicit LOCAL port that is in use stops the command. `--address` sets the local addresses (default `127.0.0.1`, comma-separated). stdout gets `Forwarding from 127.0.0.1:3306 -> mysql:3306` (`-q`: only `host:port`); stderr gets one line when a connection opens and one when it closes. Each local connection opens its own tunnel. Ctrl-C closes all connections and exits 0. + +- Errors that stop the command (exit 1): no access (401, 403), an unknown stack, service or port (404), the feature turned off on the server or on the cluster, a stack that is not deployed (409), and a revoked session or permission during a tunnel (close 1008). A 404 without an error code means that the server has no port-forward route: upgrade k8s-stack-manager. A status 200 without an upgrade means that a proxy dropped the WebSocket upgrade. +- Errors that close only one connection: the tunnel limit (429), no ready pod (503), a failed pod dial (502), the server time limit (close 4001), the idle timeout (4002), a pod stream error (4502) and network errors. +- The server can refuse API keys for port-forward: then run `stackctl login` and remove the API key from the context or the environment. +- Security: anyone who can reach the local port uses your access to the stack. On a shared machine, other local users can connect to it. Web pages in your browser can reach a forwarded HTTP service through DNS rebinding: stop the command when you do not need it. An address that is not loopback opens the port to the network (stackctl writes a warning). `--insecure` is refused, because a man in the middle could read the tunnel data; `--allow-insecure-stream` accepts that risk. + `stack action list` and `stack action run` see the actions that apply to the stack: a server can limit an action to some templates, template categories, definitions or clusters (`applies_to`). An action of another stack gives "unknown action" with the actions of this stack. When the server could not check where an action applies, `stack action list` ends with "Some actions are not shown: the server could not check where they apply. Try again.", and `-o json` has `"scope_incomplete": true`. `stack action run`: @@ -669,6 +681,7 @@ These groups have no section above. Run `stackctl --help` for the flags. | `definition link-template` | v0.11.0 or later | | `--stop-at`, `stack extend --until`, the TTL limit of `server info` and of the local check (an older server ignores `--stop-at` and `--until`: stackctl shows the result and exits with code 1; it uses 30 days as the limit) | v0.12.0 or later | | Actions limited to some stacks (`applies_to`) and the `scope_incomplete` hint of `stack action list` (an older server lists every action for every stack) | the release after v0.14.0 (omattsson/k8s-stack-manager#720) | +| `stack port-forward` (an older server answers 404 without an error code: stackctl says "upgrade k8s-stack-manager") | the release after v0.14.0 (omattsson/k8s-stack-manager#722) | ## Output Formats diff --git a/cli/cmd/addrinuse_other.go b/cli/cmd/addrinuse_other.go new file mode 100644 index 0000000..51eec82 --- /dev/null +++ b/cli/cmd/addrinuse_other.go @@ -0,0 +1,7 @@ +//go:build !windows + +package cmd + +func isAddrInUseOS(error) bool { + return false +} diff --git a/cli/cmd/addrinuse_windows.go b/cli/cmd/addrinuse_windows.go new file mode 100644 index 0000000..e4fc115 --- /dev/null +++ b/cli/cmd/addrinuse_windows.go @@ -0,0 +1,15 @@ +//go:build windows + +package cmd + +import ( + "errors" + + "golang.org/x/sys/windows" +) + +// isAddrInUseOS reports WSAEADDRINUSE, which syscall.EADDRINUSE does not +// match on Windows. +func isAddrInUseOS(err error) bool { + return errors.Is(err, windows.WSAEADDRINUSE) +} diff --git a/cli/cmd/agent_guide.md b/cli/cmd/agent_guide.md index a171803..9a21fe1 100644 --- a/cli/cmd/agent_guide.md +++ b/cli/cmd/agent_guide.md @@ -67,6 +67,9 @@ stackctl stack values my-app # merged Helm values stackctl stack preview my-app # values that the next deploy uses stackctl stack history my-app +# Reach a service of the stack: runs until interrupted, so start it in the background +stackctl stack port-forward my-app mysql 0:3306 -q # stdout: 127.0.0.1:; exit 1 on 403/404/409 + # Change values, then deploy again stackctl override set my-app my-chart --set image.tag=1.2.3 stackctl override branch set my-app my-chart feature/x diff --git a/cli/cmd/stack_portforward.go b/cli/cmd/stack_portforward.go new file mode 100644 index 0000000..a2c9008 --- /dev/null +++ b/cli/cmd/stack_portforward.go @@ -0,0 +1,450 @@ +package cmd + +import ( + "context" + "errors" + "fmt" + "io" + "net" + "net/http" + "os" + "os/signal" + "strconv" + "strings" + "sync" + "sync/atomic" + "syscall" + "time" + + "github.com/omattsson/stackctl/cli/pkg/client" + "github.com/spf13/cobra" +) + +const ( + // portForwardMinLocal is the lowest explicit LOCAL port (no privileged + // ports). + portForwardMinLocal = 1025 + // portForwardTries is how many ports after the default LOCAL port the + // command tries before it asks the operating system for a free port. + portForwardTries = 20 + // portForwardShutdownWait bounds the wait for open tunnels at the end. + portForwardShutdownWait = 3 * time.Second +) + +var stackPortForwardCmd = &cobra.Command{ + Use: "port-forward [LOCAL:]REMOTE", + Short: "Forward a local port to a service of a stack", + Long: `Open a local port that forwards to a Kubernetes service of a stack, +through the k8s-stack-manager server (GET +/api/v1/stack-instances/:id/port-forward). You need no kubeconfig and no +cluster access: the server checks your access to the stack. + +REMOTE is the port of the service (the service port, not the container +port). LOCAL is the local port: + - LOCAL must be 0 or from 1025 to 65535. 0 selects any free port. + - Without LOCAL, the local port is REMOTE when REMOTE is above 1024, else + REMOTE + 10000 (for example 80 becomes 10080). When that port is in + use, the command tries the next 20 ports, then any free port. + - An explicit LOCAL port that is in use stops the command. + +Each local connection opens its own tunnel, so one failed connection does +not stop the others. stdout shows the address, for example +"Forwarding from 127.0.0.1:3306 -> mysql:3306"; -q prints only +"host:port". stderr shows one line when a connection opens and one when it +closes (bytes, duration, close reason). -o is ignored. Ctrl-C closes all +connections and exits 0. + +Errors that stop the command (exit 1): no access (401 after a session +renewal, 403), an unknown stack, service or port, the feature or the +cluster switch turned off (404, 403), a stack that is not deployed (409), +and a revoked session or permission during a tunnel (close 1008). API keys +are not allowed by default: run 'stackctl login' and use a login session. +Errors that close only one connection: the tunnel limit (429), no ready pod +(503), a failed pod dial (502), the server time limit (4001), the idle +timeout (4002), a pod stream error (4502) and network errors. + +Security: + - Anyone who can reach the local port uses your access to the stack. + On a shared machine, other local users can connect to it. + - A local port that forwards to an HTTP service can be reached by web + pages in your browser through DNS rebinding. Stop the command when you + do not need it. + - --address with an address that is not loopback (for example 0.0.0.0) + opens the port to the network. The command writes a warning. + - --insecure is refused: without TLS verification, a man in the middle + can read the tunnel data (for example database passwords). Add + --allow-insecure-stream to accept that risk. + +Examples: + # Forward a database, then connect with the client + stackctl stack port-forward my-stack mysql 3306 + mysql -h 127.0.0.1 -P 3306 -u root -p + + # A web service on port 80 (local port 10080) + stackctl stack port-forward my-stack frontend 80 + + # Any free local port; a script reads host:port from stdout + stackctl stack port-forward my-stack mysql 0:3306 -q`, + Args: cobra.ExactArgs(3), + RunE: func(cmd *cobra.Command, args []string) error { + parent := cmd.Context() + if parent == nil { + parent = context.Background() + } + ctx, stop := signal.NotifyContext(parent, os.Interrupt) + defer stop() + return runPortForward(ctx, cmd, args) + }, +} + +func init() { + stackPortForwardCmd.Flags().String("address", "127.0.0.1", "Local addresses to listen on (comma-separated IP addresses or localhost)") + stackPortForwardCmd.Flags().Bool("allow-insecure-stream", false, "Allow port-forward with --insecure (the tunnel data is not protected against a man in the middle)") + stackCmd.AddCommand(stackPortForwardCmd) +} + +// portSpec is the parsed [LOCAL:]REMOTE argument. +type portSpec struct { + local int + remote int + // explicit is true when LOCAL was given (also 0). + explicit bool +} + +// parsePortSpec parses [LOCAL:]REMOTE (see the help of stack port-forward). +func parsePortSpec(s string) (portSpec, error) { + localStr, remoteStr, hasLocal := strings.Cut(strings.TrimSpace(s), ":") + if !hasLocal { + remoteStr, localStr = localStr, "" + } + remote, err := strconv.Atoi(remoteStr) + if err != nil || remote < 1 || remote > 65535 { + return portSpec{}, fmt.Errorf("invalid port %q: REMOTE must be a number from 1 to 65535", s) + } + if !hasLocal { + local := remote + if remote < portForwardMinLocal { + local = remote + 10000 + } + return portSpec{local: local, remote: remote}, nil + } + local, err := strconv.Atoi(localStr) + if err != nil || (local != 0 && (local < portForwardMinLocal || local > 65535)) { + return portSpec{}, fmt.Errorf("invalid port %q: LOCAL must be 0 (any free port) or a number from %d to 65535", s, portForwardMinLocal) + } + return portSpec{local: local, remote: remote, explicit: true}, nil +} + +// parseAddresses splits the --address value. It accepts IP addresses and +// "localhost". +func parseAddresses(raw string) ([]string, error) { + var addrs []string + for _, a := range strings.Split(raw, ",") { + a = strings.TrimSpace(a) + if a == "" { + continue + } + if a != "localhost" && net.ParseIP(a) == nil { + return nil, fmt.Errorf("invalid --address %q: use an IP address or localhost", a) + } + addrs = append(addrs, a) + } + if len(addrs) == 0 { + return nil, errors.New("--address must name at least one address") + } + return addrs, nil +} + +func isLoopback(addr string) bool { + if addr == "localhost" { + return true + } + ip := net.ParseIP(addr) + return ip != nil && ip.IsLoopback() +} + +// isAddrInUse reports a bind error because the port is in use. +func isAddrInUse(err error) bool { + return errors.Is(err, syscall.EADDRINUSE) || isAddrInUseOS(err) +} + +// bindError is a failed bind of one address. +type bindError struct { + addr string + err error +} + +func (e *bindError) Error() string { return "listening on " + e.addr + ": " + e.err.Error() } +func (e *bindError) Unwrap() error { return e.err } + +// listenAll binds every address to the same port. It closes what it opened +// when one bind fails. +func listenAll(addrs []string, port int) ([]net.Listener, error) { + listeners := make([]net.Listener, 0, len(addrs)) + for _, a := range addrs { + hostPort := net.JoinHostPort(a, strconv.Itoa(port)) + l, err := net.Listen("tcp", hostPort) + if err != nil { + closeListeners(listeners) + return nil, &bindError{addr: hostPort, err: err} + } + listeners = append(listeners, l) + if port == 0 { + // The other addresses take the port of the first one. + port = l.Addr().(*net.TCPAddr).Port + } + } + return listeners, nil +} + +func closeListeners(listeners []net.Listener) { + for _, l := range listeners { + _ = l.Close() + } +} + +// bindPortForward opens the local listeners (see the help for the port +// rules). It binds directly: a check before the bind can be wrong by the +// time of the bind. +func bindPortForward(addrs []string, spec portSpec) ([]net.Listener, error) { + if spec.explicit && spec.local != 0 { + ls, err := listenAll(addrs, spec.local) + var be *bindError + if errors.As(err, &be) && isAddrInUse(err) { + return nil, fmt.Errorf("%s is in use; choose another LOCAL port, or leave it out to let stackctl choose one", be.addr) + } + return ls, err + } + if !spec.explicit { + for p := spec.local; p <= spec.local+portForwardTries && p <= 65535; p++ { + ls, err := listenAll(addrs, p) + if err == nil { + return ls, nil + } + if !isAddrInUse(err) { + return nil, err + } + } + } + // Port 0: with several addresses, the port of the first one can be taken + // on another address, so try a few times. + var err error + for i := 0; i < 5; i++ { + var ls []net.Listener + if ls, err = listenAll(addrs, 0); err == nil { + return ls, nil + } + if !isAddrInUse(err) { + return nil, err + } + } + return nil, err +} + +// syncWriter serializes writes from the connection goroutines. +type syncWriter struct { + mu sync.Mutex + w io.Writer +} + +func (s *syncWriter) Write(p []byte) (int, error) { + s.mu.Lock() + defer s.mu.Unlock() + return s.w.Write(p) +} + +// streamInsecure reports whether TLS verification is off (flag or context). +func streamInsecure() bool { + if flagInsecure { + return true + } + ctx := cfg.CurrentCtx() + return ctx != nil && ctx.Insecure +} + +// portForwardFatal reports whether err stops the command: every later +// connection would fail the same way. +func portForwardFatal(err error) bool { + var hs *client.WSHandshakeError + if errors.As(err, &hs) { + return hs.Fatal() + } + var sc *client.StreamClosedError + if errors.As(err, &sc) { + return sc.Fatal() + } + // A refused session renewal: the server ended the session. + var apiErr *client.APIError + return errors.As(err, &apiErr) && + (apiErr.StatusCode == http.StatusUnauthorized || apiErr.StatusCode == http.StatusForbidden) +} + +// portForwardTunnelOptions are the tunnel timings (tests shorten them). +var portForwardTunnelOptions client.TunnelOptions + +func runPortForward(ctx context.Context, cmd *cobra.Command, args []string) error { + spec, err := parsePortSpec(args[2]) + if err != nil { + return err + } + service := strings.TrimSpace(args[1]) + if service == "" { + return errors.New("service name must not be empty") + } + rawAddr, _ := cmd.Flags().GetString("address") + addrs, err := parseAddresses(rawAddr) + if err != nil { + return err + } + stderr := &syncWriter{w: cmd.ErrOrStderr()} + + if streamInsecure() { + allow, _ := cmd.Flags().GetBool("allow-insecure-stream") + if !allow { + return errors.New("port-forward refuses --insecure: without TLS verification a man in the middle can read the tunnel data; add --allow-insecure-stream to accept that risk") + } + fmt.Fprintln(stderr, "Warning: --allow-insecure-stream: the tunnel data is not protected against a man in the middle") + } + for _, a := range addrs { + if !isLoopback(a) { + fmt.Fprintf(stderr, "Warning: %s is not a loopback address: anyone who can reach it uses your access to the stack\n", a) + } + } + + c, err := newClient() + if err != nil { + return err + } + c.WarnWriter = stderr + id, err := resolveStackID(c, args[0]) + if err != nil { + return err + } + + listeners, err := bindPortForward(addrs, spec) + if err != nil { + return err + } + for _, l := range listeners { + hostPort := l.Addr().String() + if printer.Quiet { + fmt.Fprintln(printer.Writer, hostPort) + } else { + fmt.Fprintf(printer.Writer, "Forwarding from %s -> %s:%d\n", hostPort, service, spec.remote) + } + } + + pf := &portForwarder{ + client: c, id: id, service: service, port: spec.remote, + stderr: stderr, fatal: make(chan error, 1), + } + return pf.serve(ctx, listeners) +} + +// portForwarder runs the accept loops and the tunnels of one command. +type portForwarder struct { + client *client.Client + id string + service string + port int + stderr io.Writer + fatal chan error + conns atomic.Int64 +} + +// serve accepts connections until ctx ends or an error stops the command. +func (pf *portForwarder) serve(ctx context.Context, listeners []net.Listener) error { + tunnelCtx, cancel := context.WithCancel(ctx) + defer cancel() + + var loops, tunnels sync.WaitGroup + for _, l := range listeners { + loops.Add(1) + go func(l net.Listener) { + defer loops.Done() + pf.acceptLoop(tunnelCtx, l, &tunnels) + }(l) + } + + var result error + select { + case <-ctx.Done(): + case result = <-pf.fatal: + } + cancel() + closeListeners(listeners) + loops.Wait() + + done := make(chan struct{}) + go func() { + tunnels.Wait() + close(done) + }() + select { + case <-done: + case <-time.After(portForwardShutdownWait): + fmt.Fprintln(pf.stderr, "Warning: some connections did not close in time") + } + return result +} + +func (pf *portForwarder) acceptLoop(ctx context.Context, l net.Listener, tunnels *sync.WaitGroup) { + for { + conn, err := l.Accept() + if err != nil { + if ctx.Err() != nil || errors.Is(err, net.ErrClosed) { + return + } + // For example too many open files: wait, then accept again. + fmt.Fprintf(pf.stderr, "Warning: accepting a connection on %s: %v\n", l.Addr(), err) + select { + case <-ctx.Done(): + return + case <-time.After(100 * time.Millisecond): + } + continue + } + tunnels.Add(1) + go func() { + defer tunnels.Done() + pf.handle(ctx, conn) + }() + } +} + +// stop ends the command with err (the first error wins). +func (pf *portForwarder) stop(err error) { + select { + case pf.fatal <- err: + default: + } +} + +// handle opens one tunnel for one local connection. +func (pf *portForwarder) handle(ctx context.Context, local net.Conn) { + n := pf.conns.Add(1) + ws, err := pf.client.DialPortForward(ctx, pf.id, pf.service, pf.port) + if err != nil { + _ = local.Close() + if ctx.Err() != nil { + return + } + if portForwardFatal(err) { + pf.stop(err) + return + } + fmt.Fprintf(pf.stderr, "Connection %d from %s failed: %v\n", n, local.RemoteAddr(), err) + return + } + fmt.Fprintf(pf.stderr, "Connection %d from %s opened\n", n, local.RemoteAddr()) + stats, err := pf.client.Tunnel(ctx, ws, local, portForwardTunnelOptions) + reason := stats.CloseReason + if err != nil { + reason = err.Error() + } + fmt.Fprintf(pf.stderr, "Connection %d closed: %d bytes in, %d bytes out, %s (%s)\n", + n, stats.BytesIn, stats.BytesOut, stats.Duration.Round(time.Millisecond), reason) + if err != nil && portForwardFatal(err) { + pf.stop(err) + } +} diff --git a/cli/cmd/stack_portforward_test.go b/cli/cmd/stack_portforward_test.go new file mode 100644 index 0000000..1c87ccd --- /dev/null +++ b/cli/cmd/stack_portforward_test.go @@ -0,0 +1,487 @@ +package cmd + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net" + "net/http" + "net/http/httptest" + "regexp" + "strconv" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/gorilla/websocket" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/goleak" +) + +// Tests in this file are NOT parallelized: they mutate package globals +// (cfg, printer, flag variables) through setupStackTestCmd. + +const pfTestPath = "/api/v1/stack-instances/42/port-forward" + +// lockedBuffer is a bytes.Buffer that the connection goroutines and the +// test can use at the same time. +type lockedBuffer struct { + mu sync.Mutex + b bytes.Buffer +} + +func (l *lockedBuffer) Write(p []byte) (int, error) { + l.mu.Lock() + defer l.mu.Unlock() + return l.b.Write(p) +} + +func (l *lockedBuffer) String() string { + l.mu.Lock() + defer l.mu.Unlock() + return l.b.String() +} + +// pfTestServer counts all requests and the upgrades of the port-forward +// route. refuse, when set, answers the route without an upgrade. +type pfTestServer struct { + *httptest.Server + requests atomic.Int32 + upgrades atomic.Int32 +} + +func newPFTestServer(t *testing.T, refuse func(w http.ResponseWriter) bool) *pfTestServer { + t.Helper() + s := &pfTestServer{} + up := websocket.Upgrader{} + s.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + s.requests.Add(1) + if r.URL.Path != pfTestPath { + http.NotFound(w, r) + return + } + if refuse != nil && refuse(w) { + return + } + ws, err := up.Upgrade(w, r, nil) + if err != nil { + return + } + s.upgrades.Add(1) + defer ws.Close() + for { + mt, data, err := ws.ReadMessage() + if err != nil { + return + } + if err := ws.WriteMessage(mt, data); err != nil { + return + } + } + })) + t.Cleanup(s.Close) + return s +} + +func jsonRefusal(status int, code, msg string) func(http.ResponseWriter) bool { + return func(w http.ResponseWriter) bool { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(map[string]string{"error": msg, "code": code}) + return true + } +} + +// setupPortForwardTest prepares the globals and the flags of the command. +func setupPortForwardTest(t *testing.T, apiURL string) (stdout, stderr *lockedBuffer) { + t.Helper() + setupStackTestCmd(t, apiURL) + stdout, stderr = &lockedBuffer{}, &lockedBuffer{} + printer.Writer = stdout + printer.ErrWriter = stderr + stackPortForwardCmd.SetErr(stderr) + t.Cleanup(func() { + stackPortForwardCmd.SetErr(nil) + _ = stackPortForwardCmd.Flags().Set("address", "127.0.0.1") + _ = stackPortForwardCmd.Flags().Set("allow-insecure-stream", "false") + }) + return stdout, stderr +} + +// startPortForward runs the command in a goroutine. +func startPortForward(ctx context.Context, args ...string) <-chan error { + done := make(chan error, 1) + go func() { + done <- runPortForward(ctx, stackPortForwardCmd, args) + }() + return done +} + +var forwardingLine = regexp.MustCompile(`Forwarding from (\S+) -> `) + +// waitForwarding returns the local address from stdout. +func waitForwarding(t *testing.T, stdout *lockedBuffer, done <-chan error) string { + t.Helper() + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if m := forwardingLine.FindStringSubmatch(stdout.String()); m != nil { + return m[1] + } + select { + case err := <-done: + t.Fatalf("command ended before it listened: %v", err) + case <-time.After(10 * time.Millisecond): + } + } + t.Fatalf("no Forwarding line; stdout: %q", stdout.String()) + return "" +} + +func waitDone(t *testing.T, done <-chan error) error { + t.Helper() + select { + case err := <-done: + return err + case <-time.After(10 * time.Second): + t.Fatal("command did not end") + return nil + } +} + +func portOf(t *testing.T, hostPort string) int { + t.Helper() + _, p, err := net.SplitHostPort(hostPort) + require.NoError(t, err) + n, err := strconv.Atoi(p) + require.NoError(t, err) + return n +} + +// holdPorts listens on n consecutive free ports of 127.0.0.1 above 1024 and +// returns the first one. +func holdPorts(t *testing.T, n int) int { + t.Helper() + for attempt := 0; attempt < 50; attempt++ { + probe, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + base := probe.Addr().(*net.TCPAddr).Port + _ = probe.Close() + if base < portForwardMinLocal || base+n+portForwardTries > 65535 { + continue + } + var held []net.Listener + ok := true + for p := base; p < base+n; p++ { + l, err := net.Listen("tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(p))) + if err != nil { + ok = false + break + } + held = append(held, l) + } + if ok { + t.Cleanup(func() { closeListeners(held) }) + return base + } + closeListeners(held) + } + t.Fatal("no free port range") + return 0 +} + +func TestParsePortSpec(t *testing.T) { + tests := []struct { + in string + want portSpec + wantErr string + explicit bool + }{ + {in: "3306", want: portSpec{local: 3306, remote: 3306}}, + {in: "80", want: portSpec{local: 10080, remote: 80}}, + {in: "1024", want: portSpec{local: 11024, remote: 1024}}, + {in: "1025", want: portSpec{local: 1025, remote: 1025}}, + {in: "0:3306", want: portSpec{local: 0, remote: 3306, explicit: true}}, + {in: "1025:80", want: portSpec{local: 1025, remote: 80, explicit: true}}, + {in: "65535:1", want: portSpec{local: 65535, remote: 1, explicit: true}}, + {in: "1024:3306", wantErr: "LOCAL must be 0"}, + {in: "65536:3306", wantErr: "LOCAL must be 0"}, + {in: "-1:3306", wantErr: "LOCAL must be 0"}, + {in: ":3306", wantErr: "LOCAL must be 0"}, + {in: "0", wantErr: "REMOTE must be"}, + {in: "65536", wantErr: "REMOTE must be"}, + {in: "3306:", wantErr: "REMOTE must be"}, + {in: "abc", wantErr: "REMOTE must be"}, + {in: "1:2:3", wantErr: "REMOTE must be"}, + } + for _, tt := range tests { + t.Run(tt.in, func(t *testing.T) { + got, err := parsePortSpec(tt.in) + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestPortForwardCmd_TwoParallelConnections(t *testing.T) { + srv := newPFTestServer(t, nil) + stdout, stderr := setupPortForwardTest(t, srv.URL) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done := startPortForward(ctx, "42", "mysql", "0:3306") + addr := waitForwarding(t, stdout, done) + assert.NotEqual(t, 0, portOf(t, addr)) + assert.Contains(t, stdout.String(), "-> mysql:3306") + + var wg sync.WaitGroup + errs := make(chan error, 2) + for i := 0; i < 2; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + conn, err := net.Dial("tcp", addr) + if err != nil { + errs <- err + return + } + defer conn.Close() + msg := []byte(fmt.Sprintf("connection-%d-payload", i)) + if _, err := conn.Write(msg); err != nil { + errs <- err + return + } + got := make([]byte, len(msg)) + _ = conn.SetReadDeadline(time.Now().Add(5 * time.Second)) + if _, err := io.ReadFull(conn, got); err != nil { + errs <- err + return + } + if !bytes.Equal(msg, got) { + errs <- fmt.Errorf("connection %d got %q", i, got) + } + }(i) + } + wg.Wait() + close(errs) + for err := range errs { + require.NoError(t, err) + } + assert.Equal(t, int32(2), srv.upgrades.Load(), "one WebSocket per local connection") + + cancel() + require.NoError(t, waitDone(t, done)) + assert.Contains(t, stderr.String(), "Connection 1 from") + assert.Contains(t, stderr.String(), "Connection 2 from") + assert.Contains(t, stderr.String(), "bytes in") +} + +func TestPortForwardCmd_QuietPrintsHostPort(t *testing.T) { + srv := newPFTestServer(t, nil) + stdout, _ := setupPortForwardTest(t, srv.URL) + printer.Quiet = true + ctx, cancel := context.WithCancel(context.Background()) + done := startPortForward(ctx, "42", "mysql", "0:3306") + deadline := time.Now().Add(5 * time.Second) + for stdout.String() == "" && time.Now().Before(deadline) { + time.Sleep(10 * time.Millisecond) + } + cancel() + require.NoError(t, waitDone(t, done)) + assert.Regexp(t, `^127\.0\.0\.1:\d+\n$`, stdout.String()) +} + +func TestPortForwardCmd_ForbiddenStopsCommand(t *testing.T) { + tests := []struct { + name string + status int + code string + apiKey string + wantMsg string + }{ + {"forbidden", http.StatusForbidden, "forbidden", "", "HTTP 403"}, + {"api key not allowed", http.StatusForbidden, "api_key_not_allowed", "sk_test", "API keys are not allowed for this command"}, + {"feature disabled", http.StatusNotFound, "feature_disabled", "", "turned off"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv := newPFTestServer(t, jsonRefusal(tt.status, tt.code, "refused")) + stdout, _ := setupPortForwardTest(t, srv.URL) + flagAPIKey = tt.apiKey + done := startPortForward(context.Background(), "42", "mysql", "0:3306") + addr := waitForwarding(t, stdout, done) + + conn, err := net.Dial("tcp", addr) + require.NoError(t, err) + defer conn.Close() + err = waitDone(t, done) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantMsg) + + _, err = net.DialTimeout("tcp", addr, time.Second) + assert.Error(t, err, "the listener is closed") + }) + } +} + +func TestPortForwardCmd_PerConnectionErrorKeepsListening(t *testing.T) { + var first atomic.Bool + first.Store(true) + srv := newPFTestServer(t, func(w http.ResponseWriter) bool { + if first.Swap(false) { + return jsonRefusal(http.StatusServiceUnavailable, "no_ready_pod", "No ready pod")(w) + } + return false + }) + stdout, stderr := setupPortForwardTest(t, srv.URL) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done := startPortForward(ctx, "42", "mysql", "0:3306") + addr := waitForwarding(t, stdout, done) + + c1, err := net.Dial("tcp", addr) + require.NoError(t, err) + _ = c1.SetReadDeadline(time.Now().Add(5 * time.Second)) + _, err = c1.Read(make([]byte, 1)) + assert.Error(t, err, "the refused connection is closed") + _ = c1.Close() + + c2, err := net.Dial("tcp", addr) + require.NoError(t, err) + defer c2.Close() + _, err = c2.Write([]byte("ok")) + require.NoError(t, err) + got := make([]byte, 2) + _ = c2.SetReadDeadline(time.Now().Add(5 * time.Second)) + _, err = io.ReadFull(c2, got) + require.NoError(t, err) + assert.Equal(t, "ok", string(got)) + + cancel() + require.NoError(t, waitDone(t, done)) + assert.Contains(t, stderr.String(), "Connection 1 from") + assert.Contains(t, stderr.String(), "HTTP 503") +} + +func TestPortForwardCmd_CancelReturnsNilWithoutLeaks(t *testing.T) { + srv := newPFTestServer(t, nil) + stdout, stderr := setupPortForwardTest(t, srv.URL) + ignore := goleak.IgnoreCurrent() + + ctx, cancel := context.WithCancel(context.Background()) + done := startPortForward(ctx, "42", "mysql", "0:3306") + addr := waitForwarding(t, stdout, done) + conn, err := net.Dial("tcp", addr) + require.NoError(t, err) + defer conn.Close() + _, err = conn.Write([]byte("x")) + require.NoError(t, err) + _, err = io.ReadFull(conn, make([]byte, 1)) + require.NoError(t, err) + + cancel() + require.NoError(t, waitDone(t, done)) + assert.Contains(t, stderr.String(), "(interrupted)") + _ = conn.Close() + srv.Close() + goleak.VerifyNone(t, ignore) +} + +func TestPortForwardCmd_NonLoopbackWarning(t *testing.T) { + srv := newPFTestServer(t, nil) + _, stderr := setupPortForwardTest(t, srv.URL) + // 192.0.2.1 (TEST-NET-1) is not a local address: the bind fails after + // the warning, so no port opens to the network. + require.NoError(t, stackPortForwardCmd.Flags().Set("address", "127.0.0.1,192.0.2.1")) + err := waitDone(t, startPortForward(context.Background(), "42", "mysql", "0:3306")) + require.Error(t, err) + assert.Contains(t, stderr.String(), "Warning: 192.0.2.1 is not a loopback address") + assert.NotContains(t, stderr.String(), "Warning: 127.0.0.1") + assert.Equal(t, int32(0), srv.upgrades.Load()) +} + +func TestPortForwardCmd_DefaultPortInUseTakesNext(t *testing.T) { + srv := newPFTestServer(t, nil) + stdout, _ := setupPortForwardTest(t, srv.URL) + held := holdPorts(t, 1) + ctx, cancel := context.WithCancel(context.Background()) + done := startPortForward(ctx, "42", "mysql", strconv.Itoa(held)) + got := portOf(t, waitForwarding(t, stdout, done)) + cancel() + require.NoError(t, waitDone(t, done)) + assert.Greater(t, got, held) + assert.LessOrEqual(t, got, held+portForwardTries) + assert.Contains(t, stdout.String(), fmt.Sprintf("-> mysql:%d", held)) +} + +func TestPortForwardCmd_AllTriedPortsInUseFallsBack(t *testing.T) { + srv := newPFTestServer(t, nil) + stdout, _ := setupPortForwardTest(t, srv.URL) + held := holdPorts(t, portForwardTries+1) + ctx, cancel := context.WithCancel(context.Background()) + done := startPortForward(ctx, "42", "mysql", strconv.Itoa(held)) + got := portOf(t, waitForwarding(t, stdout, done)) + cancel() + require.NoError(t, waitDone(t, done)) + assert.False(t, got >= held && got <= held+portForwardTries, "port %d is in the held range", got) +} + +func TestPortForwardCmd_ExplicitPortInUse(t *testing.T) { + srv := newPFTestServer(t, nil) + _, _ = setupPortForwardTest(t, srv.URL) + held := holdPorts(t, 1) + err := waitDone(t, startPortForward(context.Background(), "42", "mysql", fmt.Sprintf("%d:3306", held))) + require.Error(t, err) + assert.Equal(t, fmt.Sprintf("127.0.0.1:%d is in use; choose another LOCAL port, or leave it out to let stackctl choose one", held), err.Error()) + assert.Equal(t, int32(0), srv.upgrades.Load(), "no WebSocket") +} + +func TestPortForwardCmd_InsecureNeedsAllowFlag(t *testing.T) { + srv := newPFTestServer(t, nil) + stdout, stderr := setupPortForwardTest(t, srv.URL) + flagInsecure = true + + err := waitDone(t, startPortForward(context.Background(), "my-stack", "mysql", "0:3306")) + require.Error(t, err) + assert.Contains(t, err.Error(), "--allow-insecure-stream") + assert.Equal(t, int32(0), srv.requests.Load(), "refused before any request") + assert.Empty(t, stdout.String()) + + require.NoError(t, stackPortForwardCmd.Flags().Set("allow-insecure-stream", "true")) + ctx, cancel := context.WithCancel(context.Background()) + done := startPortForward(ctx, "42", "mysql", "0:3306") + waitForwarding(t, stdout, done) + cancel() + require.NoError(t, waitDone(t, done)) + assert.Contains(t, stderr.String(), "Warning: --allow-insecure-stream") +} + +func TestPortForwardCmd_InvalidArgs(t *testing.T) { + srv := newPFTestServer(t, nil) + _, _ = setupPortForwardTest(t, srv.URL) + err := waitDone(t, startPortForward(context.Background(), "42", "mysql", "80:3306")) + require.Error(t, err) + assert.Contains(t, err.Error(), "LOCAL must be 0") + + require.NoError(t, stackPortForwardCmd.Flags().Set("address", "not-an-ip")) + err = waitDone(t, startPortForward(context.Background(), "42", "mysql", "3306")) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid --address") + assert.Equal(t, int32(0), srv.requests.Load()) +} + +func TestPortForwardCmd_Registered(t *testing.T) { + found, _, err := rootCmd.Find([]string{"stack", "port-forward"}) + require.NoError(t, err) + assert.Equal(t, stackPortForwardCmd, found) + assert.True(t, strings.Contains(found.Long, "mysql -h 127.0.0.1 -P 3306")) + assert.Contains(t, found.Long, "DNS rebinding") +} diff --git a/cli/pkg/client/portforward.go b/cli/pkg/client/portforward.go new file mode 100644 index 0000000..469116c --- /dev/null +++ b/cli/pkg/client/portforward.go @@ -0,0 +1,277 @@ +package client + +import ( + "context" + "errors" + "fmt" + "io" + "net" + "net/url" + "strconv" + "sync" + "sync/atomic" + "time" + + "github.com/gorilla/websocket" +) + +const ( + // maxTunnelFrame is the largest data frame that a tunnel sends; the + // server read limit is 64 KiB. + maxTunnelFrame = 32 << 10 + // tunnelReadLimit bounds a server frame (the server sends at most 32 KiB). + tunnelReadLimit = 1 << 20 + // DefaultDrainTimeout is how long a tunnel reads after the local client + // closed its side. + DefaultDrainTimeout = 2 * time.Second + // interruptCloseTimeout bounds the close frame after a cancel. + interruptCloseTimeout = time.Second +) + +// Close reasons in TunnelStats.CloseReason for a normal close. +const ( + TunnelReasonLocalClosed = "local connection closed" + TunnelReasonServerClosed = "closed by the server" + TunnelReasonInterrupted = "interrupted" +) + +// ErrTunnelTimeout reports that the server sent no data, ping or pong within +// the read timeout. +var ErrTunnelTimeout = errors.New("tunnel closed: no response from the server") + +// TunnelOptions changes the timings of Tunnel. A zero field takes the +// default. Tests inject short values. +type TunnelOptions struct { + Keepalive KeepaliveTimings + // DrainTimeout is how long the tunnel still reads server data after the + // local client closed its side (default 2 s). + DrainTimeout time.Duration +} + +func (o TunnelOptions) withDefaults() TunnelOptions { + o.Keepalive = o.Keepalive.withDefaults() + if o.DrainTimeout <= 0 { + o.DrainTimeout = DefaultDrainTimeout + } + return o +} + +// TunnelStats describes a finished tunnel. +type TunnelStats struct { + // BytesIn counts the bytes from the server to the local connection. + BytesIn int64 + // BytesOut counts the bytes from the local connection to the server. + BytesOut int64 + Duration time.Duration + // CloseCode is the close code of the server, or CloseNormal for a close + // that the client started; 0 when the connection was lost. + CloseCode int + // CloseReason is a short text for the user, for example "server time + // limit reached" or TunnelReasonLocalClosed. + CloseReason string +} + +// DialPortForward opens the port-forward WebSocket of a stack instance +// (GET /api/v1/stack-instances/:id/port-forward?service=&port=). The route +// accepts only header auth; a refused upgrade gives a *WSHandshakeError with +// the error code of the server. +func (c *Client) DialPortForward(ctx context.Context, instanceID, service string, port int) (*websocket.Conn, error) { + q := url.Values{} + q.Set("service", service) + q.Set("port", strconv.Itoa(port)) + path := fmt.Sprintf("/api/v1/stack-instances/%s/port-forward", url.PathEscape(instanceID)) + "?" + q.Encode() + ws, err := c.dialWSOpts(ctx, path, c.WarnWriter, wsDialOptions{stream: true, op: "port-forward"}) + if err != nil { + return nil, err + } + ws.SetReadLimit(tunnelReadLimit) + return ws, nil +} + +// localWriteError is a failed write to the local connection. +type localWriteError struct{ err error } + +func (e *localWriteError) Error() string { return "writing to the local connection: " + e.err.Error() } +func (e *localWriteError) Unwrap() error { return e.err } + +// localPumpResult is the end of the local-to-server copy. +type localPumpResult struct { + // wsErr is set when the write to the server failed. + wsErr error +} + +// Tunnel copies bytes between ws (a port-forward WebSocket) and local until +// one side closes, ctx ends or the server stops answering. It closes ws and +// local before it returns and leaves no goroutine behind. +// +// A server close 1000, a local close and a cancelled ctx give a nil error. A +// server close with another code gives a *StreamClosedError (Fatal for 1008); +// a silent server gives ErrTunnelTimeout. +func (c *Client) Tunnel(ctx context.Context, ws *websocket.Conn, local net.Conn, opts TunnelOptions) (TunnelStats, error) { + opts = opts.withDefaults() + start := time.Now() + var in, out atomic.Int64 + stats := func(code int, reason string) TunnelStats { + return TunnelStats{ + BytesIn: in.Load(), BytesOut: out.Load(), Duration: time.Since(start), + CloseCode: code, CloseReason: reason, + } + } + + extend := setKeepalive(ws, opts.Keepalive) + readDone := make(chan error, 1) + writeDone := make(chan localPumpResult, 1) + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + readDone <- c.serverToLocal(ws, local, extend, &in) + }() + go func() { + defer wg.Done() + writeDone <- localToServer(ws, local, opts.Keepalive.ReadTimeout, &out) + }() + teardown := func() { + _ = ws.Close() + _ = local.Close() + wg.Wait() + } + + ticker := time.NewTicker(opts.Keepalive.PingInterval) + defer ticker.Stop() + var drain <-chan time.Time + var wsWriteErr error + localDone := false + for { + select { + case <-ctx.Done(): + _ = sendClose(ws, CloseNormal, interruptCloseTimeout) + teardown() + return stats(CloseNormal, TunnelReasonInterrupted), nil + + case <-ticker.C: + _ = sendPing(ws, opts.Keepalive) + + case r := <-writeDone: + writeDone = nil + if r.wsErr != nil { + // The reader usually holds the close frame that explains it. + wsWriteErr = r.wsErr + drain = time.After(time.Second) + continue + } + localDone = true + _ = sendClose(ws, CloseNormal, opts.Keepalive.WriteTimeout) + drain = time.After(opts.DrainTimeout) + + case <-drain: + teardown() + if wsWriteErr != nil { + return stats(0, "connection lost"), fmt.Errorf("tunnel connection lost: %w", wsWriteErr) + } + return stats(CloseNormal, TunnelReasonLocalClosed), nil + + case err := <-readDone: + code, reason, tErr := c.tunnelEnd(ctx, err, localDone, opts) + switch { + case reason == TunnelReasonServerClosed: + // Pass the EOF of the server to the local client. + if cw, ok := local.(interface{ CloseWrite() error }); ok { + _ = cw.CloseWrite() + } + case reason == TunnelReasonLocalClosed && !localDone: + _ = sendClose(ws, CloseNormal, opts.Keepalive.WriteTimeout) + } + teardown() + return stats(code, reason), tErr + } + } +} + +// tunnelEnd classifies the end of the server-to-local copy. +func (c *Client) tunnelEnd(ctx context.Context, err error, localDone bool, opts TunnelOptions) (code int, reason string, tErr error) { + var ce *websocket.CloseError + var lw *localWriteError + var ne net.Error + // gorilla reports a lost connection as close code 1006; no frame came. + closeFrame := errors.As(err, &ce) && ce.Code != websocket.CloseAbnormalClosure + switch { + case ctx.Err() != nil: + return CloseNormal, TunnelReasonInterrupted, nil + case closeFrame: + if ce.Code == CloseNormal || ce.Code == closeNoStatus { + if localDone { + return CloseNormal, TunnelReasonLocalClosed, nil + } + return CloseNormal, TunnelReasonServerClosed, nil + } + reason := sanitizeServerMessage(ce.Text) + return ce.Code, CloseCodeText(ce.Code, reason, c.APIKey != ""), + &StreamClosedError{Subject: "tunnel", Code: ce.Code, Reason: reason, APIKey: c.APIKey != ""} + case errors.As(err, &lw): + return CloseNormal, TunnelReasonLocalClosed, nil + case localDone: + // The server dropped the connection after our close frame. + return CloseNormal, TunnelReasonLocalClosed, nil + case errors.As(err, &ne) && ne.Timeout(): + return 0, "no response from the server", fmt.Errorf("%w for %s", ErrTunnelTimeout, opts.Keepalive.ReadTimeout) + default: + return 0, "connection lost", fmt.Errorf("tunnel connection lost: %w", err) + } +} + +// serverToLocal copies the binary frames of ws to local. Each read moves the +// read deadline. Text frames are ignored (their content is never logged). +func (c *Client) serverToLocal(ws *websocket.Conn, local net.Conn, extend func(), n *atomic.Int64) error { + buf := make([]byte, maxTunnelFrame) + for { + extend() + mt, r, err := ws.NextReader() + if err != nil { + return err + } + if mt != websocket.BinaryMessage { + c.debugf("port-forward: ignored a text frame from the server\n") + if _, err := io.Copy(io.Discard, r); err != nil { + return err + } + continue + } + for { + k, rerr := r.Read(buf) + if k > 0 { + extend() + if _, werr := local.Write(buf[:k]); werr != nil { + return &localWriteError{err: werr} + } + n.Add(int64(k)) + } + if errors.Is(rerr, io.EOF) { + break + } + if rerr != nil { + return rerr + } + } + } +} + +// localToServer copies local to ws in binary frames of at most 32 KiB. It is +// the only data writer of ws. It returns at the local EOF or read error, or +// at a failed write to the server. +func localToServer(ws *websocket.Conn, local net.Conn, writeTimeout time.Duration, n *atomic.Int64) localPumpResult { + buf := make([]byte, maxTunnelFrame) + for { + k, err := local.Read(buf) + if k > 0 { + _ = ws.SetWriteDeadline(time.Now().Add(writeTimeout)) + if werr := ws.WriteMessage(websocket.BinaryMessage, buf[:k]); werr != nil { + return localPumpResult{wsErr: werr} + } + n.Add(int64(k)) + } + if err != nil { + return localPumpResult{} + } + } +} diff --git a/cli/pkg/client/portforward_test.go b/cli/pkg/client/portforward_test.go new file mode 100644 index 0000000..80203f5 --- /dev/null +++ b/cli/pkg/client/portforward_test.go @@ -0,0 +1,539 @@ +package client + +import ( + "bytes" + "context" + "crypto/rand" + "errors" + "io" + "net" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/gorilla/websocket" + "github.com/omattsson/stackctl/cli/pkg/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const pfPath = "/api/v1/stack-instances/42/port-forward" + +// pfServer serves the port-forward route with handler after the upgrade. +func pfServer(t *testing.T, handler func(r *http.Request, ws *websocket.Conn)) *httptest.Server { + t.Helper() + up := websocket.Upgrader{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != pfPath { + http.NotFound(w, r) + return + } + ws, err := up.Upgrade(w, r, nil) + if err != nil { + return + } + defer ws.Close() + handler(r, ws) + })) + t.Cleanup(srv.Close) + return srv +} + +// echo returns binary frames until the client closes. It records the largest +// frame. +func echo(maxFrame *atomic.Int64) func(*http.Request, *websocket.Conn) { + return func(_ *http.Request, ws *websocket.Conn) { + for { + mt, data, err := ws.ReadMessage() + if err != nil { + return + } + if maxFrame != nil && int64(len(data)) > maxFrame.Load() { + maxFrame.Store(int64(len(data))) + } + if err := ws.WriteMessage(mt, data); err != nil { + return + } + } + } +} + +// tcpPair returns the two ends of a loopback TCP connection. +func tcpPair(t *testing.T) (tunnelSide, clientSide *net.TCPConn) { + t.Helper() + l, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer l.Close() + accepted := make(chan net.Conn, 1) + go func() { + c, _ := l.Accept() + accepted <- c + }() + cs, err := net.Dial("tcp", l.Addr().String()) + require.NoError(t, err) + ts := <-accepted + require.NotNil(t, ts) + t.Cleanup(func() { + _ = cs.Close() + _ = ts.Close() + }) + return ts.(*net.TCPConn), cs.(*net.TCPConn) +} + +type tunnelResult struct { + stats TunnelStats + err error +} + +// startTunnel dials the server and runs Tunnel in a goroutine. +func startTunnel(t *testing.T, ctx context.Context, c *Client, opts TunnelOptions) (*net.TCPConn, <-chan tunnelResult) { + t.Helper() + ws, err := c.DialPortForward(ctx, "42", "mysql", 3306) + require.NoError(t, err) + tunnelSide, clientSide := tcpPair(t) + done := make(chan tunnelResult, 1) + go func() { + st, err := c.Tunnel(ctx, ws, tunnelSide, opts) + done <- tunnelResult{st, err} + }() + return clientSide, done +} + +func waitTunnel(t *testing.T, done <-chan tunnelResult) tunnelResult { + t.Helper() + select { + case r := <-done: + return r + case <-time.After(10 * time.Second): + t.Fatal("tunnel did not end") + return tunnelResult{} + } +} + +func TestPortForward_RoundTrip1MiB(t *testing.T) { + t.Parallel() + var maxFrame atomic.Int64 + srv := pfServer(t, echo(&maxFrame)) + clientSide, done := startTunnel(t, context.Background(), New(srv.URL), TunnelOptions{}) + + payload := make([]byte, 1<<20) + _, err := rand.Read(payload) + require.NoError(t, err) + readErr := make(chan error, 1) + got := make([]byte, len(payload)) + go func() { + _, err := io.ReadFull(clientSide, got) + readErr <- err + }() + _, err = clientSide.Write(payload) + require.NoError(t, err) + require.NoError(t, <-readErr) + assert.True(t, bytes.Equal(payload, got), "echoed bytes differ") + + require.NoError(t, clientSide.CloseWrite()) + r := waitTunnel(t, done) + require.NoError(t, r.err) + assert.Equal(t, int64(1<<20), r.stats.BytesOut) + assert.Equal(t, int64(1<<20), r.stats.BytesIn) + assert.Equal(t, CloseNormal, r.stats.CloseCode) + assert.Equal(t, TunnelReasonLocalClosed, r.stats.CloseReason) + assert.LessOrEqual(t, maxFrame.Load(), int64(32<<10), "frames must stay below the server read limit") +} + +func TestDialPortForward_AuthHeadersAndQuery(t *testing.T) { + t.Parallel() + tests := []struct { + name string + token string + apiKey string + wantHeader string + wantValue string + }{ + {"bearer", "jwt-1", "", "Authorization", "Bearer jwt-1"}, + {"api key", "", "sk_test", "X-API-Key", "sk_test"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + reqs := make(chan *http.Request, 1) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + reqs <- r.Clone(context.Background()) + ws, err := (&websocket.Upgrader{}).Upgrade(w, r, nil) + if err != nil { + return + } + defer ws.Close() + _, _, _ = ws.ReadMessage() + })) + defer srv.Close() + + c := New(srv.URL) + c.Token = tt.token + c.APIKey = tt.apiKey + ws, err := c.DialPortForward(context.Background(), "42", "my svc&x=1", 8080) + require.NoError(t, err) + _ = ws.Close() + + r := <-reqs + assert.Equal(t, pfPath, r.URL.Path) + assert.Equal(t, "my svc&x=1", r.URL.Query().Get("service")) + assert.Equal(t, "8080", r.URL.Query().Get("port")) + assert.Empty(t, r.URL.Query().Get("x"), "service must be escaped") + assert.Empty(t, r.URL.Query().Get("token")) + assert.Equal(t, tt.wantValue, r.Header.Get(tt.wantHeader)) + assert.Empty(t, r.Header.Get("Origin")) + assert.True(t, strings.HasPrefix(r.Header.Get("User-Agent"), "stackctl/")) + }) + } +} + +func TestDialPortForward_NoQueryTokenRetryAfter401(t *testing.T) { + t.Parallel() + var hits, withToken atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + if r.URL.Query().Get("token") != "" { + withToken.Add(1) + } + writeJSON(w, http.StatusUnauthorized, map[string]string{"error": "Invalid or expired token"}) + })) + defer srv.Close() + + c := New(srv.URL) + c.Token = "jwt-1" + _, err := c.DialPortForward(context.Background(), "42", "mysql", 3306) + var hs *WSHandshakeError + require.ErrorAs(t, err, &hs) + assert.Equal(t, http.StatusUnauthorized, hs.StatusCode) + assert.True(t, hs.Fatal()) + assert.Contains(t, err.Error(), "stackctl login") + assert.Equal(t, int32(1), hits.Load(), "no retry") + assert.Equal(t, int32(0), withToken.Load(), "the JWT never goes into the URL") +} + +func TestDialPortForward_RenewsSessionAfter401(t *testing.T) { + t.Parallel() + renewed := makeJWT(t, "alice", time.Now().Add(time.Hour)) + var refreshCalls atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v1/auth/refresh": + refreshCalls.Add(1) + http.SetCookie(w, &http.Cookie{Name: "refresh_token", Value: "r2", Path: "/api/v1/auth"}) + writeJSON(w, http.StatusOK, map[string]string{"token": renewed}) + case pfPath: + if r.Header.Get("Authorization") != "Bearer "+renewed { + writeJSON(w, http.StatusUnauthorized, map[string]string{"error": "Invalid or expired token"}) + return + } + ws, err := (&websocket.Upgrader{}).Upgrade(w, r, nil) + if err != nil { + return + } + defer ws.Close() + _, _, _ = ws.ReadMessage() + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + + old := makeJWT(t, "alice-old", time.Now().Add(time.Hour)) + c, _ := newSessionClient(t, srv.URL, &config.StoredToken{Token: old, RefreshToken: "r1", ExpiresAt: time.Now().Add(time.Hour), APIURL: srv.URL}) + ws, err := c.DialPortForward(context.Background(), "42", "mysql", 3306) + require.NoError(t, err) + _ = ws.Close() + assert.Equal(t, int32(1), refreshCalls.Load()) +} + +func TestDialPortForward_HandshakeErrors(t *testing.T) { + t.Parallel() + tests := []struct { + name string + status int + body string + apiKey bool + check func(t *testing.T, hs *WSHandshakeError) + wantFatal bool + wantMsg []string + }{ + {name: "api key not allowed", status: 403, apiKey: true, + body: `{"error":"API keys cannot open this stream","code":"api_key_not_allowed"}`, + wantFatal: true, wantMsg: []string{"HTTP 403", "API keys are not allowed for this command", "stackctl login"}}, + {name: "forbidden", status: 403, + body: `{"error":"You do not have permission","code":"forbidden"}`, + wantFatal: true, wantMsg: []string{"port-forward refused by the server (HTTP 403): You do not have permission"}}, + {name: "feature disabled", status: 404, + body: `{"error":"This feature is disabled","code":"feature_disabled"}`, + check: func(t *testing.T, hs *WSHandshakeError) { + assert.True(t, hs.FeatureDisabled()) + assert.False(t, hs.NoRoute()) + }, + wantFatal: true, wantMsg: []string{"This feature is disabled", "turned off"}}, + {name: "service not found", status: 404, + body: `{"error":"Service not found","code":"service_not_found"}`, + wantFatal: true, wantMsg: []string{"Service not found", "service port"}}, + {name: "plain 404", status: 404, body: "404 page not found", + check: func(t *testing.T, hs *WSHandshakeError) { assert.True(t, hs.NoRoute()) }, + wantFatal: true, wantMsg: []string{"no port-forward route", "upgrade k8s-stack-manager"}}, + {name: "not deployed", status: 409, + body: `{"error":"Instance is not deployed","code":"instance_not_deployed"}`, + wantFatal: true, wantMsg: []string{"Deploy the stack first"}}, + {name: "tunnel limit", status: 429, + body: `{"error":"Too many tunnels","code":"tunnel_limit"}`, + wantFatal: false, wantMsg: []string{"HTTP 429", "Too many tunnels"}}, + {name: "no ready pod", status: 503, + body: `{"error":"No ready pod","code":"no_ready_pod"}`, + wantFatal: false, wantMsg: []string{"HTTP 503", "No ready pod"}}, + {name: "dial failed", status: 502, + body: `{"error":"Dial to the pod failed","code":"dial_failed"}`, + wantFatal: false, wantMsg: []string{"HTTP 502"}}, + {name: "200 without upgrade", status: 200, body: "ok", + check: func(t *testing.T, hs *WSHandshakeError) { assert.True(t, hs.UpgradeDropped()) }, + wantFatal: true, wantMsg: []string{"HTTP 200", "proxy"}}, + {name: "upgrade required", status: 400, + body: `{"error":"WebSocket upgrade required","code":"upgrade_required"}`, + check: func(t *testing.T, hs *WSHandshakeError) { assert.True(t, hs.UpgradeDropped()) }, + wantFatal: true, wantMsg: []string{"proxy"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(tt.status) + _, _ = io.WriteString(w, tt.body) + })) + defer srv.Close() + c := New(srv.URL) + if tt.apiKey { + c.APIKey = "sk_test" + } + _, err := c.DialPortForward(context.Background(), "42", "mysql", 3306) + var hs *WSHandshakeError + require.ErrorAs(t, err, &hs) + assert.Equal(t, tt.status, hs.StatusCode) + assert.Equal(t, tt.wantFatal, hs.Fatal()) + for _, m := range tt.wantMsg { + assert.Contains(t, err.Error(), m) + } + assert.NotContains(t, err.Error(), "sk_test") + if tt.check != nil { + tt.check(t, hs) + } + }) + } +} + +func TestTunnel_ServerCloseCodes(t *testing.T) { + t.Parallel() + tests := []struct { + name string + code int + reason string + wantMsg string + wantFatal bool + }{ + {"max duration", CloseMaxDuration, "maximum duration reached", "tunnel closed: server time limit reached", false}, + {"idle timeout", CloseIdleTimeout, "idle timeout", "tunnel closed: idle timeout", false}, + {"pod stream error", ClosePodStreamError, "connection refused", "tunnel closed: pod stream error: connection refused", false}, + {"server shutdown", CloseGoingAway, "server shutdown", "tunnel closed: the server is shutting down", false}, + {"revoked", ClosePolicy, "session revoked", "tunnel closed: access revoked (token, session or permissions); run 'stackctl login'", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + srv := pfServer(t, func(_ *http.Request, ws *websocket.Conn) { + mt, data, err := ws.ReadMessage() + if err != nil { + return + } + _ = ws.WriteMessage(mt, data) + _ = ws.WriteControl(websocket.CloseMessage, websocket.FormatCloseMessage(tt.code, tt.reason), time.Now().Add(time.Second)) + _, _, _ = ws.ReadMessage() + }) + clientSide, done := startTunnel(t, context.Background(), New(srv.URL), TunnelOptions{}) + _, err := clientSide.Write([]byte("hello")) + require.NoError(t, err) + got, _ := io.ReadAll(clientSide) + assert.Equal(t, "hello", string(got)) + + r := waitTunnel(t, done) + var sc *StreamClosedError + require.ErrorAs(t, r.err, &sc) + assert.Contains(t, r.err.Error(), tt.wantMsg) + assert.Equal(t, tt.wantFatal, sc.Fatal()) + assert.Equal(t, tt.code, r.stats.CloseCode) + assert.Equal(t, int64(5), r.stats.BytesIn) + assert.Equal(t, int64(5), r.stats.BytesOut) + assert.Positive(t, r.stats.Duration) + var wsClosed *WSClosedError + assert.Equal(t, tt.code == ClosePolicy, errors.As(r.err, &wsClosed), "1008 also gives a WSClosedError") + }) + } +} + +func TestTunnel_ServerNormalCloseEndsLocalStream(t *testing.T) { + t.Parallel() + srv := pfServer(t, func(_ *http.Request, ws *websocket.Conn) { + _ = ws.WriteMessage(websocket.BinaryMessage, []byte("bye")) + _ = ws.WriteControl(websocket.CloseMessage, websocket.FormatCloseMessage(CloseNormal, ""), time.Now().Add(time.Second)) + _, _, _ = ws.ReadMessage() + }) + clientSide, done := startTunnel(t, context.Background(), New(srv.URL), TunnelOptions{}) + got, err := io.ReadAll(clientSide) + require.NoError(t, err, "the local client gets an EOF") + assert.Equal(t, "bye", string(got)) + r := waitTunnel(t, done) + require.NoError(t, r.err) + assert.Equal(t, TunnelReasonServerClosed, r.stats.CloseReason) +} + +func TestTunnel_LocalEOFSendsCloseAndDrains(t *testing.T) { + t.Parallel() + gotClose := make(chan int, 1) + srv := pfServer(t, func(_ *http.Request, ws *websocket.Conn) { + ws.SetCloseHandler(func(code int, _ string) error { + gotClose <- code + return nil + }) + if _, _, err := ws.ReadMessage(); err != nil { + return + } + // The close frame of the client ends this read. + _, _, _ = ws.ReadMessage() + _ = ws.WriteMessage(websocket.BinaryMessage, []byte("late")) + _ = ws.WriteControl(websocket.CloseMessage, websocket.FormatCloseMessage(CloseNormal, ""), time.Now().Add(time.Second)) + }) + clientSide, done := startTunnel(t, context.Background(), New(srv.URL), TunnelOptions{DrainTimeout: 5 * time.Second}) + _, err := clientSide.Write([]byte("query")) + require.NoError(t, err) + time.Sleep(50 * time.Millisecond) + require.NoError(t, clientSide.CloseWrite()) + + got, _ := io.ReadAll(clientSide) + assert.Equal(t, "late", string(got), "data after the local EOF still reaches the local side") + assert.Equal(t, CloseNormal, <-gotClose) + r := waitTunnel(t, done) + require.NoError(t, r.err) + assert.Equal(t, TunnelReasonLocalClosed, r.stats.CloseReason) + assert.Equal(t, int64(4), r.stats.BytesIn) + assert.Equal(t, int64(5), r.stats.BytesOut) +} + +func TestTunnel_DrainTimeout(t *testing.T) { + t.Parallel() + srv := pfServer(t, func(_ *http.Request, ws *websocket.Conn) { + ws.SetCloseHandler(func(int, string) error { return nil }) + for { + if _, _, err := ws.ReadMessage(); err != nil { + break + } + } + // Never answer the close: hold the connection until the client drops it. + _, _ = io.Copy(io.Discard, ws.UnderlyingConn()) + }) + clientSide, done := startTunnel(t, context.Background(), New(srv.URL), TunnelOptions{DrainTimeout: 100 * time.Millisecond}) + require.NoError(t, clientSide.CloseWrite()) + r := waitTunnel(t, done) + require.NoError(t, r.err) + assert.Equal(t, TunnelReasonLocalClosed, r.stats.CloseReason) +} + +func TestTunnel_SilentServerTripsReadDeadline(t *testing.T) { + t.Parallel() + srv := pfServer(t, func(_ *http.Request, ws *websocket.Conn) { + // No pong replies and no data. + ws.SetPingHandler(func(string) error { return nil }) + for { + if _, _, err := ws.ReadMessage(); err != nil { + return + } + } + }) + opts := TunnelOptions{Keepalive: KeepaliveTimings{PingInterval: 50 * time.Millisecond, ReadTimeout: 200 * time.Millisecond}} + _, done := startTunnel(t, context.Background(), New(srv.URL), opts) + r := waitTunnel(t, done) + require.ErrorIs(t, r.err, ErrTunnelTimeout) + assert.Contains(t, r.err.Error(), "no response from the server") +} + +func TestTunnel_ServerPingsKeepAliveUntilCancel(t *testing.T) { + t.Parallel() + gotClose := make(chan int, 1) + srv := pfServer(t, func(_ *http.Request, ws *websocket.Conn) { + ws.SetCloseHandler(func(code int, _ string) error { + gotClose <- code + return nil + }) + stop := make(chan struct{}) + defer close(stop) + go func() { + tk := time.NewTicker(40 * time.Millisecond) + defer tk.Stop() + for { + select { + case <-stop: + return + case <-tk.C: + _ = ws.WriteControl(websocket.PingMessage, nil, time.Now().Add(time.Second)) + } + } + }() + // The client pings too; the default handler answers them. + for { + if _, _, err := ws.ReadMessage(); err != nil { + return + } + } + }) + ctx, cancel := context.WithCancel(context.Background()) + opts := TunnelOptions{Keepalive: KeepaliveTimings{PingInterval: time.Hour, ReadTimeout: 150 * time.Millisecond}} + _, done := startTunnel(t, ctx, New(srv.URL), opts) + select { + case r := <-done: + t.Fatalf("tunnel ended early: %v", r.err) + case <-time.After(500 * time.Millisecond): + } + cancel() + r := waitTunnel(t, done) + require.NoError(t, r.err) + assert.Equal(t, TunnelReasonInterrupted, r.stats.CloseReason) + select { + case code := <-gotClose: + assert.Equal(t, CloseNormal, code) + case <-time.After(2 * time.Second): + t.Fatal("the server got no close frame") + } +} + +func TestTunnel_TextFramesIgnored(t *testing.T) { + t.Parallel() + srv := pfServer(t, func(_ *http.Request, ws *websocket.Conn) { + _ = ws.WriteMessage(websocket.TextMessage, []byte(`{"secret":"x"}`)) + _ = ws.WriteMessage(websocket.BinaryMessage, []byte("data")) + _ = ws.WriteControl(websocket.CloseMessage, websocket.FormatCloseMessage(CloseNormal, ""), time.Now().Add(time.Second)) + _, _, _ = ws.ReadMessage() + }) + c := New(srv.URL) + var debug bytes.Buffer + c.Debug = true + c.DebugWriter = &debug + clientSide, done := startTunnel(t, context.Background(), c, TunnelOptions{}) + got, _ := io.ReadAll(clientSide) + assert.Equal(t, "data", string(got)) + r := waitTunnel(t, done) + require.NoError(t, r.err) + assert.Contains(t, debug.String(), "ignored a text frame") + assert.NotContains(t, debug.String(), "secret", "--debug never logs frame content") +} + +func TestCloseCodeText(t *testing.T) { + t.Parallel() + assert.Equal(t, "access revoked (API key or permissions); check the key with 'stackctl apikey list'", CloseCodeText(ClosePolicy, "", true)) + assert.Equal(t, "internal server error", CloseCodeText(CloseInternalError, "", false)) + assert.Equal(t, "closed by the server (code 4999): odd", CloseCodeText(4999, "odd", false)) + assert.Equal(t, "pod stream error", CloseCodeText(ClosePodStreamError, "", false)) +} diff --git a/cli/pkg/client/websocket.go b/cli/pkg/client/websocket.go index afd7589..510d795 100644 --- a/cli/pkg/client/websocket.go +++ b/cli/pkg/client/websocket.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "net" "net/http" "net/url" "strings" @@ -254,6 +255,22 @@ func (c *Client) StreamDeploymentLogsWithOptions(ctx context.Context, instanceID // transport triggers a warning via warnWriter (when non-nil) because we // can't extract TLS settings safely. func (c *Client) dialWS(ctx context.Context, path string, warnWriter io.Writer) (*websocket.Conn, error) { + return c.dialWSOpts(ctx, path, warnWriter, wsDialOptions{}) +} + +// wsDialOptions changes the behaviour of dialWSOpts. +type wsDialOptions struct { + // stream marks a stream route (port-forward, exec). These routes accept + // only header auth: no ?token= retry, which would only put the JWT in + // access logs. A 401 gets one forced session renewal and one more dial. + stream bool + // op names the operation in handshake errors, for example + // "port-forward". + op string +} + +// dialWSOpts is dialWS with options (see wsDialOptions). +func (c *Client) dialWSOpts(ctx context.Context, path string, warnWriter io.Writer, opts wsDialOptions) (*websocket.Conn, error) { wsURL, err := c.websocketURL(path) if err != nil { return nil, err @@ -287,6 +304,27 @@ func (c *Client) dialWS(ctx context.Context, path string, warnWriter io.Writer) if err == nil { return conn, nil } + if opts.stream { + if resp != nil && resp.StatusCode == http.StatusUnauthorized && c.APIKey == "" && token != "" { + renewed, rerr := c.RenewSession() + if rerr != nil { + return nil, rerr + } + if renewed { + c.debugf("↻ session renewed after a WebSocket 401; dialing %s again\n", opts.op) + header.Set("Authorization", "Bearer "+c.currentToken()) + conn, resp, err = dialer.DialContext(ctx, wsURL, header) + if err == nil { + return conn, nil + } + } + } + if hsErr := handshakeError(resp, err, c.APIKey != ""); hsErr != nil { + hsErr.Op = opts.op + return nil, hsErr + } + return nil, fmt.Errorf("connecting to the %s WebSocket: %w", opts.op, err) + } // 401 → fall back to ?token= query param when a JWT is configured. // The retry is gated on c.Token because an API-key-only caller has no // token to fall back to. When BOTH APIKey and Token are set, this @@ -335,15 +373,51 @@ const maxHandshakeBody = 4096 type WSHandshakeError struct { StatusCode int Message string + // Code is the stable error code of the JSON body ({"error","code"}) of + // the stream routes, for example StreamCodeFeatureDisabled. Empty when + // the body has no code. + Code string + // Op names the operation of a stream route, for example "port-forward". + // Empty for /ws. + Op string // APIKey is true when the refused request used an API key (no JWT // fallback). APIKey bool err error } +// Error codes of the stream routes (port-forward, exec) of +// k8s-stack-manager, in the JSON body of a refused upgrade. +const ( + StreamCodeFeatureDisabled = "feature_disabled" + StreamCodeAPIKeyNotAllowed = "api_key_not_allowed" + StreamCodeForbidden = "forbidden" + StreamCodeClusterDisabled = "cluster_port_forward_disabled" + StreamCodeNamespaceNotAllowed = "namespace_not_allowed" + StreamCodeOriginNotAllowed = "origin_not_allowed" + StreamCodeInstanceNotFound = "instance_not_found" + StreamCodeServiceNotFound = "service_not_found" + StreamCodePortNotFound = "port_not_found" + StreamCodeInvalidService = "invalid_service" + StreamCodeInvalidPort = "invalid_port" + StreamCodeUpgradeRequired = "upgrade_required" + StreamCodeInstanceNotDeployed = "instance_not_deployed" + StreamCodeServiceExternalName = "service_external_name" + StreamCodeServiceNoSelector = "service_no_selector" + StreamCodeTunnelLimit = "tunnel_limit" + StreamCodeNoReadyPod = "no_ready_pod" + StreamCodeKubeTimeout = "kube_timeout" + StreamCodeShuttingDown = "shutting_down" + StreamCodeDialFailed = "dial_failed" + StreamCodeInternal = "internal" +) + // Error returns a message for the user, for example "WebSocket refused by // the server (HTTP 401): Invalid or expired token. Run 'stackctl login'." func (e *WSHandshakeError) Error() string { + if e.Op != "" { + return e.streamError() + } msg := fmt.Sprintf("WebSocket refused by the server (HTTP %d): %s", e.StatusCode, e.Message) switch { case e.APIKeyNotAccepted(): @@ -357,11 +431,75 @@ func (e *WSHandshakeError) Error() string { } } +// streamError is the message of a refused stream route upgrade. +func (e *WSHandshakeError) streamError() string { + if e.UpgradeDropped() { + return fmt.Sprintf("%s failed: the server answered HTTP %d without a WebSocket upgrade; a proxy between stackctl and the server probably dropped the upgrade", e.Op, e.StatusCode) + } + if e.NoRoute() { + return fmt.Sprintf("%s failed: the server has no %s route (HTTP 404); upgrade k8s-stack-manager", e.Op, e.Op) + } + msg := fmt.Sprintf("%s refused by the server (HTTP %d): %s", e.Op, e.StatusCode, e.Message) + switch { + case e.StatusCode == http.StatusUnauthorized && e.APIKey: + return msg + ". Check the API key." + case e.StatusCode == http.StatusUnauthorized: + return msg + ". Run 'stackctl login'." + } + switch e.Code { + case StreamCodeFeatureDisabled: + return msg + ". The server has " + e.Op + " turned off; ask an administrator." + case StreamCodeAPIKeyNotAllowed: + return msg + ". API keys are not allowed for this command: run 'stackctl login' and remove the API key (--api-key, STACKCTL_API_KEY or the api-key of the context)." + case StreamCodeServiceNotFound, StreamCodePortNotFound: + return msg + ". Check the service name and the service port (not the container port)." + case StreamCodeInstanceNotDeployed: + return msg + ". Deploy the stack first." + case StreamCodeTunnelLimit: + return msg + ". Close other connections, then try again." + } + return msg +} + // Unwrap returns the dialer error (websocket.ErrBadHandshake). func (e *WSHandshakeError) Unwrap() error { return e.err } +// FeatureDisabled reports whether the server has the stream feature turned +// off (404 with code feature_disabled). +func (e *WSHandshakeError) FeatureDisabled() bool { + return e.StatusCode == http.StatusNotFound && e.Code == StreamCodeFeatureDisabled +} + +// NoRoute reports a 404 without an error code: a server without the route +// (an older k8s-stack-manager). Only meaningful for a stream route. +func (e *WSHandshakeError) NoRoute() bool { + return e.StatusCode == http.StatusNotFound && e.Code == "" +} + +// UpgradeDropped reports a success status, or a 400 without a code or with +// upgrade_required: the request reached the server without the upgrade +// headers, so a proxy dropped them. +func (e *WSHandshakeError) UpgradeDropped() bool { + if e.StatusCode >= 200 && e.StatusCode < 300 { + return true + } + return e.StatusCode == http.StatusBadRequest && (e.Code == "" || e.Code == StreamCodeUpgradeRequired) +} + +// Fatal reports whether every later dial of the same stream would fail the +// same way: 400, 401, 403, 404 and 409. 429, 5xx and network errors can pass +// on the next try. +func (e *WSHandshakeError) Fatal() bool { + switch e.StatusCode { + case http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden, + http.StatusNotFound, http.StatusConflict: + return true + } + return e.UpgradeDropped() +} + // APIKeyNotAccepted reports whether the server refused an API key because // its WebSocket endpoint accepts only a JWT (k8s-stack-manager before // v0.8.0). @@ -386,8 +524,10 @@ func handshakeError(resp *http.Response, err error, apiKey bool) *WSHandshakeErr var errResp struct { Error string `json:"error"` Message string `json:"message"` + Code string `json:"code"` } if json.Unmarshal(body, &errResp) == nil { + hsErr.Code = sanitizeServerMessage(errResp.Code) hsErr.Message = sanitizeServerMessage(errResp.Error) if hsErr.Message == "" { hsErr.Message = sanitizeServerMessage(errResp.Message) @@ -611,3 +751,153 @@ func appendQueryToken(rawURL, token string) string { } return rawURL + "?token=" + escaped } + +// Close codes of the stream routes (port-forward, exec) of +// k8s-stack-manager. +const ( + CloseNormal = websocket.CloseNormalClosure // 1000 + CloseGoingAway = websocket.CloseGoingAway // 1001: server shutdown + CloseUnsupported = websocket.CloseUnsupportedData // 1003: text frame from the client + ClosePolicy = websocket.ClosePolicyViolation // 1008: access revoked + CloseMessageTooBig = websocket.CloseMessageTooBig // 1009 + CloseInternalError = websocket.CloseInternalServerErr // 1011 + CloseMaxDuration = 4001 // server time limit + CloseIdleTimeout = 4002 // no data in either direction + ClosePodStreamError = 4502 // the reason holds the error + closeNoStatus = websocket.CloseNoStatusReceived // 1005: close frame without a code +) + +// StreamClosedError reports that the server closed a stream WebSocket +// (port-forward, exec) with a code other than 1000. +type StreamClosedError struct { + // Subject names the stream in the message, for example "tunnel". + Subject string + Code int + // Reason is the sanitized close reason of the server. + Reason string + // APIKey is true when the stream used an API key. + APIKey bool +} + +// Error returns a message for the user, for example "tunnel closed: server +// time limit reached". +func (e *StreamClosedError) Error() string { + return e.Subject + " closed: " + CloseCodeText(e.Code, e.Reason, e.APIKey) +} + +// Fatal reports whether a new stream would fail the same way (1008: access +// revoked). +func (e *StreamClosedError) Fatal() bool { + return e.Code == ClosePolicy +} + +// Unwrap returns a *WSClosedError for close code 1008, so callers that +// handle a revoked /ws connection handle a revoked stream too. +func (e *StreamClosedError) Unwrap() error { + if e.Code == ClosePolicy { + return &WSClosedError{Code: e.Code, Reason: e.Reason, APIKey: e.APIKey} + } + return nil +} + +// CloseCodeText returns a short text for the user for a close code of a +// stream route, for example "server time limit reached" for 4001. +func CloseCodeText(code int, reason string, apiKey bool) string { + reason = sanitizeServerMessage(reason) + switch code { + case CloseNormal, closeNoStatus: + return "normal close" + case CloseGoingAway: + return "the server is shutting down" + case CloseUnsupported: + return "the server refused a text frame" + case ClosePolicy: + if apiKey { + return "access revoked (API key or permissions); check the key with 'stackctl apikey list'" + } + return "access revoked (token, session or permissions); run 'stackctl login'" + case CloseMessageTooBig: + return "message too big" + case CloseInternalError: + return "internal server error" + case CloseMaxDuration: + return "server time limit reached" + case CloseIdleTimeout: + return "idle timeout (no data in either direction)" + case ClosePodStreamError: + if reason == "" { + return "pod stream error" + } + return "pod stream error: " + reason + } + if reason == "" { + return fmt.Sprintf("closed by the server (code %d)", code) + } + return fmt.Sprintf("closed by the server (code %d): %s", code, reason) +} + +// KeepaliveTimings are the keepalive deadlines of a stream WebSocket. A zero +// field takes the default. Tests inject short values. +type KeepaliveTimings struct { + // PingInterval is the interval of the client pings (default 30 s). + PingInterval time.Duration + // ReadTimeout is the read deadline, moved by data, a ping or a pong + // (default 75 s; the server pings every 30 s). + ReadTimeout time.Duration + // WriteTimeout is the deadline of a control frame (default 10 s). + WriteTimeout time.Duration +} + +// Default keepalive timings of a stream WebSocket. +const ( + DefaultPingInterval = 30 * time.Second + DefaultReadTimeout = 75 * time.Second + DefaultWriteTimeout = 10 * time.Second +) + +func (k KeepaliveTimings) withDefaults() KeepaliveTimings { + if k.PingInterval <= 0 { + k.PingInterval = DefaultPingInterval + } + if k.ReadTimeout <= 0 { + k.ReadTimeout = DefaultReadTimeout + } + if k.WriteTimeout <= 0 { + k.WriteTimeout = DefaultWriteTimeout + } + return k +} + +// setKeepalive sets the read deadline and the ping and pong handlers of ws. +// It returns the function that moves the read deadline. Call it, and read +// ws, only from the reader goroutine: gorilla runs the handlers there. +func setKeepalive(ws *websocket.Conn, k KeepaliveTimings) (extend func()) { + extend = func() { _ = ws.SetReadDeadline(time.Now().Add(k.ReadTimeout)) } + extend() + ws.SetPongHandler(func(string) error { + extend() + return nil + }) + ws.SetPingHandler(func(data string) error { + extend() + err := ws.WriteControl(websocket.PongMessage, []byte(data), time.Now().Add(k.WriteTimeout)) + var ne net.Error + if err == nil || errors.Is(err, websocket.ErrCloseSent) || (errors.As(err, &ne) && ne.Timeout()) { + return nil + } + return err + }) + return extend +} + +// sendPing writes a ping control frame. It is safe to call concurrently with +// the data writer. +func sendPing(ws *websocket.Conn, k KeepaliveTimings) error { + return ws.WriteControl(websocket.PingMessage, nil, time.Now().Add(k.WriteTimeout)) +} + +// sendClose writes a close control frame. It is safe to call concurrently +// with the data writer. +func sendClose(ws *websocket.Conn, code int, timeout time.Duration) error { + return ws.WriteControl(websocket.CloseMessage, websocket.FormatCloseMessage(code, ""), time.Now().Add(timeout)) +} diff --git a/cli/test/contract/testdata/swagger.json b/cli/test/contract/testdata/swagger.json index 68ad9f5..e939c72 100644 --- a/cli/test/contract/testdata/swagger.json +++ b/cli/test/contract/testdata/swagger.json @@ -5531,7 +5531,7 @@ } }, "put": { - "description": "Update a stack instance (branch, name, etc.). A changed name must be a DNS label (lowercase a-z, 0-9, '-', start and end alphanumeric, at most 50 characters); an unchanged name is not checked, so older instances with other names stay editable. A rename does not change the namespace.\nttl_minutes sets a new TTL from now (0 clears the expiry). expires_at (RFC 3339 with a time zone) sets a fixed stop time and ttl_minutes 0. Do not send both (400). The limits are the same as for create (MAX_INSTANCE_TTL_DAYS).", + "description": "Update a stack instance (branch, name, etc.). A changed name must be a DNS label (lowercase a-z, 0-9, '-', start and end alphanumeric, at most 50 characters); an unchanged name is not checked, so older instances with other names stay editable. A rename does not change the namespace. The namespace is set at create and cannot be changed: a namespace that differs from the stored one gives 400 (an unchanged value is accepted).\nttl_minutes sets a new TTL from now (0 clears the expiry). expires_at (RFC 3339 with a time zone) sets a fixed stop time and ttl_minutes 0. Do not send both (400). The limits are the same as for create (MAX_INSTANCE_TTL_DAYS).", "consumes": [ "application/json" ], @@ -7642,6 +7642,138 @@ } } }, + "/api/v1/stack-instances/{id}/port-forward": { + "get": { + "security": [ + { + "BearerAuth": [] + }, + { + "ApiKeyAuth": [] + } + ], + "description": "Upgrades the connection to a WebSocket that carries raw TCP bytes between the client and a service port of the instance. Binary frames carry the bytes in both directions; a text frame closes the tunnel with 1003. There is no half-close. The server pings every 30 s; pings do not count as activity. Authentication: the Authorization header (JWT) or, when STACK_STREAM_ALLOW_API_KEYS is true, the X-API-Key header. The route never accepts a token in the query string. The caller must be the owner of the instance, a devops user or an admin, and the cluster must allow port-forward (allow_port_forward). All checks run before the upgrade and give an HTTP status with a stable code in the body ({\"error\",\"code\"}). Close codes after the upgrade: 1000 normal, 1001 server shutdown, 1003 text frame, 1008 access revoked, 1009 frame larger than 64 KiB, 1011 internal error, 4001 maximum duration, 4002 idle timeout, 4502 pod stream error (the reason holds the error). An expired access token does not close an open tunnel. Delete, stop and clean of the instance close the tunnels of that replica at once and the tunnels of other replicas at the next revalidation (up to one minute).", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "stack-instances" + ], + "summary": "Open a port-forward tunnel to a service of a stack instance", + "parameters": [ + { + "type": "string", + "description": "Instance ID", + "name": "id", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Service name (DNS-1035 label)", + "name": "service", + "in": "query", + "required": true + }, + { + "type": "integer", + "description": "Service port number (1-65535)", + "name": "port", + "in": "query", + "required": true + } + ], + "responses": { + "101": { + "description": "Switching Protocols" + }, + "400": { + "description": "invalid_service, invalid_port, upgrade_required", + "schema": { + "type": "object", + "additionalProperties": { + "type": "string" + } + } + }, + "401": { + "description": "Unauthorized", + "schema": { + "type": "object", + "additionalProperties": { + "type": "string" + } + } + }, + "403": { + "description": "api_key_not_allowed, forbidden, cluster_port_forward_disabled, namespace_not_allowed, origin_not_allowed", + "schema": { + "type": "object", + "additionalProperties": { + "type": "string" + } + } + }, + "404": { + "description": "feature_disabled, instance_not_found, service_not_found, port_not_found", + "schema": { + "type": "object", + "additionalProperties": { + "type": "string" + } + } + }, + "409": { + "description": "instance_not_deployed, service_external_name, service_no_selector", + "schema": { + "type": "object", + "additionalProperties": { + "type": "string" + } + } + }, + "429": { + "description": "tunnel_limit", + "schema": { + "type": "object", + "additionalProperties": { + "type": "string" + } + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "type": "object", + "additionalProperties": { + "type": "string" + } + } + }, + "502": { + "description": "dial_failed", + "schema": { + "type": "object", + "additionalProperties": { + "type": "string" + } + } + }, + "503": { + "description": "no_ready_pod, kube_timeout, shutting_down", + "schema": { + "type": "object", + "additionalProperties": { + "type": "string" + } + } + } + } + } + }, "/api/v1/stack-instances/{id}/quota": { "get": { "security": [ @@ -9691,7 +9823,7 @@ }, "/api/v1/ui-config": { "get": { - "description": "Returns the display values of the web UI: product title, logo URL and favicon URL (APP_TITLE, APP_LOGO_URL, APP_FAVICON_URL), the start value of the TTL field in the create forms (default_instance_ttl_minutes, DEFAULT_INSTANCE_TTL_MINUTES; 0 means no expiry), and the maximum TTL and stop-time distance in days (max_instance_ttl_days, MAX_INSTANCE_TTL_DAYS). Public: no authentication, so the login page can use it. An empty logo_url means the built-in logo. A URL is a same-origin path or an https URL. Cached for 5 minutes.", + "description": "Returns the display values of the web UI: product title, logo URL and favicon URL (APP_TITLE, APP_LOGO_URL, APP_FAVICON_URL), the start value of the TTL field in the create forms (default_instance_ttl_minutes, DEFAULT_INSTANCE_TTL_MINUTES; 0 means no expiry), the maximum TTL and stop-time distance in days (max_instance_ttl_days, MAX_INSTANCE_TTL_DAYS), and whether port-forward is on (port_forward_enabled, STACK_PORT_FORWARD_ENABLED). Public: no authentication, so the login page can use it. An empty logo_url means the built-in logo. A URL is a same-origin path or an https URL. Cached for 5 minutes.", "produces": [ "application/json" ], @@ -11063,6 +11195,9 @@ "name" ], "properties": { + "allow_port_forward": { + "type": "boolean" + }, "api_server_url": { "type": "string" }, @@ -11800,6 +11935,11 @@ "type": "integer", "example": 30 }, + "port_forward_enabled": { + "description": "PortForwardEnabled is true when the port-forward tunnels are on\n(STACK_PORT_FORWARD_ENABLED). The UI shows the stackctl hint then.", + "type": "boolean", + "example": false + }, "title": { "description": "Title is the product name (APP_TITLE).", "type": "string", @@ -11810,6 +11950,9 @@ "handlers.UpdateClusterRequest": { "type": "object", "properties": { + "allow_port_forward": { + "type": "boolean" + }, "api_server_url": { "type": "string" }, @@ -13670,6 +13813,10 @@ "models.Cluster": { "type": "object", "properties": { + "allow_port_forward": { + "description": "AllowPortForward lets users open port-forward tunnels to the stacks of\nthis cluster. Off by default; only an admin sets it.", + "type": "boolean" + }, "api_server_url": { "type": "string" }, From 05c379f55dbb775b243951f9e6d7f70333e5ce45 Mon Sep 17 00:00:00 2001 From: Olof Mattsson <14146601+omattsson@users.noreply.github.com> Date: Fri, 30 Oct 2026 10:40:12 +0100 Subject: [PATCH 2/7] fix(port-forward): drain, close texts, keepalive and limits (#178) - Keep reading server data after a local EOF; send close 1000 only after the server closes, a local write fails or the drain wait ends. - Close 1008: separate texts for a stopped, deleted or cleaned stack, a revoked session and an expired API key. - Send pings from their own goroutine, so a cancel closes fast. - Renew the session once for parallel 401s (renew with the refused token). - Handle SIGTERM. - Print a fatal tunnel error once. - Refuse a plain http API URL to a non-loopback host unless --allow-insecure-stream is set. - Cap open tunnels at 16; local write deadline in the tunnel. - Check the bound address for the non-loopback warning. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- README.md | 6 +- cli/cmd/agent_guide.md | 3 + cli/cmd/stack_portforward.go | 100 +++++++++--- cli/cmd/stack_portforward_test.go | 162 ++++++++++++++++++- cli/pkg/client/auth.go | 9 +- cli/pkg/client/portforward.go | 83 +++++++--- cli/pkg/client/portforward_test.go | 248 +++++++++++++++++++++++++++-- cli/pkg/client/websocket.go | 20 ++- 9 files changed, 562 insertions(+), 71 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c1173ae..897941f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -194,7 +194,7 @@ All API calls go to `/api/v1/*`. Key route groups: - `/api/v1/audit-logs` — list/export; `--action` and `--entity-type` are exact matches (`deploy`, `stack_instance`, …; v0.7.0+, see `auditFilterValuesHelp`). Older servers: HTTP method + last route segment (`create` / `deploy`) - `/api/v1/users` — list/delete/disable/enable/password (admin). `PUT /:id/role` `{"role"}` (k8s-stack-manager v0.8.0+, `user set-role `, user by ID or username via `resolveUserID`): response `id`, `old_role`, `new_role`, `changed`; 403 own role / caller not admin, 409 SSO user / last admin, 400 invalid role; a plain 404 (no route) maps to the v0.8.0 upgrade hint (`setRoleError`) - `/ws` — the server closes a socket with code 1008 and reason `session revoked` or `token expired`. `Client.Watch` returns a `WatchStream`; `Err()` gives a `*WSClosedError` ("Connection closed by the server: . Log in again."). `stack watch` exits non-zero on it; for `token expired` it calls `Client.RenewSession` (forced renewal, false without refresh token) and reconnects, at most once per 30 s (`stackWatchMinReconnect`), then with `--id` GETs each pending instance (`watchCatchUp`) so a terminal status in the gap counts. Ctrl-C during renewal or dial exits 0. Another connection loss: `--id` with pending instances exits non-zero ("connection lost before all instances reached a terminal status"), else a stderr note and exit 0. `StreamDeploymentLogs` returns the same `WSClosedError`; `--follow` (`followLogsCtx`, op deploy/stop/clean/rollback, "" for `stack logs -f`) uses the same `renewAfterClose`; it reads the instance after the first connect (the server sets deploying/stopping/cleaning before the 202; not for `logs -f`) and after each reconnect (`StreamOptions.AfterSubscribe`). A normal close (code 1000, stream status `unknown`) reads the instance (`followEndError`): a final status counts, else it fails like a lost connection. When it cannot follow to the end it says that the operation continues on the server and points to `stack watch --id` / `stack status` (never "run again"); errors name the operation ("rollback failed (status error): ..."). Terminal statuses live in `client.IsTerminalStatus` (running, stopped, draft, error, partial, failed) and `client.IsFailedStatus` (error, partial, failed → exit 1) -- `GET /api/v1/stack-instances/:id/port-forward?service=&port=` — `stack port-forward` (omattsson/stackctl#178, k8s-stack-manager#722). WebSocket upgrade, auth only by header (`dialWSOpts` with `stream`: no `?token=` retry, a 401 gets one forced `RenewSession` and one more dial; no `Origin` header). Refusals are HTTP statuses with `{"error","code"}` (`WSHandshakeError.Code`, `StreamCode*`): `FeatureDisabled()` (404 `feature_disabled`), `NoRoute()` (404 without a code: older server), `UpgradeDropped()` (2xx, or 400 without a code or `upgrade_required`: a proxy), `Fatal()` (400, 401, 403, 404, 409 stop the command; 429, 5xx and network errors close one connection). After the upgrade: binary frames of at most 32 KiB both ways (server read limit 64 KiB), no half-close. `Client.Tunnel`: one reader and one writer goroutine, pings and the close through `WriteControl`; ping every 30 s, read deadline 75 s moved by data, ping or pong; a local EOF sends close 1000 and reads for at most 2 s; a server close 1000 does `CloseWrite` on the local socket. Close codes (`Close*` constants, `CloseCodeText`): 1001 shutdown, 1008 revoked (`StreamClosedError.Fatal`, unwraps to `WSClosedError`), 1011, 4001 time limit, 4002 idle, 4502 pod stream error. `--insecure` is refused unless `--allow-insecure-stream`. +- `GET /api/v1/stack-instances/:id/port-forward?service=&port=` — `stack port-forward` (omattsson/stackctl#178, k8s-stack-manager#722). WebSocket upgrade, auth only by header (`dialWSOpts` with `stream`: no `?token=` retry, a 401 gets one forced `RenewSession` and one more dial; no `Origin` header). Refusals are HTTP statuses with `{"error","code"}` (`WSHandshakeError.Code`, `StreamCode*`): `FeatureDisabled()` (404 `feature_disabled`), `NoRoute()` (404 without a code: older server), `UpgradeDropped()` (2xx, or 400 without a code or `upgrade_required`: a proxy), `Fatal()` (400, 401, 403, 404, 409 stop the command; 429, 5xx and network errors close one connection). After the upgrade: binary frames of at most 32 KiB both ways (server read limit 64 KiB), no half-close. `Client.Tunnel`: one reader and one writer goroutine, pings and the close through `WriteControl`; pings from their own goroutine every 30 s (a waiting ping cannot delay a cancel), read deadline 75 s moved by data, ping or pong; a local write deadline of 75 s; after a local EOF the tunnel copies server data to local until the server closes, a local write fails, or `DrainTimeout` (2 s) passes without server data, and only then sends close 1000; a server close 1000 does `CloseWrite` on the local socket. A 401 on the stream dial renews with the refused token (`renewSession(stale)`), so parallel 401s send one refresh. Close codes (`Close*` constants, `CloseCodeText`): 1001 shutdown, 1008 (`StreamClosedError.Fatal`, unwraps to `WSClosedError`; reason `session revoked` → login advice, `access revoked` (`CloseReasonAccessLost`) → stack stopped, deleted or cleaned, or access changed), 1011, 4001 time limit, 4002 idle, 4502 pod stream error. `--insecure`, and an `http://` API URL to a host that is not loopback (`plaintextRemoteURL`), are refused unless `--allow-insecure-stream`. At most 16 open tunnels (`portForwardConnLimit`); a further connection is closed at once. The non-loopback warning checks the bound address (`warnNonLoopback`). SIGTERM ends the command like Ctrl-C. A fatal tunnel error is not repeated in the per-connection close line. - `/api/v1/admin/orphaned-namespaces` — `orphaned list [--details]`; `DELETE /:ns?confirm=` is needed for a namespace without the label managed-by=k8s-stack-manager (409 without it; `orphaned delete --confirm`). `test/contract/path_contract_test.go` checks every `/api/v1` literal of `pkg/client` and `cmd` against the vendored swagger - `POST /api/v1/auth/logout-all` — `logout --all`: uses the session token of the context when a token file exists (also when an API key is set), else the API key; a 401 gets one forced `RenewSession` and one retry. On failure the command stops and keeps the token file; on success the normal logout follows. - `GET /api/v1/dashboard` — `dashboard`: covers all users; `failing_instances` has status error only, `expiring_soon` the next hour. For the role `user`, `clusters` has ID and name only (no health): the table hides clusters without `health_status`. diff --git a/README.md b/README.md index 766b5e5..b7b1fde 100644 --- a/README.md +++ b/README.md @@ -303,12 +303,12 @@ stackctl stack rollback my-app --target-log `stack get` shows `Following` and `Followers` (k8s-stack-manager v0.8.0 or later). `stack preview -q` prints the names of the charts whose values differ from the last deploy. `stack preview` writes the values-drift warning to stderr, so the YAML on stdout stays valid. -`stack port-forward [LOCAL:]REMOTE` opens a local port that forwards to a service of the stack through the server (`GET /api/v1/stack-instances/:id/port-forward`). You need no kubeconfig. REMOTE is the service port. LOCAL must be 0 (any free port) or from 1025 to 65535. Without LOCAL, the local port is REMOTE when REMOTE is above 1024, else REMOTE + 10000; when that port is in use, stackctl tries the next 20 ports, then any free port. An explicit LOCAL port that is in use stops the command. `--address` sets the local addresses (default `127.0.0.1`, comma-separated). stdout gets `Forwarding from 127.0.0.1:3306 -> mysql:3306` (`-q`: only `host:port`); stderr gets one line when a connection opens and one when it closes. Each local connection opens its own tunnel. Ctrl-C closes all connections and exits 0. +`stack port-forward [LOCAL:]REMOTE` opens a local port that forwards to a service of the stack through the server (`GET /api/v1/stack-instances/:id/port-forward`). You need no kubeconfig. REMOTE is the service port. LOCAL must be 0 (any free port) or from 1025 to 65535. Without LOCAL, the local port is REMOTE when REMOTE is above 1024, else REMOTE + 10000; when that port is in use, stackctl tries the next 20 ports, then any free port. An explicit LOCAL port that is in use stops the command. `--address` sets the local addresses (default `127.0.0.1`, comma-separated). stdout gets `Forwarding from 127.0.0.1:3306 -> mysql:3306` (`-q`: only `host:port`); stderr gets one line when a connection opens and one when it closes. Each local connection opens its own tunnel. At most 16 tunnels are open at the same time: stackctl closes a further local connection at once and writes a line to stderr. When the local client closes its side, the tunnel still passes server data to it until the server closes, a local write fails, or 2 s pass without server data. A local client that reads no data for 75 s loses its connection. Ctrl-C or SIGTERM closes all connections and exits 0. -- Errors that stop the command (exit 1): no access (401, 403), an unknown stack, service or port (404), the feature turned off on the server or on the cluster, a stack that is not deployed (409), and a revoked session or permission during a tunnel (close 1008). A 404 without an error code means that the server has no port-forward route: upgrade k8s-stack-manager. A status 200 without an upgrade means that a proxy dropped the WebSocket upgrade. +- Errors that stop the command (exit 1): no access (401, 403), an unknown stack, service or port (404), the feature turned off on the server or on the cluster, a stack that is not deployed (409), and a close 1008 during a tunnel. Close 1008 with the reason `session revoked` means that the session ended: run `stackctl login`. The reason `access revoked` means that the stack was stopped, deleted or cleaned, or that your access to it changed. A 404 without an error code means that the server has no port-forward route: upgrade k8s-stack-manager. A status 200 without an upgrade means that a proxy dropped the WebSocket upgrade. - Errors that close only one connection: the tunnel limit (429), no ready pod (503), a failed pod dial (502), the server time limit (close 4001), the idle timeout (4002), a pod stream error (4502) and network errors. - The server can refuse API keys for port-forward: then run `stackctl login` and remove the API key from the context or the environment. -- Security: anyone who can reach the local port uses your access to the stack. On a shared machine, other local users can connect to it. Web pages in your browser can reach a forwarded HTTP service through DNS rebinding: stop the command when you do not need it. An address that is not loopback opens the port to the network (stackctl writes a warning). `--insecure` is refused, because a man in the middle could read the tunnel data; `--allow-insecure-stream` accepts that risk. +- Security: anyone who can reach the local port uses your access to the stack. On a shared machine, other local users can connect to it. Web pages in your browser can reach a forwarded HTTP service through DNS rebinding: stop the command when you do not need it. An address that is not loopback opens the port to the network (stackctl writes a warning). `--insecure` is refused, because a man in the middle could read the tunnel data. An `http://` API URL is refused for the same reason, except for `localhost`, `127.0.0.1` and `[::1]`. `--allow-insecure-stream` accepts that risk. `stack action list` and `stack action run` see the actions that apply to the stack: a server can limit an action to some templates, template categories, definitions or clusters (`applies_to`). An action of another stack gives "unknown action" with the actions of this stack. When the server could not check where an action applies, `stack action list` ends with "Some actions are not shown: the server could not check where they apply. Try again.", and `-o json` has `"scope_incomplete": true`. diff --git a/cli/cmd/agent_guide.md b/cli/cmd/agent_guide.md index 9a21fe1..1886a36 100644 --- a/cli/cmd/agent_guide.md +++ b/cli/cmd/agent_guide.md @@ -69,6 +69,9 @@ stackctl stack history my-app # Reach a service of the stack: runs until interrupted, so start it in the background stackctl stack port-forward my-app mysql 0:3306 -q # stdout: 127.0.0.1:; exit 1 on 403/404/409 +# SIGTERM or Ctrl-C ends it with exit 0. At most 16 open connections; more are closed at once. +# After a local EOF the tunnel passes server data until the server closes or 2 s pass without data. +# An http:// API URL to a host that is not loopback is refused (add --allow-insecure-stream to accept the risk). # Change values, then deploy again stackctl override set my-app my-chart --set image.tag=1.2.3 diff --git a/cli/cmd/stack_portforward.go b/cli/cmd/stack_portforward.go index a2c9008..368979a 100644 --- a/cli/cmd/stack_portforward.go +++ b/cli/cmd/stack_portforward.go @@ -7,6 +7,7 @@ import ( "io" "net" "net/http" + "net/url" "os" "os/signal" "strconv" @@ -29,8 +30,14 @@ const ( portForwardTries = 20 // portForwardShutdownWait bounds the wait for open tunnels at the end. portForwardShutdownWait = 3 * time.Second + // portForwardMaxConns is the default limit of open tunnels. Each tunnel + // uses a tunnel slot, the API rate limit and an audit row on the server. + portForwardMaxConns = 16 ) +// portForwardConnLimit is the limit of open tunnels (tests lower it). +var portForwardConnLimit = portForwardMaxConns + var stackPortForwardCmd = &cobra.Command{ Use: "port-forward [LOCAL:]REMOTE", Short: "Forward a local port to a service of a stack", @@ -48,16 +55,23 @@ port). LOCAL is the local port: - An explicit LOCAL port that is in use stops the command. Each local connection opens its own tunnel, so one failed connection does -not stop the others. stdout shows the address, for example +not stop the others. At most 16 tunnels are open at the same time: the +command closes a further local connection at once and writes a line to +stderr. stdout shows the address, for example "Forwarding from 127.0.0.1:3306 -> mysql:3306"; -q prints only "host:port". stderr shows one line when a connection opens and one when it -closes (bytes, duration, close reason). -o is ignored. Ctrl-C closes all -connections and exits 0. +closes (bytes, duration, close reason). -o is ignored. Ctrl-C or SIGTERM +closes all connections and exits 0. + +When the local client closes its side, the tunnel still passes server data +to it until the server closes, or 2 s pass without server data. A local +client that reads no data for 75 s loses its connection. Errors that stop the command (exit 1): no access (401 after a session renewal, 403), an unknown stack, service or port, the feature or the cluster switch turned off (404, 403), a stack that is not deployed (409), -and a revoked session or permission during a tunnel (close 1008). API keys +and a close 1008 during a tunnel: a revoked session ('stackctl login'), or +a stack that was stopped, deleted or cleaned, or access that changed. API keys are not allowed by default: run 'stackctl login' and use a login session. Errors that close only one connection: the tunnel limit (429), no ready pod (503), a failed pod dial (502), the server time limit (4001), the idle @@ -72,8 +86,9 @@ Security: - --address with an address that is not loopback (for example 0.0.0.0) opens the port to the network. The command writes a warning. - --insecure is refused: without TLS verification, a man in the middle - can read the tunnel data (for example database passwords). Add - --allow-insecure-stream to accept that risk. + can read the tunnel data (for example database passwords). An http:// + API URL is refused for the same reason, except for localhost, + 127.0.0.1 and [::1]. Add --allow-insecure-stream to accept that risk. Examples: # Forward a database, then connect with the client @@ -91,7 +106,7 @@ Examples: if parent == nil { parent = context.Background() } - ctx, stop := signal.NotifyContext(parent, os.Interrupt) + ctx, stop := signal.NotifyContext(parent, os.Interrupt, syscall.SIGTERM) defer stop() return runPortForward(ctx, cmd, args) }, @@ -99,7 +114,7 @@ Examples: func init() { stackPortForwardCmd.Flags().String("address", "127.0.0.1", "Local addresses to listen on (comma-separated IP addresses or localhost)") - stackPortForwardCmd.Flags().Bool("allow-insecure-stream", false, "Allow port-forward with --insecure (the tunnel data is not protected against a man in the middle)") + stackPortForwardCmd.Flags().Bool("allow-insecure-stream", false, "Allow port-forward with --insecure or an http:// API URL (the tunnel data is not protected against a man in the middle)") stackCmd.AddCommand(stackPortForwardCmd) } @@ -155,6 +170,16 @@ func parseAddresses(raw string) ([]string, error) { return addrs, nil } +// plaintextRemoteURL reports whether apiURL is http:// to a host that is not +// loopback: the tunnel would then go over the network without TLS. +func plaintextRemoteURL(apiURL string) bool { + u, err := url.Parse(strings.TrimSpace(apiURL)) + if err != nil || !strings.EqualFold(u.Scheme, "http") { + return false + } + return !isLoopback(strings.ToLower(u.Hostname())) +} + func isLoopback(addr string) bool { if addr == "localhost" { return true @@ -298,23 +323,22 @@ func runPortForward(ctx context.Context, cmd *cobra.Command, args []string) erro } stderr := &syncWriter{w: cmd.ErrOrStderr()} - if streamInsecure() { - allow, _ := cmd.Flags().GetBool("allow-insecure-stream") - if !allow { - return errors.New("port-forward refuses --insecure: without TLS verification a man in the middle can read the tunnel data; add --allow-insecure-stream to accept that risk") - } - fmt.Fprintln(stderr, "Warning: --allow-insecure-stream: the tunnel data is not protected against a man in the middle") - } - for _, a := range addrs { - if !isLoopback(a) { - fmt.Fprintf(stderr, "Warning: %s is not a loopback address: anyone who can reach it uses your access to the stack\n", a) - } - } - c, err := newClient() if err != nil { return err } + insecure, plaintext := streamInsecure(), plaintextRemoteURL(c.BaseURL) + if insecure || plaintext { + allow, _ := cmd.Flags().GetBool("allow-insecure-stream") + switch { + case allow: + fmt.Fprintln(stderr, "Warning: --allow-insecure-stream: the tunnel data is not protected against a man in the middle") + case insecure: + return errors.New("port-forward refuses --insecure: without TLS verification a man in the middle can read the tunnel data; add --allow-insecure-stream to accept that risk") + default: + return errors.New("port-forward refuses an http:// API URL to a host that is not loopback: a man in the middle can read the tunnel data; use https, or add --allow-insecure-stream to accept that risk") + } + } c.WarnWriter = stderr id, err := resolveStackID(c, args[0]) if err != nil { @@ -325,6 +349,7 @@ func runPortForward(ctx context.Context, cmd *cobra.Command, args []string) erro if err != nil { return err } + warnNonLoopback(stderr, listeners) for _, l := range listeners { hostPort := l.Addr().String() if printer.Quiet { @@ -337,10 +362,22 @@ func runPortForward(ctx context.Context, cmd *cobra.Command, args []string) erro pf := &portForwarder{ client: c, id: id, service: service, port: spec.remote, stderr: stderr, fatal: make(chan error, 1), + slots: make(chan struct{}, portForwardConnLimit), } return pf.serve(ctx, listeners) } +// warnNonLoopback warns for each listener on an address that is not +// loopback. It checks the bound address, so a "localhost" that resolves to +// another address is caught too. +func warnNonLoopback(w io.Writer, listeners []net.Listener) { + for _, l := range listeners { + if a, ok := l.Addr().(*net.TCPAddr); ok && !a.IP.IsLoopback() { + fmt.Fprintf(w, "Warning: %s is not a loopback address: anyone who can reach it uses your access to the stack\n", a.IP) + } + } +} + // portForwarder runs the accept loops and the tunnels of one command. type portForwarder struct { client *client.Client @@ -349,7 +386,9 @@ type portForwarder struct { port int stderr io.Writer fatal chan error - conns atomic.Int64 + // slots limits the open tunnels of all listeners. + slots chan struct{} + conns atomic.Int64 } // serve accepts connections until ctx ends or an error stops the command. @@ -404,9 +443,17 @@ func (pf *portForwarder) acceptLoop(ctx context.Context, l net.Listener, tunnels } continue } + select { + case pf.slots <- struct{}{}: + default: + fmt.Fprintf(pf.stderr, "Connection from %s refused: %d tunnels are open (the limit)\n", conn.RemoteAddr(), cap(pf.slots)) + _ = conn.Close() + continue + } tunnels.Add(1) go func() { defer tunnels.Done() + defer func() { <-pf.slots }() pf.handle(ctx, conn) }() } @@ -438,13 +485,18 @@ func (pf *portForwarder) handle(ctx context.Context, local net.Conn) { } fmt.Fprintf(pf.stderr, "Connection %d from %s opened\n", n, local.RemoteAddr()) stats, err := pf.client.Tunnel(ctx, ws, local, portForwardTunnelOptions) + fatal := err != nil && portForwardFatal(err) reason := stats.CloseReason - if err != nil { + switch { + case fatal: + // The returned error of the command prints the text. + reason = "the command stops" + case err != nil: reason = err.Error() } fmt.Fprintf(pf.stderr, "Connection %d closed: %d bytes in, %d bytes out, %s (%s)\n", n, stats.BytesIn, stats.BytesOut, stats.Duration.Round(time.Millisecond), reason) - if err != nil && portForwardFatal(err) { + if fatal { pf.stop(err) } } diff --git a/cli/cmd/stack_portforward_test.go b/cli/cmd/stack_portforward_test.go index 1c87ccd..f9bfbb3 100644 --- a/cli/cmd/stack_portforward_test.go +++ b/cli/cmd/stack_portforward_test.go @@ -395,19 +395,169 @@ func TestPortForwardCmd_CancelReturnsNilWithoutLeaks(t *testing.T) { goleak.VerifyNone(t, ignore) } -func TestPortForwardCmd_NonLoopbackWarning(t *testing.T) { +func TestPortForwardCmd_BindFailureOpensNothing(t *testing.T) { srv := newPFTestServer(t, nil) - _, stderr := setupPortForwardTest(t, srv.URL) - // 192.0.2.1 (TEST-NET-1) is not a local address: the bind fails after - // the warning, so no port opens to the network. + stdout, stderr := setupPortForwardTest(t, srv.URL) + // 192.0.2.1 (TEST-NET-1) is not a local address: the bind fails, so no + // port opens to the network. require.NoError(t, stackPortForwardCmd.Flags().Set("address", "127.0.0.1,192.0.2.1")) err := waitDone(t, startPortForward(context.Background(), "42", "mysql", "0:3306")) require.Error(t, err) - assert.Contains(t, stderr.String(), "Warning: 192.0.2.1 is not a loopback address") - assert.NotContains(t, stderr.String(), "Warning: 127.0.0.1") + assert.Contains(t, err.Error(), "192.0.2.1") + assert.Empty(t, stdout.String()) + assert.NotContains(t, stderr.String(), "Warning") assert.Equal(t, int32(0), srv.upgrades.Load()) } +func TestPortForwardCmd_LocalhostBindsLoopbackWithoutWarning(t *testing.T) { + srv := newPFTestServer(t, nil) + stdout, stderr := setupPortForwardTest(t, srv.URL) + require.NoError(t, stackPortForwardCmd.Flags().Set("address", "localhost")) + ctx, cancel := context.WithCancel(context.Background()) + done := startPortForward(ctx, "42", "mysql", "0:3306") + waitForwarding(t, stdout, done) + cancel() + require.NoError(t, waitDone(t, done)) + assert.NotContains(t, stderr.String(), "not a loopback address") +} + +// addrListener is a net.Listener that only reports an address. +type addrListener struct { + net.Listener + addr net.Addr +} + +func (l addrListener) Addr() net.Addr { return l.addr } + +func TestWarnNonLoopback(t *testing.T) { + t.Parallel() + listeners := []net.Listener{ + addrListener{addr: &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 3306}}, + addrListener{addr: &net.TCPAddr{IP: net.ParseIP("::1"), Port: 3306}}, + addrListener{addr: &net.TCPAddr{IP: net.ParseIP("192.0.2.1"), Port: 3306}}, + addrListener{addr: &net.TCPAddr{IP: net.IPv4zero, Port: 3306}}, + } + var buf bytes.Buffer + warnNonLoopback(&buf, listeners) + assert.Equal(t, "Warning: 192.0.2.1 is not a loopback address: anyone who can reach it uses your access to the stack\n"+ + "Warning: 0.0.0.0 is not a loopback address: anyone who can reach it uses your access to the stack\n", buf.String()) +} + +func TestPlaintextRemoteURL(t *testing.T) { + t.Parallel() + tests := []struct { + url string + want bool + }{ + {"http://localhost:8081", false}, + {"http://LOCALHOST:8081", false}, + {"http://127.0.0.1:8081", false}, + {"http://127.0.0.2", false}, + {"http://[::1]:8081", false}, + {"https://stacks.example.com", false}, + {"http://stacks.example.com", true}, + {"HTTP://stacks.example.com", true}, + {"http://192.0.2.1:8081", true}, + {"http://localhost.example.com", true}, + } + for _, tt := range tests { + t.Run(tt.url, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, plaintextRemoteURL(tt.url)) + }) + } +} + +func TestPortForwardCmd_PlainHTTPRemoteNeedsAllowFlag(t *testing.T) { + // Refused before any request: stacks.example.com is never contacted. + stdout, _ := setupPortForwardTest(t, "http://stacks.example.com") + err := waitDone(t, startPortForward(context.Background(), "42", "mysql", "0:3306")) + require.Error(t, err) + assert.Contains(t, err.Error(), "refuses an http:// API URL") + assert.Contains(t, err.Error(), "--allow-insecure-stream") + assert.Empty(t, stdout.String()) +} + +func TestPortForwardCmd_ConnectionLimit(t *testing.T) { + srv := newPFTestServer(t, nil) + stdout, stderr := setupPortForwardTest(t, srv.URL) + old := portForwardConnLimit + portForwardConnLimit = 2 + t.Cleanup(func() { portForwardConnLimit = old }) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done := startPortForward(ctx, "42", "mysql", "0:3306") + addr := waitForwarding(t, stdout, done) + + roundTrip := func(conn net.Conn) error { + if _, err := conn.Write([]byte("ping")); err != nil { + return err + } + _ = conn.SetReadDeadline(time.Now().Add(5 * time.Second)) + _, err := io.ReadFull(conn, make([]byte, 4)) + return err + } + var open []net.Conn + for i := 0; i < 2; i++ { + conn, err := net.Dial("tcp", addr) + require.NoError(t, err) + defer conn.Close() + require.NoError(t, roundTrip(conn)) + open = append(open, conn) + } + + over, err := net.Dial("tcp", addr) + require.NoError(t, err) + defer over.Close() + _ = over.SetReadDeadline(time.Now().Add(5 * time.Second)) + _, err = over.Read(make([]byte, 1)) + assert.Error(t, err, "the connection over the limit is closed") + assert.Contains(t, stderr.String(), "refused: 2 tunnels are open (the limit)") + assert.Equal(t, int32(2), srv.upgrades.Load(), "no WebSocket for the refused connection") + + // A closed tunnel frees its slot. + _ = open[0].Close() + require.Eventually(t, func() bool { return strings.Contains(stderr.String(), "Connection 1 closed") }, 5*time.Second, 10*time.Millisecond) + next, err := net.Dial("tcp", addr) + require.NoError(t, err) + defer next.Close() + require.NoError(t, roundTrip(next)) + + cancel() + require.NoError(t, waitDone(t, done)) +} + +func TestPortForwardCmd_FatalCloseNotPrintedTwice(t *testing.T) { + up := websocket.Upgrader{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != pfTestPath { + http.NotFound(w, r) + return + } + ws, err := up.Upgrade(w, r, nil) + if err != nil { + return + } + defer ws.Close() + _ = ws.WriteControl(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.ClosePolicyViolation, "access revoked"), time.Now().Add(time.Second)) + _, _, _ = ws.ReadMessage() + })) + defer srv.Close() + stdout, stderr := setupPortForwardTest(t, srv.URL) + done := startPortForward(context.Background(), "42", "mysql", "0:3306") + addr := waitForwarding(t, stdout, done) + conn, err := net.Dial("tcp", addr) + require.NoError(t, err) + defer conn.Close() + + err = waitDone(t, done) + require.Error(t, err) + assert.Contains(t, err.Error(), "stopped, deleted or cleaned") + assert.Contains(t, stderr.String(), "Connection 1 closed") + assert.Contains(t, stderr.String(), "(the command stops)") + assert.NotContains(t, stderr.String(), "stopped, deleted or cleaned", "the error prints once, as the command error") +} + func TestPortForwardCmd_DefaultPortInUseTakesNext(t *testing.T) { srv := newPFTestServer(t, nil) stdout, _ := setupPortForwardTest(t, srv.URL) diff --git a/cli/pkg/client/auth.go b/cli/pkg/client/auth.go index 4763abc..97aa1e7 100644 --- a/cli/pkg/client/auth.go +++ b/cli/pkg/client/auth.go @@ -115,10 +115,17 @@ func (c *Client) EnsureFreshToken() (string, error) { // session, or a session without a refresh token (SSO login, old token // file). A 401 *APIError means that the server ended the session. func (c *Client) RenewSession() (bool, error) { + return c.renewSession(c.currentToken()) +} + +// renewSession is RenewSession for the access token stale that the server +// refused. When another goroutine already replaced stale, it uses that token +// and sends no refresh. +func (c *Client) renewSession(stale string) (bool, error) { if c.Tokens == nil || c.APIKey != "" { return false, nil } - if err := c.renew(c.currentToken()); err != nil { + if err := c.renew(stale); err != nil { if errors.Is(err, errNoRefreshToken) { return false, nil } diff --git a/cli/pkg/client/portforward.go b/cli/pkg/client/portforward.go index 469116c..9603df9 100644 --- a/cli/pkg/client/portforward.go +++ b/cli/pkg/client/portforward.go @@ -21,8 +21,8 @@ const ( maxTunnelFrame = 32 << 10 // tunnelReadLimit bounds a server frame (the server sends at most 32 KiB). tunnelReadLimit = 1 << 20 - // DefaultDrainTimeout is how long a tunnel reads after the local client - // closed its side. + // DefaultDrainTimeout is how long a tunnel waits for server data after + // the local client closed its side. DefaultDrainTimeout = 2 * time.Second // interruptCloseTimeout bounds the close frame after a cancel. interruptCloseTimeout = time.Second @@ -33,6 +33,7 @@ const ( TunnelReasonLocalClosed = "local connection closed" TunnelReasonServerClosed = "closed by the server" TunnelReasonInterrupted = "interrupted" + TunnelReasonLocalStalled = "local client stopped reading" ) // ErrTunnelTimeout reports that the server sent no data, ping or pong within @@ -43,8 +44,9 @@ var ErrTunnelTimeout = errors.New("tunnel closed: no response from the server") // default. Tests inject short values. type TunnelOptions struct { Keepalive KeepaliveTimings - // DrainTimeout is how long the tunnel still reads server data after the - // local client closed its side (default 2 s). + // DrainTimeout is how long the tunnel waits for more server data after + // the local client closed its side (default 2 s). Server data restarts + // the wait. DrainTimeout time.Duration } @@ -104,13 +106,17 @@ type localPumpResult struct { // one side closes, ctx ends or the server stops answering. It closes ws and // local before it returns and leaves no goroutine behind. // +// After a local EOF the tunnel still copies server data to local, because +// the server has no half-close. It sends close 1000 when the server closes, +// a local write fails, or DrainTimeout passes without server data. +// // A server close 1000, a local close and a cancelled ctx give a nil error. A // server close with another code gives a *StreamClosedError (Fatal for 1008); // a silent server gives ErrTunnelTimeout. func (c *Client) Tunnel(ctx context.Context, ws *websocket.Conn, local net.Conn, opts TunnelOptions) (TunnelStats, error) { opts = opts.withDefaults() start := time.Now() - var in, out atomic.Int64 + var in, out, lastIn atomic.Int64 stats := func(code int, reason string) TunnelStats { return TunnelStats{ BytesIn: in.Load(), BytesOut: out.Load(), Duration: time.Since(start), @@ -121,27 +127,32 @@ func (c *Client) Tunnel(ctx context.Context, ws *websocket.Conn, local net.Conn, extend := setKeepalive(ws, opts.Keepalive) readDone := make(chan error, 1) writeDone := make(chan localPumpResult, 1) + stopPing := make(chan struct{}) var wg sync.WaitGroup - wg.Add(2) + wg.Add(3) go func() { defer wg.Done() - readDone <- c.serverToLocal(ws, local, extend, &in) + readDone <- c.serverToLocal(ws, local, extend, opts.Keepalive.ReadTimeout, &in, &lastIn) }() go func() { defer wg.Done() writeDone <- localToServer(ws, local, opts.Keepalive.ReadTimeout, &out) }() + go func() { + defer wg.Done() + pingLoop(ws, opts.Keepalive, stopPing) + }() teardown := func() { + close(stopPing) _ = ws.Close() _ = local.Close() wg.Wait() } - ticker := time.NewTicker(opts.Keepalive.PingInterval) - defer ticker.Stop() var drain <-chan time.Time var wsWriteErr error - localDone := false + var localEOF time.Time + localDone, closeSent := false, false for { select { case <-ctx.Done(): @@ -149,9 +160,6 @@ func (c *Client) Tunnel(ctx context.Context, ws *websocket.Conn, local net.Conn, teardown() return stats(CloseNormal, TunnelReasonInterrupted), nil - case <-ticker.C: - _ = sendPing(ws, opts.Keepalive) - case r := <-writeDone: writeDone = nil if r.wsErr != nil { @@ -161,25 +169,42 @@ func (c *Client) Tunnel(ctx context.Context, ws *websocket.Conn, local net.Conn, continue } localDone = true - _ = sendClose(ws, CloseNormal, opts.Keepalive.WriteTimeout) + localEOF = time.Now() drain = time.After(opts.DrainTimeout) case <-drain: - teardown() if wsWriteErr != nil { + teardown() return stats(0, "connection lost"), fmt.Errorf("tunnel connection lost: %w", wsWriteErr) } + if !closeSent { + last := localEOF + if t := time.Unix(0, lastIn.Load()); t.After(last) { + last = t + } + if idle := time.Since(last); idle < opts.DrainTimeout { + drain = time.After(opts.DrainTimeout - idle) + continue + } + _ = sendClose(ws, CloseNormal, opts.Keepalive.WriteTimeout) + closeSent = true + drain = time.After(min(opts.DrainTimeout, interruptCloseTimeout)) + continue + } + teardown() return stats(CloseNormal, TunnelReasonLocalClosed), nil case err := <-readDone: code, reason, tErr := c.tunnelEnd(ctx, err, localDone, opts) + var ce *websocket.CloseError + gotCloseFrame := errors.As(err, &ce) && ce.Code != websocket.CloseAbnormalClosure switch { case reason == TunnelReasonServerClosed: // Pass the EOF of the server to the local client. if cw, ok := local.(interface{ CloseWrite() error }); ok { _ = cw.CloseWrite() } - case reason == TunnelReasonLocalClosed && !localDone: + case (reason == TunnelReasonLocalClosed || reason == TunnelReasonLocalStalled) && !closeSent && !gotCloseFrame: _ = sendClose(ws, CloseNormal, opts.Keepalive.WriteTimeout) } teardown() @@ -188,6 +213,21 @@ func (c *Client) Tunnel(ctx context.Context, ws *websocket.Conn, local net.Conn, } } +// pingLoop sends the keepalive pings until stop is closed. It runs apart +// from Tunnel, so a ping that waits for the write lock cannot delay a cancel. +func pingLoop(ws *websocket.Conn, k KeepaliveTimings, stop <-chan struct{}) { + t := time.NewTicker(k.PingInterval) + defer t.Stop() + for { + select { + case <-stop: + return + case <-t.C: + _ = sendPing(ws, k) + } + } +} + // tunnelEnd classifies the end of the server-to-local copy. func (c *Client) tunnelEnd(ctx context.Context, err error, localDone bool, opts TunnelOptions) (code int, reason string, tErr error) { var ce *websocket.CloseError @@ -209,6 +249,9 @@ func (c *Client) tunnelEnd(ctx context.Context, err error, localDone bool, opts return ce.Code, CloseCodeText(ce.Code, reason, c.APIKey != ""), &StreamClosedError{Subject: "tunnel", Code: ce.Code, Reason: reason, APIKey: c.APIKey != ""} case errors.As(err, &lw): + if errors.As(lw.err, &ne) && ne.Timeout() { + return CloseNormal, TunnelReasonLocalStalled, nil + } return CloseNormal, TunnelReasonLocalClosed, nil case localDone: // The server dropped the connection after our close frame. @@ -221,8 +264,10 @@ func (c *Client) tunnelEnd(ctx context.Context, err error, localDone bool, opts } // serverToLocal copies the binary frames of ws to local. Each read moves the -// read deadline. Text frames are ignored (their content is never logged). -func (c *Client) serverToLocal(ws *websocket.Conn, local net.Conn, extend func(), n *atomic.Int64) error { +// read deadline. Text frames are ignored (their content is never logged). A +// local write that takes longer than writeTimeout ends the copy, so a local +// client that stops reading cannot hold the tunnel open. +func (c *Client) serverToLocal(ws *websocket.Conn, local net.Conn, extend func(), writeTimeout time.Duration, n, last *atomic.Int64) error { buf := make([]byte, maxTunnelFrame) for { extend() @@ -241,6 +286,8 @@ func (c *Client) serverToLocal(ws *websocket.Conn, local net.Conn, extend func() k, rerr := r.Read(buf) if k > 0 { extend() + last.Store(time.Now().UnixNano()) + _ = local.SetWriteDeadline(time.Now().Add(writeTimeout)) if _, werr := local.Write(buf[:k]); werr != nil { return &localWriteError{err: werr} } diff --git a/cli/pkg/client/portforward_test.go b/cli/pkg/client/portforward_test.go index 80203f5..47447f2 100644 --- a/cli/pkg/client/portforward_test.go +++ b/cli/pkg/client/portforward_test.go @@ -10,6 +10,7 @@ import ( "net/http" "net/http/httptest" "strings" + "sync" "sync/atomic" "testing" "time" @@ -340,7 +341,9 @@ func TestTunnel_ServerCloseCodes(t *testing.T) { {"idle timeout", CloseIdleTimeout, "idle timeout", "tunnel closed: idle timeout", false}, {"pod stream error", ClosePodStreamError, "connection refused", "tunnel closed: pod stream error: connection refused", false}, {"server shutdown", CloseGoingAway, "server shutdown", "tunnel closed: the server is shutting down", false}, - {"revoked", ClosePolicy, "session revoked", "tunnel closed: access revoked (token, session or permissions); run 'stackctl login'", true}, + {"session revoked", ClosePolicy, "session revoked", "tunnel closed: access revoked (token, session or permissions); run 'stackctl login'", true}, + {"token expired", ClosePolicy, "token expired", "run 'stackctl login'", true}, + {"stack stopped or access lost", ClosePolicy, "access revoked", "tunnel closed: access revoked: the stack was stopped, deleted or cleaned, or your access to it changed", true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -391,38 +394,166 @@ func TestTunnel_ServerNormalCloseEndsLocalStream(t *testing.T) { assert.Equal(t, TunnelReasonServerClosed, r.stats.CloseReason) } +// podstreamLike serves like the server: the default close handler echoes +// the close of the client, and nothing is sent after it. send runs once +// localEOF is closed, unless the client closed first. +func podstreamLike(localEOF <-chan struct{}, gotClose chan<- int, send func(ws *websocket.Conn, closed <-chan struct{})) func(*http.Request, *websocket.Conn) { + return func(_ *http.Request, ws *websocket.Conn) { + closed := make(chan struct{}) + go func() { + defer close(closed) + for { + _, _, err := ws.ReadMessage() + if err != nil { + var ce *websocket.CloseError + if errors.As(err, &ce) { + gotClose <- ce.Code + } + return + } + } + }() + select { + case <-localEOF: + case <-closed: + return + } + send(ws, closed) + <-closed + } +} + func TestTunnel_LocalEOFSendsCloseAndDrains(t *testing.T) { t.Parallel() + localEOF := make(chan struct{}) gotClose := make(chan int, 1) - srv := pfServer(t, func(_ *http.Request, ws *websocket.Conn) { - ws.SetCloseHandler(func(code int, _ string) error { - gotClose <- code - return nil - }) - if _, _, err := ws.ReadMessage(); err != nil { + srv := pfServer(t, podstreamLike(localEOF, gotClose, func(ws *websocket.Conn, closed <-chan struct{}) { + time.Sleep(100 * time.Millisecond) + select { + case <-closed: return + default: } - // The close frame of the client ends this read. - _, _, _ = ws.ReadMessage() _ = ws.WriteMessage(websocket.BinaryMessage, []byte("late")) - _ = ws.WriteControl(websocket.CloseMessage, websocket.FormatCloseMessage(CloseNormal, ""), time.Now().Add(time.Second)) - }) - clientSide, done := startTunnel(t, context.Background(), New(srv.URL), TunnelOptions{DrainTimeout: 5 * time.Second}) + })) + clientSide, done := startTunnel(t, context.Background(), New(srv.URL), TunnelOptions{DrainTimeout: 500 * time.Millisecond}) _, err := clientSide.Write([]byte("query")) require.NoError(t, err) time.Sleep(50 * time.Millisecond) require.NoError(t, clientSide.CloseWrite()) + close(localEOF) got, _ := io.ReadAll(clientSide) assert.Equal(t, "late", string(got), "data after the local EOF still reaches the local side") - assert.Equal(t, CloseNormal, <-gotClose) r := waitTunnel(t, done) require.NoError(t, r.err) + select { + case code := <-gotClose: + assert.Equal(t, CloseNormal, code) + case <-time.After(2 * time.Second): + t.Fatal("the server got no close frame") + } assert.Equal(t, TunnelReasonLocalClosed, r.stats.CloseReason) assert.Equal(t, int64(4), r.stats.BytesIn) assert.Equal(t, int64(5), r.stats.BytesOut) } +func TestTunnel_DrainWaitRestartsOnData(t *testing.T) { + t.Parallel() + localEOF := make(chan struct{}) + gotClose := make(chan int, 1) + srv := pfServer(t, podstreamLike(localEOF, gotClose, func(ws *websocket.Conn, closed <-chan struct{}) { + for i := 0; i < 6; i++ { + select { + case <-closed: + return + case <-time.After(100 * time.Millisecond): + } + _ = ws.WriteMessage(websocket.BinaryMessage, []byte("x")) + } + })) + clientSide, done := startTunnel(t, context.Background(), New(srv.URL), TunnelOptions{DrainTimeout: 250 * time.Millisecond}) + require.NoError(t, clientSide.CloseWrite()) + close(localEOF) + + got, _ := io.ReadAll(clientSide) + assert.Equal(t, "xxxxxx", string(got), "600 ms of data with a 250 ms drain timeout") + r := waitTunnel(t, done) + require.NoError(t, r.err) + assert.Equal(t, CloseNormal, <-gotClose) +} + +func TestTunnel_ServerCloseDuringDrainEndsAtOnce(t *testing.T) { + t.Parallel() + srv := pfServer(t, func(_ *http.Request, ws *websocket.Conn) { + if _, _, err := ws.ReadMessage(); err != nil { + return + } + _ = ws.WriteMessage(websocket.BinaryMessage, []byte("reply")) + _ = ws.WriteControl(websocket.CloseMessage, websocket.FormatCloseMessage(CloseNormal, ""), time.Now().Add(time.Second)) + _, _, _ = ws.ReadMessage() + }) + clientSide, done := startTunnel(t, context.Background(), New(srv.URL), TunnelOptions{DrainTimeout: 10 * time.Second}) + start := time.Now() + _, err := clientSide.Write([]byte("q")) + require.NoError(t, err) + require.NoError(t, clientSide.CloseWrite()) + got, _ := io.ReadAll(clientSide) + assert.Equal(t, "reply", string(got)) + r := waitTunnel(t, done) + require.NoError(t, r.err) + assert.Less(t, time.Since(start), 3*time.Second, "no wait for the drain timeout") +} + +func TestTunnel_StalledLocalReaderEndsTunnel(t *testing.T) { + t.Parallel() + srv := pfServer(t, func(_ *http.Request, ws *websocket.Conn) { + chunk := make([]byte, 32<<10) + for { + _ = ws.SetWriteDeadline(time.Now().Add(5 * time.Second)) + if err := ws.WriteMessage(websocket.BinaryMessage, chunk); err != nil { + return + } + } + }) + opts := TunnelOptions{Keepalive: KeepaliveTimings{ReadTimeout: 300 * time.Millisecond}} + _, done := startTunnel(t, context.Background(), New(srv.URL), opts) + // The local client never reads. + r := waitTunnel(t, done) + require.NoError(t, r.err) + assert.Equal(t, TunnelReasonLocalStalled, r.stats.CloseReason) + assert.Positive(t, r.stats.BytesIn) +} + +func TestTunnel_CancelWhileDataWriterBlocked(t *testing.T) { + t.Parallel() + release := make(chan struct{}) + srv := pfServer(t, func(_ *http.Request, ws *websocket.Conn) { + // Read nothing, so the data writer of the client blocks. + <-release + }) + t.Cleanup(func() { close(release) }) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + opts := TunnelOptions{Keepalive: KeepaliveTimings{PingInterval: 10 * time.Millisecond}} + clientSide, done := startTunnel(t, ctx, New(srv.URL), opts) + go func() { + buf := make([]byte, 256<<10) + for { + if _, err := clientSide.Write(buf); err != nil { + return + } + } + }() + time.Sleep(500 * time.Millisecond) + start := time.Now() + cancel() + r := waitTunnel(t, done) + require.NoError(t, r.err) + assert.Equal(t, TunnelReasonInterrupted, r.stats.CloseReason) + assert.Less(t, time.Since(start), 2*time.Second, "a blocked ping must not delay the cancel") +} + func TestTunnel_DrainTimeout(t *testing.T) { t.Parallel() srv := pfServer(t, func(_ *http.Request, ws *websocket.Conn) { @@ -536,4 +667,95 @@ func TestCloseCodeText(t *testing.T) { assert.Equal(t, "internal server error", CloseCodeText(CloseInternalError, "", false)) assert.Equal(t, "closed by the server (code 4999): odd", CloseCodeText(4999, "odd", false)) assert.Equal(t, "pod stream error", CloseCodeText(ClosePodStreamError, "", false)) + + tests := []struct { + name string + reason string + apiKey bool + want []string + notWant string + }{ + {"session revoked", "session revoked", false, []string{"stackctl login"}, "stopped"}, + {"token expired", "token expired", false, []string{"stackctl login"}, "stopped"}, + {"session revoked with api key", "session revoked", true, []string{"stackctl apikey list"}, "stopped"}, + {"stack stopped or access lost", "access revoked", false, []string{"stopped, deleted or cleaned", "access to it changed"}, "stackctl login"}, + {"stack stopped with api key", "access revoked", true, []string{"stopped, deleted or cleaned"}, "stackctl login"}, + {"api key expired", "api key expired", true, []string{"stackctl apikey create"}, "stackctl login"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := CloseCodeText(ClosePolicy, tt.reason, tt.apiKey) + for _, w := range tt.want { + assert.Contains(t, got, w) + } + assert.NotContains(t, got, tt.notWant) + assert.True(t, (&StreamClosedError{Code: ClosePolicy, Reason: tt.reason}).Fatal(), "1008 stays fatal") + }) + } +} + +func TestDialPortForward_Parallel401sRenewOnce(t *testing.T) { + t.Parallel() + renewed := makeJWT(t, "alice", time.Now().Add(time.Hour)) + var refreshCalls, stale atomic.Int32 + bothStale, renewedSeen := make(chan struct{}), make(chan struct{}) + var renewedOnce sync.Once + wait := func(ch <-chan struct{}) { + select { + case <-ch: + case <-time.After(5 * time.Second): + } + } + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v1/auth/refresh": + refreshCalls.Add(1) + http.SetCookie(w, &http.Cookie{Name: "refresh_token", Value: "r2", Path: "/api/v1/auth"}) + writeJSON(w, http.StatusOK, map[string]string{"token": renewed}) + case pfPath: + if r.Header.Get("Authorization") != "Bearer "+renewed { + // Both dials hold the old token. The second 401 comes after + // the first dial has renewed and dialed again. + n := stale.Add(1) + if n == 2 { + close(bothStale) + } + wait(bothStale) + if n == 2 { + wait(renewedSeen) + } + writeJSON(w, http.StatusUnauthorized, map[string]string{"error": "Invalid or expired token"}) + return + } + renewedOnce.Do(func() { close(renewedSeen) }) + ws, err := (&websocket.Upgrader{}).Upgrade(w, r, nil) + if err != nil { + return + } + defer ws.Close() + _, _, _ = ws.ReadMessage() + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + + old := makeJWT(t, "alice-old", time.Now().Add(time.Hour)) + c, _ := newSessionClient(t, srv.URL, &config.StoredToken{Token: old, RefreshToken: "r1", ExpiresAt: time.Now().Add(time.Hour), APIURL: srv.URL}) + errs := make(chan error, 2) + for i := 0; i < 2; i++ { + go func() { + ws, err := c.DialPortForward(context.Background(), "42", "mysql", 3306) + if err == nil { + _ = ws.Close() + } + errs <- err + }() + } + for i := 0; i < 2; i++ { + require.NoError(t, <-errs) + } + assert.Equal(t, int32(2), stale.Load()) + assert.Equal(t, int32(1), refreshCalls.Load(), "the second 401 reuses the renewed token") } diff --git a/cli/pkg/client/websocket.go b/cli/pkg/client/websocket.go index 510d795..e7a3d57 100644 --- a/cli/pkg/client/websocket.go +++ b/cli/pkg/client/websocket.go @@ -24,6 +24,10 @@ const ( CloseReasonSessionRevoked = "session revoked" // CloseReasonTokenExpired: the access token of the connection expired. CloseReasonTokenExpired = "token expired" + // CloseReasonAccessLost: a stream (port-forward, exec) lost its stack: + // the stack was stopped, deleted or cleaned, or the access re-check of the + // user failed. + CloseReasonAccessLost = "access revoked" ) // WSClosedError reports that the server closed the WebSocket connection @@ -306,7 +310,7 @@ func (c *Client) dialWSOpts(ctx context.Context, path string, warnWriter io.Writ } if opts.stream { if resp != nil && resp.StatusCode == http.StatusUnauthorized && c.APIKey == "" && token != "" { - renewed, rerr := c.RenewSession() + renewed, rerr := c.renewSession(token) if rerr != nil { return nil, rerr } @@ -758,7 +762,7 @@ const ( CloseNormal = websocket.CloseNormalClosure // 1000 CloseGoingAway = websocket.CloseGoingAway // 1001: server shutdown CloseUnsupported = websocket.CloseUnsupportedData // 1003: text frame from the client - ClosePolicy = websocket.ClosePolicyViolation // 1008: access revoked + ClosePolicy = websocket.ClosePolicyViolation // 1008: session, access or stack revoked CloseMessageTooBig = websocket.CloseMessageTooBig // 1009 CloseInternalError = websocket.CloseInternalServerErr // 1011 CloseMaxDuration = 4001 // server time limit @@ -785,8 +789,8 @@ func (e *StreamClosedError) Error() string { return e.Subject + " closed: " + CloseCodeText(e.Code, e.Reason, e.APIKey) } -// Fatal reports whether a new stream would fail the same way (1008: access -// revoked). +// Fatal reports whether a new stream would fail the same way (1008: session, +// access or stack revoked). func (e *StreamClosedError) Fatal() bool { return e.Code == ClosePolicy } @@ -812,6 +816,12 @@ func CloseCodeText(code int, reason string, apiKey bool) string { case CloseUnsupported: return "the server refused a text frame" case ClosePolicy: + switch reason { + case CloseReasonAccessLost: + return "access revoked: the stack was stopped, deleted or cleaned, or your access to it changed" + case CloseReasonAPIKeyExpired: + return "the API key expired; create a new key with 'stackctl apikey create'" + } if apiKey { return "access revoked (API key or permissions); check the key with 'stackctl apikey list'" } @@ -891,7 +901,7 @@ func setKeepalive(ws *websocket.Conn, k KeepaliveTimings) (extend func()) { } // sendPing writes a ping control frame. It is safe to call concurrently with -// the data writer. +// the data writer, but it waits up to k.WriteTimeout for the write lock. func sendPing(ws *websocket.Conn, k KeepaliveTimings) error { return ws.WriteControl(websocket.PingMessage, nil, time.Now().Add(k.WriteTimeout)) } From 49fa674178e7d5ae8cb661395cc51ad88df44db1 Mon Sep 17 00:00:00 2001 From: Olof Mattsson <14146601+omattsson@users.noreply.github.com> Date: Fri, 30 Oct 2026 10:40:12 +0100 Subject: [PATCH 3/7] test(live): port-forward round trip, parallel tunnels and half-close (#178) Deploys a small echo chart, tunnels through the server and checks a round trip, two parallel connections and a half-close. Skips when the server has port-forward turned off or no cluster or chart is available. Co-Authored-By: Claude Opus 5.5 --- cli/test/live/portforward_live_test.go | 260 ++++++++++++++++++ .../live/testdata/portforward-echo/Chart.yaml | 5 + .../portforward-echo/templates/echo.yaml | 44 +++ 3 files changed, 309 insertions(+) create mode 100644 cli/test/live/portforward_live_test.go create mode 100644 cli/test/live/testdata/portforward-echo/Chart.yaml create mode 100644 cli/test/live/testdata/portforward-echo/templates/echo.yaml diff --git a/cli/test/live/portforward_live_test.go b/cli/test/live/portforward_live_test.go new file mode 100644 index 0000000..b2a340f --- /dev/null +++ b/cli/test/live/portforward_live_test.go @@ -0,0 +1,260 @@ +//go:build live + +package live + +import ( + "bytes" + "context" + "errors" + "fmt" + "io" + "net" + "os" + "path/filepath" + "runtime" + "sync" + "testing" + "time" + + "github.com/gorilla/websocket" + "github.com/omattsson/stackctl/cli/pkg/client" + "github.com/omattsson/stackctl/cli/pkg/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Environment (besides the suite variables): +// - STACKCTL_LIVE_PF_CHART — chart path that the backend can read +// (default: testdata/portforward-echo next to this file; works when the +// backend runs on the same host). +// +// The test needs a login (JWT): the route refuses API keys by default. The +// backend needs STACK_PORT_FORWARD_ENABLED=true. The test sets +// allow_port_forward on the default cluster (admin) and restores it. + +const ( + pfService = "echo" + pfPort = 8080 +) + +func pfChartPath(t *testing.T) string { + t.Helper() + if p := os.Getenv("STACKCTL_LIVE_PF_CHART"); p != "" { + return p + } + _, file, _, _ := runtime.Caller(0) + p := filepath.Join(filepath.Dir(file), "testdata", "portforward-echo") + if _, err := os.Stat(p); err != nil { + t.Skipf("echo chart not found: %v", err) + } + return p +} + +func waitStackStatus(t *testing.T, c *client.Client, id string, want string, timeout time.Duration) *types.StackInstance { + t.Helper() + deadline := time.Now().Add(timeout) + for { + inst, err := c.GetStack(id) + require.NoError(t, err, "get stack") + if inst.Status == want { + return inst + } + if inst.Status == "error" { + t.Fatalf("stack entered status error: %s", inst.ErrorMessage) + } + if time.Now().After(deadline) { + t.Fatalf("stack status %q after %s, want %q", inst.Status, timeout, want) + } + time.Sleep(2 * time.Second) + } +} + +// startPFListener accepts local connections and tunnels each one to the +// echo service through the backend, as `stackctl stack port-forward` does. +func startPFListener(t *testing.T, c *client.Client, instanceID string) string { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + ctx, cancel := context.WithCancel(context.Background()) + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + for { + conn, err := ln.Accept() + if err != nil { + return + } + wg.Add(1) + go func() { + defer wg.Done() + ws, err := dialPFRetry(ctx, c, instanceID) + if err != nil { + _ = conn.Close() + return + } + _, _ = c.Tunnel(ctx, ws, conn, client.TunnelOptions{}) + }() + } + }() + t.Cleanup(func() { + cancel() + _ = ln.Close() + wg.Wait() + }) + return ln.Addr().String() +} + +// dialPFRetry retries while the pod is not ready yet (503) or the dial fails (502). +func dialPFRetry(ctx context.Context, c *client.Client, id string) (*websocket.Conn, error) { + deadline := time.Now().Add(30 * time.Second) + for { + ws, err := c.DialPortForward(ctx, id, pfService, pfPort) + if err == nil { + return ws, nil + } + var hs *client.WSHandshakeError + retry := errors.As(err, &hs) && (hs.StatusCode == 502 || hs.StatusCode == 503) + if !retry || time.Now().After(deadline) || ctx.Err() != nil { + return nil, err + } + time.Sleep(time.Second) + } +} + +func TestLivePortForward_Tunnel(t *testing.T) { + c := newLiveClient(t) + login(t, c) + + // Probe: a feature-disabled backend answers 404 before it looks at the instance. + probeCtx, probeCancel := context.WithTimeout(context.Background(), 10*time.Second) + defer probeCancel() + if ws, err := c.DialPortForward(probeCtx, "00000000-0000-0000-0000-0000000000ff", pfService, pfPort); err == nil { + _ = ws.Close() + t.Fatal("dial of an unknown instance must fail") + } else { + var hs *client.WSHandshakeError + if errors.As(err, &hs) { + switch hs.Code { + case client.StreamCodeFeatureDisabled: + t.Skip("port-forward is disabled on this backend (STACK_PORT_FORWARD_ENABLED)") + case client.StreamCodeAPIKeyNotAllowed: + t.Skip("backend refuses API keys on the stream routes; use STACKCTL_LIVE_USER and STACKCTL_LIVE_PASS") + } + } + } + + chart := pfChartPath(t) + cluster := requireCluster(t, c) + + // allow_port_forward is not in the typed cluster request; send the raw field. + var raw struct { + AllowPortForward bool `json:"allow_port_forward"` + } + require.NoError(t, c.Get("/api/v1/clusters/"+cluster.ID, &raw), "get cluster") + if !raw.AllowPortForward { + if err := c.Put("/api/v1/clusters/"+cluster.ID, map[string]any{"allow_port_forward": true}, nil); err != nil { + t.Skipf("cannot enable allow_port_forward on the cluster (admin needed): %v", err) + } + t.Cleanup(func() { + _ = c.Put("/api/v1/clusters/"+cluster.ID, map[string]any{"allow_port_forward": false}, nil) + }) + } + + prefix := liveResourcePrefix() + def, err := c.CreateDefinition(&types.CreateDefinitionRequest{ + Name: prefix + "-pf-def", + Description: "live port-forward echo", + }) + require.NoError(t, err, "create definition") + deleteDefinitionIfExists(t, c, def.ID) + _, err = c.AddDefinitionChart(def.ID, &types.UpdateChartConfigRequest{ + ChartName: "portforward-echo", + ChartPath: chart, + ChartVersion: "0.1.0", + }) + require.NoError(t, err, "add chart") + + inst, err := c.CreateStack(&types.CreateStackRequest{ + Name: prefix + "-pf", + StackDefinitionID: def.ID, + Branch: "main", + ClusterID: cluster.ID, + }) + require.NoError(t, err, "create stack") + t.Cleanup(func() { + _, _ = c.CleanStack(inst.ID) + deadline := time.Now().Add(2 * time.Minute) + for time.Now().Before(deadline) { + cur, err := c.GetStack(inst.ID) + if err != nil || cur.Status != "cleaning" { + break + } + time.Sleep(2 * time.Second) + } + _ = c.DeleteStack(inst.ID) + }) + + _, err = c.DeployStack(inst.ID) + require.NoError(t, err, "deploy stack") + waitStackStatus(t, c, inst.ID, "running", 4*time.Minute) + + addr := startPFListener(t, c, inst.ID) + + roundTrip := func(t *testing.T, payload []byte, halfClose bool) []byte { + t.Helper() + conn, err := net.Dial("tcp", addr) + require.NoError(t, err) + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(30 * time.Second)) + go func() { + _, _ = conn.Write(payload) + if halfClose { + _ = conn.(*net.TCPConn).CloseWrite() + } + }() + got := make([]byte, len(payload)) + _, err = io.ReadFull(conn, got) + require.NoError(t, err, "read echo") + return got + } + + t.Run("round trip", func(t *testing.T) { + payload := []byte("hello through the backend\n") + assert.Equal(t, payload, roundTrip(t, payload, false)) + }) + + t.Run("parallel connections carry independent data", func(t *testing.T) { + var wg sync.WaitGroup + results := make([][]byte, 2) + payloads := [][]byte{ + bytes.Repeat([]byte("A1"), 100_000), + bytes.Repeat([]byte("B2"), 70_000), + } + for i := range payloads { + wg.Add(1) + go func() { + defer wg.Done() + results[i] = roundTrip(t, payloads[i], false) + }() + } + wg.Wait() + for i := range payloads { + assert.True(t, bytes.Equal(payloads[i], results[i]), "connection %d got foreign or damaged data", i) + } + }) + + t.Run("half-close still delivers the response", func(t *testing.T) { + conn, err := net.Dial("tcp", addr) + require.NoError(t, err) + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(30 * time.Second)) + payload := []byte(fmt.Sprintf("half-close-%d", time.Now().UnixNano())) + _, err = conn.Write(payload) + require.NoError(t, err) + require.NoError(t, conn.(*net.TCPConn).CloseWrite()) + got, err := io.ReadAll(conn) + require.NoError(t, err, "read after CloseWrite") + assert.Equal(t, payload, got) + }) +} diff --git a/cli/test/live/testdata/portforward-echo/Chart.yaml b/cli/test/live/testdata/portforward-echo/Chart.yaml new file mode 100644 index 0000000..a357311 --- /dev/null +++ b/cli/test/live/testdata/portforward-echo/Chart.yaml @@ -0,0 +1,5 @@ +apiVersion: v2 +name: portforward-echo +description: TCP echo server for the port-forward live test +version: 0.1.0 +appVersion: "1.0" diff --git a/cli/test/live/testdata/portforward-echo/templates/echo.yaml b/cli/test/live/testdata/portforward-echo/templates/echo.yaml new file mode 100644 index 0000000..f2f2e38 --- /dev/null +++ b/cli/test/live/testdata/portforward-echo/templates/echo.yaml @@ -0,0 +1,44 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: echo +spec: + replicas: 1 + selector: + matchLabels: + app: echo + template: + metadata: + labels: + app: echo + spec: + securityContext: + runAsNonRoot: true + runAsUser: 1000 + seccompProfile: + type: RuntimeDefault + containers: + - name: echo + image: alpine/socat:1.8.0.3 + args: ["TCP-LISTEN:8080,fork,reuseaddr", "EXEC:/bin/cat"] + ports: + - containerPort: 8080 + readinessProbe: + tcpSocket: + port: 8080 + periodSeconds: 2 + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: ["ALL"] +--- +apiVersion: v1 +kind: Service +metadata: + name: echo +spec: + selector: + app: echo + ports: + - port: 8080 + targetPort: 8080 From 7953e6a71601320e55ead7b8c029f067046c61c7 Mon Sep 17 00:00:00 2001 From: Olof Mattsson <14146601+omattsson@users.noreply.github.com> Date: Fri, 30 Oct 2026 10:50:00 +0100 Subject: [PATCH 4/7] feat(port-forward): stop after repeated 429s, SIGTERM test, server version hint (#178) - Count handshake 429s in a row over all tunnels; a successful upgrade resets the count. At 3 the command stops: wait and try again. - portForwardSignals lists os.Interrupt and SIGTERM. A Unix test sends a real SIGTERM to a subprocess with an open tunnel: exit 0, close 1000. - The 404 without a code names k8s-stack-manager v0.15.0 or later (client.PortForwardServerVersion); README and agent guide match. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- README.md | 6 +- cli/cmd/agent_guide.md | 4 +- cli/cmd/stack_portforward.go | 30 ++++- cli/cmd/stack_portforward_signal_test.go | 159 +++++++++++++++++++++++ cli/cmd/stack_portforward_test.go | 89 +++++++++++++ cli/pkg/client/portforward_test.go | 2 +- cli/pkg/client/websocket.go | 6 +- 8 files changed, 287 insertions(+), 11 deletions(-) create mode 100644 cli/cmd/stack_portforward_signal_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 897941f..26142d3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -194,7 +194,7 @@ All API calls go to `/api/v1/*`. Key route groups: - `/api/v1/audit-logs` — list/export; `--action` and `--entity-type` are exact matches (`deploy`, `stack_instance`, …; v0.7.0+, see `auditFilterValuesHelp`). Older servers: HTTP method + last route segment (`create` / `deploy`) - `/api/v1/users` — list/delete/disable/enable/password (admin). `PUT /:id/role` `{"role"}` (k8s-stack-manager v0.8.0+, `user set-role `, user by ID or username via `resolveUserID`): response `id`, `old_role`, `new_role`, `changed`; 403 own role / caller not admin, 409 SSO user / last admin, 400 invalid role; a plain 404 (no route) maps to the v0.8.0 upgrade hint (`setRoleError`) - `/ws` — the server closes a socket with code 1008 and reason `session revoked` or `token expired`. `Client.Watch` returns a `WatchStream`; `Err()` gives a `*WSClosedError` ("Connection closed by the server: . Log in again."). `stack watch` exits non-zero on it; for `token expired` it calls `Client.RenewSession` (forced renewal, false without refresh token) and reconnects, at most once per 30 s (`stackWatchMinReconnect`), then with `--id` GETs each pending instance (`watchCatchUp`) so a terminal status in the gap counts. Ctrl-C during renewal or dial exits 0. Another connection loss: `--id` with pending instances exits non-zero ("connection lost before all instances reached a terminal status"), else a stderr note and exit 0. `StreamDeploymentLogs` returns the same `WSClosedError`; `--follow` (`followLogsCtx`, op deploy/stop/clean/rollback, "" for `stack logs -f`) uses the same `renewAfterClose`; it reads the instance after the first connect (the server sets deploying/stopping/cleaning before the 202; not for `logs -f`) and after each reconnect (`StreamOptions.AfterSubscribe`). A normal close (code 1000, stream status `unknown`) reads the instance (`followEndError`): a final status counts, else it fails like a lost connection. When it cannot follow to the end it says that the operation continues on the server and points to `stack watch --id` / `stack status` (never "run again"); errors name the operation ("rollback failed (status error): ..."). Terminal statuses live in `client.IsTerminalStatus` (running, stopped, draft, error, partial, failed) and `client.IsFailedStatus` (error, partial, failed → exit 1) -- `GET /api/v1/stack-instances/:id/port-forward?service=&port=` — `stack port-forward` (omattsson/stackctl#178, k8s-stack-manager#722). WebSocket upgrade, auth only by header (`dialWSOpts` with `stream`: no `?token=` retry, a 401 gets one forced `RenewSession` and one more dial; no `Origin` header). Refusals are HTTP statuses with `{"error","code"}` (`WSHandshakeError.Code`, `StreamCode*`): `FeatureDisabled()` (404 `feature_disabled`), `NoRoute()` (404 without a code: older server), `UpgradeDropped()` (2xx, or 400 without a code or `upgrade_required`: a proxy), `Fatal()` (400, 401, 403, 404, 409 stop the command; 429, 5xx and network errors close one connection). After the upgrade: binary frames of at most 32 KiB both ways (server read limit 64 KiB), no half-close. `Client.Tunnel`: one reader and one writer goroutine, pings and the close through `WriteControl`; pings from their own goroutine every 30 s (a waiting ping cannot delay a cancel), read deadline 75 s moved by data, ping or pong; a local write deadline of 75 s; after a local EOF the tunnel copies server data to local until the server closes, a local write fails, or `DrainTimeout` (2 s) passes without server data, and only then sends close 1000; a server close 1000 does `CloseWrite` on the local socket. A 401 on the stream dial renews with the refused token (`renewSession(stale)`), so parallel 401s send one refresh. Close codes (`Close*` constants, `CloseCodeText`): 1001 shutdown, 1008 (`StreamClosedError.Fatal`, unwraps to `WSClosedError`; reason `session revoked` → login advice, `access revoked` (`CloseReasonAccessLost`) → stack stopped, deleted or cleaned, or access changed), 1011, 4001 time limit, 4002 idle, 4502 pod stream error. `--insecure`, and an `http://` API URL to a host that is not loopback (`plaintextRemoteURL`), are refused unless `--allow-insecure-stream`. At most 16 open tunnels (`portForwardConnLimit`); a further connection is closed at once. The non-loopback warning checks the bound address (`warnNonLoopback`). SIGTERM ends the command like Ctrl-C. A fatal tunnel error is not repeated in the per-connection close line. +- `GET /api/v1/stack-instances/:id/port-forward?service=&port=` — `stack port-forward` (omattsson/stackctl#178, k8s-stack-manager#722). WebSocket upgrade, auth only by header (`dialWSOpts` with `stream`: no `?token=` retry, a 401 gets one forced `RenewSession` and one more dial; no `Origin` header). Refusals are HTTP statuses with `{"error","code"}` (`WSHandshakeError.Code`, `StreamCode*`): `FeatureDisabled()` (404 `feature_disabled`), `NoRoute()` (404 without a code: older server; hint `client.PortForwardServerVersion`), `UpgradeDropped()` (2xx, or 400 without a code or `upgrade_required`: a proxy), `Fatal()` (400, 401, 403, 404, 409 stop the command; 429, 5xx and network errors close one connection; 3 handshake 429s in a row, with no successful upgrade between them (`portForwardMax429`, one counter for all tunnels), stop the command). After the upgrade: binary frames of at most 32 KiB both ways (server read limit 64 KiB), no half-close. `Client.Tunnel`: one reader and one writer goroutine, pings and the close through `WriteControl`; pings from their own goroutine every 30 s (a waiting ping cannot delay a cancel), read deadline 75 s moved by data, ping or pong; a local write deadline of 75 s; after a local EOF the tunnel copies server data to local until the server closes, a local write fails, or `DrainTimeout` (2 s) passes without server data, and only then sends close 1000; a server close 1000 does `CloseWrite` on the local socket. A 401 on the stream dial renews with the refused token (`renewSession(stale)`), so parallel 401s send one refresh. Close codes (`Close*` constants, `CloseCodeText`): 1001 shutdown, 1008 (`StreamClosedError.Fatal`, unwraps to `WSClosedError`; reason `session revoked` → login advice, `access revoked` (`CloseReasonAccessLost`) → stack stopped, deleted or cleaned, or access changed), 1011, 4001 time limit, 4002 idle, 4502 pod stream error. `--insecure`, and an `http://` API URL to a host that is not loopback (`plaintextRemoteURL`), are refused unless `--allow-insecure-stream`. At most 16 open tunnels (`portForwardConnLimit`); a further connection is closed at once. The non-loopback warning checks the bound address (`warnNonLoopback`). SIGTERM ends the command like Ctrl-C (`portForwardSignals`; `stack_portforward_signal_test.go` sends a real SIGTERM to a subprocess, not on Windows). A fatal tunnel error is not repeated in the per-connection close line. - `/api/v1/admin/orphaned-namespaces` — `orphaned list [--details]`; `DELETE /:ns?confirm=` is needed for a namespace without the label managed-by=k8s-stack-manager (409 without it; `orphaned delete --confirm`). `test/contract/path_contract_test.go` checks every `/api/v1` literal of `pkg/client` and `cmd` against the vendored swagger - `POST /api/v1/auth/logout-all` — `logout --all`: uses the session token of the context when a token file exists (also when an API key is set), else the API key; a 401 gets one forced `RenewSession` and one retry. On failure the command stops and keeps the token file; on success the normal logout follows. - `GET /api/v1/dashboard` — `dashboard`: covers all users; `failing_instances` has status error only, `expiring_soon` the next hour. For the role `user`, `clusters` has ID and name only (no health): the table hides clusters without `health_status`. diff --git a/README.md b/README.md index b7b1fde..67cab3d 100644 --- a/README.md +++ b/README.md @@ -305,8 +305,8 @@ stackctl stack rollback my-app --target-log `stack port-forward [LOCAL:]REMOTE` opens a local port that forwards to a service of the stack through the server (`GET /api/v1/stack-instances/:id/port-forward`). You need no kubeconfig. REMOTE is the service port. LOCAL must be 0 (any free port) or from 1025 to 65535. Without LOCAL, the local port is REMOTE when REMOTE is above 1024, else REMOTE + 10000; when that port is in use, stackctl tries the next 20 ports, then any free port. An explicit LOCAL port that is in use stops the command. `--address` sets the local addresses (default `127.0.0.1`, comma-separated). stdout gets `Forwarding from 127.0.0.1:3306 -> mysql:3306` (`-q`: only `host:port`); stderr gets one line when a connection opens and one when it closes. Each local connection opens its own tunnel. At most 16 tunnels are open at the same time: stackctl closes a further local connection at once and writes a line to stderr. When the local client closes its side, the tunnel still passes server data to it until the server closes, a local write fails, or 2 s pass without server data. A local client that reads no data for 75 s loses its connection. Ctrl-C or SIGTERM closes all connections and exits 0. -- Errors that stop the command (exit 1): no access (401, 403), an unknown stack, service or port (404), the feature turned off on the server or on the cluster, a stack that is not deployed (409), and a close 1008 during a tunnel. Close 1008 with the reason `session revoked` means that the session ended: run `stackctl login`. The reason `access revoked` means that the stack was stopped, deleted or cleaned, or that your access to it changed. A 404 without an error code means that the server has no port-forward route: upgrade k8s-stack-manager. A status 200 without an upgrade means that a proxy dropped the WebSocket upgrade. -- Errors that close only one connection: the tunnel limit (429), no ready pod (503), a failed pod dial (502), the server time limit (close 4001), the idle timeout (4002), a pod stream error (4502) and network errors. +- Errors that stop the command (exit 1): no access (401, 403), an unknown stack, service or port (404), the feature turned off on the server or on the cluster, a stack that is not deployed (409), and a close 1008 during a tunnel. Close 1008 with the reason `session revoked` means that the session ended: run `stackctl login`. The reason `access revoked` means that the stack was stopped, deleted or cleaned, or that your access to it changed. A 404 without an error code means that the server has no port-forward route: upgrade the server to k8s-stack-manager v0.15.0 or later. A status 200 without an upgrade means that a proxy dropped the WebSocket upgrade. +- Errors that close only one connection: the rate limit or the tunnel limit (429), no ready pod (503), a failed pod dial (502), the server time limit (close 4001), the idle timeout (4002), a pod stream error (4502) and network errors. 3 refusals with 429 in a row, with no successful tunnel between them, stop the command (exit 1): wait and try again. - The server can refuse API keys for port-forward: then run `stackctl login` and remove the API key from the context or the environment. - Security: anyone who can reach the local port uses your access to the stack. On a shared machine, other local users can connect to it. Web pages in your browser can reach a forwarded HTTP service through DNS rebinding: stop the command when you do not need it. An address that is not loopback opens the port to the network (stackctl writes a warning). `--insecure` is refused, because a man in the middle could read the tunnel data. An `http://` API URL is refused for the same reason, except for `localhost`, `127.0.0.1` and `[::1]`. `--allow-insecure-stream` accepts that risk. @@ -681,7 +681,7 @@ These groups have no section above. Run `stackctl --help` for the flags. | `definition link-template` | v0.11.0 or later | | `--stop-at`, `stack extend --until`, the TTL limit of `server info` and of the local check (an older server ignores `--stop-at` and `--until`: stackctl shows the result and exits with code 1; it uses 30 days as the limit) | v0.12.0 or later | | Actions limited to some stacks (`applies_to`) and the `scope_incomplete` hint of `stack action list` (an older server lists every action for every stack) | the release after v0.14.0 (omattsson/k8s-stack-manager#720) | -| `stack port-forward` (an older server answers 404 without an error code: stackctl says "upgrade k8s-stack-manager") | the release after v0.14.0 (omattsson/k8s-stack-manager#722) | +| `stack port-forward` (an older server answers 404 without an error code: stackctl says "upgrade the server to k8s-stack-manager v0.15.0 or later") | v0.15.0 or later (omattsson/k8s-stack-manager#722) | ## Output Formats diff --git a/cli/cmd/agent_guide.md b/cli/cmd/agent_guide.md index 1886a36..1ea2661 100644 --- a/cli/cmd/agent_guide.md +++ b/cli/cmd/agent_guide.md @@ -71,6 +71,8 @@ stackctl stack history my-app stackctl stack port-forward my-app mysql 0:3306 -q # stdout: 127.0.0.1:; exit 1 on 403/404/409 # SIGTERM or Ctrl-C ends it with exit 0. At most 16 open connections; more are closed at once. # After a local EOF the tunnel passes server data until the server closes or 2 s pass without data. +# 3 refusals with 429 in a row end it with exit 1: wait and try again. +# Needs k8s-stack-manager v0.15.0 or later (an older server: "upgrade the server to k8s-stack-manager v0.15.0 or later"). # An http:// API URL to a host that is not loopback is refused (add --allow-insecure-stream to accept the risk). # Change values, then deploy again @@ -112,7 +114,7 @@ When `--follow` or `watch --id` stops early for another reason (lost connection, - `Conflict: ...`: the state does not allow the change (for example a name in use; the message lists free names). - `Rate limited`: wait and try once more. - `Server error`: report it; the server logs have the cause. -- `upgrade k8s-stack-manager to vX or later`: the server is too old for this feature. Tell the user. +- `upgrade k8s-stack-manager to vX or later` or `upgrade the server to k8s-stack-manager vX or later`: the server is too old for this feature. Tell the user. - `Warning: ...` on stderr: a notice from the server (for example a deprecation). The command still worked. ## More diff --git a/cli/cmd/stack_portforward.go b/cli/cmd/stack_portforward.go index 368979a..91f72cd 100644 --- a/cli/cmd/stack_portforward.go +++ b/cli/cmd/stack_portforward.go @@ -33,8 +33,14 @@ const ( // portForwardMaxConns is the default limit of open tunnels. Each tunnel // uses a tunnel slot, the API rate limit and an audit row on the server. portForwardMaxConns = 16 + // portForwardMax429 is the number of 429 handshakes in a row, with no + // successful upgrade between them, that stops the command. + portForwardMax429 = 3 ) +// portForwardSignals end the command like Ctrl-C. +var portForwardSignals = []os.Signal{os.Interrupt, syscall.SIGTERM} + // portForwardConnLimit is the limit of open tunnels (tests lower it). var portForwardConnLimit = portForwardMaxConns @@ -73,9 +79,11 @@ cluster switch turned off (404, 403), a stack that is not deployed (409), and a close 1008 during a tunnel: a revoked session ('stackctl login'), or a stack that was stopped, deleted or cleaned, or access that changed. API keys are not allowed by default: run 'stackctl login' and use a login session. -Errors that close only one connection: the tunnel limit (429), no ready pod -(503), a failed pod dial (502), the server time limit (4001), the idle -timeout (4002), a pod stream error (4502) and network errors. +Errors that close only one connection: the rate limit or the tunnel limit +(429), no ready pod (503), a failed pod dial (502), the server time limit +(4001), the idle timeout (4002), a pod stream error (4502) and network +errors. 3 refusals with 429 in a row, with no successful tunnel between +them, stop the command. Security: - Anyone who can reach the local port uses your access to the stack. @@ -106,7 +114,7 @@ Examples: if parent == nil { parent = context.Background() } - ctx, stop := signal.NotifyContext(parent, os.Interrupt, syscall.SIGTERM) + ctx, stop := signal.NotifyContext(parent, portForwardSignals...) defer stop() return runPortForward(ctx, cmd, args) }, @@ -304,6 +312,12 @@ func portForwardFatal(err error) bool { (apiErr.StatusCode == http.StatusUnauthorized || apiErr.StatusCode == http.StatusForbidden) } +// isRateLimited reports a handshake refused with 429. +func isRateLimited(err error) bool { + var hs *client.WSHandshakeError + return errors.As(err, &hs) && hs.StatusCode == http.StatusTooManyRequests +} + // portForwardTunnelOptions are the tunnel timings (tests shorten them). var portForwardTunnelOptions client.TunnelOptions @@ -389,6 +403,9 @@ type portForwarder struct { // slots limits the open tunnels of all listeners. slots chan struct{} conns atomic.Int64 + // rateLimited counts the 429 handshakes since the last successful + // upgrade of any tunnel. + rateLimited atomic.Int32 } // serve accepts connections until ctx ends or an error stops the command. @@ -480,9 +497,14 @@ func (pf *portForwarder) handle(ctx context.Context, local net.Conn) { pf.stop(err) return } + if isRateLimited(err) && pf.rateLimited.Add(1) >= portForwardMax429 { + pf.stop(fmt.Errorf("port-forward stopped: the server rate limit or tunnel limit was hit %d times in a row; wait and try again (last error: %w)", portForwardMax429, err)) + return + } fmt.Fprintf(pf.stderr, "Connection %d from %s failed: %v\n", n, local.RemoteAddr(), err) return } + pf.rateLimited.Store(0) fmt.Fprintf(pf.stderr, "Connection %d from %s opened\n", n, local.RemoteAddr()) stats, err := pf.client.Tunnel(ctx, ws, local, portForwardTunnelOptions) fatal := err != nil && portForwardFatal(err) diff --git a/cli/cmd/stack_portforward_signal_test.go b/cli/cmd/stack_portforward_signal_test.go new file mode 100644 index 0000000..384b9d3 --- /dev/null +++ b/cli/cmd/stack_portforward_signal_test.go @@ -0,0 +1,159 @@ +//go:build !windows + +package cmd + +import ( + "bytes" + "errors" + "io" + "net" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "strings" + "syscall" + "testing" + "time" + + "github.com/gorilla/websocket" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/goleak" +) + +const ( + pfSigtermHelperEnv = "STACKCTL_TEST_PF_SIGTERM_HELPER" + pfSigtermURLEnv = "STACKCTL_TEST_PF_SIGTERM_URL" +) + +// firstLineWriter sends the first line written to it. +type firstLineWriter struct { + buf bytes.Buffer + line chan<- string + sent bool +} + +func (w *firstLineWriter) Write(p []byte) (int, error) { + if !w.sent { + w.buf.Write(p) + if line, _, ok := strings.Cut(w.buf.String(), "\n"); ok { + w.sent = true + w.line <- strings.TrimSpace(line) + } + } + return len(p), nil +} + +// TestPortForwardSigtermHelper runs the real command in the subprocess of +// TestPortForwardCmd_SigtermClosesTunnels. +func TestPortForwardSigtermHelper(t *testing.T) { + if os.Getenv(pfSigtermHelperEnv) != "1" { + t.Skip("helper for TestPortForwardCmd_SigtermClosesTunnels") + } + SetArgs([]string{"stack", "port-forward", "42", "mysql", "0:3306", "-q", + "--address", "127.0.0.1", "--api-url", os.Getenv(pfSigtermURLEnv)}) + if err := Execute(); err != nil { + os.Exit(3) + } +} + +func TestPortForwardCmd_SigtermClosesTunnels(t *testing.T) { + ignore := goleak.IgnoreCurrent() + const timeout = 10 * time.Second + + closeCodes := make(chan int, 1) + up := websocket.Upgrader{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != pfTestPath { + http.NotFound(w, r) + return + } + ws, err := up.Upgrade(w, r, nil) + if err != nil { + return + } + defer ws.Close() + for { + mt, data, err := ws.ReadMessage() + if err != nil { + code := 0 + var ce *websocket.CloseError + if errors.As(err, &ce) { + code = ce.Code + } + closeCodes <- code + return + } + if err := ws.WriteMessage(mt, data); err != nil { + return + } + } + })) + defer srv.Close() + + var env []string + for _, kv := range os.Environ() { + if !strings.HasPrefix(kv, "STACKCTL_") { + env = append(env, kv) + } + } + env = append(env, pfSigtermHelperEnv+"=1", pfSigtermURLEnv+"="+srv.URL, "STACKCTL_CONFIG_DIR="+t.TempDir()) + + sub := exec.Command(os.Args[0], "-test.run=^TestPortForwardSigtermHelper$", "-test.count=1") + sub.Env = env + var subErr lockedBuffer + sub.Stderr = &subErr + addrCh := make(chan string, 1) + sub.Stdout = &firstLineWriter{line: addrCh} + require.NoError(t, sub.Start()) + exited := make(chan error, 1) + waitStarted := false + defer func() { + if !waitStarted { + _ = sub.Process.Kill() + _ = sub.Wait() + } + }() + + var addr string + select { + case addr = <-addrCh: + case <-time.After(timeout): + t.Fatal("no host:port on stdout") + } + require.Regexp(t, `^127\.0\.0\.1:\d+$`, addr, "stderr: %s", subErr.String()) + + conn, err := net.DialTimeout("tcp", addr, timeout) + require.NoError(t, err) + defer conn.Close() + _, err = conn.Write([]byte("x")) + require.NoError(t, err) + _ = conn.SetReadDeadline(time.Now().Add(timeout)) + got := make([]byte, 1) + _, err = io.ReadFull(conn, got) + require.NoError(t, err, "the tunnel is open") + assert.Equal(t, "x", string(got)) + + require.NoError(t, sub.Process.Signal(syscall.SIGTERM)) + waitStarted = true + go func() { exited <- sub.Wait() }() + select { + case err := <-exited: + require.NoError(t, err, "exit code 0; stderr: %s", subErr.String()) + case <-time.After(timeout): + _ = sub.Process.Kill() + <-exited + t.Fatalf("the subprocess did not exit after SIGTERM; stderr: %s", subErr.String()) + } + select { + case code := <-closeCodes: + assert.Equal(t, websocket.CloseNormalClosure, code) + case <-time.After(timeout): + t.Fatal("the server got no close") + } + + _ = conn.Close() + srv.Close() + goleak.VerifyNone(t, ignore) +} diff --git a/cli/cmd/stack_portforward_test.go b/cli/cmd/stack_portforward_test.go index f9bfbb3..8a9600e 100644 --- a/cli/cmd/stack_portforward_test.go +++ b/cli/cmd/stack_portforward_test.go @@ -4,16 +4,19 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "net" "net/http" "net/http/httptest" + "os" "regexp" "strconv" "strings" "sync" "sync/atomic" + "syscall" "testing" "time" @@ -635,3 +638,89 @@ func TestPortForwardCmd_Registered(t *testing.T) { assert.True(t, strings.Contains(found.Long, "mysql -h 127.0.0.1 -P 3306")) assert.Contains(t, found.Long, "DNS rebinding") } + +func TestPortForwardSignals(t *testing.T) { + t.Parallel() + assert.Contains(t, portForwardSignals, os.Interrupt) + assert.Contains(t, portForwardSignals, syscall.SIGTERM) +} + +// dialUntilClosed opens a local connection and waits until the command +// closes it. +func dialUntilClosed(t *testing.T, addr string) { + t.Helper() + conn, err := net.Dial("tcp", addr) + require.NoError(t, err) + defer conn.Close() + _ = conn.SetReadDeadline(time.Now().Add(5 * time.Second)) + _, err = conn.Read(make([]byte, 1)) + require.Error(t, err) + var ne net.Error + require.False(t, errors.As(err, &ne) && ne.Timeout(), "the refused connection is closed") +} + +func TestPortForwardCmd_Repeated429StopsCommand(t *testing.T) { + var dials atomic.Int32 + srv := newPFTestServer(t, func(w http.ResponseWriter) bool { + dials.Add(1) + return jsonRefusal(http.StatusTooManyRequests, "tunnel_limit", "Too many tunnels")(w) + }) + stdout, stderr := setupPortForwardTest(t, srv.URL) + done := startPortForward(context.Background(), "42", "mysql", "0:3306") + addr := waitForwarding(t, stdout, done) + + for i := 0; i < portForwardMax429; i++ { + dialUntilClosed(t, addr) + } + err := waitDone(t, done) + require.Error(t, err) + assert.Contains(t, err.Error(), "rate limit or tunnel limit was hit 3 times in a row; wait and try again") + assert.Equal(t, int32(3), dials.Load()) + assert.Equal(t, int32(0), srv.upgrades.Load()) + assert.Equal(t, 2, strings.Count(stderr.String(), "HTTP 429"), "one line for each 429 below the limit") + + _, err = net.DialTimeout("tcp", addr, time.Second) + assert.Error(t, err, "the listener is closed") +} + +func TestPortForwardCmd_SuccessResets429Count(t *testing.T) { + // 429, 429, upgrade, 429, 429, upgrade: never 3 in a row. + var dials atomic.Int32 + srv := newPFTestServer(t, func(w http.ResponseWriter) bool { + if dials.Add(1)%3 == 0 { + return false + } + return jsonRefusal(http.StatusTooManyRequests, "tunnel_limit", "Too many tunnels")(w) + }) + stdout, stderr := setupPortForwardTest(t, srv.URL) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done := startPortForward(ctx, "42", "mysql", "0:3306") + addr := waitForwarding(t, stdout, done) + + roundTrip := func() { + conn, err := net.Dial("tcp", addr) + require.NoError(t, err) + defer conn.Close() + _, err = conn.Write([]byte("ok")) + require.NoError(t, err) + _ = conn.SetReadDeadline(time.Now().Add(5 * time.Second)) + _, err = io.ReadFull(conn, make([]byte, 2)) + require.NoError(t, err) + } + for round := 0; round < 2; round++ { + dialUntilClosed(t, addr) + dialUntilClosed(t, addr) + roundTrip() + } + select { + case err := <-done: + t.Fatalf("command ended: %v", err) + default: + } + cancel() + require.NoError(t, waitDone(t, done)) + assert.Equal(t, int32(6), dials.Load()) + assert.Equal(t, int32(2), srv.upgrades.Load()) + assert.Equal(t, 4, strings.Count(stderr.String(), "HTTP 429")) +} diff --git a/cli/pkg/client/portforward_test.go b/cli/pkg/client/portforward_test.go index 47447f2..79ccaf4 100644 --- a/cli/pkg/client/portforward_test.go +++ b/cli/pkg/client/portforward_test.go @@ -279,7 +279,7 @@ func TestDialPortForward_HandshakeErrors(t *testing.T) { wantFatal: true, wantMsg: []string{"Service not found", "service port"}}, {name: "plain 404", status: 404, body: "404 page not found", check: func(t *testing.T, hs *WSHandshakeError) { assert.True(t, hs.NoRoute()) }, - wantFatal: true, wantMsg: []string{"no port-forward route", "upgrade k8s-stack-manager"}}, + wantFatal: true, wantMsg: []string{"no port-forward route", "upgrade the server to k8s-stack-manager v0.15.0 or later"}}, {name: "not deployed", status: 409, body: `{"error":"Instance is not deployed","code":"instance_not_deployed"}`, wantFatal: true, wantMsg: []string{"Deploy the stack first"}}, diff --git a/cli/pkg/client/websocket.go b/cli/pkg/client/websocket.go index e7a3d57..233b493 100644 --- a/cli/pkg/client/websocket.go +++ b/cli/pkg/client/websocket.go @@ -366,6 +366,10 @@ func baseWSHeader() http.Header { // "Invalid API key" or "API key expired" instead. const wsAuthRequiredMessage = "Authentication required" +// PortForwardServerVersion is the first server release with the +// port-forward route (k8s-stack-manager#722). +const PortForwardServerVersion = "k8s-stack-manager v0.15.0 or later" + // maxHandshakeBody is the number of response body bytes that // handshakeError reads. gorilla/websocket keeps at most 1024 bytes. const maxHandshakeBody = 4096 @@ -441,7 +445,7 @@ func (e *WSHandshakeError) streamError() string { return fmt.Sprintf("%s failed: the server answered HTTP %d without a WebSocket upgrade; a proxy between stackctl and the server probably dropped the upgrade", e.Op, e.StatusCode) } if e.NoRoute() { - return fmt.Sprintf("%s failed: the server has no %s route (HTTP 404); upgrade k8s-stack-manager", e.Op, e.Op) + return fmt.Sprintf("%s failed: the server has no %s route (HTTP 404); upgrade the server to %s", e.Op, e.Op, PortForwardServerVersion) } msg := fmt.Sprintf("%s refused by the server (HTTP %d): %s", e.Op, e.StatusCode, e.Message) switch { From eb418c3272adf3f6e64eb54737a1b0362c1461b9 Mon Sep 17 00:00:00 2001 From: Olof Mattsson <14146601+omattsson@users.noreply.github.com> Date: Fri, 30 Oct 2026 10:53:03 +0100 Subject: [PATCH 5/7] build: Go 1.27.3 and staticcheck 2026.2.2 - cli/go.mod: go 1.27.3 (all workflows read go-version-file: cli/go.mod). go mod tidy changes nothing in go.sum. - Makefile: staticcheck 2026.2.2; 2026.1 cannot read the Go 1.27 export data. - govulncheck v1.1.4 and goreleaser v2.18.2 work with Go 1.27.3 and stay. - README, CLAUDE.md and copilot instructions name 1.27.3. Co-Authored-By: Claude Opus 5.5 --- .github/copilot-instructions.md | 2 +- CLAUDE.md | 2 +- README.md | 2 +- cli/Makefile | 2 +- cli/go.mod | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index b1fd502..e8df8b4 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -7,7 +7,7 @@ stackctl is a Go CLI tool (Cobra) for managing Kubernetes stack deployments. It ## Architecture - All code lives under `cli/` — the Go module root (`cli/go.mod`). Run Go commands from `cli/`. -- Go version: see `cli/go.mod` (1.26.10 or later). CI uses `go-version-file: cli/go.mod`. +- Go version: see `cli/go.mod` (1.27.3 or later). CI uses `go-version-file: cli/go.mod`. - Commands: `cli/cmd/` (one file per command group, subcommands registered in `init()`) - HTTP client: `cli/pkg/client/client.go` (REST), `cli/pkg/client/auth.go` (session renewal), `cli/pkg/client/websocket.go` (deployment log streaming, stream dial, close codes, keepalive), `cli/pkg/client/portforward.go` (port-forward tunnel) - Types: `cli/pkg/types/types.go` (client-side structs matching API responses) diff --git a/CLAUDE.md b/CLAUDE.md index 26142d3..853997e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,7 +4,7 @@ Go CLI tool (Cobra + yaml.v3) for managing Kubernetes stack deployments. Pure API client — talks to the [k8s-stack-manager](https://github.com/omattsson/k8s-stack-manager) backend. No backend logic, no frontend, no database, no direct K8s interaction. -Go version: see `cli/go.mod` (1.26.10 or later); the Dockerfile builder image may be newer. +Go version: see `cli/go.mod` (1.27.3 or later); the Dockerfile builder image may be newer. ## Project Structure diff --git a/README.md b/README.md index 67cab3d..fdbe4ae 100644 --- a/README.md +++ b/README.md @@ -746,7 +746,7 @@ stackctl completion powershell | Out-String | Invoke-Expression ### Prerequisites -- Go version: see `cli/go.mod` (1.26.10 or later) +- Go version: see `cli/go.mod` (1.27.3 or later) - A running [k8s-stack-manager](https://github.com/omattsson/k8s-stack-manager) backend for integration/e2e tests (`make dev` in that repo) ### Getting Started diff --git a/cli/Makefile b/cli/Makefile index 38e7b3f..2bbb302 100644 --- a/cli/Makefile +++ b/cli/Makefile @@ -6,7 +6,7 @@ COMMIT ?= $(shell git rev-parse --short HEAD 2>/dev/null || echo "none") DATE ?= $(shell date -u +%Y-%m-%dT%H:%M:%SZ) LDFLAGS = -s -w -X main.version=$(VERSION) -X main.commit=$(COMMIT) -X main.date=$(DATE) -STATICCHECK_VERSION ?= 2026.1 +STATICCHECK_VERSION ?= 2026.2.2 # Detect Windows for .exe extension ifdef GOOS diff --git a/cli/go.mod b/cli/go.mod index 1376e72..a16e882 100644 --- a/cli/go.mod +++ b/cli/go.mod @@ -1,6 +1,6 @@ module github.com/omattsson/stackctl/cli -go 1.26.10 +go 1.27.3 require ( github.com/gorilla/websocket v1.5.3 From 437ab7a445a4a22db5db11db0330e53150d77051 Mon Sep 17 00:00:00 2001 From: Olof Mattsson <14146601+omattsson@users.noreply.github.com> Date: Fri, 30 Oct 2026 11:02:43 +0100 Subject: [PATCH 6/7] build: lint uses the staticcheck that make tools installs make lint looked up staticcheck on PATH and did not check its version. With GOBIN not on PATH, lint failed after make tools. A stale binary from an older pin ran without a warning. Co-Authored-By: Claude Opus 5.5 --- cli/Makefile | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cli/Makefile b/cli/Makefile index 2bbb302..842d0d0 100644 --- a/cli/Makefile +++ b/cli/Makefile @@ -7,6 +7,8 @@ DATE ?= $(shell date -u +%Y-%m-%dT%H:%M:%SZ) LDFLAGS = -s -w -X main.version=$(VERSION) -X main.commit=$(COMMIT) -X main.date=$(DATE) STATICCHECK_VERSION ?= 2026.2.2 +# Use the binary that `make tools` installs, so lint works when GOBIN is not on PATH. +STATICCHECK := $(or $(shell go env GOBIN),$(shell go env GOPATH)/bin)/staticcheck # Detect Windows for .exe extension ifdef GOOS @@ -44,11 +46,11 @@ test: lint: go vet ./... - @if ! command -v staticcheck >/dev/null 2>&1; then \ - echo "Error: staticcheck is not installed. Run 'make tools' to install staticcheck@$(STATICCHECK_VERSION)"; \ + @if ! $(STATICCHECK) -version 2>/dev/null | grep -qF "staticcheck $(STATICCHECK_VERSION) "; then \ + echo "Error: $(STATICCHECK) is missing or not version $(STATICCHECK_VERSION). Run 'make tools'"; \ exit 1; \ fi - staticcheck ./... + $(STATICCHECK) ./... coverage: go test ./... -coverprofile=coverage.out From f2f4ceb1366f9d2909ecbfa12d0076861b4fd8b9 Mon Sep 17 00:00:00 2001 From: Olof Mattsson <14146601+omattsson@users.noreply.github.com> Date: Fri, 30 Oct 2026 11:06:04 +0100 Subject: [PATCH 7/7] ci: govulncheck v1.9.0 for Go 1.27 govulncheck v1.1.4 uses x/tools v0.29.0, which panics on Go 1.27 code (unexpected expr: *ast.KeyValueExpr). Co-Authored-By: Claude Opus 5.5 --- .github/workflows/security.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index cbe6d35..1d8f3cb 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -27,7 +27,7 @@ jobs: cache-dependency-path: cli/go.sum - name: Install govulncheck - run: go install golang.org/x/vuln/cmd/govulncheck@v1.1.4 + run: go install golang.org/x/vuln/cmd/govulncheck@v1.9.0 - name: Run govulncheck working-directory: cli