diff --git a/.agents/skills/brev-cli/SKILL.md b/.agents/skills/brev-cli/SKILL.md index 548f14bd0..e8466f285 100644 --- a/.agents/skills/brev-cli/SKILL.md +++ b/.agents/skills/brev-cli/SKILL.md @@ -47,7 +47,7 @@ brev create my-instance --type g5.xlarge # List your instances brev ls -# List external nodes (a separate list from `brev ls`) +# List Brev Connect machines (a separate list from `brev ls`) brev ls nodes # SSH into an instance (interactive) @@ -171,34 +171,39 @@ brev copy my-instance:/remote/file ./local-path/ # Port forward brev port-forward my-instance -p 8080:8080 -# List Brev-managed HTTP and network ports for an instance or external node +# List Brev-managed ports for an instance or Brev Connect machine brev ports ls my-instance -brev ports ls my-node --json - -# Create a public port (TCP by default) -brev ports create my-instance 8080 -brev ports create my-node 53 --protocol udp --allow 203.0.113.10/32 -brev ports create my-instance 3000 --protocol http --public -brev ports create my-instance 8888 --protocol http --authorize me@example.com - -# Update one mapping interactively or by exact ID -brev ports update my-instance --destination-port 8081 -brev ports update my-instance --id nport-abc123 --allow 203.0.113.10/32 -brev ports update my-instance --id nport-abc123 --public - -# Close one port interactively or by exact ID -brev ports close my-instance -brev ports close my-instance --id nport-abc123 --approve +brev ports ls my-connect-machine --json + +# Get all data for one port by exact ID +brev ports get nport-abc123 +brev ports get nport-abc123 --json + +# Open a port or an inclusive sequential TCP/UDP range (TCP by default) +brev ports open my-instance 8080 +brev ports open my-instance 8000-8031 +brev ports open my-connect-machine 53 --protocol udp --allow 203.0.113.10/32 +brev ports open my-instance 3000 --protocol http --public +brev ports open my-instance 8888 --protocol http --authorize me@example.com + +# Update one mapping by its globally unique ID +brev ports update nport-abc123 --destination-port 8081 +brev ports update nport-abc123 --allow 203.0.113.10/32 +brev ports update nport-abc123 --public + +# Remove one port by destination port or exact ID +brev ports remove my-instance 8080 +brev ports rm nport-abc123 ``` ### Listing Instances and Nodes -`brev ls` covers two separate namespaces. External nodes never appear in +`brev ls` covers two separate namespaces. Brev Connect machines never appear in `brev ls`, so check `brev ls nodes` before concluding a machine doesn't exist. ```bash brev ls # cloud instances brev ls instances # same as above, explicit -brev ls nodes # external nodes only (machines registered to the org) +brev ls nodes # Brev Connect machines only brev ls orgs # organizations brev ls --json # machine-readable brev ls nodes --json @@ -319,7 +324,7 @@ Do this proactively when: **"Instance not found":** - Run `brev ls` to see available instances -- Run `brev ls nodes` — external nodes are a separate list and never show up in `brev ls` +- Run `brev ls nodes` — Brev Connect machines are a separate list and never show up in `brev ls` - Check if you're in the correct org: `brev org ls` **"Failed to create instance":** diff --git a/.agents/skills/brev-cli/reference/commands.md b/.agents/skills/brev-cli/reference/commands.md index cca995005..692b2b832 100644 --- a/.agents/skills/brev-cli/reference/commands.md +++ b/.agents/skills/brev-cli/reference/commands.md @@ -219,7 +219,7 @@ brev ls [subcommand] [flags] |---|---| | *(none)* | cloud instances | | `instances` | cloud instances (explicit form) | -| `nodes` | external nodes only | +| `nodes` | Brev Connect machines only | | `orgs` | organizations | **Flags:** @@ -229,9 +229,9 @@ brev ls [subcommand] [flags] | `--all` | | Show all instances in org | | `--json` | | Output as JSON | -#### Instances vs. external nodes +#### Instances vs. Brev Connect machines -Two separate namespaces — an external node never appears in `brev ls`. +Two separate namespaces — a Brev Connect machine never appears in `brev ls`. ```bash $ brev ls @@ -467,11 +467,10 @@ brev port-forward my-instance -p 3000:3000 ``` ### brev ports ls -List Brev-managed HTTP applications and raw network port mappings for a -managed instance or registered compute node. +List Brev-managed ports for a managed instance or Brev Connect machine. ```bash -brev ports ls [flags] +brev ports ls [flags] ``` **Flags:** @@ -480,8 +479,10 @@ brev ports ls [flags] |------|-------------| | `--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. +The compact table output includes ID, endpoint, public port, destination port, +and protocol. Ports are ordered by protocol (`HTTPS`, `HTTP`, `TCP`, `UDP`, +`SSH`), then by ascending destination port. Use `brev ports get` to inspect +authorization, IP restrictions, and the remaining data for one mapping. For managed instances, this command reads the Brev-managed network configuration. It does not synthesize the legacy secure-link or firewall rows @@ -507,18 +508,41 @@ The JSON output is an array with the following stable fields: **Examples:** ```bash brev ports ls my-instance -brev ports ls my-node +brev ports ls my-connect-machine brev ports ls my-instance --json ``` -### Create a port +### Get a port -Create a raw TCP, UDP, or SSH port, or an HTTP application endpoint, on a -managed instance or registered compute node. `open` and `add` are aliases for -`create`. +Get all data for one port mapping by its exact `port_id`. ```bash -brev ports create [flags] +brev ports get [flags] +``` + +**Flags:** + +| Flag | Description | +|------|-------------| +| `--json` | Output the port mapping as a JSON object | + +Human-readable output includes ID, endpoint, public and destination ports, +protocol, allowed sources, authorized emails, public-access state, and type. +The JSON object uses the same stable fields documented for `brev ports ls`. + +**Examples:** +```bash +brev ports get nport-abc123 +brev ports get nport-abc123 --json +``` + +### Open a port + +Open a TCP, UDP, SSH, HTTP, or HTTPS port on a managed instance or Brev +Connect machine. TCP and UDP also accept inclusive sequential ranges. + +```bash +brev ports open [flags] ``` **Flags:** @@ -529,7 +553,7 @@ brev ports create [flags] | `--authorize` | Email authorized for an HTTP endpoint; repeat to add more than one | | `--hostname` | HTTP endpoint hostname prefix; defaults to the destination port | | `--public` | Disable authentication for an HTTP endpoint | -| `--json` | Output the created port as JSON | +| `--json` | Output the opened port as JSON | Omit `--allow` to allow raw-port connections from any source. HTTP endpoints default to authorizing the current user's email; use `--public` to make one @@ -538,28 +562,29 @@ endpoint to a plain-HTTP service, while `https` expects TLS on the destination. **Examples:** ```bash -brev ports create my-instance 8080 -brev ports create my-node 53 --protocol udp -brev ports create my-instance 8080 --allow 203.0.113.10/32 -brev ports create my-node 2222 --protocol ssh --json -brev ports create my-instance 3000 --protocol http --public -brev ports create my-instance 8888 --protocol http --authorize me@example.com +brev ports open my-instance 8080 +brev ports open my-instance 8000-8031 +brev ports open my-connect-machine 53 --protocol udp +brev ports open my-instance 8080 --allow 203.0.113.10/32 +brev ports open my-connect-machine 2222 --protocol ssh --json +brev ports open my-instance 3000 --protocol http --public +brev ports open my-instance 8888 --protocol http --authorize me@example.com ``` ### Update a port -Update a mapping in place while preserving its `port_id` and public endpoint. -Omit `--id` to select a mapping interactively. `edit` is an alias for `update`. +Update a mapping in place while preserving its globally unique `port_id` and +public endpoint. The CLI resolves the owning environment or Brev Connect +machine automatically. `edit` is an alias for `update`. ```bash -brev ports update [flags] +brev ports update [flags] ``` **Flags:** | Flag | Description | |------|-------------| -| `--id` | Update the exact mapping with this `port_id`; omit to select interactively | | `--destination-port` | Change the destination port (1-65535) | | `--allow` | Replace source restrictions with this CIDR; repeat to add more than one | | `--allow-anywhere` | Clear all source restrictions | @@ -576,38 +601,37 @@ separate mutations are not transactional. **Examples:** ```bash -brev ports update my-instance --destination-port 8081 -brev ports update my-instance --id nport-abc123 --allow 203.0.113.10/32 -brev ports edit my-node --id nport-abc123 --allow-anywhere -brev ports update my-instance --id nport-abc123 --protocol https -brev ports update my-instance --id nport-abc123 --authorize me@example.com -brev ports update my-instance --id nport-abc123 --public --json +brev ports update nport-abc123 --destination-port 8081 +brev ports update nport-abc123 --allow 203.0.113.10/32 +brev ports edit nport-abc123 --allow-anywhere +brev ports update nport-abc123 --protocol https +brev ports update nport-abc123 --authorize me@example.com +brev ports update nport-abc123 --public --json ``` -### Close ports +### Remove ports -Select and close one port interactively or close an exact mapping by its -`port_id`. `remove` is an alias for `close`. +Remove one port by its globally unique `port_id` without specifying its owner. +To identify a port by destination number instead, also provide the environment +or Brev Connect machine. If multiple mappings use that destination, use an +exact `port_id`. `rm` is an alias for `remove`. ```bash -brev ports close [flags] +brev ports remove +brev ports remove ``` -**Flags:** - -| Flag | Description | -|------|-------------| -| `--id` | Close the exact mapping with this `port_id` | -| `--approve` | Skip the confirmation prompt | - -Use `brev ports ls --json` to obtain stable `port_id` values +Use `brev ports ls --json` to find port IDs for automation. +On success, the command reports the protocol, destination port, and owning +machine, for example: `Removed TCP port 8080 on my-instance.` + **Examples:** ```bash -brev ports close my-instance -brev ports close my-instance --id nport-abc123 --approve -brev ports remove my-node --id nport-abc123 --approve +brev ports remove my-instance 8080 +brev ports rm nport-abc123 +brev ports remove nport-abc123 ``` ## Organization Commands diff --git a/go.mod b/go.mod index ff6fb9616..4d0829606 100644 --- a/go.mod +++ b/go.mod @@ -3,9 +3,9 @@ module github.com/brevdev/brev-cli go 1.25.0 require ( - buf.build/gen/go/brevdev/devplane/connectrpc/go v1.20.0-20260911001103-64122a9c386c.1 - buf.build/gen/go/brevdev/devplane/protocolbuffers/go v1.36.12-20260911001103-64122a9c386c.1 - connectrpc.com/connect v1.20.0 + buf.build/gen/go/brevdev/devplane/connectrpc/go v1.21.0-20260915224432-6a5d0c858507.1 + buf.build/gen/go/brevdev/devplane/protocolbuffers/go v1.36.12-20260915224432-6a5d0c858507.2 + connectrpc.com/connect v1.21.0 github.com/NVIDIA/go-nvml v0.13.0-1 github.com/alessio/shellescape v1.4.1 github.com/brevdev/parse v0.0.11 @@ -50,7 +50,7 @@ require ( ) require ( - buf.build/gen/go/brevdev/protoc-gen-gotag/protocolbuffers/go v1.36.12-20220906235457-8b4922735da5.1 // indirect + buf.build/gen/go/brevdev/protoc-gen-gotag/protocolbuffers/go v1.36.12-20220906235457-8b4922735da5.2 // indirect dario.cat/mergo v1.0.0 // indirect github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect github.com/Microsoft/go-winio v0.6.2 // indirect diff --git a/go.sum b/go.sum index ff5e42948..294c9e6c4 100644 --- a/go.sum +++ b/go.sum @@ -1,9 +1,9 @@ -buf.build/gen/go/brevdev/devplane/connectrpc/go v1.20.0-20260911001103-64122a9c386c.1 h1:nrPJ9Iw5plC+jbx2szj9CvdDRaYkg+dgGHms7Jsb500= -buf.build/gen/go/brevdev/devplane/connectrpc/go v1.20.0-20260911001103-64122a9c386c.1/go.mod h1:K41bEH5jxz70cfjH6X8P5ykdEpM/+iT2TMr1sYdnPLk= -buf.build/gen/go/brevdev/devplane/protocolbuffers/go v1.36.12-20260911001103-64122a9c386c.1 h1:lpcaoFSjHRkbcehomGsn6JlRGqt+gzFyK+VjlN5zgIE= -buf.build/gen/go/brevdev/devplane/protocolbuffers/go v1.36.12-20260911001103-64122a9c386c.1/go.mod h1:N18pnR0HL6srurI7G19FpSEki71wA1u4e2c5zbfeTV8= -buf.build/gen/go/brevdev/protoc-gen-gotag/protocolbuffers/go v1.36.12-20220906235457-8b4922735da5.1 h1:Qk/4GJyWVWvWsfEFeX4T+k7KouZdRUxxUnIUwJ3hmZg= -buf.build/gen/go/brevdev/protoc-gen-gotag/protocolbuffers/go v1.36.12-20220906235457-8b4922735da5.1/go.mod h1:SacJAYqnICCQAsBA46cSA/hxhqhxYkiYzseucf6/fhQ= +buf.build/gen/go/brevdev/devplane/connectrpc/go v1.21.0-20260915224432-6a5d0c858507.1 h1:CFXTDUzEoMYQzJXAG+hnMO5Cs6h3zVq734k/VnyWuCM= +buf.build/gen/go/brevdev/devplane/connectrpc/go v1.21.0-20260915224432-6a5d0c858507.1/go.mod h1:dS34xJi+QXqWDdKwUHTBfzkcbQFNK3u4UH3F3L6izx0= +buf.build/gen/go/brevdev/devplane/protocolbuffers/go v1.36.12-20260915224432-6a5d0c858507.2 h1:CjGsO8kcUR0YcKzjKy/L5m2jIhnNW6VW6ArYliA2p6o= +buf.build/gen/go/brevdev/devplane/protocolbuffers/go v1.36.12-20260915224432-6a5d0c858507.2/go.mod h1:SBVLUYGc/4vgMlhOJL5SkwIrSh/FL9EEimlpI8zQSXo= +buf.build/gen/go/brevdev/protoc-gen-gotag/protocolbuffers/go v1.36.12-20220906235457-8b4922735da5.2 h1:W5SVs96P8ZgLElYzL93rJu+S+XmkdGQSaVWzPvR5lvg= +buf.build/gen/go/brevdev/protoc-gen-gotag/protocolbuffers/go v1.36.12-20220906235457-8b4922735da5.2/go.mod h1:SacJAYqnICCQAsBA46cSA/hxhqhxYkiYzseucf6/fhQ= cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go v0.38.0/go.mod h1:990N+gfupTy94rShfmMCWGDn0LpTmnzTp2qbd1dvSRU= @@ -41,8 +41,8 @@ cloud.google.com/go/storage v1.6.0/go.mod h1:N7U0C8pVQ/+NIKOBQyamJIeKQKkZ+mxpohl cloud.google.com/go/storage v1.8.0/go.mod h1:Wv1Oy7z6Yz3DshWRJFhqM/UCfaWIRTdp0RXyy7KQOVs= cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9ullr3+Kg0= cloud.google.com/go/storage v1.14.0/go.mod h1:GrKmX003DSIwi9o29oFT7YDnHYwZoctc3fOKtUw0Xmo= -connectrpc.com/connect v1.20.0 h1:6TNDAB+WeNd2uolWNlYczB5E0KNNaVMNUEx8JEUsPmQ= -connectrpc.com/connect v1.20.0/go.mod h1:A2ygJrukXwWy32vkCAAHNVguZrqZ+jeZ9rGRnGR4dN4= +connectrpc.com/connect v1.21.0 h1:LhqSJt7jHf5NJBo9Jq/t/9FjcYAideif0mg+qe2jCUs= +connectrpc.com/connect v1.21.0/go.mod h1:A2ygJrukXwWy32vkCAAHNVguZrqZ+jeZ9rGRnGR4dN4= dario.cat/mergo v1.0.0 h1:AGCNq9Evsj31mOgNPcLyXc+4PNABt905YmuqPYYpBWk= dario.cat/mergo v1.0.0/go.mod h1:uNxQE+84aUszobStD9th8a29P2fMDhsBdgRYvZOxGmk= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= diff --git a/pkg/cmd/cmd.go b/pkg/cmd/cmd.go index 356a158bc..9a44c9c58 100644 --- a/pkg/cmd/cmd.go +++ b/pkg/cmd/cmd.go @@ -233,6 +233,8 @@ func NewBrevCommand() *cobra.Command { //nolint:funlen,gocognit,gocyclo // defin cobra.AddTemplateFunc("providerDependentCommands", providerDependentCommands) cobra.AddTemplateFunc("hasAccessCommands", hasAccessCommands) cobra.AddTemplateFunc("accessCommands", accessCommands) + cobra.AddTemplateFunc("hasNetworkingCommands", hasNetworkingCommands) + cobra.AddTemplateFunc("networkingCommands", networkingCommands) cobra.AddTemplateFunc("hasOrganizationCommands", hasOrganizationCommands) cobra.AddTemplateFunc("organizationCommands", organizationCommands) cobra.AddTemplateFunc("hasConfigurationCommands", hasConfigurationCommands) @@ -358,6 +360,10 @@ func hasAccessCommands(cmd *cobra.Command) bool { return len(accessCommands(cmd)) > 0 } +func hasNetworkingCommands(cmd *cobra.Command) bool { + return len(networkingCommands(cmd)) > 0 +} + func hasOrganizationCommands(cmd *cobra.Command) bool { return len(organizationCommands(cmd)) > 0 } @@ -398,6 +404,16 @@ func accessCommands(cmd *cobra.Command) []*cobra.Command { return cmds } +func networkingCommands(cmd *cobra.Command) []*cobra.Command { + cmds := []*cobra.Command{} + for _, sub := range cmd.Commands() { + if sub.IsAvailableCommand() && isNetworkingCommand(sub) { + cmds = append(cmds, sub) + } + } + return cmds +} + func organizationCommands(cmd *cobra.Command) []*cobra.Command { cmds := []*cobra.Command{} for _, sub := range cmd.Commands() { @@ -458,6 +474,11 @@ func isAccessCommand(cmd *cobra.Command) bool { return ok } +func isNetworkingCommand(cmd *cobra.Command) bool { + _, ok := cmd.Annotations["networking"] + return ok +} + func isOrganizationCommand(cmd *cobra.Command) bool { _, ok := cmd.Annotations["organization"] return ok @@ -520,6 +541,13 @@ Instance Access: {{rpad .Name .NamePadding }} {{.Short}} {{- end}}{{- end}} +{{- if hasNetworkingCommands . }} + +Networking: +{{- range networkingCommands . }} + {{rpad .Name .NamePadding }} {{.Short}} +{{- end}}{{- end}} + {{- if hasOrganizationCommands . }} Organization Management: diff --git a/pkg/cmd/ls/ls.go b/pkg/cmd/ls/ls.go index cd4954bcd..4df3e204c 100644 --- a/pkg/cmd/ls/ls.go +++ b/pkg/cmd/ls/ls.go @@ -61,7 +61,7 @@ func NewCmdLs(t *terminal.Terminal, loginLsStore LsStore, noLoginLsStore LsStore Subcommands: instances List cloud instances - nodes List external nodes only + nodes List Brev Connect machines only orgs List organizations When stdout is piped, outputs instance names only (one per line) for easy chaining @@ -109,7 +109,7 @@ with other commands like stop, start, or delete.`, fmt.Print(breverrors.WrapAndTrace(err)) } - cmd.Flags().BoolVar(&showAll, "all", false, "show all instances and external nodes in org") + cmd.Flags().BoolVar(&showAll, "all", false, "show all instances and Brev Connect machines in org") cmd.Flags().BoolVar(&jsonOutput, "json", false, "output as JSON") return cmd @@ -474,7 +474,7 @@ func (ls Ls) RunWorkspaces(cliAuth auth.CLIAuth, org *entity.Organization, showA nodes, err = ls.listNodes(org) if err != nil { if featureflag.Debug() { - _, _ = fmt.Fprintf(os.Stderr, "debug: failed to list external nodes: %v\n", err) + _, _ = fmt.Fprintf(os.Stderr, "debug: failed to list Brev Connect machines: %v\n", err) } } }() @@ -522,7 +522,7 @@ func (ls Ls) RunWorkspaces(cliAuth auth.CLIAuth, org *entity.Organization, showA if showAll { ls.ShowAllWorkspaces(org, orgs, workspacesToShow, gpuLookup) if len(nodes) > 0 { - ls.terminal.Vprintf("\nYou have %d external node(s) in Org %s\n", len(nodes), ls.terminal.Yellow(org.Name)) + ls.terminal.Vprintf("\nYou have %d Brev Connect machine(s) in Org %s\n", len(nodes), ls.terminal.Yellow(org.Name)) displayNodesTable(ls.terminal, nodes, ls.piped) } } else { @@ -761,7 +761,7 @@ func getStatusColoredText(t *terminal.Terminal, status string) string { } } -// NodeInfo represents external node data for JSON output. +// NodeInfo represents Brev Connect machine data for JSON output. type NodeInfo struct { Name string `json:"name"` OrgID string `json:"org_id"` @@ -779,7 +779,7 @@ func (ls Ls) listNodes(org *entity.Organization) ([]*nodev1.ExternalNode, error) return resp.Msg.GetItems(), nil } -// RunNodes lists external nodes for the given org. +// RunNodes lists Brev Connect machines for the given org. func (ls Ls) RunNodes(org *entity.Organization) error { nodes, err := ls.listNodes(org) if err != nil { @@ -794,7 +794,7 @@ func (ls Ls) RunNodes(org *entity.Organization) error { if ls.piped { return nil } - ls.terminal.Vprint(ls.terminal.Yellow("No external nodes in this org.")) + ls.terminal.Vprint(ls.terminal.Yellow("No Brev Connect machines in this org.")) return nil } @@ -802,7 +802,7 @@ func (ls Ls) RunNodes(org *entity.Organization) error { return ls.outputNodesJSON(nodes) } if !ls.piped { - ls.terminal.Vprintf("\nYou have %d external node(s) in Org %s\n", len(nodes), ls.terminal.Yellow(org.Name)) + ls.terminal.Vprintf("\nYou have %d Brev Connect machine(s) in Org %s\n", len(nodes), ls.terminal.Yellow(org.Name)) } displayNodesTable(ls.terminal, nodes, ls.piped) return nil diff --git a/pkg/cmd/mintcert/mintcert.go b/pkg/cmd/mintcert/mintcert.go index eadfa78bf..f2b7d4a6f 100644 --- a/pkg/cmd/mintcert/mintcert.go +++ b/pkg/cmd/mintcert/mintcert.go @@ -96,7 +96,7 @@ func NewCmdMintCert(store Store) *cobra.Command { ) cmd := &cobra.Command{ Use: "mint-cert", - Short: "Mint a short-lived SSH certificate for an environment or external node", + Short: "Mint a short-lived SSH certificate for an environment or Brev Connect machine", Args: cobra.NoArgs, Hidden: true, RunE: func(cmd *cobra.Command, args []string) error { @@ -116,7 +116,7 @@ func NewCmdMintCert(store Store) *cobra.Command { }, } cmd.Flags().StringVar(&env, "env", "", "environment ID to mint a certificate for") - cmd.Flags().StringVar(&node, "node", "", "external node ID to mint a certificate for") + cmd.Flags().StringVar(&node, "node", "", "Brev Connect machine ID to mint a certificate for") cmd.Flags().StringVar(&port, "port", "", "network-member port ID for the SSH access") cmd.Flags().StringVar(&user, "linux-user", "", "Linux user for the certificate principal") cmd.Flags().StringVar(&outKey, "out-key", "", "private-key path (certificate goes to -cert.pub)") diff --git a/pkg/cmd/ports/close.go b/pkg/cmd/ports/close.go index 55c34cd34..b35b989f4 100644 --- a/pkg/cmd/ports/close.go +++ b/pkg/cmd/ports/close.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "io" + "strings" devplanev1 "buf.build/gen/go/brevdev/devplane/protocolbuffers/go/devplaneapi/v1" "connectrpc.com/connect" @@ -14,151 +15,128 @@ import ( cmdutil "github.com/brevdev/brev-cli/pkg/cmd/util" "github.com/brevdev/brev-cli/pkg/config" breverrors "github.com/brevdev/brev-cli/pkg/errors" - "github.com/brevdev/brev-cli/pkg/terminal" ) -type closeOptions struct { - portID string - approve bool -} - -type closePrompter interface { - terminal.Selector - terminal.Confirmer -} - -// NewCmdClosePort creates the `brev ports close` command. +// NewCmdClosePort creates the `brev ports remove` command. func NewCmdClosePort(portStore Store) *cobra.Command { - return newCmdClosePort(portStore, register.TerminalPrompter{}) -} - -func newCmdClosePort(portStore Store, prompter closePrompter) *cobra.Command { - var opts closeOptions - cmd := &cobra.Command{ - Annotations: map[string]string{"access": ""}, - Use: "close ", - Aliases: []string{"remove"}, - Hidden: true, + Annotations: map[string]string{"networking": ""}, + Use: "remove | ", + Aliases: []string{"rm"}, DisableFlagsInUseLine: true, - Short: "[beta] Close public ports on an instance or external node", + Short: "Remove a Brev-managed port by ID or destination port", + Long: `Remove a Brev-managed port. + +Use a globally unique port ID without specifying its owner. To select by +destination port instead, also provide the environment or Brev Connect machine. +When multiple mappings share that destination, use an exact ID instead.`, Example: ` - brev ports close my-instance - brev ports close my-instance --id nport-abc123 --approve`, - Args: cmderrors.TransformToValidationError(cobra.ExactArgs(1)), + brev ports remove nport-abc123 + brev ports remove my-instance 8080 + brev ports rm nport-abc123`, + Args: cmderrors.TransformToValidationError(cobra.RangeArgs(1, 2)), RunE: func(cmd *cobra.Command, args []string) error { - if err := runClose(cmd.Context(), cmd.OutOrStdout(), portStore, prompter, args[0], opts); err != nil { - return breverrors.WrapAndTrace(err) + if len(args) == 1 { + return breverrors.WrapAndTrace(runRemoveByID( + cmd.Context(), cmd.OutOrStdout(), portStore, args[0], + )) } - return nil + return breverrors.WrapAndTrace(runRemoveByDestination( + cmd.Context(), cmd.OutOrStdout(), portStore, args[0], args[1], + )) }, } - - cmd.Flags().StringVar(&opts.portID, "id", "", "close the exact port mapping with this port_id") - cmd.Flags().BoolVar(&opts.approve, "approve", false, "skip confirmation prompt (assume yes)") return cmd } -func runClose( +func runRemoveByID( ctx context.Context, out io.Writer, portStore Store, - prompter closePrompter, - nameOrID string, - opts closeOptions, + portID string, ) error { - target, apiPorts, err := resolveTargetPorts(ctx, portStore, nameOrID) + target, port, err := resolvePortID(ctx, portStore, portID) if err != nil { return breverrors.WrapAndTrace(err) } - - removable := removablePorts(apiPorts) - if len(removable) == 0 { - return fmt.Errorf("no removable ports are open on %s", nameOrID) + if err := closePort(ctx, portStore, target, port.GetPortId()); err != nil { + return breverrors.WrapAndTrace(fmt.Errorf("remove port_id %q: %w", port.GetPortId(), err)) } + return writeRemoveResult(out, target, port) +} - selected, err := selectPortToClose(prompter, removable, opts) +func runRemoveByDestination( + ctx context.Context, + out io.Writer, + portStore Store, + nameOrID string, + destination string, +) error { + target, apiPorts, err := resolveTargetPorts(ctx, portStore, nameOrID) if err != nil { return breverrors.WrapAndTrace(err) } - if err := displayCloseConfirmation(out, nameOrID, selected); err != nil { - return breverrors.WrapAndTrace(err) - } - if !opts.approve && !prompter.ConfirmYesNo(closeConfirmationLabel(nameOrID)) { - _, err := fmt.Fprintln(out, "No ports were closed.") + selected, err := resolvePortByDestination(apiPorts, destination) + if err != nil { return breverrors.WrapAndTrace(err) } if err := closePort(ctx, portStore, target, selected.GetPortId()); err != nil { - return breverrors.WrapAndTrace(fmt.Errorf("close port_id %q: %w", selected.GetPortId(), err)) + return breverrors.WrapAndTrace(fmt.Errorf("remove port_id %q: %w", selected.GetPortId(), err)) } - _, err = fmt.Fprintf(out, "Closed 1 port on %s.\n", nameOrID) - return breverrors.WrapAndTrace(err) + return writeRemoveResult(out, target, selected) } -func removablePorts(apiPorts []*devplanev1.Port) []*devplanev1.Port { - ports := make([]*devplanev1.Port, 0, len(apiPorts)) - for _, port := range apiPorts { - if port != nil && port.GetPortId() != "" { - ports = append(ports, port) +func writeRemoveResult(out io.Writer, target *cmdutil.WorkspaceOrNode, port *devplanev1.Port) error { + name := "unknown target" + if target.Workspace != nil { + name = target.Workspace.Name + if name == "" { + name = target.Workspace.ID } - } - return ports -} - -func selectPortToClose( - prompter terminal.Selector, - ports []*devplanev1.Port, - opts closeOptions, -) (*devplanev1.Port, error) { - if opts.portID != "" { - for _, port := range ports { - if port.GetPortId() == opts.portID { - return port, nil - } + } else if target.Node != nil { + name = target.Node.GetName() + if name == "" { + name = target.Node.GetExternalNodeId() } - return nil, fmt.Errorf("port_id %q is not open on this target", opts.portID) } + _, err := fmt.Fprintf( + out, + "Removed %s port %d on %s.\n", + protocolLabel(port, isHTTPPort(port)), + port.GetServerPort(), + name, + ) + return breverrors.WrapAndTrace(err) +} - labels := make([]string, len(ports)) - for i, port := range ports { - labels[i] = closeSelectionLabel(i, port) +func resolvePortByDestination(ports []*devplanev1.Port, value string) (*devplanev1.Port, error) { + value = strings.TrimSpace(value) + destinationPort, err := parsePortNumber(value) + if err != nil { + return nil, breverrors.NewValidationError("destination port must be a number between 1 and 65535") } - chosen := prompter.Select("Select a port to close", labels) - for i, label := range labels { - if label == chosen { - return ports[i], nil + matches := make([]*devplanev1.Port, 0, 1) + for _, port := range ports { + if port != nil && port.GetPortId() != "" && port.GetServerPort() == destinationPort { + matches = append(matches, port) } } - return nil, fmt.Errorf("selected item did not match any open port") -} - -func closeSelectionLabel(index int, port *devplanev1.Port) string { - info := toPortInfos([]*devplanev1.Port{port})[0] - return fmt.Sprintf( - "%d. %s %s public %s -> destination %s", - index+1, - info.Protocol, - valueOrDash(info.Endpoint), - portNumberLabel(info.PublicPort), - portNumberLabel(info.DestinationPort), - ) -} - -func displayCloseConfirmation(out io.Writer, nameOrID string, port *devplanev1.Port) error { - if _, err := fmt.Fprintf(out, "The following port mapping will be permanently removed from %s:\n\n", nameOrID); err != nil { - return breverrors.WrapAndTrace(err) + if len(matches) == 0 { + return nil, fmt.Errorf("destination port %d is not open on this target", destinationPort) } - if err := displayTables(out, nameOrID, toPortInfos([]*devplanev1.Port{port})); err != nil { - return breverrors.WrapAndTrace(err) + if len(matches) > 1 { + portIDs := make([]string, len(matches)) + for i, port := range matches { + portIDs[i] = port.GetPortId() + } + return nil, fmt.Errorf( + "destination port %d matches multiple ports (%s); use an exact port_id from `brev ports ls`", + destinationPort, strings.Join(portIDs, ", "), + ) } - _, err := fmt.Fprintln(out, "\nActive connections may be dropped and this action cannot be undone.") - return breverrors.WrapAndTrace(err) -} - -func closeConfirmationLabel(nameOrID string) string { - return fmt.Sprintf("Close 1 port on %s?", nameOrID) + return matches[0], nil } func closePort( @@ -181,5 +159,5 @@ func closePort( })) return breverrors.WrapAndTrace(err) } - return fmt.Errorf("resolved target has no instance or external node") + return fmt.Errorf("resolved target has no instance or Brev Connect machine") } diff --git a/pkg/cmd/ports/close_test.go b/pkg/cmd/ports/close_test.go index 5c0f6a645..8f4e60469 100644 --- a/pkg/cmd/ports/close_test.go +++ b/pkg/cmd/ports/close_test.go @@ -3,7 +3,6 @@ package ports import ( "bytes" "context" - "errors" "testing" devplanev1connect "buf.build/gen/go/brevdev/devplane/connectrpc/go/devplaneapi/v1/devplaneapiv1connect" @@ -15,35 +14,12 @@ import ( "github.com/brevdev/brev-cli/pkg/entity" ) -type fakeClosePrompter struct { - selectIndex int - confirm bool - selectCalls int - confirmCalls int - items []string -} - -func (p *fakeClosePrompter) Select(_ string, items []string) string { - p.selectCalls++ - p.items = append([]string{}, items...) - if p.selectIndex < 0 || p.selectIndex >= len(items) { - return "" - } - return items[p.selectIndex] -} - -func (p *fakeClosePrompter) ConfirmYesNo(_ string) bool { - p.confirmCalls++ - return p.confirm -} - type fakeCloseEnvironmentService struct { devplanev1connect.UnimplementedEnvironmentServiceHandler t *testing.T expectedEnvID string ports []*devplanev1.Port closedPortIDs []string - failPortID string } func (s *fakeCloseEnvironmentService) GetNetworkInfo( @@ -65,9 +41,6 @@ func (s *fakeCloseEnvironmentService) ClosePort( req *connect.Request[devplanev1.EnvironmentServiceClosePortRequest], ) (*connect.Response[devplanev1.EnvironmentServiceClosePortResponse], error) { s.t.Helper() - if req.Msg.GetPortId() == s.failPortID { - return nil, connect.NewError(connect.CodeInternal, errors.New("close failed")) - } s.closedPortIDs = append(s.closedPortIDs, req.Msg.GetPortId()) return connect.NewResponse(&devplanev1.EnvironmentServiceClosePortResponse{}), nil } @@ -117,50 +90,35 @@ func testTCPPort(id string, publicPort int32) *devplanev1.Port { } } -func TestCloseInteractivelySelectsOnePort(t *testing.T) { +func TestRemoveByDestinationPort(t *testing.T) { + second := testTCPPort("nport-two", 52002) + second.ServerPort = 9090 service := &fakeCloseEnvironmentService{ t: t, expectedEnvID: "env123", ports: []*devplanev1.Port{ testTCPPort("nport-one", 41001), - testTCPPort("nport-two", 52002), + second, }, } _, handler := devplanev1connect.NewEnvironmentServiceHandler(service) newTestServer(t, handler) - prompter := &fakeClosePrompter{selectIndex: 1, confirm: true} var out bytes.Buffer - err := runClose( + err := runRemoveByDestination( context.Background(), &out, newCloseEnvironmentStore(), - prompter, "my-instance", - closeOptions{}, + "9090", ) require.NoError(t, err) assert.Equal(t, []string{"nport-two"}, service.closedPortIDs) - assert.Equal(t, 1, prompter.selectCalls) - assert.Equal(t, 1, prompter.confirmCalls) - require.Len(t, prompter.items, 2) - assert.Contains(t, prompter.items[0], "public 41001 -> destination 8080") - assert.Contains(t, prompter.items[1], "public 52002 -> destination 8080") - assert.Contains(t, out.String(), "Closed 1 port on my-instance.") -} - -func TestCloseSelectionLabelMissingDestinationDoesNotUsePublicPort(t *testing.T) { - label := closeSelectionLabel(0, &devplanev1.Port{ - Protocol: devplanev1.PortProtocol_PORT_PROTOCOL_TCP, - PortNumber: 443, - }) - - assert.Contains(t, label, "public 443 -> destination -") - assert.NotContains(t, label, "destination 443") + assert.Equal(t, "Removed TCP port 9090 on my-instance.\n", out.String()) } -func TestCloseByExactIDOnExternalNode(t *testing.T) { +func TestRemoveByExactIDOnBrevConnectMachine(t *testing.T) { service := &fakeCloseNodeService{ t: t, node: &devplanev1.ExternalNode{ @@ -174,55 +132,25 @@ func TestCloseByExactIDOnExternalNode(t *testing.T) { } _, handler := devplanev1connect.NewExternalNodeServiceHandler(service) newTestServer(t, handler) - prompter := &fakeClosePrompter{selectIndex: -1} store := &fakeStore{ user: &entity.User{ID: "user1"}, org: &entity.Organization{ID: "org1"}, } var out bytes.Buffer - err := runClose( + err := runRemoveByID( context.Background(), &out, store, - prompter, - "unode123", - closeOptions{portID: "nport-one", approve: true}, + "nport-one", ) require.NoError(t, err) assert.Equal(t, []string{"nport-one"}, service.closedPortIDs) - assert.Zero(t, prompter.selectCalls) - assert.Zero(t, prompter.confirmCalls) - assert.Contains(t, out.String(), "global.prd.ga.run.brev.nvidia.com:41001") -} - -func TestCloseCancellationDoesNotClosePort(t *testing.T) { - service := &fakeCloseEnvironmentService{ - t: t, - expectedEnvID: "env123", - ports: []*devplanev1.Port{testTCPPort("nport-one", 41001)}, - } - _, handler := devplanev1connect.NewEnvironmentServiceHandler(service) - newTestServer(t, handler) - prompter := &fakeClosePrompter{selectIndex: 0, confirm: false} - var out bytes.Buffer - - err := runClose( - context.Background(), - &out, - newCloseEnvironmentStore(), - prompter, - "my-instance", - closeOptions{}, - ) - - require.NoError(t, err) - assert.Empty(t, service.closedPortIDs) - assert.Contains(t, out.String(), "No ports were closed.") + assert.Equal(t, "Removed TCP port 8080 on my-node.\n", out.String()) } -func TestCloseRejectsUnknownID(t *testing.T) { +func TestRemoveRejectsUnknownDestination(t *testing.T) { service := &fakeCloseEnvironmentService{ t: t, expectedEnvID: "env123", @@ -230,52 +158,27 @@ func TestCloseRejectsUnknownID(t *testing.T) { } _, handler := devplanev1connect.NewEnvironmentServiceHandler(service) newTestServer(t, handler) - var out bytes.Buffer - err := runClose( - context.Background(), - &out, - newCloseEnvironmentStore(), - &fakeClosePrompter{}, - "my-instance", - closeOptions{portID: "nport-missing", approve: true}, - ) + err := runRemoveByDestination(context.Background(), &bytes.Buffer{}, newCloseEnvironmentStore(), "my-instance", "9090") - assert.ErrorContains(t, err, `port_id "nport-missing" is not open on this target`) + assert.ErrorContains(t, err, "destination port 9090 is not open on this target") assert.Empty(t, service.closedPortIDs) } -func TestCloseByExactIDReportsFailure(t *testing.T) { +func TestRemoveRejectsAmbiguousDestination(t *testing.T) { service := &fakeCloseEnvironmentService{ t: t, expectedEnvID: "env123", - ports: []*devplanev1.Port{testTCPPort("nport-one", 41001)}, - failPortID: "nport-one", + ports: []*devplanev1.Port{ + testTCPPort("nport-one", 41001), + testTCPPort("nport-two", 52002), + }, } _, handler := devplanev1connect.NewEnvironmentServiceHandler(service) newTestServer(t, handler) - var out bytes.Buffer - err := runClose( - context.Background(), - &out, - newCloseEnvironmentStore(), - &fakeClosePrompter{}, - "my-instance", - closeOptions{portID: "nport-one", approve: true}, - ) + err := runRemoveByDestination(context.Background(), &bytes.Buffer{}, newCloseEnvironmentStore(), "my-instance", "8080") - assert.ErrorContains(t, err, `close port_id "nport-one"`) + assert.ErrorContains(t, err, "destination port 8080 matches multiple ports (nport-one, nport-two); use an exact port_id from `brev ports ls`") assert.Empty(t, service.closedPortIDs) } - -func TestRemovablePortsRequiresPortID(t *testing.T) { - got := removablePorts([]*devplanev1.Port{ - nil, - {PortNumber: 1234}, - testTCPPort("nport-one", 41001), - }) - - require.Len(t, got, 1) - assert.Equal(t, "nport-one", got[0].GetPortId()) -} diff --git a/pkg/cmd/ports/get.go b/pkg/cmd/ports/get.go new file mode 100644 index 000000000..30eb38b17 --- /dev/null +++ b/pkg/cmd/ports/get.go @@ -0,0 +1,94 @@ +package ports + +import ( + "context" + "encoding/json" + "fmt" + "io" + "strconv" + "strings" + + devplanev1 "buf.build/gen/go/brevdev/devplane/protocolbuffers/go/devplaneapi/v1" + "github.com/jedib0t/go-pretty/v6/table" + "github.com/spf13/cobra" + + "github.com/brevdev/brev-cli/pkg/cmd/cmderrors" + breverrors "github.com/brevdev/brev-cli/pkg/errors" +) + +// NewCmdGetPort creates the `brev ports get` command. +func NewCmdGetPort(portStore Store) *cobra.Command { + var jsonOutput bool + + cmd := &cobra.Command{ + Annotations: map[string]string{"networking": ""}, + Use: "get ", + DisableFlagsInUseLine: true, + Short: "Get a Brev-managed port by ID", + Example: ` + brev ports get nport-abc123 + brev ports get nport-abc123 --json`, + Args: cmderrors.TransformToValidationError(cobra.ExactArgs(1)), + RunE: func(cmd *cobra.Command, args []string) error { + return breverrors.WrapAndTrace(Get( + cmd.Context(), cmd.OutOrStdout(), portStore, args[0], jsonOutput, + )) + }, + } + + cmd.Flags().BoolVar(&jsonOutput, "json", false, "output as JSON") + return cmd +} + +// Get resolves a port by its exact ID and displays all of its data. +func Get( + ctx context.Context, + out io.Writer, + portStore Store, + portID string, + jsonOutput bool, +) error { + _, port, err := resolvePortID(ctx, portStore, portID) + if err != nil { + return breverrors.WrapAndTrace(err) + } + portInfo := toPortInfos([]*devplanev1.Port{port})[0] + if jsonOutput { + return writePortJSON(out, portInfo) + } + displayPortDetails(out, portInfo) + return nil +} + +func writePortJSON(out io.Writer, portInfo PortInfo) error { + encoded, err := json.MarshalIndent(portInfo, "", " ") + if err != nil { + return breverrors.WrapAndTrace(err) + } + _, err = fmt.Fprintln(out, string(encoded)) + return breverrors.WrapAndTrace(err) +} + +func displayPortDetails(out io.Writer, port PortInfo) { + tw := newTable(out) + tw.AppendHeader(table.Row{"FIELD", "VALUE"}) + tw.AppendRows([]table.Row{ + {"ID", valueOrDash(port.PortID)}, + {"ENDPOINT", valueOrDash(port.Endpoint)}, + {"PUBLIC PORT", portNumberLabel(port.PublicPort)}, + {"DESTINATION PORT", portNumberLabel(port.DestinationPort)}, + {"PROTOCOL", port.Protocol}, + {"ALLOWED SOURCES", allowedSourcesLabel(port.AllowedSources)}, + {"AUTHORIZED EMAILS", stringSliceLabel(port.AuthorizedEmails)}, + {"PUBLIC UNAUTHENTICATED", strconv.FormatBool(port.AllowPublicUnauthenticated)}, + {"TYPE", port.Type}, + }) + tw.Render() +} + +func stringSliceLabel(values []string) string { + if len(values) == 0 { + return "-" + } + return strings.Join(values, ", ") +} diff --git a/pkg/cmd/ports/get_test.go b/pkg/cmd/ports/get_test.go new file mode 100644 index 000000000..5e294952b --- /dev/null +++ b/pkg/cmd/ports/get_test.go @@ -0,0 +1,107 @@ +package ports + +import ( + "bytes" + "context" + "net/http" + "testing" + + devplanev1connect "buf.build/gen/go/brevdev/devplane/connectrpc/go/devplaneapi/v1/devplaneapiv1connect" + devplanev1 "buf.build/gen/go/brevdev/devplane/protocolbuffers/go/devplaneapi/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/brevdev/brev-cli/pkg/entity" +) + +func testGetPort() *devplanev1.Port { + hostname := "app.example.com" + public := true + return &devplanev1.Port{ + PortId: "nport-abc123", + HttpProtocol: devplanev1.HttpPortProtocol_HTTP_PORT_PROTOCOL_HTTPS, + PortNumber: 443, + ServerPort: 8443, + Hostname: &hostname, + AllowedSources: []string{"203.0.113.10/32"}, + AuthorizedEmails: []string{"user@example.com"}, + AllowPublicUnauthenticated: &public, + Type: devplanev1.PortType_PORT_TYPE_USER, + } +} + +func newGetEnvironmentStore() *fakeStore { + return &fakeStore{ + workspaces: []entity.Workspace{{ID: "env123", Name: "my-instance", CreatedByUserID: "user1"}}, + user: &entity.User{ID: "user1"}, + org: &entity.Organization{ID: "org1"}, + } +} + +func serveGetPort(t *testing.T) { + t.Helper() + service := &fakeEnvironmentService{ + t: t, + expectedEnvID: "env123", + networkInfo: &devplanev1.EnvironmentNetworkInfo{ + Status: devplanev1.NetworkMemberStatus_NETWORK_MEMBER_STATUS_CONNECTED, + Ports: []*devplanev1.Port{testGetPort()}, + }, + } + environmentPath, environmentHandler := devplanev1connect.NewEnvironmentServiceHandler(service) + nodePath, nodeHandler := devplanev1connect.NewExternalNodeServiceHandler(&fakeNodeService{}) + mux := http.NewServeMux() + mux.Handle(environmentPath, environmentHandler) + mux.Handle(nodePath, nodeHandler) + newTestServer(t, mux) +} + +func TestGetDisplaysAllPortData(t *testing.T) { + serveGetPort(t) + var out bytes.Buffer + + err := Get(context.Background(), &out, newGetEnvironmentStore(), "nport-abc123", false) + + require.NoError(t, err) + for _, value := range []string{ + "ID", "nport-abc123", + "ENDPOINT", "https://app.example.com", + "PUBLIC PORT", "443", + "DESTINATION PORT", "8443", + "PROTOCOL", "HTTPS", + "ALLOWED SOURCES", "203.0.113.10/32", + "AUTHORIZED EMAILS", "user@example.com", + "PUBLIC UNAUTHENTICATED", "true", + "TYPE", "user", + } { + assert.Contains(t, out.String(), value) + } +} + +func TestGetJSONIsSingleCompletePort(t *testing.T) { + serveGetPort(t) + var out bytes.Buffer + + err := Get(context.Background(), &out, newGetEnvironmentStore(), "nport-abc123", true) + + require.NoError(t, err) + assert.JSONEq(t, `{ + "port_id":"nport-abc123", + "endpoint":"https://app.example.com", + "public_port":443, + "destination_port":8443, + "protocol":"HTTPS", + "allowed_sources":["203.0.113.10/32"], + "authorized_emails":["user@example.com"], + "allow_public_unauthenticated":true, + "type":"user" + }`, out.String()) +} + +func TestGetRejectsUnknownPortID(t *testing.T) { + serveGetPort(t) + + err := Get(context.Background(), &bytes.Buffer{}, newGetEnvironmentStore(), "missing", false) + + assert.ErrorContains(t, err, `port_id "missing" is not open in the active organization`) +} diff --git a/pkg/cmd/ports/open.go b/pkg/cmd/ports/open.go index 3e0cb7a67..02edd0886 100644 --- a/pkg/cmd/ports/open.go +++ b/pkg/cmd/ports/open.go @@ -21,21 +21,26 @@ import ( breverrors "github.com/brevdev/brev-cli/pkg/errors" ) -// NewCmdCreatePort creates the `brev ports create` command. +// NewCmdCreatePort creates the `brev ports open` command. func NewCmdCreatePort(portStore Store) *cobra.Command { var opts openOptions cmd := &cobra.Command{ - Annotations: map[string]string{"access": ""}, - Use: "create ", - Aliases: []string{"open", "add"}, - Hidden: true, + Annotations: map[string]string{"networking": ""}, + Use: "open ", DisableFlagsInUseLine: true, - Short: "[beta] Create a public port on an instance or external node", - Example: "\n brev ports create my-instance 8080" + - "\n brev ports create my-node 53 --protocol udp" + - "\n brev ports create my-instance 8080 --allow 203.0.113.10/32" + - "\n brev ports create my-instance 3000 --protocol http --public", + Short: "Open a public port on an instance or Brev Connect machine", + Long: `Open a Brev-managed port on an instance or Brev Connect machine. + +TCP is the default protocol. Use --protocol to open UDP, SSH, HTTP, or HTTPS. +TCP and UDP accept an inclusive FROM-TO range. +Use --allow for TCP, UDP, and SSH source restrictions. Use --authorize, +--hostname, or --public only with HTTP and HTTPS ports.`, + Example: "\n brev ports open my-instance 8080" + + "\n brev ports open my-instance 8000-8031" + + "\n brev ports open my-connect-machine 53 --protocol udp" + + "\n brev ports open my-instance 8080 --allow 203.0.113.10/32" + + "\n brev ports open my-instance 3000 --protocol http --public", Args: cmderrors.TransformToValidationError(cobra.ExactArgs(2)), RunE: func(cmd *cobra.Command, args []string) error { return runOpenCommand(cmd.Context(), cmd.OutOrStdout(), portStore, args[0], args[1], opts) @@ -47,7 +52,7 @@ func NewCmdCreatePort(portStore Store) *cobra.Command { cmd.Flags().StringArrayVar(&opts.authorizedEmails, "authorize", nil, "email authorized for an HTTP port (repeatable; defaults to you)") cmd.Flags().StringVar(&opts.customHostname, "hostname", "", "hostname prefix for an HTTP port (defaults to the destination port)") cmd.Flags().BoolVar(&opts.allowPublicUnauthenticated, "public", false, "disable authentication for an HTTP port") - cmd.Flags().BoolVar(&opts.jsonOutput, "json", false, "output the created port as JSON") + cmd.Flags().BoolVar(&opts.jsonOutput, "json", false, "output the opened port as JSON") _ = cmd.RegisterFlagCompletionFunc("protocol", cobra.FixedCompletions( []string{"tcp", "udp", "ssh", "http", "https"}, cobra.ShellCompDirectiveNoFileComp, @@ -73,14 +78,17 @@ func runOpenCommand( portValue string, opts openOptions, ) error { - portNumber, err := parsePortNumber(portValue) + fromPort, toPort, isRange, err := parsePortRange(portValue) if err != nil { return err } if isHTTPProtocol(opts.protocol) { - return runOpenHTTPCommand(ctx, out, portStore, nameOrID, portNumber, opts) + if isRange { + return breverrors.NewValidationError("port ranges are only supported for tcp and udp ports") + } + return runOpenHTTPCommand(ctx, out, portStore, nameOrID, fromPort, opts) } - return runOpenNetworkCommand(ctx, out, portStore, nameOrID, portNumber, opts) + return runOpenNetworkCommand(ctx, out, portStore, nameOrID, fromPort, toPort, isRange, opts) } func runOpenHTTPCommand( @@ -119,26 +127,34 @@ func runOpenNetworkCommand( out io.Writer, portStore Store, nameOrID string, - portNumber int32, + fromPort int32, + toPort int32, + isRange bool, opts openOptions, ) error { - if opts.customHostname != "" || len(opts.authorizedEmails) > 0 || opts.allowPublicUnauthenticated { - return breverrors.NewValidationError("--hostname, --authorize, and --public are only supported for http and https ports") - } portProtocol, err := parseProtocol(opts.protocol) if err != nil { return err } + if opts.customHostname != "" || len(opts.authorizedEmails) > 0 || opts.allowPublicUnauthenticated { + return breverrors.NewValidationError("--hostname, --authorize, and --public are only supported for http and https ports") + } + if isRange && portProtocol == devplanev1.PortProtocol_PORT_PROTOCOL_SSH { + return breverrors.NewValidationError("port ranges are only supported for tcp and udp ports") + } allowedSources, err := normalizeAllowedSources(opts.allowedSources) if err != nil { return err } - return breverrors.WrapAndTrace(Open( - ctx, out, portStore, nameOrID, portNumber, portProtocol, allowedSources, opts.jsonOutput, - )) + if isRange { + return breverrors.WrapAndTrace(OpenSequential( + ctx, out, portStore, nameOrID, fromPort, toPort, portProtocol, allowedSources, opts.jsonOutput, + )) + } + return breverrors.WrapAndTrace(Open(ctx, out, portStore, nameOrID, fromPort, portProtocol, allowedSources, opts.jsonOutput)) } -// OpenHTTP resolves a managed instance or registered compute node and creates +// OpenHTTP resolves a managed instance or Brev Connect machine and creates // an authenticated or public HTTP application endpoint. func OpenHTTP( ctx context.Context, @@ -204,7 +220,7 @@ func OpenHTTP( AllowPublicUnauthenticated: allowPublicUnauthenticated, })) if err != nil { - return fmt.Errorf("open HTTP port on external node %q: %w", nameOrID, err) + return fmt.Errorf("open HTTP port on Brev Connect machine %q: %w", nameOrID, err) } if resp != nil { openedPort = resp.Msg.GetPort() @@ -217,7 +233,7 @@ func OpenHTTP( return writeOpenResult(out, nameOrID, openedPort, jsonOutput) } -// Open resolves a managed instance or registered compute node and opens a port. +// Open resolves a managed instance or Brev Connect machine and opens a port. func Open( ctx context.Context, out io.Writer, @@ -257,7 +273,7 @@ func Open( AllowedSources: allowedSources, })) if err != nil { - return fmt.Errorf("open port on external node %q: %w", nameOrID, err) + return fmt.Errorf("open port on Brev Connect machine %q: %w", nameOrID, err) } if resp != nil && resp.Msg != nil { openedPort = resp.Msg.GetPort() @@ -270,6 +286,87 @@ func Open( return writeOpenResult(out, nameOrID, openedPort, jsonOutput) } +// OpenSequential resolves a managed instance or Brev Connect machine and +// opens an inclusive range whose public ports are allocated sequentially. +func OpenSequential( + ctx context.Context, + out io.Writer, + portStore Store, + nameOrID string, + fromPort int32, + toPort int32, + protocol devplanev1.PortProtocol, + allowedSources []string, + jsonOutput bool, +) error { + target, err := cmdutil.ResolveWorkspaceOrNodeWithContext(ctx, portStore, nameOrID) + if err != nil { + return breverrors.WrapAndTrace(err) + } + + var openedPorts []*devplanev1.Port + if target.Workspace != nil { + client := register.NewEnvironmentServiceClient(portStore, config.GlobalConfig.GetBrevPublicAPIURL()) + resp, err := client.OpenSequentialPorts(ctx, connect.NewRequest(&devplanev1.EnvironmentServiceOpenSequentialPortsRequest{ + EnvironmentId: target.Workspace.ID, + Protocol: protocol, + FromPortNumber: fromPort, + ToPortNumber: toPort, + AllowedSources: allowedSources, + })) + if err != nil { + return fmt.Errorf("open sequential ports on instance %q: %w", nameOrID, err) + } + if resp != nil && resp.Msg != nil { + openedPorts = resp.Msg.GetPorts() + } + } else if target.Node != nil { + client := register.NewNodeServiceClient(portStore, config.GlobalConfig.GetBrevPublicAPIURL()) + resp, err := client.OpenSequentialPorts(ctx, connect.NewRequest(&devplanev1.OpenSequentialPortsRequest{ + ExternalNodeId: target.Node.GetExternalNodeId(), + Protocol: protocol, + FromPortNumber: fromPort, + ToPortNumber: toPort, + AllowedSources: allowedSources, + })) + if err != nil { + return fmt.Errorf("open sequential ports on Brev Connect machine %q: %w", nameOrID, err) + } + if resp != nil && resp.Msg != nil { + openedPorts = resp.Msg.GetPorts() + } + } + + if len(openedPorts) == 0 { + return fmt.Errorf("open sequential ports on %q: API returned no ports", nameOrID) + } + return writeOpenResults(out, nameOrID, openedPorts, jsonOutput) +} + +func parsePortRange(value string) (int32, int32, bool, error) { + value = strings.TrimSpace(value) + fromValue, toValue, found := strings.Cut(value, "-") + if !found { + portNumber, err := parsePortNumber(value) + return portNumber, portNumber, false, err + } + if strings.Contains(toValue, "-") { + return 0, 0, false, fmt.Errorf("invalid port range %q: use FROM-TO, for example 8000-8031", value) + } + fromPort, err := parsePortNumber(fromValue) + if err != nil { + return 0, 0, false, fmt.Errorf("invalid port range %q: start port must be between 1 and 65535", value) + } + toPort, err := parsePortNumber(toValue) + if err != nil { + return 0, 0, false, fmt.Errorf("invalid port range %q: end port must be between 1 and 65535", value) + } + if fromPort >= toPort { + return 0, 0, false, fmt.Errorf("invalid port range %q: start port must be less than end port", value) + } + return fromPort, toPort, true, nil +} + func parsePortNumber(value string) (int32, error) { portNumber, err := strconv.ParseInt(value, 10, 32) if err != nil || portNumber < 1 || portNumber > 65535 { @@ -381,9 +478,17 @@ func buildHTTPHostname(value string, portNumber int32, targetID string) (string, } func writeOpenResult(out io.Writer, nameOrID string, port *devplanev1.Port, jsonOutput bool) error { - portInfo := toPortInfos([]*devplanev1.Port{port})[0] + return writeOpenResults(out, nameOrID, []*devplanev1.Port{port}, jsonOutput) +} + +func writeOpenResults(out io.Writer, nameOrID string, ports []*devplanev1.Port, jsonOutput bool) error { + portInfos := toPortInfos(ports) if jsonOutput { - encoded, err := json.MarshalIndent(portInfo, "", " ") + var value any = portInfos + if len(portInfos) == 1 { + value = portInfos[0] + } + encoded, err := json.MarshalIndent(value, "", " ") if err != nil { return breverrors.WrapAndTrace(err) } @@ -391,9 +496,23 @@ func writeOpenResult(out io.Writer, nameOrID string, port *devplanev1.Port, json return breverrors.WrapAndTrace(err) } - _, err := fmt.Fprintf(out, "Created %s port %d on %s.\n", portInfo.Protocol, port.GetServerPort(), nameOrID) + if len(portInfos) == 0 { + return fmt.Errorf("API returned no ports") + } + if len(portInfos) == 1 { + _, err := fmt.Fprintf(out, "Opened %s port %d on %s.\n", portInfos[0].Protocol, portInfos[0].DestinationPort, nameOrID) + if err != nil { + return breverrors.WrapAndTrace(err) + } + return displayTables(out, nameOrID, portInfos) + } + _, err := fmt.Fprintf( + out, "Opened %d %s ports %d-%d on %s.\n", + len(portInfos), portInfos[0].Protocol, portInfos[0].DestinationPort, + portInfos[len(portInfos)-1].DestinationPort, nameOrID, + ) if err != nil { return breverrors.WrapAndTrace(err) } - return displayTables(out, nameOrID, []PortInfo{portInfo}) + return displayTables(out, nameOrID, portInfos) } diff --git a/pkg/cmd/ports/open_test.go b/pkg/cmd/ports/open_test.go index 0d15404a2..ebf1752ad 100644 --- a/pkg/cmd/ports/open_test.go +++ b/pkg/cmd/ports/open_test.go @@ -26,11 +26,13 @@ func newTestServer(t *testing.T, handler http.Handler) { type fakeOpenEnvironmentService struct { devplanev1connect.UnimplementedEnvironmentServiceHandler - t *testing.T - wantReq *devplanev1.EnvironmentServiceOpenPortRequest - wantHTTPReq *devplanev1.EnvironmentServiceOpenHTTPPortRequest - port *devplanev1.Port - httpPort *devplanev1.Port + t *testing.T + wantReq *devplanev1.EnvironmentServiceOpenPortRequest + wantSequentialReq *devplanev1.EnvironmentServiceOpenSequentialPortsRequest + wantHTTPReq *devplanev1.EnvironmentServiceOpenHTTPPortRequest + port *devplanev1.Port + sequentialPorts []*devplanev1.Port + httpPort *devplanev1.Port } type httpOpenRequest interface { @@ -41,6 +43,13 @@ type httpOpenRequest interface { GetAllowPublicUnauthenticated() bool } +type sequentialOpenRequest interface { + GetFromPortNumber() int32 + GetToPortNumber() int32 + GetProtocol() devplanev1.PortProtocol + GetAllowedSources() []string +} + func assertHTTPOpenRequest(t *testing.T, want, got httpOpenRequest) { t.Helper() assert.Equal(t, want.GetPortNumber(), got.GetPortNumber()) @@ -50,6 +59,14 @@ func assertHTTPOpenRequest(t *testing.T, want, got httpOpenRequest) { assert.Equal(t, want.GetAllowPublicUnauthenticated(), got.GetAllowPublicUnauthenticated()) } +func assertSequentialOpenRequest(t *testing.T, want, got sequentialOpenRequest) { + t.Helper() + assert.Equal(t, want.GetFromPortNumber(), got.GetFromPortNumber()) + assert.Equal(t, want.GetToPortNumber(), got.GetToPortNumber()) + assert.Equal(t, want.GetProtocol(), got.GetProtocol()) + assert.Equal(t, want.GetAllowedSources(), got.GetAllowedSources()) +} + func (s *fakeOpenEnvironmentService) OpenHTTPPort( _ context.Context, req *connect.Request[devplanev1.EnvironmentServiceOpenHTTPPortRequest], @@ -72,14 +89,26 @@ func (s *fakeOpenEnvironmentService) OpenPort( return connect.NewResponse(&devplanev1.EnvironmentServiceOpenPortResponse{Port: s.port}), nil } +func (s *fakeOpenEnvironmentService) OpenSequentialPorts( + _ context.Context, + req *connect.Request[devplanev1.EnvironmentServiceOpenSequentialPortsRequest], +) (*connect.Response[devplanev1.EnvironmentServiceOpenSequentialPortsResponse], error) { + s.t.Helper() + assert.Equal(s.t, s.wantSequentialReq.GetEnvironmentId(), req.Msg.GetEnvironmentId()) + assertSequentialOpenRequest(s.t, s.wantSequentialReq, req.Msg) + return connect.NewResponse(&devplanev1.EnvironmentServiceOpenSequentialPortsResponse{Ports: s.sequentialPorts}), nil +} + type fakeOpenNodeService struct { devplanev1connect.UnimplementedExternalNodeServiceHandler - t *testing.T - node *devplanev1.ExternalNode - wantReq *devplanev1.OpenPortRequest - wantHTTPReq *devplanev1.OpenHTTPPortRequest - port *devplanev1.Port - httpPort *devplanev1.Port + t *testing.T + node *devplanev1.ExternalNode + wantReq *devplanev1.OpenPortRequest + wantSequentialReq *devplanev1.OpenSequentialPortsRequest + wantHTTPReq *devplanev1.OpenHTTPPortRequest + port *devplanev1.Port + sequentialPorts []*devplanev1.Port + httpPort *devplanev1.Port } func (s *fakeOpenNodeService) OpenHTTPPort( @@ -111,6 +140,23 @@ func (s *fakeOpenNodeService) OpenPort( return connect.NewResponse(&devplanev1.OpenPortResponse{Port: s.port}), nil } +func (s *fakeOpenNodeService) OpenSequentialPorts( + _ context.Context, + req *connect.Request[devplanev1.OpenSequentialPortsRequest], +) (*connect.Response[devplanev1.OpenSequentialPortsResponse], error) { + s.t.Helper() + assert.Equal(s.t, s.wantSequentialReq.GetExternalNodeId(), req.Msg.GetExternalNodeId()) + assertSequentialOpenRequest(s.t, s.wantSequentialReq, req.Msg) + return connect.NewResponse(&devplanev1.OpenSequentialPortsResponse{Ports: s.sequentialPorts}), nil +} + +func sequentialPort(id string, protocol devplanev1.PortProtocol, publicPort, serverPort int32) *devplanev1.Port { + hostname := "global.prd.ga.run.brev.nvidia.com" + return &devplanev1.Port{ + PortId: id, Protocol: protocol, PortNumber: publicPort, ServerPort: serverPort, Hostname: &hostname, + } +} + func TestOpenEnvironment(t *testing.T) { service := &fakeOpenEnvironmentService{ t: t, @@ -150,9 +196,69 @@ func TestOpenEnvironment(t *testing.T) { ) require.NoError(t, err) - assert.Contains(t, out.String(), "Created TCP port 8080 on my-instance.") + assert.Contains(t, out.String(), "Opened TCP port 8080 on my-instance.") assert.Contains(t, out.String(), "19001") - assert.Contains(t, out.String(), "203.0.113.10/32") +} + +func TestOpenSequentialEnvironment(t *testing.T) { + service := &fakeOpenEnvironmentService{ + t: t, + wantSequentialReq: &devplanev1.EnvironmentServiceOpenSequentialPortsRequest{ + EnvironmentId: "env123", Protocol: devplanev1.PortProtocol_PORT_PROTOCOL_TCP, + FromPortNumber: 8000, ToPortNumber: 8002, AllowedSources: []string{"10.0.0.0/8"}, + }, + sequentialPorts: []*devplanev1.Port{ + sequentialPort("port-8002", devplanev1.PortProtocol_PORT_PROTOCOL_TCP, 19002, 8002), + sequentialPort("port-8000", devplanev1.PortProtocol_PORT_PROTOCOL_TCP, 19000, 8000), + sequentialPort("port-8001", devplanev1.PortProtocol_PORT_PROTOCOL_TCP, 19001, 8001), + }, + } + _, handler := devplanev1connect.NewEnvironmentServiceHandler(service) + newTestServer(t, handler) + store := &fakeStore{ + workspaces: []entity.Workspace{{ID: "env123", Name: "my-instance", CreatedByUserID: "user1"}}, + user: &entity.User{ID: "user1"}, + org: &entity.Organization{ID: "org1"}, + } + cmd := NewCmdPorts(store) + cmd.SetArgs([]string{"open", "my-instance", "8000-8002", "--allow", "10.0.0.0/8"}) + var out bytes.Buffer + cmd.SetOut(&out) + + err := cmd.Execute() + + require.NoError(t, err) + assert.Contains(t, out.String(), "Opened 3 TCP ports 8000-8002 on my-instance.") +} + +func TestOpenSequentialExternalNodeJSON(t *testing.T) { + service := &fakeOpenNodeService{ + t: t, + node: &devplanev1.ExternalNode{ExternalNodeId: "unode123", Name: "my-node"}, + wantSequentialReq: &devplanev1.OpenSequentialPortsRequest{ + ExternalNodeId: "unode123", Protocol: devplanev1.PortProtocol_PORT_PROTOCOL_UDP, + FromPortNumber: 9000, ToPortNumber: 9001, + }, + sequentialPorts: []*devplanev1.Port{ + sequentialPort("port-9000", devplanev1.PortProtocol_PORT_PROTOCOL_UDP, 20000, 9000), + sequentialPort("port-9001", devplanev1.PortProtocol_PORT_PROTOCOL_UDP, 20001, 9001), + }, + } + _, handler := devplanev1connect.NewExternalNodeServiceHandler(service) + newTestServer(t, handler) + store := &fakeStore{user: &entity.User{ID: "user1"}, org: &entity.Organization{ID: "org1"}} + cmd := NewCmdPorts(store) + cmd.SetArgs([]string{"open", "my-node", "9000-9001", "--protocol", "udp", "--json"}) + var out bytes.Buffer + cmd.SetOut(&out) + + err := cmd.Execute() + + require.NoError(t, err) + assert.JSONEq(t, `[ + {"port_id":"port-9000","endpoint":"global.prd.ga.run.brev.nvidia.com:20000","public_port":20000,"destination_port":9000,"protocol":"UDP","allowed_sources":[],"authorized_emails":[],"allow_public_unauthenticated":false,"type":"unspecified"}, + {"port_id":"port-9001","endpoint":"global.prd.ga.run.brev.nvidia.com:20001","public_port":20001,"destination_port":9001,"protocol":"UDP","allowed_sources":[],"authorized_emails":[],"allow_public_unauthenticated":false,"type":"unspecified"} + ]`, out.String()) } func TestOpenExternalNodeByIDJSON(t *testing.T) { @@ -250,9 +356,8 @@ func TestOpenHTTPEnvironmentDefaultsToCurrentUser(t *testing.T) { ) require.NoError(t, err) - assert.Contains(t, out.String(), "Created HTTP port 3000 on my-instance.") + assert.Contains(t, out.String(), "Opened HTTP port 3000 on my-instance.") assert.Contains(t, out.String(), "https://3000-env123.apps.run.brev.nvidia.com") - assert.Contains(t, out.String(), "me@example.com") } func TestOpenHTTPExternalNodePublicJSON(t *testing.T) { @@ -285,7 +390,7 @@ func TestOpenHTTPExternalNodePublicJSON(t *testing.T) { } cmd := NewCmdPorts(store) cmd.SetArgs([]string{ - "create", "my-node", "8443", "--protocol", "HTTPS", + "open", "my-node", "8443", "--protocol", "HTTPS", "--hostname", "demo", "--public", "--json", }) var out bytes.Buffer @@ -333,7 +438,7 @@ func TestNewCmdCreatePortParsesFlags(t *testing.T) { } cmd := NewCmdPorts(store) cmd.SetArgs([]string{ - "create", "my-instance", "2222", "--protocol", "SSH", + "open", "my-instance", "2222", "--protocol", "SSH", "--allow", "10.0.0.0/8", "--allow", "192.0.2.0/24", "--allow", " 10.0.0.0/8 ", }) @@ -343,12 +448,12 @@ func TestNewCmdCreatePortParsesFlags(t *testing.T) { err := cmd.Execute() require.NoError(t, err) - assert.Contains(t, out.String(), "Created SSH port 2222 on my-instance.") + assert.Contains(t, out.String(), "Opened SSH port 2222 on my-instance.") } func TestNewCmdCreatePortRejectsNonCIDRAllowedSource(t *testing.T) { cmd := NewCmdPorts(&fakeStore{}) - cmd.SetArgs([]string{"create", "my-instance", "2222", "--allow", "203.0.113.10"}) + cmd.SetArgs([]string{"open", "my-instance", "2222", "--allow", "203.0.113.10"}) err := cmd.Execute() @@ -380,6 +485,40 @@ func TestParsePortNumber(t *testing.T) { } } +func TestParsePortRange(t *testing.T) { + from, to, isRange, err := parsePortRange("8000-8031") + require.NoError(t, err) + assert.Equal(t, int32(8000), from) + assert.Equal(t, int32(8031), to) + assert.True(t, isRange) + + from, to, isRange, err = parsePortRange("8080") + require.NoError(t, err) + assert.Equal(t, int32(8080), from) + assert.Equal(t, int32(8080), to) + assert.False(t, isRange) + + for _, value := range []string{"8000-8000", "8001-8000", "0-2", "65534-65536", "1-2-3"} { + t.Run(value, func(t *testing.T) { + _, _, _, err := parsePortRange(value) + assert.Error(t, err) + }) + } +} + +func TestOpenRangeProtocolValidation(t *testing.T) { + for _, protocol := range []string{"http", "https", "ssh"} { + t.Run(protocol, func(t *testing.T) { + cmd := NewCmdPorts(&fakeStore{}) + cmd.SetArgs([]string{"open", "my-instance", "8000-8001", "--protocol", protocol}) + + err := cmd.Execute() + + assert.EqualError(t, err, "port ranges are only supported for tcp and udp ports") + }) + } +} + func TestParseProtocol(t *testing.T) { tests := []struct { input string @@ -407,22 +546,22 @@ func TestOpenHTTPFlagValidation(t *testing.T) { }{ { name: "public with authorized email", - args: []string{"create", "my-instance", "8080", "--protocol", "http", "--public", "--authorize", "me@example.com"}, + args: []string{"open", "my-instance", "8080", "--protocol", "http", "--public", "--authorize", "me@example.com"}, want: "--public and --authorize cannot be used together", }, { name: "IP allow-list on HTTP", - args: []string{"create", "my-instance", "8080", "--protocol", "http", "--allow", "10.0.0.0/8"}, + args: []string{"open", "my-instance", "8080", "--protocol", "http", "--allow", "10.0.0.0/8"}, want: "--allow is only supported for tcp, udp, and ssh ports", }, { name: "HTTP flag on TCP", - args: []string{"create", "my-instance", "8080", "--public"}, + args: []string{"open", "my-instance", "8080", "--public"}, want: "--hostname, --authorize, and --public are only supported for http and https ports", }, { name: "invalid hostname", - args: []string{"create", "my-instance", "8080", "--protocol", "http", "--hostname", "Not Valid"}, + args: []string{"open", "my-instance", "8080", "--protocol", "http", "--hostname", "Not Valid"}, want: "hostname must contain only lowercase letters", }, } diff --git a/pkg/cmd/ports/ports.go b/pkg/cmd/ports/ports.go index bd428c983..7339e3dc5 100644 --- a/pkg/cmd/ports/ports.go +++ b/pkg/cmd/ports/ports.go @@ -1,4 +1,4 @@ -// Package ports displays Brev-managed public port mappings for an instance or external node. +// Package ports displays Brev-managed public port mappings for an instance or Brev Connect machine. package ports import ( @@ -7,6 +7,7 @@ import ( "fmt" "io" "net" + "sort" "strconv" "strings" @@ -19,13 +20,15 @@ import ( "github.com/brevdev/brev-cli/pkg/cmd/register" cmdutil "github.com/brevdev/brev-cli/pkg/cmd/util" "github.com/brevdev/brev-cli/pkg/config" + "github.com/brevdev/brev-cli/pkg/entity" breverrors "github.com/brevdev/brev-cli/pkg/errors" ) // Store contains the dependencies needed to resolve both managed instances and -// registered compute nodes. +// Brev Connect machines. type Store interface { cmdutil.WorkspaceOrNodeResolver + GetContextWorkspaces() ([]entity.Workspace, error) } // PortInfo is the stable JSON representation of a port mapping. @@ -39,22 +42,27 @@ 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 group. func NewCmdPorts(portStore Store) *cobra.Command { cmd := &cobra.Command{ - Annotations: map[string]string{"access": ""}, + Annotations: map[string]string{"networking": ""}, Use: "ports", - Hidden: true, - Short: "[beta] Manage ports for an instance or external node", + Short: "Manage ports for environments or Brev Connect machines.", Args: cmderrors.TransformToValidationError(cobra.NoArgs), Example: ` brev ports ls my-instance - brev ports ls my-node --json`, + brev ports get nport-abc123 + brev ports open my-instance 8080 + brev ports open my-instance 8000-8031 --protocol tcp + brev ports open my-instance 3000 --protocol http --public + brev ports remove my-instance 8080 + brev ports rm nport-abc123 + brev ports ls my-connect-machine --json`, } cmd.AddCommand( + NewCmdGetPort(portStore), NewCmdPortsLs(portStore), NewCmdCreatePort(portStore), NewCmdUpdatePort(portStore), @@ -68,14 +76,13 @@ func NewCmdPortsLs(portStore Store) *cobra.Command { var jsonOutput bool cmd := &cobra.Command{ - Annotations: map[string]string{"access": ""}, - Use: "ls ", - Hidden: true, + Annotations: map[string]string{"networking": ""}, + Use: "ls ", DisableFlagsInUseLine: true, - Short: "[beta] List Brev-managed ports for an instance or external node", + Short: "List Brev-managed ports for an instance or Brev Connect machine", Example: ` brev ports ls my-instance - brev ports ls my-node --json`, + brev ports ls my-connect-machine --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 { @@ -89,7 +96,7 @@ func NewCmdPortsLs(portStore Store) *cobra.Command { return cmd } -// Run resolves a managed instance or registered compute node and displays its ports. +// Run resolves a managed instance or Brev Connect machine and displays its ports. func Run(ctx context.Context, out io.Writer, portStore Store, nameOrID string, jsonOutput bool) error { _, apiPorts, err := resolveTargetPorts(ctx, portStore, nameOrID) if err != nil { @@ -144,6 +151,82 @@ func resolveTargetPorts( return target, apiPorts, nil } +// resolvePortID finds the owner of a globally unique port ID without requiring +// the caller to know whether it belongs to an environment or Brev Connect machine. +func resolvePortID( //nolint:gocyclo // A port may belong to either supported owner type. + ctx context.Context, + portStore Store, + portID string, +) (*cmdutil.WorkspaceOrNode, *devplanev1.Port, error) { + portID = strings.TrimSpace(portID) + if portID == "" { + return nil, nil, breverrors.NewValidationError("port ID cannot be empty") + } + + workspaces, err := portStore.GetContextWorkspaces() + if err != nil { + return nil, nil, breverrors.WrapAndTrace(err) + } + environmentClient := register.NewEnvironmentServiceClient(portStore, config.GlobalConfig.GetBrevPublicAPIURL()) + var lookupErr error + for i := range workspaces { + workspace := &workspaces[i] + resp, getErr := environmentClient.GetNetworkInfo(ctx, connect.NewRequest(&devplanev1.EnvironmentServiceGetNetworkInfoRequest{ + EnvironmentId: workspace.ID, + })) + if getErr != nil { + if lookupErr == nil { + lookupErr = fmt.Errorf("get ports for instance %q: %w", workspace.Name, getErr) + } + continue + } + if resp == nil || resp.Msg == nil { + continue + } + if port := portWithID(resp.Msg.GetNetworkInfo().GetPorts(), portID); port != nil { + return &cmdutil.WorkspaceOrNode{Workspace: workspace}, port, nil + } + } + + org, err := portStore.GetActiveOrganizationOrDefault() + if err != nil { + return nil, nil, breverrors.WrapAndTrace(err) + } + nodeClient := register.NewNodeServiceClient(portStore, config.GlobalConfig.GetBrevPublicAPIURL()) + nodesResp, err := nodeClient.ListNodes(ctx, connect.NewRequest(&devplanev1.ListNodesRequest{ + OrganizationId: org.ID, + Options: &devplanev1.ListNodesOptions{ + ExcludeConnectivityInfo: true, + }, + })) + if err != nil { + return nil, nil, breverrors.WrapAndTrace(err) + } + if nodesResp != nil && nodesResp.Msg != nil { + for _, node := range nodesResp.Msg.GetItems() { + if node == nil { + continue + } + if port := portWithID(node.GetPorts(), portID); port != nil { + return &cmdutil.WorkspaceOrNode{Node: node}, port, nil + } + } + } + if lookupErr != nil { + return nil, nil, breverrors.WrapAndTrace(lookupErr) + } + return nil, nil, fmt.Errorf("port_id %q is not open in the active organization", portID) +} + +func portWithID(ports []*devplanev1.Port, portID string) *devplanev1.Port { + for _, port := range ports { + if port != nil && port.GetPortId() == portID { + return port + } + } + return nil +} + func toPortInfos(apiPorts []*devplanev1.Port) []PortInfo { portInfos := make([]PortInfo, 0, len(apiPorts)) for _, port := range apiPorts { @@ -161,12 +244,41 @@ func toPortInfos(apiPorts []*devplanev1.Port) []PortInfo { AuthorizedEmails: append([]string{}, port.GetAuthorizedEmails()...), AllowPublicUnauthenticated: port.GetAllowPublicUnauthenticated(), Type: portTypeLabel(port.GetType()), - isHTTP: isHTTP, }) } + sort.SliceStable(portInfos, func(i, j int) bool { + left, right := portInfos[i], portInfos[j] + if protocolOrder(left.Protocol) != protocolOrder(right.Protocol) { + return protocolOrder(left.Protocol) < protocolOrder(right.Protocol) + } + if left.DestinationPort != right.DestinationPort { + return left.DestinationPort < right.DestinationPort + } + if left.PublicPort != right.PublicPort { + return left.PublicPort < right.PublicPort + } + return left.PortID < right.PortID + }) return portInfos } +func protocolOrder(protocol string) int { + switch protocol { + case "HTTPS": + return 0 + case "HTTP": + return 1 + case "TCP": + return 2 + case "UDP": + return 3 + case "SSH": + return 4 + default: + return 5 + } +} + func endpoint(port *devplanev1.Port, isHTTP bool) string { hostname := port.GetHostname() if hostname == "" { @@ -233,59 +345,17 @@ func displayTables(out io.Writer, nameOrID string, portInfos []PortInfo) error { return breverrors.WrapAndTrace(err) } - httpPorts := make([]PortInfo, 0, len(portInfos)) - networkPorts := make([]PortInfo, 0, len(portInfos)) - for _, port := range portInfos { - if port.isHTTP { - 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, "NETWORK PORTS"); err != nil { - return breverrors.WrapAndTrace(err) - } - displayNetworkTable(out, networkPorts) - } + displayPortTable(out, portInfos) return nil } -func displayHTTPTable(out io.Writer, portInfos []PortInfo) { +func displayPortTable(out io.Writer, portInfos []PortInfo) { tw := newTable(out) - tw.AppendHeader(table.Row{"ENDPOINT", "AUTHORIZATION", "IP RESTRICTIONS", "PUBLIC PORT", "DESTINATION PORT", "PROTOCOL"}) + tw.AppendHeader(table.Row{"ID", "ENDPOINT", "PUBLIC PORT", "DESTINATION PORT", "PROTOCOL"}) for _, port := range portInfos { tw.AppendRow(table.Row{ + valueOrDash(port.PortID), valueOrDash(port.Endpoint), - authorizationLabel(port), - allowedSourcesLabel(port.AllowedSources), - portNumberLabel(port.PublicPort), - portNumberLabel(port.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, @@ -306,16 +376,6 @@ func newTable(out io.Writer) table.Writer { 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" diff --git a/pkg/cmd/ports/ports_test.go b/pkg/cmd/ports/ports_test.go index f50344c2e..e47f83cb8 100644 --- a/pkg/cmd/ports/ports_test.go +++ b/pkg/cmd/ports/ports_test.go @@ -33,6 +33,10 @@ func (s *fakeStore) GetWorkspaceByNameOrID(_ string, _ string) ([]entity.Workspa return s.workspaces, nil } +func (s *fakeStore) GetContextWorkspaces() ([]entity.Workspace, error) { + return s.workspaces, nil +} + func (s *fakeStore) GetCurrentUser() (*entity.User, error) { return s.user, nil } @@ -73,37 +77,25 @@ func (s *fakeNodeService) ListNodes( func TestPortsCommandUsesSubcommands(t *testing.T) { cmd := NewCmdPorts(&fakeStore{}) - assert.Equal(t, "[beta] Manage ports for an instance or external node", cmd.Short) - assert.True(t, cmd.Hidden) - - listCmd, _, err := cmd.Find([]string{"ls"}) - require.NoError(t, err) - assert.Equal(t, "ls ", listCmd.Use) - assert.Equal(t, "[beta] List Brev-managed ports for an instance or external node", listCmd.Short) - assert.True(t, listCmd.Hidden) - assert.Contains(t, listCmd.Annotations, "access") - assert.Nil(t, cmd.Flags().Lookup("json")) - assert.NotNil(t, listCmd.Flags().Lookup("json")) - - createCmd, _, err := cmd.Find([]string{"create"}) - require.NoError(t, err) - assert.Equal(t, "create ", createCmd.Use) - assert.Equal(t, "[beta] Create a public port on an instance or external node", createCmd.Short) - assert.True(t, createCmd.Hidden) - assert.ElementsMatch(t, []string{"open", "add"}, createCmd.Aliases) - - closeCmd, _, err := cmd.Find([]string{"close"}) - require.NoError(t, err) - assert.Equal(t, "close ", closeCmd.Use) - assert.Equal(t, "[beta] Close public ports on an instance or external node", closeCmd.Short) - assert.True(t, closeCmd.Hidden) - assert.Nil(t, closeCmd.Flags().Lookup("all")) + assert.Equal(t, "Manage ports for environments or Brev Connect machines.", cmd.Short) + assert.Contains(t, cmd.Annotations, "networking") + + uses := map[string]string{ + "get": "get ", + "ls": "ls ", + "open": "open ", + "remove": "remove | ", + "update": "update ", + } + for name, use := range uses { + subcommand, _, err := cmd.Find([]string{name}) + require.NoError(t, err) + assert.Equal(t, use, subcommand.Use) + } - updateCmd, _, err := cmd.Find([]string{"update"}) + removeCmd, _, err := cmd.Find([]string{"remove"}) require.NoError(t, err) - assert.Equal(t, "update ", updateCmd.Use) - assert.Equal(t, "[beta] Update a public port on an instance or external node", updateCmd.Short) - assert.True(t, updateCmd.Hidden) + assert.Equal(t, []string{"rm"}, removeCmd.Aliases) } func TestRunEnvironmentJSON(t *testing.T) { @@ -158,7 +150,7 @@ func TestRunEnvironmentJSON(t *testing.T) { ]`, out.String()) } -func TestRunExternalNodeByIDDisplaysTables(t *testing.T) { +func TestRunBrevConnectMachineDisplaysPorts(t *testing.T) { httpHostname := "jupyter-node.apps.run.brev.nvidia.com" tcpHostname := "global.prd.ga.run.brev.nvidia.com" service := &fakeNodeService{nodes: []*devplanev1.ExternalNode{ @@ -199,49 +191,10 @@ func TestRunExternalNodeByIDDisplaysTables(t *testing.T) { err := Run(context.Background(), &out, store, "unode123", false) require.NoError(t, err) - assert.Contains(t, out.String(), "HTTP APPLICATIONS") + assert.Contains(t, out.String(), "port-http") 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(), "NETWORK PORTS") - assert.Contains(t, out.String(), "PUBLIC PORT") - assert.Contains(t, out.String(), "DESTINATION PORT") + assert.Contains(t, out.String(), "port-tcp") 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 TestDisplayTablesSSHUsesNetworkHeading(t *testing.T) { - var out bytes.Buffer - ports := []PortInfo{ - { - 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 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) { @@ -271,11 +224,9 @@ func TestToPortInfosHandlesPublicHTTPAndRestrictedUDP(t *testing.T) { }) require.Len(t, got, 2) - 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.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) @@ -284,6 +235,24 @@ func TestToPortInfosHandlesPublicHTTPAndRestrictedUDP(t *testing.T) { assert.Equal(t, "unknown", portTypeLabel(devplanev1.PortType(99))) } +func TestToPortInfosSortsByProtocolThenDestinationPort(t *testing.T) { + hostname := "example.com" + ports := []*devplanev1.Port{ + {PortId: "ssh", Protocol: devplanev1.PortProtocol_PORT_PROTOCOL_SSH, ServerPort: 22}, + {PortId: "udp", Protocol: devplanev1.PortProtocol_PORT_PROTOCOL_UDP, ServerPort: 53}, + {PortId: "tcp-later", Protocol: devplanev1.PortProtocol_PORT_PROTOCOL_TCP, ServerPort: 9000}, + {PortId: "http", HttpProtocol: devplanev1.HttpPortProtocol_HTTP_PORT_PROTOCOL_HTTP, ServerPort: 80, Hostname: &hostname}, + {PortId: "https", HttpProtocol: devplanev1.HttpPortProtocol_HTTP_PORT_PROTOCOL_HTTPS, ServerPort: 443, Hostname: &hostname}, + {PortId: "tcp-first", Protocol: devplanev1.PortProtocol_PORT_PROTOCOL_TCP, ServerPort: 8000}, + } + + got := toPortInfos(ports) + + assert.Equal(t, []string{"https", "http", "tcp-first", "tcp-later", "udp", "ssh"}, []string{ + got[0].PortID, got[1].PortID, got[2].PortID, got[3].PortID, got[4].PortID, got[5].PortID, + }) +} + func TestRunEmptyPortsJSONIsArray(t *testing.T) { service := &fakeEnvironmentService{ t: t, @@ -406,51 +375,3 @@ func TestRunEnvironmentWithPortsAndUnspecifiedStatusStillLists(t *testing.T) { 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", - "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/ports/update.go b/pkg/cmd/ports/update.go index 8570c1d7f..cc4a2183e 100644 --- a/pkg/cmd/ports/update.go +++ b/pkg/cmd/ports/update.go @@ -2,10 +2,8 @@ package ports import ( "context" - "encoding/json" "fmt" "io" - "strings" devplanev1 "buf.build/gen/go/brevdev/devplane/protocolbuffers/go/devplaneapi/v1" "connectrpc.com/connect" @@ -16,11 +14,9 @@ import ( cmdutil "github.com/brevdev/brev-cli/pkg/cmd/util" "github.com/brevdev/brev-cli/pkg/config" breverrors "github.com/brevdev/brev-cli/pkg/errors" - "github.com/brevdev/brev-cli/pkg/terminal" ) type updateOptions struct { - portID string destinationPort string allowedSources []string allowAnywhere bool @@ -45,31 +41,22 @@ type portUpdates struct { public bool } -type updatePrompter interface { - terminal.Selector -} - // NewCmdUpdatePort creates the `brev ports update` command. func NewCmdUpdatePort(portStore Store) *cobra.Command { - return newCmdUpdatePort(portStore, register.TerminalPrompter{}) -} - -func newCmdUpdatePort(portStore Store, prompter updatePrompter) *cobra.Command { var opts updateOptions cmd := &cobra.Command{ - Annotations: map[string]string{"access": ""}, - Use: "update ", + Annotations: map[string]string{"networking": ""}, + Use: "update ", Aliases: []string{"edit"}, - Hidden: true, DisableFlagsInUseLine: true, - Short: "[beta] Update a public port on an instance or external node", + Short: "Update a Brev-managed port by ID", Example: ` - brev ports update my-instance --id nport-abc123 --destination-port 8081 - brev ports update my-instance --id nport-abc123 --allow 203.0.113.10/32 - brev ports update my-node --id nport-abc123 --allow-anywhere - brev ports update my-instance --id nport-abc123 --protocol https - brev ports update my-instance --id nport-abc123 --public`, + brev ports update nport-abc123 --destination-port 8081 + brev ports update nport-abc123 --allow 203.0.113.10/32 + brev ports update nport-abc123 --allow-anywhere + brev ports update nport-abc123 --protocol https + brev ports update nport-abc123 --public`, Args: cmderrors.TransformToValidationError(cobra.ExactArgs(1)), RunE: func(cmd *cobra.Command, args []string) error { opts.destinationPortSet = cmd.Flags().Changed("destination-port") @@ -84,12 +71,11 @@ func newCmdUpdatePort(portStore Store, prompter updatePrompter) *cobra.Command { return breverrors.WrapAndTrace(err) } return breverrors.WrapAndTrace(runUpdate( - cmd.Context(), cmd.OutOrStdout(), portStore, prompter, args[0], opts.portID, updates, opts.jsonOutput, + cmd.Context(), cmd.OutOrStdout(), portStore, args[0], updates, opts.jsonOutput, )) }, } - cmd.Flags().StringVar(&opts.portID, "id", "", "update the exact port mapping with this port_id (omit to select interactively)") cmd.Flags().StringVar(&opts.destinationPort, "destination-port", "", "new destination port (1-65535)") cmd.Flags().StringArrayVar(&opts.allowedSources, "allow", nil, "replace source restrictions with this CIDR (repeatable)") cmd.Flags().BoolVar(&opts.allowAnywhere, "allow-anywhere", false, "clear all source restrictions") @@ -207,18 +193,11 @@ func runUpdate( ctx context.Context, out io.Writer, portStore Store, - prompter updatePrompter, - nameOrID string, portID string, updates portUpdates, jsonOutput bool, ) error { - target, apiPorts, err := resolveTargetPorts(ctx, portStore, nameOrID) - if err != nil { - return breverrors.WrapAndTrace(err) - } - - port, err := selectPortToUpdate(prompter, apiPorts, strings.TrimSpace(portID)) + target, port, err := resolvePortID(ctx, portStore, portID) if err != nil { return breverrors.WrapAndTrace(err) } @@ -230,38 +209,7 @@ func runUpdate( if err != nil { return breverrors.WrapAndTrace(err) } - return writeUpdateResult(out, nameOrID, updated, jsonOutput) -} - -func selectPortToUpdate( - prompter terminal.Selector, - apiPorts []*devplanev1.Port, - portID string, -) (*devplanev1.Port, error) { - ports := removablePorts(apiPorts) - if len(ports) == 0 { - return nil, fmt.Errorf("no updatable ports are open on this target") - } - if portID != "" { - for _, port := range ports { - if port.GetPortId() == portID { - return port, nil - } - } - return nil, fmt.Errorf("port_id %q is not open on this target", portID) - } - - labels := make([]string, len(ports)) - for i, port := range ports { - labels[i] = closeSelectionLabel(i, port) - } - chosen := prompter.Select("Select a port to update", labels) - for i, label := range labels { - if label == chosen { - return ports[i], nil - } - } - return nil, fmt.Errorf("selected item did not match any open port") + return writeUpdateResult(out, updated, jsonOutput) } func isHTTPPort(port *devplanev1.Port) bool { @@ -343,7 +291,7 @@ func setPortTarget( } return resp.Msg.GetPort(), nil } - return nil, fmt.Errorf("resolved target has no instance or external node") + return nil, fmt.Errorf("resolved target has no instance or Brev Connect machine") } func setPortAllowedSources( @@ -382,7 +330,7 @@ func setPortAllowedSources( } return resp.Msg.GetPort(), nil } - return nil, fmt.Errorf("resolved target has no instance or external node") + return nil, fmt.Errorf("resolved target has no instance or Brev Connect machine") } //nolint:dupl // Environment and node RPCs intentionally have parallel request types. @@ -419,7 +367,7 @@ func setHTTPPortProtocol( } return resp.Msg.GetPort(), nil } - return nil, fmt.Errorf("resolved target has no instance or external node") + return nil, fmt.Errorf("resolved target has no instance or Brev Connect machine") } func setHTTPPortAccess( @@ -462,22 +410,18 @@ func setHTTPPortAccess( } return resp.Msg.GetPort(), nil } - return nil, fmt.Errorf("resolved target has no instance or external node") + return nil, fmt.Errorf("resolved target has no instance or Brev Connect machine") } -func writeUpdateResult(out io.Writer, nameOrID string, port *devplanev1.Port, jsonOutput bool) error { +func writeUpdateResult(out io.Writer, port *devplanev1.Port, jsonOutput bool) error { portInfo := toPortInfos([]*devplanev1.Port{port})[0] if jsonOutput { - encoded, err := json.MarshalIndent(portInfo, "", " ") - if err != nil { - return breverrors.WrapAndTrace(err) - } - _, err = fmt.Fprintln(out, string(encoded)) - return breverrors.WrapAndTrace(err) + return writePortJSON(out, portInfo) } - if _, err := fmt.Fprintf(out, "Updated port %s on %s.\n", port.GetPortId(), nameOrID); err != nil { + if _, err := fmt.Fprintf(out, "Updated port %s.\n", port.GetPortId()); err != nil { return breverrors.WrapAndTrace(err) } - return displayTables(out, nameOrID, []PortInfo{portInfo}) + displayPortTable(out, []PortInfo{portInfo}) + return nil } diff --git a/pkg/cmd/ports/update_test.go b/pkg/cmd/ports/update_test.go index a48ef47b2..de16827a5 100644 --- a/pkg/cmd/ports/update_test.go +++ b/pkg/cmd/ports/update_test.go @@ -3,7 +3,6 @@ package ports import ( "bytes" "context" - "errors" "testing" devplanev1connect "buf.build/gen/go/brevdev/devplane/connectrpc/go/devplaneapi/v1/devplaneapiv1connect" @@ -15,21 +14,6 @@ import ( "github.com/brevdev/brev-cli/pkg/entity" ) -type fakeUpdatePrompter struct { - selectIndex int - selectCalls int - items []string -} - -func (p *fakeUpdatePrompter) Select(_ string, items []string) string { - p.selectCalls++ - p.items = append([]string{}, items...) - if p.selectIndex < 0 || p.selectIndex >= len(items) { - return "" - } - return items[p.selectIndex] -} - type fakeUpdateEnvironmentService struct { devplanev1connect.UnimplementedEnvironmentServiceHandler t *testing.T @@ -40,10 +24,6 @@ type fakeUpdateEnvironmentService struct { sourcesReq *devplanev1.EnvironmentServiceSetPortAllowedSourcesRequest protocolReq *devplanev1.EnvironmentServiceSetHTTPPortProtocolRequest accessReq *devplanev1.EnvironmentServiceSetHTTPPortAccessRequest - failMethod string - nilPortMethod string - rpcErr error - calls []string } func (s *fakeUpdateEnvironmentService) GetNetworkInfo( @@ -65,15 +45,7 @@ func (s *fakeUpdateEnvironmentService) SetPortTarget( req *connect.Request[devplanev1.EnvironmentServiceSetPortTargetRequest], ) (*connect.Response[devplanev1.EnvironmentServiceSetPortTargetResponse], error) { s.targetReq = req.Msg - s.calls = append(s.calls, "target") - if s.failMethod == "target" { - return nil, s.rpcErr - } - port := s.responsePort - if s.nilPortMethod == "target" { - port = nil - } - return connect.NewResponse(&devplanev1.EnvironmentServiceSetPortTargetResponse{Port: port}), nil + return connect.NewResponse(&devplanev1.EnvironmentServiceSetPortTargetResponse{Port: s.responsePort}), nil } func (s *fakeUpdateEnvironmentService) SetPortAllowedSources( @@ -81,15 +53,7 @@ func (s *fakeUpdateEnvironmentService) SetPortAllowedSources( req *connect.Request[devplanev1.EnvironmentServiceSetPortAllowedSourcesRequest], ) (*connect.Response[devplanev1.EnvironmentServiceSetPortAllowedSourcesResponse], error) { s.sourcesReq = req.Msg - s.calls = append(s.calls, "sources") - if s.failMethod == "sources" { - return nil, s.rpcErr - } - port := s.responsePort - if s.nilPortMethod == "sources" { - port = nil - } - return connect.NewResponse(&devplanev1.EnvironmentServiceSetPortAllowedSourcesResponse{Port: port}), nil + return connect.NewResponse(&devplanev1.EnvironmentServiceSetPortAllowedSourcesResponse{Port: s.responsePort}), nil } func (s *fakeUpdateEnvironmentService) SetHTTPPortProtocol( @@ -97,15 +61,7 @@ func (s *fakeUpdateEnvironmentService) SetHTTPPortProtocol( req *connect.Request[devplanev1.EnvironmentServiceSetHTTPPortProtocolRequest], ) (*connect.Response[devplanev1.EnvironmentServiceSetHTTPPortProtocolResponse], error) { s.protocolReq = req.Msg - s.calls = append(s.calls, "protocol") - if s.failMethod == "protocol" { - return nil, s.rpcErr - } - port := s.responsePort - if s.nilPortMethod == "protocol" { - port = nil - } - return connect.NewResponse(&devplanev1.EnvironmentServiceSetHTTPPortProtocolResponse{Port: port}), nil + return connect.NewResponse(&devplanev1.EnvironmentServiceSetHTTPPortProtocolResponse{Port: s.responsePort}), nil } func (s *fakeUpdateEnvironmentService) SetHTTPPortAccess( @@ -113,23 +69,13 @@ func (s *fakeUpdateEnvironmentService) SetHTTPPortAccess( req *connect.Request[devplanev1.EnvironmentServiceSetHTTPPortAccessRequest], ) (*connect.Response[devplanev1.EnvironmentServiceSetHTTPPortAccessResponse], error) { s.accessReq = req.Msg - s.calls = append(s.calls, "access") - if s.failMethod == "access" { - return nil, s.rpcErr - } - port := s.responsePort - if s.nilPortMethod == "access" { - port = nil - } - return connect.NewResponse(&devplanev1.EnvironmentServiceSetHTTPPortAccessResponse{Port: port}), nil + return connect.NewResponse(&devplanev1.EnvironmentServiceSetHTTPPortAccessResponse{Port: s.responsePort}), nil } type fakeUpdateNodeService struct { devplanev1connect.UnimplementedExternalNodeServiceHandler node *devplanev1.ExternalNode responsePort *devplanev1.Port - targetReq *devplanev1.SetPortTargetRequest - sourcesReq *devplanev1.SetPortAllowedSourcesRequest protocolReq *devplanev1.SetHTTPPortProtocolRequest accessReq *devplanev1.SetHTTPPortAccessRequest } @@ -143,22 +89,6 @@ func (s *fakeUpdateNodeService) ListNodes( }), nil } -func (s *fakeUpdateNodeService) SetPortTarget( - _ context.Context, - req *connect.Request[devplanev1.SetPortTargetRequest], -) (*connect.Response[devplanev1.SetPortTargetResponse], error) { - s.targetReq = req.Msg - return connect.NewResponse(&devplanev1.SetPortTargetResponse{Port: s.responsePort}), nil -} - -func (s *fakeUpdateNodeService) SetPortAllowedSources( - _ context.Context, - req *connect.Request[devplanev1.SetPortAllowedSourcesRequest], -) (*connect.Response[devplanev1.SetPortAllowedSourcesResponse], error) { - s.sourcesReq = req.Msg - return connect.NewResponse(&devplanev1.SetPortAllowedSourcesResponse{Port: s.responsePort}), nil -} - func (s *fakeUpdateNodeService) SetHTTPPortProtocol( _ context.Context, req *connect.Request[devplanev1.SetHTTPPortProtocolRequest], @@ -207,9 +137,9 @@ func TestUpdateEnvironmentDestinationAndAllowedSources(t *testing.T) { } _, handler := devplanev1connect.NewEnvironmentServiceHandler(service) newTestServer(t, handler) - cmd := newCmdUpdatePort(newUpdateEnvironmentStore(), &fakeUpdatePrompter{selectIndex: -1}) + cmd := NewCmdUpdatePort(newUpdateEnvironmentStore()) cmd.SetArgs([]string{ - "my-instance", "--id", "nport-one", "--destination-port", "9090", + "nport-one", "--destination-port", "9090", "--allow", "203.0.113.10/32", "--allow", "198.51.100.0/24", "--json", }) var out bytes.Buffer @@ -256,8 +186,8 @@ func TestUpdateExternalNodeHTTPProtocolAndPublicAccess(t *testing.T) { user: &entity.User{ID: "user1", Email: "me@example.com"}, org: &entity.Organization{ID: "org1"}, } - cmd := newCmdUpdatePort(store, &fakeUpdatePrompter{selectIndex: -1}) - cmd.SetArgs([]string{"my-node", "--id", "http-one", "--protocol", "https", "--public", "--json"}) + cmd := NewCmdUpdatePort(store) + cmd.SetArgs([]string{"http-one", "--protocol", "https", "--public", "--json"}) var out bytes.Buffer cmd.SetOut(&out) @@ -273,112 +203,6 @@ func TestUpdateExternalNodeHTTPProtocolAndPublicAccess(t *testing.T) { assert.Contains(t, out.String(), `"allow_public_unauthenticated": true`) } -func TestUpdateExternalNodeDestinationAndAllowedSources(t *testing.T) { - updated := testTCPPort("nport-one", 41001) - updated.ServerPort = 9090 - updated.AllowedSources = []string{"203.0.113.10/32"} - service := &fakeUpdateNodeService{ - node: &devplanev1.ExternalNode{ - ExternalNodeId: "unode123", - Name: "my-node", - Ports: []*devplanev1.Port{testTCPPort("nport-one", 41001)}, - }, - responsePort: updated, - } - _, handler := devplanev1connect.NewExternalNodeServiceHandler(service) - newTestServer(t, handler) - store := &fakeStore{ - user: &entity.User{ID: "user1", Email: "me@example.com"}, - org: &entity.Organization{ID: "org1"}, - } - cmd := newCmdUpdatePort(store, &fakeUpdatePrompter{selectIndex: -1}) - cmd.SetArgs([]string{ - "my-node", "--id", "nport-one", "--destination-port", "9090", - "--allow", "203.0.113.10/32", - }) - - err := cmd.Execute() - - require.NoError(t, err) - require.NotNil(t, service.targetReq) - assert.Equal(t, "nport-one", service.targetReq.GetPortId()) - assert.Equal(t, int32(9090), service.targetReq.GetPortNumber()) - require.NotNil(t, service.sourcesReq) - assert.Equal(t, []string{"203.0.113.10/32"}, service.sourcesReq.GetAllowedSources()) -} - -func TestUpdateExternalNodeAllowAnywhere(t *testing.T) { - updated := testTCPPort("nport-one", 41001) - updated.AllowedSources = []string{"0.0.0.0/0"} - service := &fakeUpdateNodeService{ - node: &devplanev1.ExternalNode{ - ExternalNodeId: "unode123", - Name: "my-node", - Ports: []*devplanev1.Port{testTCPPort("nport-one", 41001)}, - }, - responsePort: updated, - } - _, handler := devplanev1connect.NewExternalNodeServiceHandler(service) - newTestServer(t, handler) - store := &fakeStore{ - user: &entity.User{ID: "user1", Email: "me@example.com"}, - org: &entity.Organization{ID: "org1"}, - } - cmd := newCmdUpdatePort(store, &fakeUpdatePrompter{selectIndex: -1}) - cmd.SetArgs([]string{"my-node", "--id", "nport-one", "--allow-anywhere"}) - - err := cmd.Execute() - - require.NoError(t, err) - require.NotNil(t, service.sourcesReq) - assert.Equal(t, []string{"0.0.0.0/0"}, service.sourcesReq.GetAllowedSources()) -} - -func TestUpdateStopsAfterMutationFailure(t *testing.T) { - tests := []struct { - name string - failMethod string - nilPortMethod string - wantCalls []string - wantErr string - }{ - {name: "target RPC error", failMethod: "target", wantCalls: []string{"target"}, wantErr: "update destination port"}, - {name: "target missing port", nilPortMethod: "target", wantCalls: []string{"target"}, wantErr: "set destination port: API returned no port"}, - {name: "sources RPC error", failMethod: "sources", wantCalls: []string{"target", "sources"}, wantErr: "update allowed sources"}, - {name: "sources missing port", nilPortMethod: "sources", wantCalls: []string{"target", "sources"}, wantErr: "set allowed sources: API returned no port"}, - {name: "protocol RPC error", failMethod: "protocol", wantCalls: []string{"target", "sources", "protocol"}, wantErr: "update HTTP protocol"}, - {name: "protocol missing port", nilPortMethod: "protocol", wantCalls: []string{"target", "sources", "protocol"}, wantErr: "set HTTP protocol: API returned no port"}, - {name: "access RPC error", failMethod: "access", wantCalls: []string{"target", "sources", "protocol", "access"}, wantErr: "update HTTP access"}, - {name: "access missing port", nilPortMethod: "access", wantCalls: []string{"target", "sources", "protocol", "access"}, wantErr: "set HTTP access: API returned no port"}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - service := &fakeUpdateEnvironmentService{ - t: t, - expectedEnvID: "env123", - ports: []*devplanev1.Port{testHTTPPort(8080)}, - responsePort: testHTTPPort(9090), - failMethod: tt.failMethod, - nilPortMethod: tt.nilPortMethod, - rpcErr: connect.NewError(connect.CodeInternal, errors.New("boom")), - } - _, handler := devplanev1connect.NewEnvironmentServiceHandler(service) - newTestServer(t, handler) - cmd := newCmdUpdatePort(newUpdateEnvironmentStore(), &fakeUpdatePrompter{selectIndex: -1}) - cmd.SetArgs([]string{ - "my-instance", "--id", "http-one", "--destination-port", "9090", - "--allow", "203.0.113.10/32", "--protocol", "https", "--authorize", "next@example.com", - }) - - err := cmd.Execute() - - assert.ErrorContains(t, err, tt.wantErr) - assert.Equal(t, tt.wantCalls, service.calls) - }) - } -} - func TestUpdateHTTPAuthorizedEmails(t *testing.T) { updated := testHTTPPort(8080) updated.AuthorizedEmails = []string{"one@example.com", "two@example.com"} @@ -390,9 +214,9 @@ func TestUpdateHTTPAuthorizedEmails(t *testing.T) { } _, handler := devplanev1connect.NewEnvironmentServiceHandler(service) newTestServer(t, handler) - cmd := newCmdUpdatePort(newUpdateEnvironmentStore(), &fakeUpdatePrompter{selectIndex: -1}) + cmd := NewCmdUpdatePort(newUpdateEnvironmentStore()) cmd.SetArgs([]string{ - "my-instance", "--id", "http-one", + "http-one", "--authorize", "one@example.com", "--authorize", "two@example.com", }) var out bytes.Buffer @@ -404,53 +228,7 @@ func TestUpdateHTTPAuthorizedEmails(t *testing.T) { require.NotNil(t, service.accessReq) assert.Equal(t, []string{"one@example.com", "two@example.com"}, service.accessReq.GetAuthorizedEmails().GetEmails()) assert.False(t, service.accessReq.GetAllowPublicUnauthenticated()) - assert.Contains(t, out.String(), "Updated port http-one on my-instance.") -} - -func TestUpdateInteractiveSelectionCanDisambiguateDuplicateDestinations(t *testing.T) { - updated := testTCPPort("nport-two", 52002) - updated.AllowedSources = []string{"0.0.0.0/0"} - service := &fakeUpdateEnvironmentService{ - t: t, - expectedEnvID: "env123", - ports: []*devplanev1.Port{ - testTCPPort("nport-one", 41001), - testTCPPort("nport-two", 52002), - }, - responsePort: updated, - } - _, handler := devplanev1connect.NewEnvironmentServiceHandler(service) - newTestServer(t, handler) - prompter := &fakeUpdatePrompter{selectIndex: 1} - cmd := newCmdUpdatePort(newUpdateEnvironmentStore(), prompter) - cmd.SetArgs([]string{"my-instance", "--allow-anywhere"}) - - err := cmd.Execute() - - require.NoError(t, err) - assert.Equal(t, 1, prompter.selectCalls) - require.Len(t, prompter.items, 2) - assert.Contains(t, prompter.items[0], "public 41001 -> destination 8080") - assert.Contains(t, prompter.items[1], "public 52002 -> destination 8080") - require.NotNil(t, service.sourcesReq) - assert.Equal(t, "nport-two", service.sourcesReq.GetPortId()) - assert.Equal(t, []string{"0.0.0.0/0"}, service.sourcesReq.GetAllowedSources().GetCidrBlocks()) -} - -func TestUpdateRejectsUnknownID(t *testing.T) { - service := &fakeUpdateEnvironmentService{ - t: t, - expectedEnvID: "env123", - ports: []*devplanev1.Port{testTCPPort("nport-one", 41001)}, - } - _, handler := devplanev1connect.NewEnvironmentServiceHandler(service) - newTestServer(t, handler) - cmd := newCmdUpdatePort(newUpdateEnvironmentStore(), &fakeUpdatePrompter{selectIndex: -1}) - cmd.SetArgs([]string{"my-instance", "--id", "missing", "--destination-port", "9090"}) - - err := cmd.Execute() - - assert.ErrorContains(t, err, `port_id "missing" is not open on this target`) + assert.Contains(t, out.String(), "Updated port http-one.") } func TestUpdateRejectsHTTPFlagsForRawPort(t *testing.T) { @@ -461,8 +239,8 @@ func TestUpdateRejectsHTTPFlagsForRawPort(t *testing.T) { } _, handler := devplanev1connect.NewEnvironmentServiceHandler(service) newTestServer(t, handler) - cmd := newCmdUpdatePort(newUpdateEnvironmentStore(), &fakeUpdatePrompter{selectIndex: -1}) - cmd.SetArgs([]string{"my-instance", "--id", "nport-one", "--public"}) + cmd := NewCmdUpdatePort(newUpdateEnvironmentStore()) + cmd.SetArgs([]string{"nport-one", "--public"}) err := cmd.Execute() diff --git a/pkg/cmd/util/externalnode.go b/pkg/cmd/util/externalnode.go index acd7d2abd..7d37e43fc 100644 --- a/pkg/cmd/util/externalnode.go +++ b/pkg/cmd/util/externalnode.go @@ -15,7 +15,7 @@ import ( "github.com/brevdev/brev-cli/pkg/ssh" ) -// ExternalNodeStore is the minimal interface needed for external node lookup and SSH resolution. +// ExternalNodeStore is the minimal interface needed for Brev Connect machine lookup and SSH resolution. type ExternalNodeStore interface { GetActiveOrganizationOrDefault() (*entity.Organization, error) GetAccessToken() (string, error) @@ -33,7 +33,7 @@ type WorkspaceOrNode struct { Node *nodev1.ExternalNode } -// ResolveWorkspaceOrNode looks up a workspace first; if not found, falls back to external nodes. +// ResolveWorkspaceOrNode looks up a workspace first; if not found, falls back to Brev Connect machines. // The store must satisfy both GetWorkspaceByNameOrIDErrStore and ExternalNodeStore. func ResolveWorkspaceOrNode(store WorkspaceOrNodeResolver, nameOrID string, ) (*WorkspaceOrNode, error) { @@ -41,7 +41,7 @@ func ResolveWorkspaceOrNode(store WorkspaceOrNodeResolver, nameOrID string, } // ResolveWorkspaceOrNodeWithContext looks up a workspace first; if not found, falls back to -// external nodes. The context is used for the external-node service request. +// Brev Connect machines. The context is used for the machine service request. func ResolveWorkspaceOrNodeWithContext(ctx context.Context, store WorkspaceOrNodeResolver, nameOrID string, ) (*WorkspaceOrNode, error) { workspace, workspaceFound, err := findUserWorkspaceByNameOrID(store, nameOrID) @@ -57,14 +57,14 @@ func ResolveWorkspaceOrNodeWithContext(ctx context.Context, store WorkspaceOrNod } if node == nil { return nil, breverrors.NewValidationError(fmt.Sprintf( - "instance or external node with id/name %q not found", + "instance or Brev Connect machine with id/name %q not found", nameOrID, )) } return &WorkspaceOrNode{Node: node}, nil } -// ExternalNodeSSHInfo holds resolved SSH connection details for an external node. +// ExternalNodeSSHInfo holds resolved SSH connection details for a Brev Connect machine. type ExternalNodeSSHInfo struct { Node *nodev1.ExternalNode LinuxUser string @@ -127,7 +127,7 @@ func resolvePortForSSHAccess(node *nodev1.ExternalNode, access *nodev1.SSHAccess return nil } -// OpenPort calls the OpenPort RPC to open a port on an external node via netbird. +// OpenPort calls the OpenPort RPC to open a port on a Brev Connect machine via netbird. // 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()) @@ -142,13 +142,13 @@ func OpenPort(store ExternalNodeStore, nodeID string, portNumber int32, protocol return resp.Msg.GetPort(), nil } -// FindExternalNode searches for an external node by name or ID in the user's active organization. +// FindExternalNode searches for a Brev Connect machine 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 +// FindExternalNodeWithContext searches for a Brev Connect machine 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 FindExternalNodeWithContext(ctx context.Context, store ExternalNodeStore, nameOrID string) (*nodev1.ExternalNode, error) { @@ -184,7 +184,7 @@ func findExternalNode(nodes []*nodev1.ExternalNode, nameOrID string) *nodev1.Ext return nil } -// ResolveExternalNodeSSH resolves the SSH connection details for an external node +// ResolveExternalNodeSSH resolves the SSH connection details for a Brev Connect machine // by finding the current user's SSH access and the allocated port for that access. func ResolveExternalNodeSSH(store ExternalNodeStore, node *nodev1.ExternalNode) (*ExternalNodeSSHInfo, error) { user, err := store.GetCurrentUser() diff --git a/pkg/cmd/util/externalnode_test.go b/pkg/cmd/util/externalnode_test.go index 45e18644f..2674b9ebd 100644 --- a/pkg/cmd/util/externalnode_test.go +++ b/pkg/cmd/util/externalnode_test.go @@ -182,7 +182,7 @@ func TestResolveWorkspaceOrNodeWithContext_NoMatchesReturnsWorkspaceError(t *tes 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`) { + if !strings.Contains(err.Error(), `instance or Brev Connect machine with id/name "missing" not found`) { t.Fatalf("expected a combined target not-found error, got %v", err) } var validationErr breverrors.ValidationError diff --git a/pkg/ssh/sshconfigurer_test.go b/pkg/ssh/sshconfigurer_test.go index 75a37c650..ef8959c39 100644 --- a/pkg/ssh/sshconfigurer_test.go +++ b/pkg/ssh/sshconfigurer_test.go @@ -768,6 +768,10 @@ func makeMockWSLFS() SSHConfigurerV2Store { } func TestSSHConfigurerV2_Update(t *testing.T) { //nolint // this is a test + orig := isSSHCertRequired + isSSHCertRequired = func() bool { return false } + t.Cleanup(func() { isSSHCertRequired = orig }) + type fields struct { store SSHConfigurerV2Store runRemoteCMD bool