From ac17212ecece7e6255a98dda51ed136517c06af4 Mon Sep 17 00:00:00 2001 From: omattsson Date: Sat, 31 Oct 2026 08:51:59 +0100 Subject: [PATCH] fix: port-forward 429 handling, quick-deploy exit code, exec hints, live tests Fixes from a live test run against k8s-stack-manager on Docker Desktop. - stack port-forward: a 429 tunnel_limit gets 3 dial retries (0.5, 1, 2 s) for tunnels that drain or close; the rate limiter's 429 is not retried. 3 connections in a row refused with 429 stop the command only while no other connection of the command dials or tunnels, so a full server tunnel limit never closes working tunnels (8 parallel downloads against a limit of 5: 0 of 8 completed before, 8 of 8 now). - template quick-deploy: exits 1 when the returned stack has status error. Without a log_id the deploy did not start ("fix the cause, then run 'stackctl stack deploy '"); with one it started and failed. Server fields are sanitized. QuickDeployTemplateLog returns the log_id. - stack exec: refusal hints no longer repeat the server message; without a server text the hint says what the code means. - Live tests: FullLifecycle creates a stack from the instantiated definition; FullLifecycle and BulkOperations wait for each operation; the cleanup waits until no operation runs and only a 404 counts as deleted; waitStackStatus fails fast on every failed status. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- README.md | 2 +- cli/cmd/agent_guide.md | 5 +- cli/cmd/stack_exec_test.go | 8 +- cli/cmd/stack_portforward.go | 76 +++++++++-- cli/cmd/stack_portforward_test.go | 179 +++++++++++++++++++++---- cli/cmd/template.go | 31 ++++- cli/cmd/template_test.go | 81 +++++++++++ cli/pkg/client/client.go | 29 +++- cli/pkg/client/exec_test.go | 42 +++--- cli/pkg/client/websocket.go | 48 +++++-- cli/test/e2e/cli_e2e_test.go | 5 +- cli/test/live/live_test.go | 129 ++++++++++++++---- cli/test/live/portforward_live_test.go | 4 +- 14 files changed, 525 insertions(+), 116 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 7bd5d25..77295c2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -200,7 +200,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; 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` (`checkStreamTransport`, shared with `stack exec`; "insecure" is `effectiveInsecure()` in `root.go`, the same check as the TLS warning and `applyInsecureTLS`). 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. +- `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; a 429 `tunnel_limit` first gets 3 dial retries (`portForward429Delays`: 0.5, 1, 2 s, for tunnels that drain or close; the rate limiter's 429 is not retried); 3 connections in a row refused with 429, each while no other connection of the command dials or tunnels (`portForwarder.active`, decremented before the refused local connection closes; a refusal with others active resets `rateLimited`), stop the command (`portForwardMax429`), so a full server tunnel limit never closes working tunnels). 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` (`checkStreamTransport`, shared with `stack exec`; "insecure" is `effectiveInsecure()` in `root.go`, the same check as the TLS warning and `applyInsecureTLS`). 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 8fa6744..07c706a 100644 --- a/README.md +++ b/README.md @@ -310,7 +310,7 @@ 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 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. +- Errors that close only one connection: the rate limit or the tunnel limit (429; the tunnel limit, code `tunnel_limit`, first gets 3 dial retries within 3.5 s, while a tunnel that closes or drains frees its server slot), 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 open tunnels stay open. 3 connections in a row refused with 429 while no other connection of the command is open 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. diff --git a/cli/cmd/agent_guide.md b/cli/cmd/agent_guide.md index 607a3da..e2dd9f6 100644 --- a/cli/cmd/agent_guide.md +++ b/cli/cmd/agent_guide.md @@ -71,7 +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. +# A 429 closes only that connection (the server tunnel limit is retried 3 times first). +# 3 refused connections in a row while no other connection is open 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). @@ -109,7 +110,7 @@ Paged lists (`stack list`, `definition list`, `template list`) return one page a ## Long-running operations -`stack deploy`, `stop`, `clean`, `rollback`, `template quick-deploy` and the `bulk` commands return when the server has accepted the operation; the work continues on the server (a `bulk` result `success` means that the operation started). To wait for the result: +`stack deploy`, `stop`, `clean`, `rollback`, `template quick-deploy` and the `bulk` commands return when the server has accepted the operation; the work continues on the server (a `bulk` result `success` means that the operation started). When `template quick-deploy` creates the stack but its deploy cannot start (for example the namespace of an earlier stack with the same name is still terminating), it prints the stack (status `error`) and exits 1 with the reason: fix the cause, then run `stackctl stack deploy `; do not run the quick deploy again (the stack exists). To wait for the result: - add `--follow` (`-f`, only `stack deploy`, `stop`, `clean` and `rollback`): stackctl streams the log and exits when the stack reaches a final status. With `-o json` the log goes to stderr and stdout gets the stack after the operation (also after a failed one); - or run `stackctl stack watch --id [,...]`, which exits when every listed stack reaches a final status; diff --git a/cli/cmd/stack_exec_test.go b/cli/cmd/stack_exec_test.go index 6f8e9c8..79a8009 100644 --- a/cli/cmd/stack_exec_test.go +++ b/cli/cmd/stack_exec_test.go @@ -365,8 +365,8 @@ func TestStackExecCmd_ExitMapping(t *testing.T) { handler: func(ws *websocket.Conn) { wsCloseAndDrain(ws, websocket.CloseGoingAway, "server shutdown") }, wantMsg: []string{"exec closed: the server is shutting down; the server did not start the command"}}, {name: "403 container not allowed", args: []string{"42", "web", "-c", "php", "--", "ls"}, wantCode: 255, - refuse: jsonRefusal(http.StatusForbidden, "container_not_allowed", "Container does not allow exec"), - wantMsg: []string{"exec refused by the server (HTTP 403)", `Container "php" does not allow exec`}}, + refuse: jsonRefusal(http.StatusForbidden, "container_not_allowed", "The container is not allowed for exec"), + wantMsg: []string{`exec refused by the server (HTTP 403): The container is not allowed for exec: "php". Use -c with a container that allows exec, or ask an administrator which containers do.`}}, {name: "404 feature disabled", wantCode: 255, refuse: jsonRefusal(http.StatusNotFound, "feature_disabled", "This feature is disabled"), wantMsg: []string{"The server has exec turned off"}}, @@ -377,8 +377,8 @@ func TestStackExecCmd_ExitMapping(t *testing.T) { }, wantMsg: []string{"the server has no exec route (HTTP 404); upgrade the server to " + client.ExecServerVersion}}, {name: "503 no ready pod", wantCode: 255, - refuse: jsonRefusal(http.StatusServiceUnavailable, "no_ready_pod", "No ready pod"), - wantMsg: []string{"HTTP 503", "no running and ready pod"}}, + refuse: jsonRefusal(http.StatusServiceUnavailable, "no_ready_pod", "No ready pod for the chart"), + wantMsg: []string{"exec refused by the server (HTTP 503): No ready pod for the chart. Check 'stackctl stack pods', then try again."}}, {name: "unknown stack name", args: []string{"nope", "web", "--", "ls"}, wantCode: 255, handler: exitWith(0, "", ""), wantMsg: []string{`no stack found with name "nope"`}}, diff --git a/cli/cmd/stack_portforward.go b/cli/cmd/stack_portforward.go index d5fa342..80d8e8b 100644 --- a/cli/cmd/stack_portforward.go +++ b/cli/cmd/stack_portforward.go @@ -17,6 +17,7 @@ import ( "syscall" "time" + "github.com/gorilla/websocket" "github.com/omattsson/stackctl/cli/pkg/client" "github.com/spf13/cobra" ) @@ -33,11 +34,20 @@ 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 is the number of connections in a row that the + // server refused with 429 (after the retries of portForward429Delays), + // each while no other connection of the command was open, that stops + // the command. portForwardMax429 = 3 ) +// portForward429Delays are the waits before the dial retries of one local +// connection that the server refused with 429 tunnel_limit (its tunnel +// limit per user; the API rate limiter's 429 is not retried). A tunnel of +// this command that ends or drains (up to client.DrainTimeout after its +// local client closed) frees a server slot within them. Tests shorten them. +var portForward429Delays = []time.Duration{500 * time.Millisecond, time.Second, 2 * time.Second} + // portForwardSignals end the command like Ctrl-C. var portForwardSignals = []os.Signal{os.Interrupt, syscall.SIGTERM} @@ -80,10 +90,11 @@ 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 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. +(429; the tunnel limit gets 3 dial retries within 3.5 s first), 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. The open +tunnels stay open. 3 connections in a row refused with 429 while no other +connection of the command is open stop the command. Security: - Anyone who can reach the local port uses your access to the stack. @@ -337,6 +348,12 @@ func portForwardFatal(err error) bool { (apiErr.StatusCode == http.StatusUnauthorized || apiErr.StatusCode == http.StatusForbidden) } +// isTunnelLimit reports a handshake refused with 429 tunnel_limit. +func isTunnelLimit(err error) bool { + var hs *client.WSHandshakeError + return errors.As(err, &hs) && hs.StatusCode == http.StatusTooManyRequests && hs.Code == client.StreamCodeTunnelLimit +} + // isRateLimited reports a handshake refused with 429. func isRateLimited(err error) bool { var hs *client.WSHandshakeError @@ -423,9 +440,11 @@ 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 counts the connections in a row refused with 429 while + // no other connection was active. rateLimited atomic.Int32 + // active counts the connections that dial or tunnel. + active atomic.Int32 } // serve accepts connections until ctx ends or an error stops the command. @@ -504,11 +523,45 @@ func (pf *portForwarder) stop(err error) { } } +// dial opens the tunnel of one connection. A 429 refusal gets the retries +// of portForward429Delays; the local client waits meanwhile. +func (pf *portForwarder) dial(ctx context.Context) (*websocket.Conn, error) { + ws, err := pf.client.DialPortForward(ctx, pf.id, pf.service, pf.port) + for _, d := range portForward429Delays { + if err == nil || !isTunnelLimit(err) { + break + } + select { + case <-ctx.Done(): + return nil, err + case <-time.After(d): + } + ws, err = pf.client.DialPortForward(ctx, pf.id, pf.service, pf.port) + } + return ws, err +} + // 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) + pf.active.Add(1) + ws, err := pf.dial(ctx) if err != nil { + // Before the local close: a client that connects again after the + // close must not count this connection as active. + others := pf.active.Add(-1) + limited := isRateLimited(err) + stopNow := false + if limited { + // A refusal while other connections of this command dial or + // tunnel closes only this one: those hold (or wait for) the + // server slots and keep working. + if others > 0 { + pf.rateLimited.Store(0) + } else { + stopNow = pf.rateLimited.Add(1) >= portForwardMax429 + } + } _ = local.Close() if ctx.Err() != nil { return @@ -517,13 +570,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)) + if stopNow { + pf.stop(fmt.Errorf("port-forward stopped: the server refused %d connections in a row with 429 (rate limit or tunnel limit) while no other connection was open; 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 } + defer pf.active.Add(-1) 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) diff --git a/cli/cmd/stack_portforward_test.go b/cli/cmd/stack_portforward_test.go index 718694b..2557015 100644 --- a/cli/cmd/stack_portforward_test.go +++ b/cli/cmd/stack_portforward_test.go @@ -659,13 +659,69 @@ func dialUntilClosed(t *testing.T, addr string) { 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 { +// fast429Retries shortens the dial retries after a 429. +func fast429Retries(t *testing.T) { + t.Helper() + old := portForward429Delays + portForward429Delays = []time.Duration{time.Millisecond, time.Millisecond, time.Millisecond} + t.Cleanup(func() { portForward429Delays = old }) +} + +// pfRoundTrip writes "ok" on conn and reads the echo. +func pfRoundTrip(t *testing.T, conn net.Conn) { + t.Helper() + _, 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) +} + +// refuse429Unless returns a refusal function for newPFTestServer: 429 for +// every dial while allow is false. +func refuse429Unless(dials *atomic.Int32, allow *atomic.Bool) func(http.ResponseWriter) bool { + return func(w http.ResponseWriter) bool { dials.Add(1) + if allow.Load() { + return false + } return jsonRefusal(http.StatusTooManyRequests, "tunnel_limit", "Too many tunnels")(w) + } +} + +func TestPortForwardCmd_429RetrySucceeds(t *testing.T) { + fast429Retries(t) + var dials atomic.Int32 + srv := newPFTestServer(t, func(w http.ResponseWriter) bool { + if dials.Add(1) <= 2 { + return jsonRefusal(http.StatusTooManyRequests, "tunnel_limit", "Too many tunnels")(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) + + conn, err := net.Dial("tcp", addr) + require.NoError(t, err) + pfRoundTrip(t, conn) + _ = conn.Close() + + cancel() + require.NoError(t, waitDone(t, done)) + assert.Equal(t, int32(3), dials.Load(), "two retries, then the upgrade") + assert.Equal(t, int32(1), srv.upgrades.Load()) + assert.NotContains(t, stderr.String(), "HTTP 429", "a retried refusal is not a failed connection") +} + +func TestPortForwardCmd_Repeated429StopsCommand(t *testing.T) { + fast429Retries(t) + var dials atomic.Int32 + var allow atomic.Bool + srv := newPFTestServer(t, refuse429Unless(&dials, &allow)) + stdout, stderr := setupPortForwardTest(t, srv.URL) done := startPortForward(context.Background(), "42", "mysql", "0:3306") addr := waitForwarding(t, stdout, done) @@ -674,55 +730,122 @@ func TestPortForwardCmd_Repeated429StopsCommand(t *testing.T) { } 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.Contains(t, err.Error(), "the server refused 3 connections in a row with 429 (rate limit or tunnel limit) while no other connection was open; wait and try again") + assert.Equal(t, int32(3*(1+len(portForward429Delays))), dials.Load(), "each connection retries the dial") 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") + assert.Equal(t, 2, strings.Count(stderr.String(), "HTTP 429"), "one line for each refused connection 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. + // refused, refused, upgrade, refused, refused: never 3 in a row. + fast429Retries(t) 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) - }) + var allow atomic.Bool + srv := newPFTestServer(t, refuse429Unless(&dials, &allow)) 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) + dialUntilClosed(t, addr) + dialUntilClosed(t, addr) + allow.Store(true) + conn, err := net.Dial("tcp", addr) + require.NoError(t, err) + pfRoundTrip(t, conn) + _ = conn.Close() + allow.Store(false) + require.Eventually(t, func() bool { return strings.Contains(stderr.String(), "Connection 3 closed") }, 5*time.Second, 10*time.Millisecond) + dialUntilClosed(t, addr) + dialUntilClosed(t, addr) + + select { + case err := <-done: + t.Fatalf("command ended: %v", err) + default: } - for round := 0; round < 2; round++ { - dialUntilClosed(t, addr) + cancel() + require.NoError(t, waitDone(t, done)) + assert.Equal(t, int32(1), srv.upgrades.Load()) + assert.Equal(t, 4, strings.Count(stderr.String(), "HTTP 429")) +} + +// The server tunnel limit is full with tunnels of this command: further +// connections are refused, the open tunnel keeps working and the command +// keeps running. +func TestPortForwardCmd_429WithOpenTunnelKeepsCommand(t *testing.T) { + fast429Retries(t) + var dials atomic.Int32 + var allow atomic.Bool + allow.Store(true) + srv := newPFTestServer(t, refuse429Unless(&dials, &allow)) + 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) + + open, err := net.Dial("tcp", addr) + require.NoError(t, err) + defer open.Close() + pfRoundTrip(t, open) + allow.Store(false) + + for i := 0; i < portForwardMax429+1; i++ { dialUntilClosed(t, addr) - roundTrip() } select { case err := <-done: t.Fatalf("command ended: %v", err) default: } + pfRoundTrip(t, open) + assert.Equal(t, portForwardMax429+1, strings.Count(stderr.String(), "HTTP 429")) + + // The refusals while the tunnel was open do not count: after it + // closes, one refusal does not stop the command; 3 in a row do. + _ = open.Close() + require.Eventually(t, func() bool { return strings.Contains(stderr.String(), "Connection 1 closed") }, 5*time.Second, 10*time.Millisecond) + dialUntilClosed(t, addr) + select { + case err := <-done: + t.Fatalf("command ended after one refusal without a tunnel: %v", err) + case <-time.After(100 * time.Millisecond): + } + dialUntilClosed(t, addr) + dialUntilClosed(t, addr) + err = waitDone(t, done) + require.Error(t, err) + assert.Contains(t, err.Error(), "refused 3 connections in a row") +} + +// A 429 of the API rate limiter (no tunnel_limit code) is not retried: the +// retries would only add requests to the rate limit window. +func TestPortForwardCmd_RateLimiter429NotRetried(t *testing.T) { + fast429Retries(t) + var dials atomic.Int32 + srv := newPFTestServer(t, func(w http.ResponseWriter) bool { + dials.Add(1) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusTooManyRequests) + _, _ = w.Write([]byte(`{"error":"Rate limit exceeded"}`)) + return true + }) + 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) + + dialUntilClosed(t, addr) + require.Eventually(t, func() bool { return strings.Contains(stderr.String(), "HTTP 429") }, 5*time.Second, 10*time.Millisecond) + assert.Equal(t, int32(1), dials.Load()) 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")) } func TestPortForwardCmd_InsecureTextsUnchanged(t *testing.T) { diff --git a/cli/cmd/template.go b/cli/cmd/template.go index 8cbca08..2ace2c3 100644 --- a/cli/cmd/template.go +++ b/cli/cmd/template.go @@ -213,6 +213,13 @@ Examples: stackctl template quick-deploy 8c9d0e1f-2a3b-4c5d-9e6f-7a8b9c0d1e2f --name my-stack --stop-at "2026-10-17 18:00" Without --ttl and --stop-at the stack gets the default TTL of the server. + +The command returns when the server has accepted the deploy; run +'stackctl stack watch --id ' to wait for the result. When the server +creates the stack but cannot start its deploy (for example the namespace +of a stack with the same name is still terminating), the stack has status +error: the command prints it and exits 1 with the reason. Fix the cause, +then run 'stackctl stack deploy '. ` + expiryFlagsHelp, Args: cobra.ExactArgs(1), SilenceUsage: true, @@ -246,7 +253,7 @@ Without --ttl and --stop-at the stack gets the default TTL of the server. return err } - instance, err := c.QuickDeployTemplate(id, req) + instance, logID, err := c.QuickDeployTemplateLog(id, req) if err != nil { return templateReleaseError(err, c, id, args[0]) } @@ -254,10 +261,30 @@ Without --ttl and --stop-at the stack gets the default TTL of the server. if err := printInstance(instance); err != nil { return err } - return checkStopApplied("stop-at", expiry.stopAt, instance) + return errors.Join(quickDeployStartError(instance, logID), checkStopApplied("stop-at", expiry.stopAt, instance)) }, } +// quickDeployStartError returns an error when the stack of a quick deploy +// has status error in the response. Without a deployment log (log_id "" +// or absent) the server created the stack but did not start its deploy; +// with one, the deploy started and failed at once. +func quickDeployStartError(inst *types.StackInstance, logID *string) error { + if inst.Status != "error" { + return nil + } + reason := client.SanitizeServerText(inst.ErrorMessage) + if reason == "" { + reason = "the server gave no reason" + } + name, id := client.SanitizeServerText(inst.Name), client.SanitizeServerText(inst.ID) + if logID != nil && *logID != "" { + return fmt.Errorf("the deploy of stack %s (%s) failed: %s; see 'stackctl stack logs %s'", name, id, reason, id) + } + return fmt.Errorf("the stack %s (%s) was created, but its deploy did not start: %s; fix the cause, then run 'stackctl stack deploy %s'", + name, id, reason, id) +} + var templateDeleteCmd = &cobra.Command{ Use: "delete ", Short: "Delete a stack template", diff --git a/cli/cmd/template_test.go b/cli/cmd/template_test.go index c3abd49..d98f56d 100644 --- a/cli/cmd/template_test.go +++ b/cli/cmd/template_test.go @@ -504,6 +504,87 @@ func TestTemplateQuickDeployCmd_JSONOutput(t *testing.T) { assert.Equal(t, "deploying", result.Status) } +func TestTemplateQuickDeployCmd_DeployNotStarted(t *testing.T) { + instance := types.StackInstance{ + Base: types.Base{ID: "61"}, Name: "quick-stack", Status: "error", + ErrorMessage: "Deployment failed: namespace \"stack-quick-stack-admin\" is still terminating; wait for it to be fully removed, then retry", + } + tests := []struct { + name string + format output.Format + quiet bool + check func(t *testing.T, stdout string) + }{ + {name: "table", format: output.FormatTable, check: func(t *testing.T, stdout string) { + assert.Contains(t, stdout, "quick-stack") + assert.Contains(t, stdout, "error") + }}, + {name: "json", format: output.FormatJSON, check: func(t *testing.T, stdout string) { + var got types.StackInstance + require.NoError(t, json.Unmarshal([]byte(stdout), &got)) + assert.Equal(t, "error", got.Status) + }}, + {name: "yaml", format: output.FormatYAML, check: func(t *testing.T, stdout string) { + assert.Contains(t, stdout, "status: error") + }}, + {name: "quiet", format: output.FormatTable, quiet: true, check: func(t *testing.T, stdout string) { + assert.Equal(t, "61\n", stdout) + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusAccepted) + json.NewEncoder(w).Encode(map[string]any{"instance": instance, "log_id": ""}) + })) + defer server.Close() + + buf := setupStackTestCmd(t, server.URL) + printer.Format = tt.format + printer.Quiet = tt.quiet + templateQuickDeployCmd.Flags().Set("name", "quick-stack") + t.Cleanup(func() { templateQuickDeployCmd.Flags().Set("name", "") }) + + err := templateQuickDeployCmd.RunE(templateQuickDeployCmd, []string{"10"}) + require.Error(t, err) + assert.Equal(t, `the stack quick-stack (61) was created, but its deploy did not start: Deployment failed: namespace "stack-quick-stack-admin" is still terminating; wait for it to be fully removed, then retry; fix the cause, then run 'stackctl stack deploy 61'`, err.Error()) + tt.check(t, buf.String()) + }) + } +} + +func TestQuickDeployStartError(t *testing.T) { + t.Parallel() + empty, started := "", "log-1" + tests := []struct { + name string + inst types.StackInstance + logID *string + want string + }{ + {name: "deploying", inst: types.StackInstance{Status: "deploying"}, logID: &started}, + {name: "not started, older server without log_id", inst: types.StackInstance{Base: types.Base{ID: "7"}, Name: "a\x1b[31mb", Status: "error"}, + want: "the stack a [31mb (7) was created, but its deploy did not start: the server gave no reason; fix the cause, then run 'stackctl stack deploy 7'"}, + {name: "not started, empty log_id", inst: types.StackInstance{Base: types.Base{ID: "7\u202e"}, Name: "a", Status: "error", ErrorMessage: "Deployment failed"}, logID: &empty, + want: "the stack a (7) was created, but its deploy did not start: Deployment failed; fix the cause, then run 'stackctl stack deploy 7'"}, + {name: "started and failed at once", inst: types.StackInstance{Base: types.Base{ID: "7"}, Name: "a", Status: "error", ErrorMessage: "pre-deploy hook denied"}, logID: &started, + want: "the deploy of stack a (7) failed: pre-deploy hook denied; see 'stackctl stack logs 7'"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := quickDeployStartError(&tt.inst, tt.logID) + if tt.want == "" { + assert.NoError(t, err) + return + } + require.Error(t, err) + assert.Equal(t, tt.want, err.Error()) + }) + } +} + // ---------- template list auth error ---------- func TestTemplateListCmd_Unauthorized(t *testing.T) { diff --git a/cli/pkg/client/client.go b/cli/pkg/client/client.go index c11d5c1..ccce8e9 100644 --- a/cli/pkg/client/client.go +++ b/cli/pkg/client/client.go @@ -1052,30 +1052,45 @@ func (c *Client) InstantiateTemplate(id string, req *types.InstantiateTemplateRe // The server answers {"instance": {...}, "definition": {...}, "log_id": "..."}; // a body without "instance" is read as the instance itself. func (c *Client) QuickDeployTemplate(id string, req *types.QuickDeployRequest) (*types.StackInstance, error) { + instance, _, err := c.QuickDeployTemplateLog(id, req) + return instance, err +} + +// QuickDeployTemplateLog is QuickDeployTemplate that also returns the +// log_id of the response: "" when the server created the stack but did not +// start its deploy, nil when the response has no log_id (an older server +// that sends only the instance). +func (c *Client) QuickDeployTemplateLog(id string, req *types.QuickDeployRequest) (*types.StackInstance, *string, error) { var raw json.RawMessage err := c.Post(fmt.Sprintf("/api/v1/templates/%s/quick-deploy", id), req, &raw) if err != nil { - return nil, err + return nil, nil, err } var fields map[string]json.RawMessage if err := json.Unmarshal(raw, &fields); err != nil { - return nil, fmt.Errorf("decoding quick-deploy response: %w", err) + return nil, nil, fmt.Errorf("decoding quick-deploy response: %w", err) } if inst, ok := fields["instance"]; ok { var instance *types.StackInstance if err := json.Unmarshal(inst, &instance); err != nil { - return nil, fmt.Errorf("decoding quick-deploy response: %w", err) + return nil, nil, fmt.Errorf("decoding quick-deploy response: %w", err) } if instance == nil { - return nil, fmt.Errorf("decoding quick-deploy response: the instance is null") + return nil, nil, fmt.Errorf("decoding quick-deploy response: the instance is null") } - return instance, nil + var logID *string + if rawLog, ok := fields["log_id"]; ok { + if err := json.Unmarshal(rawLog, &logID); err != nil { + return nil, nil, fmt.Errorf("decoding quick-deploy response: %w", err) + } + } + return instance, logID, nil } var instance types.StackInstance if err := json.Unmarshal(raw, &instance); err != nil { - return nil, fmt.Errorf("decoding quick-deploy response: %w", err) + return nil, nil, fmt.Errorf("decoding quick-deploy response: %w", err) } - return &instance, nil + return &instance, nil, nil } // GetUIConfig returns the public UI config of the server (GET diff --git a/cli/pkg/client/exec_test.go b/cli/pkg/client/exec_test.go index e3b5ed3..e334caa 100644 --- a/cli/pkg/client/exec_test.go +++ b/cli/pkg/client/exec_test.go @@ -465,8 +465,8 @@ func TestExec_HandshakeErrors(t *testing.T) { check func(t *testing.T, hs *WSHandshakeError) }{ {name: "container not allowed names the container", status: 403, container: "php", - body: `{"error":"Container does not allow exec","code":"container_not_allowed"}`, - wantMsg: []string{"exec refused by the server (HTTP 403): Container does not allow exec", `Container "php" does not allow exec`}}, + body: `{"error":"The container is not allowed for exec","code":"container_not_allowed"}`, + wantMsg: []string{`exec refused by the server (HTTP 403): The container is not allowed for exec: "php". Use -c with a container that allows exec, or ask an administrator which containers do.`}}, {name: "container not allowed without -c", status: 403, body: `{"error":"No container allows exec","code":"container_not_allowed"}`, wantMsg: []string{"No container of the chart allows exec"}}, @@ -477,11 +477,17 @@ func TestExec_HandshakeErrors(t *testing.T) { body: `{"error":"You do not have permission","code":"forbidden"}`, wantMsg: []string{"exec refused by the server (HTTP 403): You do not have permission"}}, {name: "service account token", status: 403, - body: `{"error":"Container holds a token","code":"service_account_token"}`, - wantMsg: []string{"service-account token"}}, + body: `{"error":"The container holds a service-account token","code":"service_account_token"}`, + wantMsg: []string{"exec refused by the server (HTTP 403): The container holds a service-account token. The server refuses exec into such a container; ask an administrator."}}, {name: "cluster exec disabled", status: 403, - body: `{"error":"Exec is disabled for the cluster","code":"cluster_exec_disabled"}`, - wantMsg: []string{"The cluster of the stack does not allow exec"}}, + body: `{"error":"This cluster does not allow exec","code":"cluster_exec_disabled"}`, + wantMsg: []string{"exec refused by the server (HTTP 403): This cluster does not allow exec. Ask an administrator."}}, + {name: "cluster exec disabled without a server text", status: 403, + body: `{"code":"cluster_exec_disabled"}`, + wantMsg: []string{"exec refused by the server (HTTP 403): Forbidden. The cluster of the stack does not allow exec. Ask an administrator."}}, + {name: "exec limit without a server text", status: 429, + body: `{"code":"exec_limit"}`, + wantMsg: []string{"exec refused by the server (HTTP 429): Too Many Requests. Too many of your exec sessions are open. Wait until one of your exec sessions ends, then try again."}}, {name: "feature disabled", status: 404, body: `{"error":"This feature is disabled","code":"feature_disabled"}`, wantMsg: []string{"This feature is disabled", "The server has exec turned off"}, @@ -497,15 +503,15 @@ func TestExec_HandshakeErrors(t *testing.T) { body: `{"error":"Chart not found","code":"chart_not_found"}`, wantMsg: []string{"Chart not found", "CHART column of 'stackctl stack pods'"}}, {name: "container not found", status: 404, container: "nope", - body: `{"error":"Container not found","code":"container_not_found"}`, - wantMsg: []string{`runs container "nope"`}}, + body: `{"error":"No ready pod runs the container","code":"container_not_found"}`, + wantMsg: []string{`exec refused by the server (HTTP 404): No ready pod runs the container: "nope". Check the container name and 'stackctl stack pods'.`}}, {name: "container not found without -c", status: 404, - body: `{"error":"No allowed container","code":"container_not_found"}`, - wantMsg: []string{"exec refused by the server (HTTP 404): No allowed container. No ready pod of the chart runs an allowed container."}, + body: `{"error":"No ready pod runs the container","code":"container_not_found"}`, + wantMsg: []string{"exec refused by the server (HTTP 404): No ready pod runs the container. Check 'stackctl stack pods'."}, notMsg: []string{`container ""`}}, {name: "invalid chart", status: 400, - body: `{"error":"Invalid chart name","code":"invalid_chart"}`, - wantMsg: []string{"exec refused by the server (HTTP 400): Invalid chart name. Check the chart name (the Helm release name): the CHART column of 'stackctl stack pods'."}, + body: `{"error":"chart must be a Helm release name (a DNS-1123 name of at most 53 characters)","code":"invalid_chart"}`, + wantMsg: []string{"exec refused by the server (HTTP 400): chart must be a Helm release name (a DNS-1123 name of at most 53 characters). The CHART column of 'stackctl stack pods' shows the chart names."}, check: func(t *testing.T, hs *WSHandshakeError) { assert.False(t, hs.UpgradeDropped(), "a 400 with a code is a refusal") }}, @@ -513,14 +519,14 @@ func TestExec_HandshakeErrors(t *testing.T) { body: `{"error":"Invalid container name","code":"invalid_container"}`, wantMsg: []string{"exec refused by the server (HTTP 400): Invalid container name. Check the container name."}}, {name: "token check forbidden", status: 403, - body: `{"error":"Cannot check the pod","code":"token_check_forbidden"}`, - wantMsg: []string{"exec refused by the server (HTTP 403): Cannot check the pod. The server may not check the pod for a service-account token; ask an administrator."}}, + body: `{"error":"The backend may not read the service account or secrets of the pod","code":"token_check_forbidden"}`, + wantMsg: []string{"exec refused by the server (HTTP 403): The backend may not read the service account or secrets of the pod. Without that right the server cannot check the container for a service-account token and refuses exec; ask an administrator."}}, {name: "no ready pod", status: 503, - body: `{"error":"No ready pod","code":"no_ready_pod"}`, - wantMsg: []string{"exec refused by the server (HTTP 503): No ready pod", "no running and ready pod"}}, + body: `{"error":"No ready pod for the chart","code":"no_ready_pod"}`, + wantMsg: []string{"exec refused by the server (HTTP 503): No ready pod for the chart. Check 'stackctl stack pods', then try again."}}, {name: "exec limit", status: 429, - body: `{"error":"Too many exec sessions","code":"exec_limit"}`, - wantMsg: []string{"Too many of your exec sessions"}}, + body: `{"error":"Too many open exec sessions","code":"exec_limit"}`, + wantMsg: []string{"exec refused by the server (HTTP 429): Too many open exec sessions. Wait until one of your exec sessions ends, then try again."}}, {name: "401 with an api key", status: 401, apiKey: true, body: `{"error":"Invalid API key"}`, wantMsg: []string{"HTTP 401", "Check the API key"}}, diff --git a/cli/pkg/client/websocket.go b/cli/pkg/client/websocket.go index 6c0d95d..5bb9d44 100644 --- a/cli/pkg/client/websocket.go +++ b/cli/pkg/client/websocket.go @@ -481,7 +481,10 @@ type WSHandshakeError struct { // Container is the container that an exec asked for; empty when the // server chooses it. Only used in the message. Container string - err error + // noServerText is true when the body had no error text and Message is + // the HTTP status text: a hint then says what the code means. + noServerText bool + err error } // Error codes of the stream routes (port-forward, exec) of @@ -573,37 +576,59 @@ func (e *WSHandshakeError) streamError() string { return msg } -// execHint is the advice for a refused exec upgrade, or "". +// execMeaning is what an exec refusal code means, for a body without an +// error text (the server message usually says it). +var execMeaning = map[string]string{ + StreamCodeServiceAccountToken: "The container holds a service-account token", + StreamCodeTokenCheckForbidden: "The server may not read the service account or secrets of the pod", + StreamCodeClusterExecDisabled: "The cluster of the stack does not allow exec", + StreamCodeContainerNotFound: "No ready pod of the chart runs the container", + StreamCodeNoReadyPod: "The chart has no running and ready pod", + StreamCodeExecLimit: "Too many of your exec sessions are open", +} + +// execHint is the advice for a refused exec upgrade, or "". The server +// message comes first, so a hint does not repeat it; without a server text +// the hint first says what the code means. func (e *WSHandshakeError) execHint() string { + hint := e.execAdvice() + if meaning, ok := execMeaning[e.Code]; ok && e.noServerText && hint != "" { + return ". " + meaning + hint + } + return hint +} + +// execAdvice is the advice part of execHint. +func (e *WSHandshakeError) execAdvice() string { container := sanitizeServerMessage(e.Container) switch e.Code { case StreamCodeContainerNotAllowed: if container != "" { - return fmt.Sprintf(". Container %q does not allow exec; choose another container, or ask an administrator.", container) + return fmt.Sprintf(": %q. Use -c with a container that allows exec, or ask an administrator which containers do.", container) } return ". No container of the chart allows exec; ask an administrator." case StreamCodeServiceAccountToken: - return ". The container holds a service-account token, so the server refuses exec into it; ask an administrator." + return ". The server refuses exec into such a container; ask an administrator." case StreamCodeTokenCheckForbidden: - return ". The server may not check the pod for a service-account token; ask an administrator." + return ". Without that right the server cannot check the container for a service-account token and refuses exec; ask an administrator." case StreamCodeClusterExecDisabled: - return ". The cluster of the stack does not allow exec; ask an administrator." + return ". Ask an administrator." case StreamCodeInvalidChart: // The server checks the name (DNS-1123); stackctl does not repeat it. - return ". Check the chart name (the Helm release name): the CHART column of 'stackctl stack pods'." + return ". The CHART column of 'stackctl stack pods' shows the chart names." case StreamCodeInvalidContainer: return ". Check the container name." case StreamCodeChartNotFound: return ". Check the chart name: the CHART column of 'stackctl stack pods'." case StreamCodeContainerNotFound: if container != "" { - return fmt.Sprintf(". No ready pod of the chart runs container %q; check the container name.", container) + return fmt.Sprintf(": %q. Check the container name and 'stackctl stack pods'.", container) } - return ". No ready pod of the chart runs an allowed container." + return ". Check 'stackctl stack pods'." case StreamCodeNoReadyPod: - return ". The chart has no running and ready pod; check 'stackctl stack pods', then try again." + return ". Check 'stackctl stack pods', then try again." case StreamCodeExecLimit: - return ". Too many of your exec sessions are open; wait until one ends, then try again." + return ". Wait until one of your exec sessions ends, then try again." } return "" } @@ -682,6 +707,7 @@ func handshakeError(resp *http.Response, err error, apiKey bool) *WSHandshakeErr } } if hsErr.Message == "" { + hsErr.noServerText = true hsErr.Message = http.StatusText(resp.StatusCode) } if hsErr.Message == "" { diff --git a/cli/test/e2e/cli_e2e_test.go b/cli/test/e2e/cli_e2e_test.go index 30da744..f533dc4 100644 --- a/cli/test/e2e/cli_e2e_test.go +++ b/cli/test/e2e/cli_e2e_test.go @@ -3089,7 +3089,7 @@ func TestE2E_StackExecExitCodes(t *testing.T) { server, _ := startE2EExecMockServer(t, func(w http.ResponseWriter) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusForbidden) - _ = json.NewEncoder(w).Encode(map[string]string{"error": "Container does not allow exec", "code": "container_not_allowed"}) + _ = json.NewEncoder(w).Encode(map[string]string{"error": "The container is not allowed for exec", "code": "container_not_allowed"}) }) dir := t.TempDir() setupE2EStackContext(t, dir, server.URL) @@ -3099,8 +3099,7 @@ func TestE2E_StackExecExitCodes(t *testing.T) { require.ErrorAs(t, err, &exitErr) assert.Equal(t, 255, exitErr.ExitCode()) assert.Empty(t, stdout) - assert.Contains(t, stderr, "exec refused by the server (HTTP 403): Container does not allow exec") - assert.Contains(t, stderr, `Container "php" does not allow exec`) + assert.Contains(t, stderr, `exec refused by the server (HTTP 403): The container is not allowed for exec: "php". Use -c with a container that allows exec, or ask an administrator which containers do.`) }) t.Run("usage error exits 1", func(t *testing.T) { diff --git a/cli/test/live/live_test.go b/cli/test/live/live_test.go index ae9d263..bda5b21 100644 --- a/cli/test/live/live_test.go +++ b/cli/test/live/live_test.go @@ -3,6 +3,7 @@ package live import ( + "errors" "fmt" "net/http" "os" @@ -74,24 +75,74 @@ func login(t *testing.T, c *client.Client) *types.LoginResponse { return resp } -// cleanupInstance registers a t.Cleanup that best-effort deletes the instance. +// liveOpTimeout bounds the wait for one deploy, stop, clean or delete of a +// heavy live test. +const liveOpTimeout = 10 * time.Minute + +// cleanupInstance registers a t.Cleanup that best-effort deletes the +// instance. It first waits until no operation runs (the server refuses a +// delete during one); a delete of a deployed stack cleans it first. func cleanupInstance(t *testing.T, c *client.Client, id string) { t.Helper() t.Cleanup(func() { - // Best-effort: stop -> clean -> delete. Ignore errors because the - // instance may already be in a terminal state. - _, _ = c.StopStack(id) - _, _ = c.CleanStack(id) - _ = c.DeleteStack(id) + deadline := time.Now().Add(liveOpTimeout) + for time.Now().Before(deadline) { + inst, err := c.GetStack(id) + if isNotFound(err) { + return // already deleted + } + if err == nil && client.IsTerminalStatus(inst.Status) { + break + } + if err != nil { + t.Logf("cleanup: get stack %s: %v", id, err) + } + time.Sleep(2 * time.Second) + } + if err := c.DeleteStack(id); err != nil { + t.Logf("cleanup: delete stack %s: %v", id, err) + return + } + waitStackGone(t, c, id, liveOpTimeout) }) } +// isNotFound reports a 404 of the API. +func isNotFound(err error) bool { + var apiErr *client.APIError + return errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound +} + +// waitStackGone waits until GET of the instance returns 404 (deleted). +// Other errors (a network error, a 5xx) are retried until the timeout. +func waitStackGone(t *testing.T, c *client.Client, id string, timeout time.Duration) { + t.Helper() + deadline := time.Now().Add(timeout) + for { + inst, err := c.GetStack(id) + if isNotFound(err) { + return + } + if time.Now().After(deadline) { + if err != nil { + t.Errorf("stack %s: no 404 after %s (last error: %v)", id, timeout, err) + } else { + t.Errorf("stack %s still exists after %s (status %q)", id, timeout, inst.Status) + } + return + } + time.Sleep(2 * time.Second) + } +} + // TestLiveWorkflow_FullLifecycle exercises the complete deploy-stop-clean- // delete path against a real backend by actually rolling out a stack // instance. It pulls images, allocates PVs, and burns cluster quota — so // it's gated behind STACKCTL_LIVE_HEAVY=1 to keep it off the default // suite. CI smoke + per-endpoint wire-contract coverage lives in the -// other *_live_test.go files in this package. +// other *_live_test.go files in this package. Each operation is awaited +// before the next one: the server refuses a clean during a stop and a +// delete during an operation. func TestLiveWorkflow_FullLifecycle(t *testing.T) { if os.Getenv("STACKCTL_LIVE_HEAVY") != "1" { t.Skip("set STACKCTL_LIVE_HEAVY=1 to run real-workload lifecycle tests (~5–10 min, requires cluster capacity)") @@ -115,17 +166,28 @@ func TestLiveWorkflow_FullLifecycle(t *testing.T) { tmpl := templates.Data[0] t.Logf("Using template %s (%s)", tmpl.ID, tmpl.Name) - // Step 3: Instantiate template - t.Log("Step 3: Instantiate template") - instance, err := c.InstantiateTemplate(tmpl.ID, &types.InstantiateTemplateRequest{ - Name: fmt.Sprintf("live-test-%d", time.Now().UnixMilli()), + // Step 3: Instantiate the template (a definition), then create a stack + // from the definition. + t.Log("Step 3: Instantiate template and create a stack") + name := fmt.Sprintf("live-test-%d", time.Now().UnixMilli()) + def, err := c.InstantiateTemplate(tmpl.ID, &types.InstantiateTemplateRequest{ + Name: name, Branch: "main", }) require.NoError(t, err, "instantiate template") + require.NotEmpty(t, def.ID, "definition ID must be set") + deleteDefinitionIfExists(t, c, def.ID) + instance, err := c.CreateStack(&types.CreateStackRequest{ + Name: name, + StackDefinitionID: def.ID, + Branch: "main", + }) + require.NoError(t, err, "create stack") require.NotEmpty(t, instance.ID, "instance ID must be set") - t.Logf("Created instance %s (%s)", instance.ID, instance.Name) + t.Logf("Created definition %s and instance %s (%s)", def.ID, instance.ID, instance.Name) - // Register cleanup so resources are removed even on failure. + // Register cleanup so resources are removed even on failure. It runs + // before the definition cleanup (t.Cleanup is last in, first out). cleanupInstance(t, c, instance.ID) // Step 4: Deploy @@ -133,6 +195,7 @@ func TestLiveWorkflow_FullLifecycle(t *testing.T) { deployLog, err := c.DeployStack(instance.ID) require.NoError(t, err, "deploy stack") assert.NotEmpty(t, deployLog.LogID, "deploy log ID must be set") + waitStackStatus(t, c, instance.ID, "running", liveOpTimeout) // Step 5: Check status t.Log("Step 5: Check status") @@ -145,15 +208,18 @@ func TestLiveWorkflow_FullLifecycle(t *testing.T) { } t.Logf("Status: %s, charts: %d, pods: %d", status.Status, len(status.Charts), pods) - // Step 6–7: Set overrides and redeploy (only if template has charts). - // Note: templates.Data[0] from ListTemplates doesn't carry charts unless - // the backend includes them in the list response, so this branch usually - // gets skipped against the live backend (charts are populated by a - // per-template GET, not on the list endpoint). - if len(tmpl.Charts) == 0 { - t.Log("Step 6–7: Skipped — template list response carries no charts") + // Step 6–7: Set overrides and redeploy. Overrides are per chart config + // of the definition. + charts := def.Charts + if len(charts) == 0 { + full, err := c.GetDefinition(def.ID) + require.NoError(t, err, "get definition") + charts = full.Charts + } + if len(charts) == 0 { + t.Log("Step 6–7: Skipped — the definition has no charts") } else { - chartID := tmpl.Charts[0].ID + chartID := charts[0].ID t.Log("Step 6: Set overrides") _, err = c.SetValueOverride(instance.ID, chartID, &types.SetValueOverrideRequest{ @@ -187,6 +253,7 @@ func TestLiveWorkflow_FullLifecycle(t *testing.T) { redeployLog, err := c.DeployStack(instance.ID) require.NoError(t, err, "redeploy stack") assert.NotEmpty(t, redeployLog.LogID, "redeploy log ID must be set") + waitStackStatus(t, c, instance.ID, "running", liveOpTimeout) } // Step 8: View logs @@ -200,12 +267,14 @@ func TestLiveWorkflow_FullLifecycle(t *testing.T) { stopLog, err := c.StopStack(instance.ID) require.NoError(t, err, "stop stack") assert.NotEmpty(t, stopLog.LogID, "stop log ID must be set") + waitStackStatus(t, c, instance.ID, "stopped", liveOpTimeout) // Step 10: Clean t.Log("Step 10: Clean") cleanLog, err := c.CleanStack(instance.ID) require.NoError(t, err, "clean stack") assert.NotEmpty(t, cleanLog.LogID, "clean log ID must be set") + waitStackStatus(t, c, instance.ID, "draft", liveOpTimeout) // Step 11: Delete t.Log("Step 11: Delete") @@ -214,8 +283,7 @@ func TestLiveWorkflow_FullLifecycle(t *testing.T) { // Step 12: Verify deleted — GetStack should return an error. t.Log("Step 12: Verify deleted") - _, err = c.GetStack(instance.ID) - assert.Error(t, err, "GetStack on deleted instance should fail") + waitStackGone(t, c, instance.ID, liveOpTimeout) } // TestLiveWorkflow_BulkOperations deploys 3 real stack instances and bulk- @@ -223,7 +291,8 @@ func TestLiveWorkflow_FullLifecycle(t *testing.T) { // plus golden-db pulls. Wire-shape coverage for bulk lives in // TestLiveBulk_StackInstanceWireShape / TestLiveBulk_TemplateRoundTrip; // this test only adds the actual deploy/stop/clean assertions, so it's -// gated behind STACKCTL_LIVE_HEAVY=1. +// gated behind STACKCTL_LIVE_HEAVY=1. Each bulk operation is awaited on +// every instance before the next one starts. func TestLiveWorkflow_BulkOperations(t *testing.T) { if os.Getenv("STACKCTL_LIVE_HEAVY") != "1" { t.Skip("set STACKCTL_LIVE_HEAVY=1 to run real-workload bulk tests (creates 3 instances)") @@ -262,6 +331,12 @@ func TestLiveWorkflow_BulkOperations(t *testing.T) { for _, id := range ids { cleanupInstance(t, c, id) } + waitAll := func(status string) { + t.Helper() + for _, id := range ids { + waitStackStatus(t, c, id, status, liveOpTimeout) + } + } // Step 3: Bulk deploy t.Log("Step 3: Bulk deploy") @@ -271,6 +346,7 @@ func TestLiveWorkflow_BulkOperations(t *testing.T) { for _, r := range deployResp.Results { assert.True(t, r.Success(), "deploy should succeed for id %s: %s", r.ID(), r.Error) } + waitAll("running") // Step 4: Bulk stop t.Log("Step 4: Bulk stop") @@ -280,6 +356,7 @@ func TestLiveWorkflow_BulkOperations(t *testing.T) { for _, r := range stopResp.Results { assert.True(t, r.Success(), "stop should succeed for id %s: %s", r.ID(), r.Error) } + waitAll("stopped") // Step 5: Bulk clean t.Log("Step 5: Bulk clean") @@ -289,6 +366,7 @@ func TestLiveWorkflow_BulkOperations(t *testing.T) { for _, r := range cleanResp.Results { assert.True(t, r.Success(), "clean should succeed for id %s: %s", r.ID(), r.Error) } + waitAll("draft") // Step 6: Bulk delete t.Log("Step 6: Bulk delete") @@ -302,7 +380,6 @@ func TestLiveWorkflow_BulkOperations(t *testing.T) { // Step 7: Verify all deleted t.Log("Step 7: Verify all deleted") for _, id := range ids { - _, err := c.GetStack(id) - assert.Error(t, err, "GetStack on deleted instance %s should fail", id) + waitStackGone(t, c, id, liveOpTimeout) } } diff --git a/cli/test/live/portforward_live_test.go b/cli/test/live/portforward_live_test.go index b2a340f..3487858 100644 --- a/cli/test/live/portforward_live_test.go +++ b/cli/test/live/portforward_live_test.go @@ -59,8 +59,8 @@ func waitStackStatus(t *testing.T, c *client.Client, id string, want string, tim if inst.Status == want { return inst } - if inst.Status == "error" { - t.Fatalf("stack entered status error: %s", inst.ErrorMessage) + if inst.Status != want && client.IsFailedStatus(inst.Status) { + t.Fatalf("stack entered status %s: %s", inst.Status, inst.ErrorMessage) } if time.Now().After(deadline) { t.Fatalf("stack status %q after %s, want %q", inst.Status, timeout, want)