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) {