Skip to content

Conversation

@nwilkens
Copy link
Member

@nwilkens nwilkens commented Oct 8, 2025

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

  • New endpoint: GET /:account/machines/:machine/console
  • WebSocket upgrade with binary protocol
  • FSM-based connection management
  • TCP proxy to vmadmd console via CNAPI
  • Error handling for stopped instances, missing instances, auth failures
  • API version bumped to 9.0.0
  • Modified: lib/app.js - Endpoint registration
  • New file: lib/endpoints/console.js - WebSocket console endpoint implementation
  • Modified: lib/errors.js - Console-specific error types

Features

  • Works with all VM brands (KVM, Bhyve, Joyent, Joyent-minimal, LX)
  • Binary WebSocket protocol for efficient console data transfer
  • Proper authentication and authorization via CloudAPI
  • Graceful error handling and connection cleanup

Related PRs

Dependencies

Requires:

  • Platform changes from smartos-live PR deployed on compute nodes
  • CNAPI changes from sdc-cnapi PR deployed

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>
@nwilkens nwilkens requested a review from Copilot October 8, 2025 17:41
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +44 to +45
// All brands support console access
// KVM: serial console, others: zone console
Copy link

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
// 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.

Copilot uses AI. Check for mistakes.
this.ws = shed.accept(this.req, this.upgrade.socket, this.upgrade.head,
false, ['binary', 'console']);
} catch (ex) {
this.log.error(ex, 'websocket upgrade failed');
Copy link

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
@danmcd danmcd requested a review from a team October 8, 2025 18:10
@danmcd
Copy link
Contributor

danmcd commented Oct 8, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants