From 14be7cdf9d4daeed6aecafbcf91f36b7ad4d18d0 Mon Sep 17 00:00:00 2001 From: Allen Chen Date: Mon, 10 Aug 2026 14:26:49 -0700 Subject: [PATCH 1/7] Add ports command for instances and nodes --- .agents/skills/brev-cli/SKILL.md | 4 + .agents/skills/brev-cli/reference/commands.md | 44 ++ pkg/cmd/cmd.go | 2 + pkg/cmd/ports/ports.go | 327 +++++++++++++++ pkg/cmd/ports/ports_test.go | 389 ++++++++++++++++++ pkg/cmd/util/externalnode.go | 57 ++- pkg/cmd/util/externalnode_test.go | 170 +++++++- pkg/cmd/util/util.go | 36 +- 8 files changed, 1004 insertions(+), 25 deletions(-) create mode 100644 pkg/cmd/ports/ports.go create mode 100644 pkg/cmd/ports/ports_test.go diff --git a/.agents/skills/brev-cli/SKILL.md b/.agents/skills/brev-cli/SKILL.md index f054e452..50ce1101 100644 --- a/.agents/skills/brev-cli/SKILL.md +++ b/.agents/skills/brev-cli/SKILL.md @@ -170,6 +170,10 @@ brev copy my-instance:/remote/file ./local-path/ # Port forward brev port-forward my-instance -p 8080:8080 + +# List Skybridge-managed HTTP and network ports for an instance or external node +brev ports my-instance +brev ports my-node --json ``` ### Listing Instances and Nodes diff --git a/.agents/skills/brev-cli/reference/commands.md b/.agents/skills/brev-cli/reference/commands.md index fe91cbbd..2111490b 100644 --- a/.agents/skills/brev-cli/reference/commands.md +++ b/.agents/skills/brev-cli/reference/commands.md @@ -466,6 +466,50 @@ brev port-forward my-instance -p 8080:8080 brev port-forward my-instance -p 3000:3000 ``` +### brev ports +List Skybridge-managed HTTP applications and raw network port mappings for a +managed instance or registered compute node. + +```bash +brev ports [flags] +``` + +**Flags:** +| Flag | Description | +|------|-------------| +| `--json` | Output the port mappings as JSON | + +The table output includes endpoint, IP restrictions, public port, destination +port, and protocol. HTTP applications also include their authorization policy. + +For managed instances, this command reads the Skybridge network member. It does +not synthesize the legacy secure-link or firewall rows shown by the Brev console, +because those rows do not have real port IDs and cannot be used by port-management +automation. If the instance is still provisioning or uses legacy network access, +the command returns an error directing the user to the console. + +The JSON output is an array with the following stable fields: + +| Field | Meaning | +|------|---------| +| `port_id` | Unique port mapping ID used by automation | +| `kind` | `http` or `network` | +| `endpoint` | Public URL or `host:port` endpoint | +| `public_port` | Externally addressable port | +| `destination_port` | Port listening on the target machine | +| `protocol` | `HTTP`, `HTTPS`, `SSH`, `TCP`, `UDP`, or `UNKNOWN` | +| `allowed_sources` | Allowed IP addresses or CIDR blocks | +| `authorized_emails` | Identities authorized for an HTTP application | +| `allow_public_unauthenticated` | Whether an HTTP application is public | +| `type` | `system`, `user`, `unspecified`, or `unknown` | + +**Examples:** +```bash +brev ports my-instance +brev ports my-node +brev ports my-instance --json +``` + ## Organization Commands ### brev org ls diff --git a/pkg/cmd/cmd.go b/pkg/cmd/cmd.go index 49b5254d..831a2aa9 100644 --- a/pkg/cmd/cmd.go +++ b/pkg/cmd/cmd.go @@ -35,6 +35,7 @@ import ( "github.com/brevdev/brev-cli/pkg/cmd/open" "github.com/brevdev/brev-cli/pkg/cmd/org" "github.com/brevdev/brev-cli/pkg/cmd/portforward" + "github.com/brevdev/brev-cli/pkg/cmd/ports" "github.com/brevdev/brev-cli/pkg/cmd/profile" "github.com/brevdev/brev-cli/pkg/cmd/proxy" "github.com/brevdev/brev-cli/pkg/cmd/redeem" @@ -276,6 +277,7 @@ func createCmdTree(cmd *cobra.Command, t *terminal.Terminal, loginCmdStore *stor cmd.AddCommand(invite.NewCmdInvite(t, loginCmdStore, noLoginCmdStore)) cmd.AddCommand(redeem.NewCmdRedeem(t, loginCmdStore, noLoginCmdStore)) cmd.AddCommand(portforward.NewCmdPortForwardSSH(loginCmdStore, t)) + cmd.AddCommand(ports.NewCmdPorts(loginCmdStore)) cmd.AddCommand(login.NewCmdLogin(t, noLoginCmdStore, loginAuth)) cmd.AddCommand(logout.NewCmdLogout(loginAuth, noLoginCmdStore)) cmd.AddCommand(tasks.NewCmdTasks(t, noLoginCmdStore)) diff --git a/pkg/cmd/ports/ports.go b/pkg/cmd/ports/ports.go new file mode 100644 index 00000000..868bc0b3 --- /dev/null +++ b/pkg/cmd/ports/ports.go @@ -0,0 +1,327 @@ +// Package ports displays Skybridge-managed public port mappings for an instance or external node. +package ports + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net" + "strconv" + "strings" + + devplanev1 "buf.build/gen/go/brevdev/devplane/protocolbuffers/go/devplaneapi/v1" + "connectrpc.com/connect" + "github.com/jedib0t/go-pretty/v6/table" + "github.com/spf13/cobra" + + "github.com/brevdev/brev-cli/pkg/cmd/cmderrors" + "github.com/brevdev/brev-cli/pkg/cmd/register" + cmdutil "github.com/brevdev/brev-cli/pkg/cmd/util" + "github.com/brevdev/brev-cli/pkg/config" + breverrors "github.com/brevdev/brev-cli/pkg/errors" +) + +const ( + portKindHTTP = "http" + portKindNetwork = "network" +) + +// Store contains the dependencies needed to resolve both managed instances and +// registered compute nodes. +type Store interface { + cmdutil.WorkspaceOrNodeResolver +} + +// PortInfo is the stable JSON representation of a port mapping. +type PortInfo struct { + PortID string `json:"port_id"` + Kind string `json:"kind"` + Endpoint string `json:"endpoint"` + PublicPort int32 `json:"public_port"` + DestinationPort int32 `json:"destination_port"` + Protocol string `json:"protocol"` + AllowedSources []string `json:"allowed_sources"` + AuthorizedEmails []string `json:"authorized_emails"` + AllowPublicUnauthenticated bool `json:"allow_public_unauthenticated"` + Type string `json:"type"` +} + +// NewCmdPorts creates the `brev ports` command. +func NewCmdPorts(portStore Store) *cobra.Command { + var jsonOutput bool + + cmd := &cobra.Command{ + Annotations: map[string]string{"access": ""}, + Use: "ports ", + DisableFlagsInUseLine: true, + Short: "List Skybridge-managed ports for an instance or external node", + Example: ` + brev ports my-instance + brev ports my-node --json`, + Args: cmderrors.TransformToValidationError(cobra.ExactArgs(1)), + RunE: func(cmd *cobra.Command, args []string) error { + if err := Run(cmd.Context(), cmd.OutOrStdout(), portStore, args[0], jsonOutput); err != nil { + return breverrors.WrapAndTrace(err) + } + return nil + }, + } + + cmd.Flags().BoolVar(&jsonOutput, "json", false, "output as JSON") + return cmd +} + +// Run resolves a managed instance or registered compute node and displays its ports. +func Run(ctx context.Context, out io.Writer, portStore Store, nameOrID string, jsonOutput bool) error { + target, err := cmdutil.ResolveWorkspaceOrNodeWithContext(ctx, portStore, nameOrID) + if err != nil { + return breverrors.WrapAndTrace(err) + } + + var apiPorts []*devplanev1.Port + if target.Workspace != nil { + client := register.NewEnvironmentServiceClient(portStore, config.GlobalConfig.GetBrevPublicAPIURL()) + resp, err := client.GetNetworkInfo(ctx, connect.NewRequest(&devplanev1.EnvironmentServiceGetNetworkInfoRequest{ + EnvironmentId: target.Workspace.ID, + })) + if err != nil { + return fmt.Errorf("get ports for instance %q: %w", nameOrID, err) + } + var networkInfo *devplanev1.EnvironmentNetworkInfo + if resp != nil && resp.Msg != nil { + networkInfo = resp.Msg.GetNetworkInfo() + } + // A connected or disconnected member with no ports is a valid empty result. + // The API uses an unspecified status and no ports when no Skybridge member exists yet. + if networkInfo == nil || + (networkInfo.GetStatus() == devplanev1.NetworkMemberStatus_NETWORK_MEMBER_STATUS_UNSPECIFIED && + len(networkInfo.GetPorts()) == 0) { + return breverrors.NewValidationError(fmt.Sprintf( + "cannot list ports for instance %q: no Skybridge network member is available; "+ + "the instance may still be provisioning or may use legacy network access. "+ + "Try again when it is running, or view legacy secure links and firewall rules in the Brev console", + nameOrID, + )) + } + apiPorts = networkInfo.GetPorts() + } else if target.Node != nil { + apiPorts = target.Node.GetPorts() + } + portInfos := toPortInfos(apiPorts) + if jsonOutput { + return writeJSON(out, portInfos) + } + return displayTables(out, nameOrID, portInfos) +} + +func toPortInfos(apiPorts []*devplanev1.Port) []PortInfo { + portInfos := make([]PortInfo, 0, len(apiPorts)) + for _, port := range apiPorts { + if port == nil { + continue + } + isHTTP := port.GetHttpProtocol() != devplanev1.HttpPortProtocol_HTTP_PORT_PROTOCOL_UNSPECIFIED + kind := portKindNetwork + if isHTTP { + kind = portKindHTTP + } + portInfos = append(portInfos, PortInfo{ + PortID: port.GetPortId(), + Kind: kind, + Endpoint: endpoint(port, isHTTP), + PublicPort: port.GetPortNumber(), + DestinationPort: port.GetServerPort(), + Protocol: protocolLabel(port, isHTTP), + AllowedSources: append([]string{}, port.GetAllowedSources()...), + AuthorizedEmails: append([]string{}, port.GetAuthorizedEmails()...), + AllowPublicUnauthenticated: port.GetAllowPublicUnauthenticated(), + Type: portTypeLabel(port.GetType()), + }) + } + return portInfos +} + +func endpoint(port *devplanev1.Port, isHTTP bool) string { + hostname := port.GetHostname() + if hostname == "" { + return "" + } + if isHTTP { + return "https://" + hostname + } + if port.GetPortNumber() == 0 { + return hostname + } + return net.JoinHostPort(hostname, strconv.Itoa(int(port.GetPortNumber()))) +} + +func protocolLabel(port *devplanev1.Port, isHTTP bool) string { + if isHTTP { + switch port.GetHttpProtocol() { + case devplanev1.HttpPortProtocol_HTTP_PORT_PROTOCOL_HTTP: + return "HTTP" + case devplanev1.HttpPortProtocol_HTTP_PORT_PROTOCOL_HTTPS: + return "HTTPS" + default: + return "UNKNOWN" + } + } + + switch port.GetProtocol() { + case devplanev1.PortProtocol_PORT_PROTOCOL_SSH: + return "SSH" + case devplanev1.PortProtocol_PORT_PROTOCOL_TCP: + return "TCP" + case devplanev1.PortProtocol_PORT_PROTOCOL_UDP: + return "UDP" + default: + return "UNKNOWN" + } +} + +func portTypeLabel(portType devplanev1.PortType) string { + switch portType { + case devplanev1.PortType_PORT_TYPE_UNSPECIFIED: + return "unspecified" + case devplanev1.PortType_PORT_TYPE_SYSTEM: + return "system" + case devplanev1.PortType_PORT_TYPE_USER: + return "user" + default: + return "unknown" + } +} + +func writeJSON(out io.Writer, portInfos []PortInfo) error { + encoded, err := json.MarshalIndent(portInfos, "", " ") + if err != nil { + return breverrors.WrapAndTrace(err) + } + _, err = fmt.Fprintln(out, string(encoded)) + return breverrors.WrapAndTrace(err) +} + +func displayTables(out io.Writer, nameOrID string, portInfos []PortInfo) error { + if len(portInfos) == 0 { + _, err := fmt.Fprintf(out, "No ports are open on %s.\n", nameOrID) + return breverrors.WrapAndTrace(err) + } + + httpPorts := make([]PortInfo, 0, len(portInfos)) + networkPorts := make([]PortInfo, 0, len(portInfos)) + for _, port := range portInfos { + if port.Kind == portKindHTTP { + httpPorts = append(httpPorts, port) + } else { + networkPorts = append(networkPorts, port) + } + } + + if len(httpPorts) > 0 { + if _, err := fmt.Fprintln(out, "HTTP APPLICATIONS"); err != nil { + return breverrors.WrapAndTrace(err) + } + displayHTTPTable(out, httpPorts) + } + if len(httpPorts) > 0 && len(networkPorts) > 0 { + if _, err := fmt.Fprintln(out); err != nil { + return breverrors.WrapAndTrace(err) + } + } + if len(networkPorts) > 0 { + if _, err := fmt.Fprintln(out, "TCP/UDP PORTS"); err != nil { + return breverrors.WrapAndTrace(err) + } + displayNetworkTable(out, networkPorts) + } + return nil +} + +func displayHTTPTable(out io.Writer, portInfos []PortInfo) { + tw := newTable(out) + tw.AppendHeader(table.Row{"ENDPOINT", "AUTHORIZATION", "IP RESTRICTIONS", "PUBLIC PORT", "DESTINATION PORT", "PROTOCOL"}) + for _, port := range portInfos { + destinationPort := port.DestinationPort + if destinationPort == 0 { + destinationPort = port.PublicPort + } + tw.AppendRow(table.Row{ + valueOrDash(port.Endpoint), + authorizationLabel(port), + allowedSourcesLabel(port.AllowedSources), + portNumberLabel(port.PublicPort), + portNumberLabel(destinationPort), + port.Protocol, + }) + } + tw.Render() +} + +func displayNetworkTable(out io.Writer, portInfos []PortInfo) { + tw := newTable(out) + tw.AppendHeader(table.Row{"ENDPOINT", "IP RESTRICTIONS", "PUBLIC PORT", "DESTINATION PORT", "PROTOCOL"}) + for _, port := range portInfos { + tw.AppendRow(table.Row{ + valueOrDash(port.Endpoint), + allowedSourcesLabel(port.AllowedSources), + portNumberLabel(port.PublicPort), + portNumberLabel(port.DestinationPort), + port.Protocol, + }) + } + tw.Render() +} + +func newTable(out io.Writer) table.Writer { + tw := table.NewWriter() + tw.SetOutputMirror(out) + options := table.OptionsDefault + options.DrawBorder = false + options.SeparateColumns = false + options.SeparateRows = false + options.SeparateHeader = false + tw.Style().Options = options + return tw +} + +func authorizationLabel(port PortInfo) string { + if port.AllowPublicUnauthenticated { + return "Public" + } + if len(port.AuthorizedEmails) > 0 { + return strings.Join(port.AuthorizedEmails, ", ") + } + return "-" +} + +func allowedSourcesLabel(allowedSources []string) string { + if len(allowedSources) == 0 { + return "Anywhere" + } + allAnywhere := true + for _, source := range allowedSources { + if source != "0.0.0.0/0" { + allAnywhere = false + break + } + } + if allAnywhere { + return "Anywhere" + } + return strings.Join(allowedSources, ", ") +} + +func portNumberLabel(port int32) string { + if port == 0 { + return "-" + } + return strconv.Itoa(int(port)) +} + +func valueOrDash(value string) string { + if value == "" { + return "-" + } + return value +} diff --git a/pkg/cmd/ports/ports_test.go b/pkg/cmd/ports/ports_test.go new file mode 100644 index 00000000..95eae00d --- /dev/null +++ b/pkg/cmd/ports/ports_test.go @@ -0,0 +1,389 @@ +package ports + +import ( + "bytes" + "context" + "net/http/httptest" + "testing" + + devplanev1connect "buf.build/gen/go/brevdev/devplane/connectrpc/go/devplaneapi/v1/devplaneapiv1connect" + devplanev1 "buf.build/gen/go/brevdev/devplane/protocolbuffers/go/devplaneapi/v1" + "connectrpc.com/connect" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/brevdev/brev-cli/pkg/entity" +) + +type fakeStore struct { + workspaces []entity.Workspace + user *entity.User + org *entity.Organization +} + +func (s *fakeStore) GetAuthTokens() (*entity.AuthTokens, error) { + return nil, nil +} + +func (s *fakeStore) GetActiveOrganizationOrDefault() (*entity.Organization, error) { + return s.org, nil +} + +func (s *fakeStore) GetWorkspaceByNameOrID(_ string, _ string) ([]entity.Workspace, error) { + return s.workspaces, nil +} + +func (s *fakeStore) GetCurrentUser() (*entity.User, error) { + return s.user, nil +} + +func (s *fakeStore) GetAccessToken() (string, error) { + return "test-token", nil +} + +type fakeEnvironmentService struct { + devplanev1connect.UnimplementedEnvironmentServiceHandler + t *testing.T + expectedEnvID string + networkInfo *devplanev1.EnvironmentNetworkInfo +} + +func (s *fakeEnvironmentService) GetNetworkInfo( + _ context.Context, + req *connect.Request[devplanev1.EnvironmentServiceGetNetworkInfoRequest], +) (*connect.Response[devplanev1.EnvironmentServiceGetNetworkInfoResponse], error) { + s.t.Helper() + assert.Equal(s.t, s.expectedEnvID, req.Msg.GetEnvironmentId()) + return connect.NewResponse(&devplanev1.EnvironmentServiceGetNetworkInfoResponse{ + NetworkInfo: s.networkInfo, + }), nil +} + +type fakeNodeService struct { + devplanev1connect.UnimplementedExternalNodeServiceHandler + nodes []*devplanev1.ExternalNode +} + +func (s *fakeNodeService) ListNodes( + _ context.Context, + _ *connect.Request[devplanev1.ListNodesRequest], +) (*connect.Response[devplanev1.ListNodesResponse], error) { + return connect.NewResponse(&devplanev1.ListNodesResponse{Items: s.nodes}), nil +} + +func TestRunEnvironmentJSON(t *testing.T) { + public := false + hostname := "jupyter-env123.apps.run.brev.nvidia.com" + service := &fakeEnvironmentService{ + t: t, + expectedEnvID: "env123", + networkInfo: &devplanev1.EnvironmentNetworkInfo{ + Status: devplanev1.NetworkMemberStatus_NETWORK_MEMBER_STATUS_CONNECTED, + Ports: []*devplanev1.Port{ + { + PortId: "port-http", + HttpProtocol: devplanev1.HttpPortProtocol_HTTP_PORT_PROTOCOL_HTTP, + PortNumber: 443, + ServerPort: 8888, + Hostname: &hostname, + AuthorizedEmails: []string{"user@example.com"}, + AllowPublicUnauthenticated: &public, + Type: devplanev1.PortType_PORT_TYPE_SYSTEM, + }, + }, + }, + } + _, handler := devplanev1connect.NewEnvironmentServiceHandler(service) + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + t.Setenv("BREV_PUBLIC_API_URL", server.URL) + + store := &fakeStore{ + workspaces: []entity.Workspace{{ID: "env123", Name: "my-instance", CreatedByUserID: "user1"}}, + user: &entity.User{ID: "user1"}, + org: &entity.Organization{ID: "org1"}, + } + var out bytes.Buffer + + err := Run(context.Background(), &out, store, "my-instance", true) + + require.NoError(t, err) + assert.JSONEq(t, `[ + { + "port_id": "port-http", + "kind": "http", + "endpoint": "https://jupyter-env123.apps.run.brev.nvidia.com", + "public_port": 443, + "destination_port": 8888, + "protocol": "HTTP", + "allowed_sources": [], + "authorized_emails": ["user@example.com"], + "allow_public_unauthenticated": false, + "type": "system" + } + ]`, out.String()) +} + +func TestRunExternalNodeByIDDisplaysTables(t *testing.T) { + httpHostname := "jupyter-node.apps.run.brev.nvidia.com" + tcpHostname := "global.prd.ga.run.brev.nvidia.com" + service := &fakeNodeService{nodes: []*devplanev1.ExternalNode{ + { + ExternalNodeId: "unode123", + Name: "my-node", + Ports: []*devplanev1.Port{ + { + PortId: "port-http", + HttpProtocol: devplanev1.HttpPortProtocol_HTTP_PORT_PROTOCOL_HTTP, + PortNumber: 443, + ServerPort: 8888, + Hostname: &httpHostname, + AuthorizedEmails: []string{"user@example.com"}, + }, + { + PortId: "port-tcp", + Protocol: devplanev1.PortProtocol_PORT_PROTOCOL_TCP, + PortNumber: 18928, + ServerPort: 22, + Hostname: &tcpHostname, + AllowedSources: []string{"0.0.0.0/0"}, + }, + }, + }, + }} + _, handler := devplanev1connect.NewExternalNodeServiceHandler(service) + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + t.Setenv("BREV_PUBLIC_API_URL", server.URL) + + store := &fakeStore{ + user: &entity.User{ID: "user1"}, + org: &entity.Organization{ID: "org1"}, + } + var out bytes.Buffer + + err := Run(context.Background(), &out, store, "unode123", false) + + require.NoError(t, err) + assert.Contains(t, out.String(), "HTTP APPLICATIONS") + assert.Contains(t, out.String(), "https://jupyter-node.apps.run.brev.nvidia.com") + assert.Contains(t, out.String(), "user@example.com") + assert.Contains(t, out.String(), "TCP/UDP PORTS") + assert.Contains(t, out.String(), "PUBLIC PORT") + assert.Contains(t, out.String(), "DESTINATION PORT") + assert.Contains(t, out.String(), "global.prd.ga.run.brev.nvidia.com:18928") + assert.Contains(t, out.String(), "Anywhere") + assert.Contains(t, out.String(), "22") + assert.Contains(t, out.String(), "TCP") +} + +func TestToPortInfosHandlesPublicHTTPAndRestrictedUDP(t *testing.T) { + public := true + httpHostname := "app.example.com" + udpHostname := "gateway.example.com" + + got := toPortInfos([]*devplanev1.Port{ + { + PortId: "http", + HttpProtocol: devplanev1.HttpPortProtocol_HTTP_PORT_PROTOCOL_HTTPS, + PortNumber: 443, + ServerPort: 8443, + Hostname: &httpHostname, + AllowPublicUnauthenticated: &public, + }, + { + PortId: "udp", + Protocol: devplanev1.PortProtocol_PORT_PROTOCOL_UDP, + PortNumber: 5000, + ServerPort: 5001, + Hostname: &udpHostname, + AllowedSources: []string{"10.0.0.0/8"}, + Type: devplanev1.PortType_PORT_TYPE_USER, + }, + nil, + }) + + require.Len(t, got, 2) + assert.Equal(t, "http", got[0].Kind) + assert.Equal(t, "HTTPS", got[0].Protocol) + assert.Equal(t, "https://app.example.com", got[0].Endpoint) + assert.True(t, got[0].AllowPublicUnauthenticated) + assert.Equal(t, "network", got[1].Kind) + assert.Equal(t, "UDP", got[1].Protocol) + assert.Equal(t, "gateway.example.com:5000", got[1].Endpoint) + assert.Equal(t, []string{"10.0.0.0/8"}, got[1].AllowedSources) + assert.Equal(t, "user", got[1].Type) + assert.Equal(t, "unspecified", portTypeLabel(devplanev1.PortType_PORT_TYPE_UNSPECIFIED)) + assert.Equal(t, "unknown", portTypeLabel(devplanev1.PortType(99))) +} + +func TestRunEmptyPortsJSONIsArray(t *testing.T) { + service := &fakeEnvironmentService{ + t: t, + expectedEnvID: "env-empty", + networkInfo: &devplanev1.EnvironmentNetworkInfo{ + Status: devplanev1.NetworkMemberStatus_NETWORK_MEMBER_STATUS_DISCONNECTED, + }, + } + _, handler := devplanev1connect.NewEnvironmentServiceHandler(service) + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + t.Setenv("BREV_PUBLIC_API_URL", server.URL) + + store := &fakeStore{ + workspaces: []entity.Workspace{{ID: "env-empty", Name: "empty", CreatedByUserID: "user1"}}, + user: &entity.User{ID: "user1"}, + org: &entity.Organization{ID: "org1"}, + } + var out bytes.Buffer + + err := Run(context.Background(), &out, store, "empty", true) + + require.NoError(t, err) + assert.JSONEq(t, `[]`, out.String()) +} + +func TestRunEmptyPortsHumanOutput(t *testing.T) { + service := &fakeEnvironmentService{ + t: t, + expectedEnvID: "env-empty", + networkInfo: &devplanev1.EnvironmentNetworkInfo{ + Status: devplanev1.NetworkMemberStatus_NETWORK_MEMBER_STATUS_CONNECTED, + }, + } + _, handler := devplanev1connect.NewEnvironmentServiceHandler(service) + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + t.Setenv("BREV_PUBLIC_API_URL", server.URL) + + store := &fakeStore{ + workspaces: []entity.Workspace{{ID: "env-empty", Name: "empty", CreatedByUserID: "user1"}}, + user: &entity.User{ID: "user1"}, + org: &entity.Organization{ID: "org1"}, + } + var out bytes.Buffer + + err := Run(context.Background(), &out, store, "empty", false) + + require.NoError(t, err) + assert.Equal(t, "No ports are open on empty.\n", out.String()) +} + +func TestRunEnvironmentWithoutNetworkMemberReturnsActionableError(t *testing.T) { + testCases := map[string]*devplanev1.EnvironmentNetworkInfo{ + "missing network info": nil, + "unspecified status": {}, + } + for name, networkInfo := range testCases { + t.Run(name, func(t *testing.T) { + service := &fakeEnvironmentService{ + t: t, + expectedEnvID: "env-legacy", + networkInfo: networkInfo, + } + _, handler := devplanev1connect.NewEnvironmentServiceHandler(service) + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + t.Setenv("BREV_PUBLIC_API_URL", server.URL) + + store := &fakeStore{ + workspaces: []entity.Workspace{{ID: "env-legacy", Name: "legacy", CreatedByUserID: "user1"}}, + user: &entity.User{ID: "user1"}, + org: &entity.Organization{ID: "org1"}, + } + var out bytes.Buffer + + err := Run(context.Background(), &out, store, "legacy", true) + + require.Error(t, err) + assert.Empty(t, out.String()) + assert.Contains(t, err.Error(), "no Skybridge network member is available") + assert.Contains(t, err.Error(), "may still be provisioning or may use legacy network access") + assert.Contains(t, err.Error(), "Brev console") + }) + } +} + +func TestRunEnvironmentWithPortsAndUnspecifiedStatusStillLists(t *testing.T) { + hostname := "app.example.com" + service := &fakeEnvironmentService{ + t: t, + expectedEnvID: "env-partial", + networkInfo: &devplanev1.EnvironmentNetworkInfo{ + Ports: []*devplanev1.Port{ + { + PortId: "port-http", + HttpProtocol: devplanev1.HttpPortProtocol_HTTP_PORT_PROTOCOL_HTTP, + PortNumber: 443, + ServerPort: 8080, + Hostname: &hostname, + }, + }, + }, + } + _, handler := devplanev1connect.NewEnvironmentServiceHandler(service) + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + t.Setenv("BREV_PUBLIC_API_URL", server.URL) + + store := &fakeStore{ + workspaces: []entity.Workspace{{ID: "env-partial", Name: "partial", CreatedByUserID: "user1"}}, + user: &entity.User{ID: "user1"}, + org: &entity.Organization{ID: "org1"}, + } + var out bytes.Buffer + + err := Run(context.Background(), &out, store, "partial", true) + + require.NoError(t, err) + assert.Contains(t, out.String(), `"port_id": "port-http"`) +} + +func TestRunExternalNodeJSONContract(t *testing.T) { + hostname := "global.prd.ga.run.brev.nvidia.com" + service := &fakeNodeService{nodes: []*devplanev1.ExternalNode{ + { + ExternalNodeId: "unode-json", + Name: "json-node", + Ports: []*devplanev1.Port{ + { + PortId: "port-ssh", + Protocol: devplanev1.PortProtocol_PORT_PROTOCOL_SSH, + PortNumber: 18928, + ServerPort: 22, + Hostname: &hostname, + AllowedSources: []string{"10.0.0.0/8"}, + Type: devplanev1.PortType_PORT_TYPE_USER, + }, + }, + }, + }} + _, handler := devplanev1connect.NewExternalNodeServiceHandler(service) + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + t.Setenv("BREV_PUBLIC_API_URL", server.URL) + + store := &fakeStore{ + user: &entity.User{ID: "user1"}, + org: &entity.Organization{ID: "org1"}, + } + var out bytes.Buffer + + err := Run(context.Background(), &out, store, "unode-json", true) + + require.NoError(t, err) + assert.JSONEq(t, `[ + { + "port_id": "port-ssh", + "kind": "network", + "endpoint": "global.prd.ga.run.brev.nvidia.com:18928", + "public_port": 18928, + "destination_port": 22, + "protocol": "SSH", + "allowed_sources": ["10.0.0.0/8"], + "authorized_emails": [], + "allow_public_unauthenticated": false, + "type": "user" + } + ]`, out.String()) +} diff --git a/pkg/cmd/util/externalnode.go b/pkg/cmd/util/externalnode.go index 0db4b3ab..90b57b45 100644 --- a/pkg/cmd/util/externalnode.go +++ b/pkg/cmd/util/externalnode.go @@ -37,13 +37,29 @@ type WorkspaceOrNode struct { // The store must satisfy both GetWorkspaceByNameOrIDErrStore and ExternalNodeStore. func ResolveWorkspaceOrNode(store WorkspaceOrNodeResolver, nameOrID string, ) (*WorkspaceOrNode, error) { - workspace, wsErr := GetUserWorkspaceByNameOrIDErr(store, nameOrID) - if wsErr == nil { + return ResolveWorkspaceOrNodeWithContext(context.Background(), store, nameOrID) +} + +// ResolveWorkspaceOrNodeWithContext looks up a workspace first; if not found, falls back to +// external nodes. The context is used for the external-node service request. +func ResolveWorkspaceOrNodeWithContext(ctx context.Context, store WorkspaceOrNodeResolver, nameOrID string, +) (*WorkspaceOrNode, error) { + workspace, workspaceFound, err := findUserWorkspaceByNameOrID(store, nameOrID) + if err != nil { + return nil, err + } + if workspaceFound { return &WorkspaceOrNode{Workspace: workspace}, nil } - node, nodeErr := FindExternalNode(store, nameOrID) - if nodeErr != nil || node == nil { - return nil, wsErr // return original workspace error + node, nodeErr := FindExternalNodeWithContext(ctx, store, nameOrID) + if nodeErr != nil { + return nil, nodeErr + } + if node == nil { + return nil, breverrors.NewValidationError(fmt.Sprintf( + "instance or external node with id/name %q not found", + nameOrID, + )) } return &WorkspaceOrNode{Node: node}, nil } @@ -124,26 +140,43 @@ func OpenPort(store ExternalNodeStore, nodeID string, portNumber int32, protocol return resp.Msg.GetPort(), nil } -// FindExternalNode searches for an external node by name in the user's active organization. +// FindExternalNode searches for an external node by name or ID in the user's active organization. +// Returns (nil, nil) if no matching node is found. +func FindExternalNode(store ExternalNodeStore, nameOrID string) (*nodev1.ExternalNode, error) { + return FindExternalNodeWithContext(context.Background(), store, nameOrID) +} + +// FindExternalNodeWithContext searches for an external node by name or ID in the user's active +// organization. Exact IDs take precedence over case-insensitive names. // Returns (nil, nil) if no matching node is found. -func FindExternalNode(store ExternalNodeStore, name string) (*nodev1.ExternalNode, error) { +func FindExternalNodeWithContext(ctx context.Context, store ExternalNodeStore, nameOrID string) (*nodev1.ExternalNode, error) { org, err := store.GetActiveOrganizationOrDefault() if err != nil { return nil, breverrors.WrapAndTrace(err) } client := register.NewNodeServiceClient(store, config.GlobalConfig.GetBrevPublicAPIURL()) - resp, err := client.ListNodes(context.Background(), connect.NewRequest(&nodev1.ListNodesRequest{ + resp, err := client.ListNodes(ctx, connect.NewRequest(&nodev1.ListNodesRequest{ OrganizationId: org.ID, })) if err != nil { return nil, breverrors.WrapAndTrace(err) } - for _, node := range resp.Msg.GetItems() { - if strings.EqualFold(node.GetName(), name) { - return node, nil + return findExternalNode(resp.Msg.GetItems(), nameOrID), nil +} + +func findExternalNode(nodes []*nodev1.ExternalNode, nameOrID string) *nodev1.ExternalNode { + // IDs are unique and unambiguous, so they must win even if an earlier node's name collides. + for _, node := range nodes { + if node != nil && node.GetExternalNodeId() == nameOrID { + return node } } - return nil, nil + for _, node := range nodes { + if node != nil && strings.EqualFold(node.GetName(), nameOrID) { + return node + } + } + return nil } // ResolveExternalNodeSSH resolves the SSH connection details for an external node diff --git a/pkg/cmd/util/externalnode_test.go b/pkg/cmd/util/externalnode_test.go index 39583e26..45e18644 100644 --- a/pkg/cmd/util/externalnode_test.go +++ b/pkg/cmd/util/externalnode_test.go @@ -1,22 +1,28 @@ package util import ( + "context" + "errors" "fmt" + "net/http/httptest" "strings" "testing" + nodev1connect "buf.build/gen/go/brevdev/devplane/connectrpc/go/devplaneapi/v1/devplaneapiv1connect" nodev1 "buf.build/gen/go/brevdev/devplane/protocolbuffers/go/devplaneapi/v1" + "connectrpc.com/connect" "github.com/brevdev/brev-cli/pkg/entity" breverrors "github.com/brevdev/brev-cli/pkg/errors" ) -// mockExternalNodeStore satisfies ExternalNodeStore for unit tests that -// only exercise ResolveExternalNodeSSH (no RPC calls). +// mockExternalNodeStore satisfies the shared node and workspace lookup interfaces. type mockExternalNodeStore struct { - user *entity.User - org *entity.Organization - err error + user *entity.User + org *entity.Organization + workspaces []entity.Workspace + workspaceErr error + err error } func (m *mockExternalNodeStore) GetActiveOrganizationOrDefault() (*entity.Organization, error) { @@ -25,6 +31,12 @@ func (m *mockExternalNodeStore) GetActiveOrganizationOrDefault() (*entity.Organi func (m *mockExternalNodeStore) GetAccessToken() (string, error) { return "tok", nil } +func (m *mockExternalNodeStore) GetAuthTokens() (*entity.AuthTokens, error) { return nil, nil } + +func (m *mockExternalNodeStore) GetWorkspaceByNameOrID(_, _ string) ([]entity.Workspace, error) { + return m.workspaces, m.workspaceErr +} + func (m *mockExternalNodeStore) GetCurrentUser() (*entity.User, error) { if m.err != nil { return nil, m.err @@ -54,6 +66,154 @@ func makeTestNode(name, userID, linuxUser, hostname string, portNumber int32) *n } } +type fakeExternalNodeResolverService struct { + nodev1connect.UnimplementedExternalNodeServiceHandler + nodes []*nodev1.ExternalNode + listErr error + listCalls int +} + +func (s *fakeExternalNodeResolverService) ListNodes( + _ context.Context, + _ *connect.Request[nodev1.ListNodesRequest], +) (*connect.Response[nodev1.ListNodesResponse], error) { + s.listCalls++ + if s.listErr != nil { + return nil, s.listErr + } + return connect.NewResponse(&nodev1.ListNodesResponse{Items: s.nodes}), nil +} + +func withExternalNodeResolverAPI(t *testing.T, service *fakeExternalNodeResolverService) { + t.Helper() + _, handler := nodev1connect.NewExternalNodeServiceHandler(service) + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + t.Setenv("BREV_PUBLIC_API_URL", server.URL) +} + +func TestFindExternalNodeWithContext_IDTakesPrecedenceOverName(t *testing.T) { + service := &fakeExternalNodeResolverService{nodes: []*nodev1.ExternalNode{ + {ExternalNodeId: "unode_name_collision", Name: "unode_target"}, + {ExternalNodeId: "unode_target", Name: "actual-node-name"}, + }} + withExternalNodeResolverAPI(t, service) + store := &mockExternalNodeStore{org: &entity.Organization{ID: "org_1"}} + + node, err := FindExternalNodeWithContext(context.Background(), store, "unode_target") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if node == nil { + t.Fatal("expected a matching node") + } + if node.GetExternalNodeId() != "unode_target" { + t.Fatalf("expected exact ID match, got node %q with ID %q", node.GetName(), node.GetExternalNodeId()) + } +} + +func TestFindExternalNodeWithContext_NameMatchIsCaseInsensitive(t *testing.T) { + service := &fakeExternalNodeResolverService{nodes: []*nodev1.ExternalNode{ + {ExternalNodeId: "unode_1", Name: "GPU-Node"}, + }} + withExternalNodeResolverAPI(t, service) + store := &mockExternalNodeStore{org: &entity.Organization{ID: "org_1"}} + + node, err := FindExternalNodeWithContext(context.Background(), store, "gpu-node") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if node == nil || node.GetExternalNodeId() != "unode_1" { + t.Fatalf("expected case-insensitive name match, got %+v", node) + } +} + +func TestFindExternalNodeWithContext_UsesCallerContext(t *testing.T) { + service := &fakeExternalNodeResolverService{nodes: []*nodev1.ExternalNode{ + {ExternalNodeId: "unode_1", Name: "gpu-node"}, + }} + withExternalNodeResolverAPI(t, service) + store := &mockExternalNodeStore{org: &entity.Organization{ID: "org_1"}} + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + node, err := FindExternalNodeWithContext(ctx, store, "gpu-node") + if err == nil { + t.Fatalf("expected canceled request to fail, got node %+v", node) + } + if connect.CodeOf(err) != connect.CodeCanceled { + t.Fatalf("expected canceled error, got %v", err) + } +} + +func TestResolveWorkspaceOrNodeWithContext_PropagatesNodeServiceError(t *testing.T) { + service := &fakeExternalNodeResolverService{ + listErr: connect.NewError(connect.CodeUnavailable, errors.New("node service unavailable")), + } + withExternalNodeResolverAPI(t, service) + store := &mockExternalNodeStore{ + user: &entity.User{ID: "user_1"}, + org: &entity.Organization{ID: "org_1"}, + } + + resolved, err := ResolveWorkspaceOrNodeWithContext(context.Background(), store, "gpu-node") + if err == nil { + t.Fatalf("expected node service error, got %+v", resolved) + } + if connect.CodeOf(err) != connect.CodeUnavailable { + t.Fatalf("expected unavailable error, got %v", err) + } + if !strings.Contains(err.Error(), "node service unavailable") { + t.Fatalf("expected node service error details, got %v", err) + } + if strings.Contains(err.Error(), "instance with id/name") { + t.Fatalf("node service error was masked by workspace lookup error: %v", err) + } +} + +func TestResolveWorkspaceOrNodeWithContext_NoMatchesReturnsWorkspaceError(t *testing.T) { + withExternalNodeResolverAPI(t, &fakeExternalNodeResolverService{}) + store := &mockExternalNodeStore{ + user: &entity.User{ID: "user_1"}, + org: &entity.Organization{ID: "org_1"}, + } + + resolved, err := ResolveWorkspaceOrNodeWithContext(context.Background(), store, "missing") + if err == nil { + t.Fatalf("expected not-found error, got %+v", resolved) + } + if !strings.Contains(err.Error(), `instance or external node with id/name "missing" not found`) { + t.Fatalf("expected a combined target not-found error, got %v", err) + } + var validationErr breverrors.ValidationError + if !breverrors.As(err, &validationErr) { + t.Fatalf("expected a validation error, got %T: %v", err, err) + } +} + +func TestResolveWorkspaceOrNodeWithContext_DoesNotFallbackAfterWorkspaceLookupFailure(t *testing.T) { + service := &fakeExternalNodeResolverService{nodes: []*nodev1.ExternalNode{ + {ExternalNodeId: "unode_1", Name: "shared-name"}, + }} + withExternalNodeResolverAPI(t, service) + store := &mockExternalNodeStore{ + user: &entity.User{ID: "user_1"}, + org: &entity.Organization{ID: "org_1"}, + workspaceErr: errors.New("workspace service unavailable"), + } + + resolved, err := ResolveWorkspaceOrNodeWithContext(context.Background(), store, "shared-name") + if err == nil { + t.Fatalf("expected workspace lookup error, got %+v", resolved) + } + if !strings.Contains(err.Error(), "workspace service unavailable") { + t.Fatalf("expected the workspace lookup error, got %v", err) + } + if service.listCalls != 0 { + t.Fatalf("expected no external-node fallback, got %d ListNodes call(s)", service.listCalls) + } +} + func TestResolveExternalNodeSSH_HappyPath(t *testing.T) { store := &mockExternalNodeStore{ user: &entity.User{ID: "user_1"}, diff --git a/pkg/cmd/util/util.go b/pkg/cmd/util/util.go index 3ac0f5fb..3dfac99e 100644 --- a/pkg/cmd/util/util.go +++ b/pkg/cmd/util/util.go @@ -17,36 +17,56 @@ type GetWorkspaceByNameOrIDErrStore interface { } func GetUserWorkspaceByNameOrIDErr(storeQ GetWorkspaceByNameOrIDErrStore, workspaceNameOrID string) (*entity.Workspace, error) { + workspace, found, err := findUserWorkspaceByNameOrID(storeQ, workspaceNameOrID) + if err != nil { + return nil, err + } + if !found { + return nil, breverrors.NewValidationError(fmt.Sprintf("instance with id/name %s not found", workspaceNameOrID)) + } + return workspace, nil +} + +// findUserWorkspaceByNameOrID distinguishes a missing workspace from lookup failures so callers +// that support other target types only fall back after a confirmed absence. +func findUserWorkspaceByNameOrID( + storeQ GetWorkspaceByNameOrIDErrStore, + workspaceNameOrID string, +) (*entity.Workspace, bool, error) { if auth.IsAPIKeyAuthStore(storeQ) { org, err := storeQ.GetActiveOrganizationOrDefault() if err != nil { - return nil, breverrors.WrapAndTrace(err) + return nil, false, breverrors.WrapAndTrace(err) } workspaces, err := storeQ.GetWorkspaceByNameOrID(org.ID, workspaceNameOrID) if err != nil { - return nil, breverrors.WrapAndTrace(err) + return nil, false, breverrors.WrapAndTrace(err) } - return selectWorkspaceByNameOrID(workspaces, workspaceNameOrID) + if len(workspaces) == 0 { + return nil, false, nil + } + workspace, err := selectWorkspaceByNameOrID(workspaces, workspaceNameOrID) + return workspace, true, err } user, err := storeQ.GetCurrentUser() if err != nil { - return nil, breverrors.WrapAndTrace(err) + return nil, false, breverrors.WrapAndTrace(err) } org, err := storeQ.GetActiveOrganizationOrDefault() if err != nil { - return nil, breverrors.WrapAndTrace(err) + return nil, false, breverrors.WrapAndTrace(err) } workspaces, err := storeQ.GetWorkspaceByNameOrID(org.ID, workspaceNameOrID) if err != nil { - return nil, breverrors.WrapAndTrace(err) + return nil, false, breverrors.WrapAndTrace(err) } workspaces = store.FilterForUserWorkspaces(workspaces, user.ID) if len(workspaces) == 0 { - return nil, breverrors.NewValidationError(fmt.Sprintf("instance with id/name %s not found", workspaceNameOrID)) + return nil, false, nil } - return &workspaces[0], nil + return &workspaces[0], true, nil } func GetAnyWorkspaceByIDOrNameInActiveOrgErr(storeQ GetWorkspaceByNameOrIDErrStore, workspaceNameOrID string) (*entity.Workspace, error) { From bf8511f30f3f46a0ff3300d3a2626acc90b3eb6c Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Wed, 19 Aug 2026 03:24:27 +0000 Subject: [PATCH 2/7] =?UTF-8?q?=F0=9F=93=9D=20Add=20docstrings=20to=20`age?= =?UTF-8?q?nt/list-instance-ports`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Docstrings generation was requested by @callen-bot. The following files were modified: * `pkg/cmd/cmd.go` * `pkg/cmd/ports/ports.go` * `pkg/cmd/util/externalnode.go` * `pkg/cmd/util/util.go` These files were ignored: * `pkg/cmd/ports/ports_test.go` * `pkg/cmd/util/externalnode_test.go` These file types are not supported: * `.agents/skills/brev-cli/SKILL.md` * `.agents/skills/brev-cli/reference/commands.md` --- pkg/cmd/cmd.go | 1 + pkg/cmd/ports/ports.go | 15 +++++++++++++++ pkg/cmd/util/externalnode.go | 17 ++++++++++++----- pkg/cmd/util/util.go | 7 ++++++- 4 files changed, 34 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/cmd.go b/pkg/cmd/cmd.go index 831a2aa9..ed1ded14 100644 --- a/pkg/cmd/cmd.go +++ b/pkg/cmd/cmd.go @@ -270,6 +270,7 @@ func NewBrevCommand() *cobra.Command { //nolint:funlen,gocognit,gocyclo // defin return cmds } +// createCmdTree registers the Brev CLI's commands and development-only commands when enabled. func createCmdTree(cmd *cobra.Command, t *terminal.Terminal, loginCmdStore *store.AuthHTTPStore, noLoginCmdStore *store.AuthHTTPStore, loginAuth *auth.LoginAuth, externalNodeCmdStore *store.AuthHTTPStore) { //nolint:funlen // define brev command cmd.AddCommand(set.NewCmdSet(t, loginCmdStore, noLoginCmdStore)) cmd.AddCommand(ls.NewCmdLs(t, loginCmdStore, noLoginCmdStore)) diff --git a/pkg/cmd/ports/ports.go b/pkg/cmd/ports/ports.go index 868bc0b3..9a58c444 100644 --- a/pkg/cmd/ports/ports.go +++ b/pkg/cmd/ports/ports.go @@ -115,6 +115,8 @@ func Run(ctx context.Context, out io.Writer, portStore Store, nameOrID string, j return displayTables(out, nameOrID, portInfos) } +// toPortInfos converts API port definitions into port information for display and serialization. +// Nil port entries are skipped. func toPortInfos(apiPorts []*devplanev1.Port) []PortInfo { portInfos := make([]PortInfo, 0, len(apiPorts)) for _, port := range apiPorts { @@ -142,6 +144,7 @@ func toPortInfos(apiPorts []*devplanev1.Port) []PortInfo { return portInfos } +// endpoint builds the public endpoint for a port, using an HTTPS URL for HTTP ports and host-port notation for network ports. func endpoint(port *devplanev1.Port, isHTTP bool) string { hostname := port.GetHostname() if hostname == "" { @@ -156,6 +159,7 @@ func endpoint(port *devplanev1.Port, isHTTP bool) string { return net.JoinHostPort(hostname, strconv.Itoa(int(port.GetPortNumber()))) } +// protocolLabel returns the display label for a port's protocol, distinguishing HTTP and network port protocols. func protocolLabel(port *devplanev1.Port, isHTTP bool) string { if isHTTP { switch port.GetHttpProtocol() { @@ -180,6 +184,7 @@ func protocolLabel(port *devplanev1.Port, isHTTP bool) string { } } +// portTypeLabel returns the display label for a port type. func portTypeLabel(portType devplanev1.PortType) string { switch portType { case devplanev1.PortType_PORT_TYPE_UNSPECIFIED: @@ -193,6 +198,7 @@ func portTypeLabel(portType devplanev1.PortType) string { } } +// writeJSON writes port information as indented JSON to out and returns any serialization or output error. func writeJSON(out io.Writer, portInfos []PortInfo) error { encoded, err := json.MarshalIndent(portInfos, "", " ") if err != nil { @@ -202,6 +208,8 @@ func writeJSON(out io.Writer, portInfos []PortInfo) error { return breverrors.WrapAndTrace(err) } +// displayTables writes port information as separate HTTP and network port tables. +// It reports when no ports are open and returns any output error encountered. func displayTables(out io.Writer, nameOrID string, portInfos []PortInfo) error { if len(portInfos) == 0 { _, err := fmt.Fprintf(out, "No ports are open on %s.\n", nameOrID) @@ -238,6 +246,7 @@ func displayTables(out io.Writer, nameOrID string, portInfos []PortInfo) error { return nil } +// displayHTTPTable renders HTTP port mappings with endpoint, authorization, source restrictions, public port, destination port, and protocol. If the destination port is unset, the public port is displayed as the destination. func displayHTTPTable(out io.Writer, portInfos []PortInfo) { tw := newTable(out) tw.AppendHeader(table.Row{"ENDPOINT", "AUTHORIZATION", "IP RESTRICTIONS", "PUBLIC PORT", "DESTINATION PORT", "PROTOCOL"}) @@ -258,6 +267,7 @@ func displayHTTPTable(out io.Writer, portInfos []PortInfo) { tw.Render() } +// displayNetworkTable renders network port mappings in a table. func displayNetworkTable(out io.Writer, portInfos []PortInfo) { tw := newTable(out) tw.AppendHeader(table.Row{"ENDPOINT", "IP RESTRICTIONS", "PUBLIC PORT", "DESTINATION PORT", "PROTOCOL"}) @@ -273,6 +283,7 @@ func displayNetworkTable(out io.Writer, portInfos []PortInfo) { tw.Render() } +// newTable creates a borderless table writer with row, column, and header separators disabled. func newTable(out io.Writer) table.Writer { tw := table.NewWriter() tw.SetOutputMirror(out) @@ -285,6 +296,7 @@ func newTable(out io.Writer) table.Writer { return tw } +// authorizationLabel formats the access authorization for a port. func authorizationLabel(port PortInfo) string { if port.AllowPublicUnauthenticated { return "Public" @@ -295,6 +307,7 @@ func authorizationLabel(port PortInfo) string { return "-" } +// allowedSourcesLabel formats allowed source ranges for display, using "Anywhere" for unrestricted access. func allowedSourcesLabel(allowedSources []string) string { if len(allowedSources) == 0 { return "Anywhere" @@ -312,6 +325,7 @@ func allowedSourcesLabel(allowedSources []string) string { return strings.Join(allowedSources, ", ") } +// portNumberLabel formats a port number for display, using "-" when the port is zero. func portNumberLabel(port int32) string { if port == 0 { return "-" @@ -319,6 +333,7 @@ func portNumberLabel(port int32) string { return strconv.Itoa(int(port)) } +// valueOrDash replaces an empty string with a dash. func valueOrDash(value string) string { if value == "" { return "-" diff --git a/pkg/cmd/util/externalnode.go b/pkg/cmd/util/externalnode.go index 90b57b45..6c16b65c 100644 --- a/pkg/cmd/util/externalnode.go +++ b/pkg/cmd/util/externalnode.go @@ -34,14 +34,18 @@ type WorkspaceOrNode struct { } // ResolveWorkspaceOrNode looks up a workspace first; if not found, falls back to external nodes. -// The store must satisfy both GetWorkspaceByNameOrIDErrStore and ExternalNodeStore. +// ResolveWorkspaceOrNode resolves a workspace or external node by name or ID. +// It searches for a workspace first, then an external node, and returns a validation +// error when neither is found. Lookup and service errors are propagated. func ResolveWorkspaceOrNode(store WorkspaceOrNodeResolver, nameOrID string, ) (*WorkspaceOrNode, error) { return ResolveWorkspaceOrNodeWithContext(context.Background(), store, nameOrID) } // ResolveWorkspaceOrNodeWithContext looks up a workspace first; if not found, falls back to -// external nodes. The context is used for the external-node service request. +// ResolveWorkspaceOrNodeWithContext resolves a workspace or external node by name or ID. +// It searches workspaces first, then external nodes, using ctx for external-node requests. +// It returns a validation error when neither resource exists. func ResolveWorkspaceOrNodeWithContext(ctx context.Context, store WorkspaceOrNodeResolver, nameOrID string, ) (*WorkspaceOrNode, error) { workspace, workspaceFound, err := findUserWorkspaceByNameOrID(store, nameOrID) @@ -126,7 +130,8 @@ func resolvePortForSSHAccess(node *nodev1.ExternalNode, access *nodev1.SSHAccess } // OpenPort calls the OpenPort RPC to open a port on an external node via netbird. -// This must be called before attempting to connect to a non-SSH port on a node. +// OpenPort requests access to a non-SSH port on an external node. +// It returns the opened port or an error. func OpenPort(store ExternalNodeStore, nodeID string, portNumber int32, protocol nodev1.PortProtocol) (*nodev1.Port, error) { client := register.NewNodeServiceClient(store, config.GlobalConfig.GetBrevPublicAPIURL()) resp, err := client.OpenPort(context.Background(), connect.NewRequest(&nodev1.OpenPortRequest{ @@ -141,14 +146,15 @@ func OpenPort(store ExternalNodeStore, nodeID string, portNumber int32, protocol } // FindExternalNode searches for an external node by name or ID in the user's active organization. -// Returns (nil, nil) if no matching node is found. +// FindExternalNode locates an external node by ID or case-insensitive name. +// It returns nil, nil when no matching node exists. func FindExternalNode(store ExternalNodeStore, nameOrID string) (*nodev1.ExternalNode, error) { return FindExternalNodeWithContext(context.Background(), store, nameOrID) } // FindExternalNodeWithContext searches for an external node by name or ID in the user's active // organization. Exact IDs take precedence over case-insensitive names. -// Returns (nil, nil) if no matching node is found. +// It returns the matching node, or nil when no matching node is found. func FindExternalNodeWithContext(ctx context.Context, store ExternalNodeStore, nameOrID string) (*nodev1.ExternalNode, error) { org, err := store.GetActiveOrganizationOrDefault() if err != nil { @@ -164,6 +170,7 @@ func FindExternalNodeWithContext(ctx context.Context, store ExternalNodeStore, n return findExternalNode(resp.Msg.GetItems(), nameOrID), nil } +// findExternalNode returns the external node matching nameOrID by ID or, if no ID matches, by case-insensitive name. It returns nil when no match exists. func findExternalNode(nodes []*nodev1.ExternalNode, nameOrID string) *nodev1.ExternalNode { // IDs are unique and unambiguous, so they must win even if an earlier node's name collides. for _, node := range nodes { diff --git a/pkg/cmd/util/util.go b/pkg/cmd/util/util.go index 3dfac99e..78ed431a 100644 --- a/pkg/cmd/util/util.go +++ b/pkg/cmd/util/util.go @@ -16,6 +16,8 @@ type GetWorkspaceByNameOrIDErrStore interface { GetCurrentUser() (*entity.User, error) } +// GetUserWorkspaceByNameOrIDErr finds a workspace accessible to the current user by name or ID. +// It returns a validation error when no matching workspace is found. func GetUserWorkspaceByNameOrIDErr(storeQ GetWorkspaceByNameOrIDErrStore, workspaceNameOrID string) (*entity.Workspace, error) { workspace, found, err := findUserWorkspaceByNameOrID(storeQ, workspaceNameOrID) if err != nil { @@ -28,7 +30,8 @@ func GetUserWorkspaceByNameOrIDErr(storeQ GetWorkspaceByNameOrIDErrStore, worksp } // findUserWorkspaceByNameOrID distinguishes a missing workspace from lookup failures so callers -// that support other target types only fall back after a confirmed absence. +// findUserWorkspaceByNameOrID looks up a workspace accessible to the current user by name or ID. +// It reports whether a matching workspace was found and returns any lookup or selection error. func findUserWorkspaceByNameOrID( storeQ GetWorkspaceByNameOrIDErrStore, workspaceNameOrID string, @@ -69,6 +72,8 @@ func findUserWorkspaceByNameOrID( return &workspaces[0], true, nil } +// GetAnyWorkspaceByIDOrNameInActiveOrgErr finds a workspace by name or ID within the active organization. +// It returns an error when the workspace cannot be found or the match is ambiguous. func GetAnyWorkspaceByIDOrNameInActiveOrgErr(storeQ GetWorkspaceByNameOrIDErrStore, workspaceNameOrID string) (*entity.Workspace, error) { org, err := storeQ.GetActiveOrganizationOrDefault() if err != nil { From e0d93f5277ee3e0b5d9385669d542c79dd9af708 Mon Sep 17 00:00:00 2001 From: Allen Chen Date: Tue, 18 Aug 2026 20:30:47 -0700 Subject: [PATCH 3/7] Address ports review feedback --- .agents/skills/brev-cli/reference/commands.md | 1 + pkg/cmd/cmd.go | 1 - pkg/cmd/ports/ports.go | 17 +---------------- pkg/cmd/util/externalnode.go | 17 +++++------------ pkg/cmd/util/util.go | 7 +------ 5 files changed, 8 insertions(+), 35 deletions(-) diff --git a/.agents/skills/brev-cli/reference/commands.md b/.agents/skills/brev-cli/reference/commands.md index 2111490b..fab3d905 100644 --- a/.agents/skills/brev-cli/reference/commands.md +++ b/.agents/skills/brev-cli/reference/commands.md @@ -475,6 +475,7 @@ brev ports [flags] ``` **Flags:** + | Flag | Description | |------|-------------| | `--json` | Output the port mappings as JSON | diff --git a/pkg/cmd/cmd.go b/pkg/cmd/cmd.go index ed1ded14..831a2aa9 100644 --- a/pkg/cmd/cmd.go +++ b/pkg/cmd/cmd.go @@ -270,7 +270,6 @@ func NewBrevCommand() *cobra.Command { //nolint:funlen,gocognit,gocyclo // defin return cmds } -// createCmdTree registers the Brev CLI's commands and development-only commands when enabled. func createCmdTree(cmd *cobra.Command, t *terminal.Terminal, loginCmdStore *store.AuthHTTPStore, noLoginCmdStore *store.AuthHTTPStore, loginAuth *auth.LoginAuth, externalNodeCmdStore *store.AuthHTTPStore) { //nolint:funlen // define brev command cmd.AddCommand(set.NewCmdSet(t, loginCmdStore, noLoginCmdStore)) cmd.AddCommand(ls.NewCmdLs(t, loginCmdStore, noLoginCmdStore)) diff --git a/pkg/cmd/ports/ports.go b/pkg/cmd/ports/ports.go index 9a58c444..f387f7e3 100644 --- a/pkg/cmd/ports/ports.go +++ b/pkg/cmd/ports/ports.go @@ -86,7 +86,7 @@ func Run(ctx context.Context, out io.Writer, portStore Store, nameOrID string, j EnvironmentId: target.Workspace.ID, })) if err != nil { - return fmt.Errorf("get ports for instance %q: %w", nameOrID, err) + return breverrors.WrapAndTrace(fmt.Errorf("get ports for instance %q: %w", nameOrID, err)) } var networkInfo *devplanev1.EnvironmentNetworkInfo if resp != nil && resp.Msg != nil { @@ -115,8 +115,6 @@ func Run(ctx context.Context, out io.Writer, portStore Store, nameOrID string, j return displayTables(out, nameOrID, portInfos) } -// toPortInfos converts API port definitions into port information for display and serialization. -// Nil port entries are skipped. func toPortInfos(apiPorts []*devplanev1.Port) []PortInfo { portInfos := make([]PortInfo, 0, len(apiPorts)) for _, port := range apiPorts { @@ -144,7 +142,6 @@ func toPortInfos(apiPorts []*devplanev1.Port) []PortInfo { return portInfos } -// endpoint builds the public endpoint for a port, using an HTTPS URL for HTTP ports and host-port notation for network ports. func endpoint(port *devplanev1.Port, isHTTP bool) string { hostname := port.GetHostname() if hostname == "" { @@ -159,7 +156,6 @@ func endpoint(port *devplanev1.Port, isHTTP bool) string { return net.JoinHostPort(hostname, strconv.Itoa(int(port.GetPortNumber()))) } -// protocolLabel returns the display label for a port's protocol, distinguishing HTTP and network port protocols. func protocolLabel(port *devplanev1.Port, isHTTP bool) string { if isHTTP { switch port.GetHttpProtocol() { @@ -184,7 +180,6 @@ func protocolLabel(port *devplanev1.Port, isHTTP bool) string { } } -// portTypeLabel returns the display label for a port type. func portTypeLabel(portType devplanev1.PortType) string { switch portType { case devplanev1.PortType_PORT_TYPE_UNSPECIFIED: @@ -198,7 +193,6 @@ func portTypeLabel(portType devplanev1.PortType) string { } } -// writeJSON writes port information as indented JSON to out and returns any serialization or output error. func writeJSON(out io.Writer, portInfos []PortInfo) error { encoded, err := json.MarshalIndent(portInfos, "", " ") if err != nil { @@ -208,8 +202,6 @@ func writeJSON(out io.Writer, portInfos []PortInfo) error { return breverrors.WrapAndTrace(err) } -// displayTables writes port information as separate HTTP and network port tables. -// It reports when no ports are open and returns any output error encountered. func displayTables(out io.Writer, nameOrID string, portInfos []PortInfo) error { if len(portInfos) == 0 { _, err := fmt.Fprintf(out, "No ports are open on %s.\n", nameOrID) @@ -246,7 +238,6 @@ func displayTables(out io.Writer, nameOrID string, portInfos []PortInfo) error { return nil } -// displayHTTPTable renders HTTP port mappings with endpoint, authorization, source restrictions, public port, destination port, and protocol. If the destination port is unset, the public port is displayed as the destination. func displayHTTPTable(out io.Writer, portInfos []PortInfo) { tw := newTable(out) tw.AppendHeader(table.Row{"ENDPOINT", "AUTHORIZATION", "IP RESTRICTIONS", "PUBLIC PORT", "DESTINATION PORT", "PROTOCOL"}) @@ -267,7 +258,6 @@ func displayHTTPTable(out io.Writer, portInfos []PortInfo) { tw.Render() } -// displayNetworkTable renders network port mappings in a table. func displayNetworkTable(out io.Writer, portInfos []PortInfo) { tw := newTable(out) tw.AppendHeader(table.Row{"ENDPOINT", "IP RESTRICTIONS", "PUBLIC PORT", "DESTINATION PORT", "PROTOCOL"}) @@ -283,7 +273,6 @@ func displayNetworkTable(out io.Writer, portInfos []PortInfo) { tw.Render() } -// newTable creates a borderless table writer with row, column, and header separators disabled. func newTable(out io.Writer) table.Writer { tw := table.NewWriter() tw.SetOutputMirror(out) @@ -296,7 +285,6 @@ func newTable(out io.Writer) table.Writer { return tw } -// authorizationLabel formats the access authorization for a port. func authorizationLabel(port PortInfo) string { if port.AllowPublicUnauthenticated { return "Public" @@ -307,7 +295,6 @@ func authorizationLabel(port PortInfo) string { return "-" } -// allowedSourcesLabel formats allowed source ranges for display, using "Anywhere" for unrestricted access. func allowedSourcesLabel(allowedSources []string) string { if len(allowedSources) == 0 { return "Anywhere" @@ -325,7 +312,6 @@ func allowedSourcesLabel(allowedSources []string) string { return strings.Join(allowedSources, ", ") } -// portNumberLabel formats a port number for display, using "-" when the port is zero. func portNumberLabel(port int32) string { if port == 0 { return "-" @@ -333,7 +319,6 @@ func portNumberLabel(port int32) string { return strconv.Itoa(int(port)) } -// valueOrDash replaces an empty string with a dash. func valueOrDash(value string) string { if value == "" { return "-" diff --git a/pkg/cmd/util/externalnode.go b/pkg/cmd/util/externalnode.go index 6c16b65c..90b57b45 100644 --- a/pkg/cmd/util/externalnode.go +++ b/pkg/cmd/util/externalnode.go @@ -34,18 +34,14 @@ type WorkspaceOrNode struct { } // ResolveWorkspaceOrNode looks up a workspace first; if not found, falls back to external nodes. -// ResolveWorkspaceOrNode resolves a workspace or external node by name or ID. -// It searches for a workspace first, then an external node, and returns a validation -// error when neither is found. Lookup and service errors are propagated. +// The store must satisfy both GetWorkspaceByNameOrIDErrStore and ExternalNodeStore. func ResolveWorkspaceOrNode(store WorkspaceOrNodeResolver, nameOrID string, ) (*WorkspaceOrNode, error) { return ResolveWorkspaceOrNodeWithContext(context.Background(), store, nameOrID) } // ResolveWorkspaceOrNodeWithContext looks up a workspace first; if not found, falls back to -// ResolveWorkspaceOrNodeWithContext resolves a workspace or external node by name or ID. -// It searches workspaces first, then external nodes, using ctx for external-node requests. -// It returns a validation error when neither resource exists. +// external nodes. The context is used for the external-node service request. func ResolveWorkspaceOrNodeWithContext(ctx context.Context, store WorkspaceOrNodeResolver, nameOrID string, ) (*WorkspaceOrNode, error) { workspace, workspaceFound, err := findUserWorkspaceByNameOrID(store, nameOrID) @@ -130,8 +126,7 @@ func resolvePortForSSHAccess(node *nodev1.ExternalNode, access *nodev1.SSHAccess } // OpenPort calls the OpenPort RPC to open a port on an external node via netbird. -// OpenPort requests access to a non-SSH port on an external node. -// It returns the opened port or an error. +// This must be called before attempting to connect to a non-SSH port on a node. func OpenPort(store ExternalNodeStore, nodeID string, portNumber int32, protocol nodev1.PortProtocol) (*nodev1.Port, error) { client := register.NewNodeServiceClient(store, config.GlobalConfig.GetBrevPublicAPIURL()) resp, err := client.OpenPort(context.Background(), connect.NewRequest(&nodev1.OpenPortRequest{ @@ -146,15 +141,14 @@ func OpenPort(store ExternalNodeStore, nodeID string, portNumber int32, protocol } // FindExternalNode searches for an external node by name or ID in the user's active organization. -// FindExternalNode locates an external node by ID or case-insensitive name. -// It returns nil, nil when no matching node exists. +// Returns (nil, nil) if no matching node is found. func FindExternalNode(store ExternalNodeStore, nameOrID string) (*nodev1.ExternalNode, error) { return FindExternalNodeWithContext(context.Background(), store, nameOrID) } // FindExternalNodeWithContext searches for an external node by name or ID in the user's active // organization. Exact IDs take precedence over case-insensitive names. -// It returns the matching node, or nil when no matching node is found. +// Returns (nil, nil) if no matching node is found. func FindExternalNodeWithContext(ctx context.Context, store ExternalNodeStore, nameOrID string) (*nodev1.ExternalNode, error) { org, err := store.GetActiveOrganizationOrDefault() if err != nil { @@ -170,7 +164,6 @@ func FindExternalNodeWithContext(ctx context.Context, store ExternalNodeStore, n return findExternalNode(resp.Msg.GetItems(), nameOrID), nil } -// findExternalNode returns the external node matching nameOrID by ID or, if no ID matches, by case-insensitive name. It returns nil when no match exists. func findExternalNode(nodes []*nodev1.ExternalNode, nameOrID string) *nodev1.ExternalNode { // IDs are unique and unambiguous, so they must win even if an earlier node's name collides. for _, node := range nodes { diff --git a/pkg/cmd/util/util.go b/pkg/cmd/util/util.go index 78ed431a..3dfac99e 100644 --- a/pkg/cmd/util/util.go +++ b/pkg/cmd/util/util.go @@ -16,8 +16,6 @@ type GetWorkspaceByNameOrIDErrStore interface { GetCurrentUser() (*entity.User, error) } -// GetUserWorkspaceByNameOrIDErr finds a workspace accessible to the current user by name or ID. -// It returns a validation error when no matching workspace is found. func GetUserWorkspaceByNameOrIDErr(storeQ GetWorkspaceByNameOrIDErrStore, workspaceNameOrID string) (*entity.Workspace, error) { workspace, found, err := findUserWorkspaceByNameOrID(storeQ, workspaceNameOrID) if err != nil { @@ -30,8 +28,7 @@ func GetUserWorkspaceByNameOrIDErr(storeQ GetWorkspaceByNameOrIDErrStore, worksp } // findUserWorkspaceByNameOrID distinguishes a missing workspace from lookup failures so callers -// findUserWorkspaceByNameOrID looks up a workspace accessible to the current user by name or ID. -// It reports whether a matching workspace was found and returns any lookup or selection error. +// that support other target types only fall back after a confirmed absence. func findUserWorkspaceByNameOrID( storeQ GetWorkspaceByNameOrIDErrStore, workspaceNameOrID string, @@ -72,8 +69,6 @@ func findUserWorkspaceByNameOrID( return &workspaces[0], true, nil } -// GetAnyWorkspaceByIDOrNameInActiveOrgErr finds a workspace by name or ID within the active organization. -// It returns an error when the workspace cannot be found or the match is ambiguous. func GetAnyWorkspaceByIDOrNameInActiveOrgErr(storeQ GetWorkspaceByNameOrIDErrStore, workspaceNameOrID string) (*entity.Workspace, error) { org, err := storeQ.GetActiveOrganizationOrDefault() if err != nil { From d1012d1c0a50286f1d8904ba043db175a87ed09d Mon Sep 17 00:00:00 2001 From: Allen Chen Date: Tue, 18 Aug 2026 20:40:51 -0700 Subject: [PATCH 4/7] Label SSH ports as network ports --- pkg/cmd/ports/ports.go | 2 +- pkg/cmd/ports/ports_test.go | 23 ++++++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/ports/ports.go b/pkg/cmd/ports/ports.go index f387f7e3..4957fc38 100644 --- a/pkg/cmd/ports/ports.go +++ b/pkg/cmd/ports/ports.go @@ -230,7 +230,7 @@ func displayTables(out io.Writer, nameOrID string, portInfos []PortInfo) error { } } if len(networkPorts) > 0 { - if _, err := fmt.Fprintln(out, "TCP/UDP PORTS"); err != nil { + if _, err := fmt.Fprintln(out, "NETWORK PORTS"); err != nil { return breverrors.WrapAndTrace(err) } displayNetworkTable(out, networkPorts) diff --git a/pkg/cmd/ports/ports_test.go b/pkg/cmd/ports/ports_test.go index 95eae00d..ab985736 100644 --- a/pkg/cmd/ports/ports_test.go +++ b/pkg/cmd/ports/ports_test.go @@ -168,7 +168,7 @@ func TestRunExternalNodeByIDDisplaysTables(t *testing.T) { assert.Contains(t, out.String(), "HTTP APPLICATIONS") assert.Contains(t, out.String(), "https://jupyter-node.apps.run.brev.nvidia.com") assert.Contains(t, out.String(), "user@example.com") - assert.Contains(t, out.String(), "TCP/UDP PORTS") + assert.Contains(t, out.String(), "NETWORK PORTS") assert.Contains(t, out.String(), "PUBLIC PORT") assert.Contains(t, out.String(), "DESTINATION PORT") assert.Contains(t, out.String(), "global.prd.ga.run.brev.nvidia.com:18928") @@ -177,6 +177,27 @@ func TestRunExternalNodeByIDDisplaysTables(t *testing.T) { assert.Contains(t, out.String(), "TCP") } +func TestDisplayTablesSSHUsesNetworkHeading(t *testing.T) { + var out bytes.Buffer + ports := []PortInfo{ + { + Kind: portKindNetwork, + Endpoint: "gateway.example.com:18928", + PublicPort: 18928, + DestinationPort: 22, + Protocol: "SSH", + }, + } + + err := displayTables(&out, "ssh-node", ports) + + require.NoError(t, err) + assert.Contains(t, out.String(), "NETWORK PORTS") + assert.Contains(t, out.String(), "gateway.example.com:18928") + assert.Contains(t, out.String(), "SSH") + assert.NotContains(t, out.String(), "TCP/UDP PORTS") +} + func TestToPortInfosHandlesPublicHTTPAndRestrictedUDP(t *testing.T) { public := true httpHostname := "app.example.com" From dce481be06e8b34aece277fb0b156528d0f7f6d0 Mon Sep 17 00:00:00 2001 From: Allen Chen Date: Wed, 19 Aug 2026 09:38:54 -0700 Subject: [PATCH 5/7] Refine ports list command --- .agents/skills/brev-cli/SKILL.md | 6 +-- .agents/skills/brev-cli/reference/commands.md | 24 +++++----- pkg/cmd/ports/ports.go | 47 ++++++++++--------- pkg/cmd/ports/ports_test.go | 20 +++++--- 4 files changed, 55 insertions(+), 42 deletions(-) diff --git a/.agents/skills/brev-cli/SKILL.md b/.agents/skills/brev-cli/SKILL.md index 50ce1101..a7c862f9 100644 --- a/.agents/skills/brev-cli/SKILL.md +++ b/.agents/skills/brev-cli/SKILL.md @@ -171,9 +171,9 @@ brev copy my-instance:/remote/file ./local-path/ # Port forward brev port-forward my-instance -p 8080:8080 -# List Skybridge-managed HTTP and network ports for an instance or external node -brev ports my-instance -brev ports my-node --json +# List Brev-managed HTTP and network ports for an instance or external node +brev ports ls my-instance +brev ports ls my-node --json ``` ### Listing Instances and Nodes diff --git a/.agents/skills/brev-cli/reference/commands.md b/.agents/skills/brev-cli/reference/commands.md index fab3d905..e1943e3e 100644 --- a/.agents/skills/brev-cli/reference/commands.md +++ b/.agents/skills/brev-cli/reference/commands.md @@ -466,12 +466,12 @@ brev port-forward my-instance -p 8080:8080 brev port-forward my-instance -p 3000:3000 ``` -### brev ports -List Skybridge-managed HTTP applications and raw network port mappings for a +### brev ports ls +List Brev-managed HTTP applications and raw network port mappings for a managed instance or registered compute node. ```bash -brev ports [flags] +brev ports ls [flags] ``` **Flags:** @@ -483,18 +483,18 @@ brev ports [flags] The table output includes endpoint, IP restrictions, public port, destination port, and protocol. HTTP applications also include their authorization policy. -For managed instances, this command reads the Skybridge network member. It does -not synthesize the legacy secure-link or firewall rows shown by the Brev console, -because those rows do not have real port IDs and cannot be used by port-management -automation. If the instance is still provisioning or uses legacy network access, -the command returns an error directing the user to the console. +For managed instances, this command reads the Brev-managed network +configuration. It does not synthesize the legacy secure-link or firewall rows +shown by the Brev console, because those rows do not have real port IDs and +cannot be used by port-management automation. If the instance is still +provisioning or uses legacy network access, the command returns an error +directing the user to the console. The JSON output is an array with the following stable fields: | Field | Meaning | |------|---------| | `port_id` | Unique port mapping ID used by automation | -| `kind` | `http` or `network` | | `endpoint` | Public URL or `host:port` endpoint | | `public_port` | Externally addressable port | | `destination_port` | Port listening on the target machine | @@ -506,9 +506,9 @@ The JSON output is an array with the following stable fields: **Examples:** ```bash -brev ports my-instance -brev ports my-node -brev ports my-instance --json +brev ports ls my-instance +brev ports ls my-node +brev ports ls my-instance --json ``` ## Organization Commands diff --git a/pkg/cmd/ports/ports.go b/pkg/cmd/ports/ports.go index 4957fc38..b68e3ae3 100644 --- a/pkg/cmd/ports/ports.go +++ b/pkg/cmd/ports/ports.go @@ -1,4 +1,4 @@ -// Package ports displays Skybridge-managed public port mappings for an instance or external node. +// Package ports displays Brev-managed public port mappings for an instance or external node. package ports import ( @@ -22,11 +22,6 @@ import ( breverrors "github.com/brevdev/brev-cli/pkg/errors" ) -const ( - portKindHTTP = "http" - portKindNetwork = "network" -) - // Store contains the dependencies needed to resolve both managed instances and // registered compute nodes. type Store interface { @@ -36,7 +31,6 @@ type Store interface { // PortInfo is the stable JSON representation of a port mapping. type PortInfo struct { PortID string `json:"port_id"` - Kind string `json:"kind"` Endpoint string `json:"endpoint"` PublicPort int32 `json:"public_port"` DestinationPort int32 `json:"destination_port"` @@ -45,20 +39,35 @@ type PortInfo struct { AuthorizedEmails []string `json:"authorized_emails"` AllowPublicUnauthenticated bool `json:"allow_public_unauthenticated"` Type string `json:"type"` + isHTTP bool } -// NewCmdPorts creates the `brev ports` command. +// NewCmdPorts creates the `brev ports` command group. func NewCmdPorts(portStore Store) *cobra.Command { + cmd := &cobra.Command{ + Annotations: map[string]string{"access": ""}, + Use: "ports", + Short: "Manage ports for an instance or external node", + Args: cmderrors.TransformToValidationError(cobra.NoArgs), + Example: ` + brev ports ls my-instance + brev ports ls my-node --json`, + } + cmd.AddCommand(NewCmdPortsLs(portStore)) + return cmd +} + +// NewCmdPortsLs creates the `brev ports ls` command. +func NewCmdPortsLs(portStore Store) *cobra.Command { var jsonOutput bool cmd := &cobra.Command{ - Annotations: map[string]string{"access": ""}, - Use: "ports ", + Use: "ls ", DisableFlagsInUseLine: true, - Short: "List Skybridge-managed ports for an instance or external node", + Short: "List Brev-managed ports for an instance or external node", Example: ` - brev ports my-instance - brev ports my-node --json`, + brev ports ls my-instance + brev ports ls my-node --json`, Args: cmderrors.TransformToValidationError(cobra.ExactArgs(1)), RunE: func(cmd *cobra.Command, args []string) error { if err := Run(cmd.Context(), cmd.OutOrStdout(), portStore, args[0], jsonOutput); err != nil { @@ -93,12 +102,12 @@ func Run(ctx context.Context, out io.Writer, portStore Store, nameOrID string, j networkInfo = resp.Msg.GetNetworkInfo() } // A connected or disconnected member with no ports is a valid empty result. - // The API uses an unspecified status and no ports when no Skybridge member exists yet. + // The API uses an unspecified status and no ports when no Brev-managed network is available. if networkInfo == nil || (networkInfo.GetStatus() == devplanev1.NetworkMemberStatus_NETWORK_MEMBER_STATUS_UNSPECIFIED && len(networkInfo.GetPorts()) == 0) { return breverrors.NewValidationError(fmt.Sprintf( - "cannot list ports for instance %q: no Skybridge network member is available; "+ + "cannot list ports for instance %q: no Brev-managed network configuration is available; "+ "the instance may still be provisioning or may use legacy network access. "+ "Try again when it is running, or view legacy secure links and firewall rules in the Brev console", nameOrID, @@ -122,13 +131,8 @@ func toPortInfos(apiPorts []*devplanev1.Port) []PortInfo { continue } isHTTP := port.GetHttpProtocol() != devplanev1.HttpPortProtocol_HTTP_PORT_PROTOCOL_UNSPECIFIED - kind := portKindNetwork - if isHTTP { - kind = portKindHTTP - } portInfos = append(portInfos, PortInfo{ PortID: port.GetPortId(), - Kind: kind, Endpoint: endpoint(port, isHTTP), PublicPort: port.GetPortNumber(), DestinationPort: port.GetServerPort(), @@ -137,6 +141,7 @@ func toPortInfos(apiPorts []*devplanev1.Port) []PortInfo { AuthorizedEmails: append([]string{}, port.GetAuthorizedEmails()...), AllowPublicUnauthenticated: port.GetAllowPublicUnauthenticated(), Type: portTypeLabel(port.GetType()), + isHTTP: isHTTP, }) } return portInfos @@ -211,7 +216,7 @@ func displayTables(out io.Writer, nameOrID string, portInfos []PortInfo) error { httpPorts := make([]PortInfo, 0, len(portInfos)) networkPorts := make([]PortInfo, 0, len(portInfos)) for _, port := range portInfos { - if port.Kind == portKindHTTP { + if port.isHTTP { httpPorts = append(httpPorts, port) } else { networkPorts = append(networkPorts, port) diff --git a/pkg/cmd/ports/ports_test.go b/pkg/cmd/ports/ports_test.go index ab985736..f64ba276 100644 --- a/pkg/cmd/ports/ports_test.go +++ b/pkg/cmd/ports/ports_test.go @@ -71,6 +71,17 @@ func (s *fakeNodeService) ListNodes( return connect.NewResponse(&devplanev1.ListNodesResponse{Items: s.nodes}), nil } +func TestPortsCommandUsesListSubcommand(t *testing.T) { + cmd := NewCmdPorts(&fakeStore{}) + commands := cmd.Commands() + + require.Len(t, commands, 1) + assert.Equal(t, "ls", commands[0].Name()) + assert.Equal(t, "ls ", commands[0].Use) + assert.Nil(t, cmd.Flags().Lookup("json")) + assert.NotNil(t, commands[0].Flags().Lookup("json")) +} + func TestRunEnvironmentJSON(t *testing.T) { public := false hostname := "jupyter-env123.apps.run.brev.nvidia.com" @@ -111,7 +122,6 @@ func TestRunEnvironmentJSON(t *testing.T) { assert.JSONEq(t, `[ { "port_id": "port-http", - "kind": "http", "endpoint": "https://jupyter-env123.apps.run.brev.nvidia.com", "public_port": 443, "destination_port": 8888, @@ -181,7 +191,6 @@ func TestDisplayTablesSSHUsesNetworkHeading(t *testing.T) { var out bytes.Buffer ports := []PortInfo{ { - Kind: portKindNetwork, Endpoint: "gateway.example.com:18928", PublicPort: 18928, DestinationPort: 22, @@ -225,11 +234,11 @@ func TestToPortInfosHandlesPublicHTTPAndRestrictedUDP(t *testing.T) { }) require.Len(t, got, 2) - assert.Equal(t, "http", got[0].Kind) + assert.True(t, got[0].isHTTP) assert.Equal(t, "HTTPS", got[0].Protocol) assert.Equal(t, "https://app.example.com", got[0].Endpoint) assert.True(t, got[0].AllowPublicUnauthenticated) - assert.Equal(t, "network", got[1].Kind) + assert.False(t, got[1].isHTTP) assert.Equal(t, "UDP", got[1].Protocol) assert.Equal(t, "gateway.example.com:5000", got[1].Endpoint) assert.Equal(t, []string{"10.0.0.0/8"}, got[1].AllowedSources) @@ -318,7 +327,7 @@ func TestRunEnvironmentWithoutNetworkMemberReturnsActionableError(t *testing.T) require.Error(t, err) assert.Empty(t, out.String()) - assert.Contains(t, err.Error(), "no Skybridge network member is available") + assert.Contains(t, err.Error(), "no Brev-managed network configuration is available") assert.Contains(t, err.Error(), "may still be provisioning or may use legacy network access") assert.Contains(t, err.Error(), "Brev console") }) @@ -396,7 +405,6 @@ func TestRunExternalNodeJSONContract(t *testing.T) { assert.JSONEq(t, `[ { "port_id": "port-ssh", - "kind": "network", "endpoint": "global.prd.ga.run.brev.nvidia.com:18928", "public_port": 18928, "destination_port": 22, From 67824194d8d95baaa55ec3b06c1fcd2cfbc70aed Mon Sep 17 00:00:00 2001 From: Allen Chen Date: Wed, 19 Aug 2026 09:51:58 -0700 Subject: [PATCH 6/7] Show ports list in command help --- pkg/cmd/ports/ports.go | 1 + pkg/cmd/ports/ports_test.go | 1 + 2 files changed, 2 insertions(+) diff --git a/pkg/cmd/ports/ports.go b/pkg/cmd/ports/ports.go index b68e3ae3..14f0fcc5 100644 --- a/pkg/cmd/ports/ports.go +++ b/pkg/cmd/ports/ports.go @@ -62,6 +62,7 @@ func NewCmdPortsLs(portStore Store) *cobra.Command { var jsonOutput bool cmd := &cobra.Command{ + Annotations: map[string]string{"access": ""}, Use: "ls ", DisableFlagsInUseLine: true, Short: "List Brev-managed ports for an instance or external node", diff --git a/pkg/cmd/ports/ports_test.go b/pkg/cmd/ports/ports_test.go index f64ba276..4ddd0f13 100644 --- a/pkg/cmd/ports/ports_test.go +++ b/pkg/cmd/ports/ports_test.go @@ -78,6 +78,7 @@ func TestPortsCommandUsesListSubcommand(t *testing.T) { require.Len(t, commands, 1) assert.Equal(t, "ls", commands[0].Name()) assert.Equal(t, "ls ", commands[0].Use) + assert.Contains(t, commands[0].Annotations, "access") assert.Nil(t, cmd.Flags().Lookup("json")) assert.NotNil(t, commands[0].Flags().Lookup("json")) } From 7e8ee3d5c0105cf8bc0fcf03ab4bfee7d4ff66ab Mon Sep 17 00:00:00 2001 From: Allen Chen Date: Wed, 19 Aug 2026 17:04:52 -0700 Subject: [PATCH 7/7] Avoid guessing missing destination ports --- pkg/cmd/ports/ports.go | 6 +----- pkg/cmd/ports/ports_test.go | 13 +++++++++++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/ports/ports.go b/pkg/cmd/ports/ports.go index 14f0fcc5..6ae613d5 100644 --- a/pkg/cmd/ports/ports.go +++ b/pkg/cmd/ports/ports.go @@ -248,16 +248,12 @@ func displayHTTPTable(out io.Writer, portInfos []PortInfo) { tw := newTable(out) tw.AppendHeader(table.Row{"ENDPOINT", "AUTHORIZATION", "IP RESTRICTIONS", "PUBLIC PORT", "DESTINATION PORT", "PROTOCOL"}) for _, port := range portInfos { - destinationPort := port.DestinationPort - if destinationPort == 0 { - destinationPort = port.PublicPort - } tw.AppendRow(table.Row{ valueOrDash(port.Endpoint), authorizationLabel(port), allowedSourcesLabel(port.AllowedSources), portNumberLabel(port.PublicPort), - portNumberLabel(destinationPort), + portNumberLabel(port.DestinationPort), port.Protocol, }) } diff --git a/pkg/cmd/ports/ports_test.go b/pkg/cmd/ports/ports_test.go index 4ddd0f13..ac973144 100644 --- a/pkg/cmd/ports/ports_test.go +++ b/pkg/cmd/ports/ports_test.go @@ -208,6 +208,19 @@ func TestDisplayTablesSSHUsesNetworkHeading(t *testing.T) { assert.NotContains(t, out.String(), "TCP/UDP PORTS") } +func TestDisplayHTTPTableMissingDestinationDoesNotUsePublicPort(t *testing.T) { + var out bytes.Buffer + + displayHTTPTable(&out, []PortInfo{{ + Endpoint: "https://app.example.com", + PublicPort: 443, + Protocol: "HTTP", + }}) + + assert.Regexp(t, `443\s+-\s+HTTP`, out.String()) + assert.NotRegexp(t, `443\s+443\s+HTTP`, out.String()) +} + func TestToPortInfosHandlesPublicHTTPAndRestrictedUDP(t *testing.T) { public := true httpHostname := "app.example.com"