Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 46 additions & 1 deletion pkg/cmd/util/externalnode.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func ResolveExternalNodeSSH(store ExternalNodeStore, node *nodev1.ExternalNode)

entry := ResolveNodeSSHEntry(user.ID, node)
if entry == nil {
return nil, breverrors.New(fmt.Sprintf("cannot resolve SSH for node %q — no access, no port, or no hostname", node.GetName()))
return nil, classifyNodeSSHFailure(user, node)
}

return &ExternalNodeSSHInfo{
Expand All @@ -166,3 +166,48 @@ func ResolveExternalNodeSSH(store ExternalNodeStore, node *nodev1.ExternalNode)
Port: entry.Port,
}, nil
}

// classifyNodeSSHFailure returns an actionable error explaining why SSH cannot
// be resolved for the user on this node. The cases mirror ResolveNodeSSHEntry:
// the user may have no access grant, an access grant whose port is not yet
// allocated, or a port that has not yet reported its hostname. Each case has a
// distinct remedy, so we surface them separately rather than as one catch-all.
func classifyNodeSSHFailure(user *entity.User, node *nodev1.ExternalNode) error {
nodeName := node.GetName()

var access *nodev1.SSHAccess
for _, a := range node.GetSshAccess() {
if a.GetUserId() == user.ID {
access = a
break
}
}

// Most common case (per the issue): the node is visible in `brev ls nodes`
// because the user is in the org, but no SSH access has been granted.
if access == nil || access.GetLinuxUser() == "" {
who := user.Email
if who == "" {
who = user.ID
}
return breverrors.New(fmt.Sprintf(
"you don't have SSH access to node %q.\n"+
"Ask an org admin to grant you access, e.g.:\n"+
" brev grant-ssh --node %s --user %s",
nodeName, nodeName, who))
}

port := resolvePortForSSHAccess(node, access)
if port == nil {
return breverrors.New(fmt.Sprintf(
"SSH access to node %q is granted but its SSH port isn't allocated yet.\n"+
"The node may still be connecting — try again shortly, or run 'brev refresh'.",
nodeName))
}

// Access and port exist, but the port has no hostname yet.
return breverrors.New(fmt.Sprintf(
"SSH access to node %q is granted but the connection details aren't ready yet.\n"+
"The node may still be connecting — try again shortly, or run 'brev refresh'.",
nodeName))
}
39 changes: 38 additions & 1 deletion pkg/cmd/util/externalnode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package util

import (
"fmt"
"strings"
"testing"

nodev1 "buf.build/gen/go/brevdev/devplane/protocolbuffers/go/devplaneapi/v1"
Expand Down Expand Up @@ -104,14 +105,47 @@ func TestResolveExternalNodeSSH_UsesServerPortNotPortNumber(t *testing.T) {

func TestResolveExternalNodeSSH_NoAccess(t *testing.T) {
store := &mockExternalNodeStore{
user: &entity.User{ID: "user_1"},
user: &entity.User{ID: "user_1", Email: "me@example.com"},
}
node := makeTestNode("box", "other_user", "ubuntu", "10.0.0.5", 22)

_, err := ResolveExternalNodeSSH(store, node)
if err == nil {
t.Fatal("expected error for no SSH access")
}
// The message should tell the user they lack access and how to get it,
// not the old confusing "no access, no port, or no hostname".
if !strings.Contains(err.Error(), "don't have SSH access") {
t.Errorf("expected a 'no access' message, got: %v", err)
}
if !strings.Contains(err.Error(), "brev grant-ssh") {
t.Errorf("expected the message to suggest 'brev grant-ssh', got: %v", err)
}
if !strings.Contains(err.Error(), "me@example.com") {
t.Errorf("expected the message to reference the user's email, got: %v", err)
}
}

func TestResolveExternalNodeSSH_NoPortAllocated(t *testing.T) {
store := &mockExternalNodeStore{
user: &entity.User{ID: "user_1"},
}
// User has an access grant, but no port matches its PortId.
node := &nodev1.ExternalNode{
Name: "box",
SshAccess: []*nodev1.SSHAccess{
{UserId: "user_1", LinuxUser: "ubuntu", PortId: "port_missing"},
},
Ports: []*nodev1.Port{},
}

_, err := ResolveExternalNodeSSH(store, node)
if err == nil {
t.Fatal("expected error when access exists but no port is allocated")
}
if !strings.Contains(err.Error(), "is granted") {
t.Errorf("expected a 'granted but not ready' message, got: %v", err)
}
}

func TestResolveExternalNodeSSH_UsesAccessPortNotProtocol(t *testing.T) {
Expand Down Expand Up @@ -155,6 +189,9 @@ func TestResolveExternalNodeSSH_EmptyHostname(t *testing.T) {
if err == nil {
t.Fatal("expected error for empty hostname")
}
if !strings.Contains(err.Error(), "is granted") {
t.Errorf("expected a 'granted but not ready' message, got: %v", err)
}
}

func TestResolveExternalNodeSSH_GetUserError(t *testing.T) {
Expand Down
Loading