RFD 189: Console Access - Add WebSocket console endpoint#152
RFD 189: Console Access - Add WebSocket console endpoint#152
Conversation
Add GET /:account/machines/:machine/console endpoint for WebSocket-based console access to all VM brands. Changes: - Add lib/endpoints/console.js with ConsoleConnectionFSM - Add MachineHasNoConsoleError error type - Mount console endpoint in app.js - WebSocket upgrade with binary protocol - TCP proxy to console (vmadmd) via CNAPI - Support for all brands (KVM serial console, zone console for others) Console connections: - Upgrade HTTP to WebSocket (binary protocol) - Query CNAPI for console host/port - Establish TCP connection to vmadmd console proxy - Bidirectional byte stream proxying - Proper error handling and cleanup API version: 9.0.0 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull Request Overview
This PR implements WebSocket console access functionality for CloudAPI as part of RFD 189. The change enables remote access to instance consoles without requiring SSH access to compute nodes, supporting all instance types including KVM, Bhyve, SmartOS native zones, and LX.
Key changes include:
- New WebSocket endpoint for console connections at
GET /:account/machines/:machine/console - FSM-based connection management with TCP proxy to vmadmd console via CNAPI
- Console-specific error handling for unsupported instance types
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/errors.js | Adds MachineHasNoConsoleError for instances that don't support console connections |
| lib/endpoints/console.js | New WebSocket console endpoint with FSM-based connection management and TCP proxy |
| lib/app.js | Registers the new console endpoint in the server routing |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
lib/endpoints/console.js
Outdated
| // All brands support console access | ||
| // KVM: serial console, others: zone console |
There was a problem hiding this comment.
The comment claims all brands support console access, but the code imports and exports MachineHasNoConsoleError, suggesting some brands may not support console access. This creates a contradiction between the comment and the available error handling.
| // All brands support console access | |
| // KVM: serial console, others: zone console | |
| // Most brands support console access (KVM: serial console, others: zone console), | |
| // but some brands may not. Handle accordingly. |
| this.ws = shed.accept(this.req, this.upgrade.socket, this.upgrade.head, | ||
| false, ['binary', 'console']); | ||
| } catch (ex) { | ||
| this.log.error(ex, 'websocket upgrade failed'); |
There was a problem hiding this comment.
The this.log property is undefined at this point since it's only initialized in the start() method on line 293. This will cause a runtime error when the websocket upgrade fails.
|
1.) Thank you for the draft PR. 2.) Copilot does not count as an official reviewer for our purposes. It'll be interesting to see if its feedback is helpful or distracting. |
Initialize this.log early in state_upgrade to prevent crash when WebSocket upgrade fails. Previously, this.log was only set in start(), causing 'this.log.error(ex, ...)' in the catch block to fail with "Cannot read property 'error' of undefined". Also update comment to clarify that most (not all) brands support console access, matching the existence of MachineHasNoConsoleError. Addresses PR review feedback from #152 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Initialize this.log early in state_upgrade to prevent crash when WebSocket upgrade fails. Previously, this.log was only set in start(), causing 'this.log.error(ex, ...)' in the catch block to fail with "Cannot read property 'error' of undefined". Also update comment to clarify that most (not all) brands support console access, matching the existence of MachineHasNoConsoleError. Addresses PR review feedback from #152 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
nwilkens
force-pushed
the
rfd-0189-console-access
branch
from
4cbf2c8 to
d4e5a47
Compare
Summary
Adds WebSocket console endpoint to CloudAPI to support RFD 189 Console Access Through CloudAPI.
Enables users to access instance consoles remotely without requiring SSH access to compute nodes. Supports all instance types: KVM, Bhyve, SmartOS native zones, and LX.
Changes
GET /:account/machines/:machine/consolelib/app.js- Endpoint registrationlib/endpoints/console.js- WebSocket console endpoint implementationlib/errors.js- Console-specific error typesFeatures
Related PRs
Dependencies
Requires: