diff --git a/pkg/cmd/util/externalnode.go b/pkg/cmd/util/externalnode.go index ef78d8e4..c899c68f 100644 --- a/pkg/cmd/util/externalnode.go +++ b/pkg/cmd/util/externalnode.go @@ -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{ @@ -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)) +} diff --git a/pkg/cmd/util/externalnode_test.go b/pkg/cmd/util/externalnode_test.go index 9e5244ae..765942ff 100644 --- a/pkg/cmd/util/externalnode_test.go +++ b/pkg/cmd/util/externalnode_test.go @@ -2,6 +2,7 @@ package util import ( "fmt" + "strings" "testing" nodev1 "buf.build/gen/go/brevdev/devplane/protocolbuffers/go/devplaneapi/v1" @@ -104,7 +105,7 @@ 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) @@ -112,6 +113,39 @@ func TestResolveExternalNodeSSH_NoAccess(t *testing.T) { 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) { @@ -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) {