From b1dbbccdaf00250876f8a2d84103c17777abe031 Mon Sep 17 00:00:00 2001 From: Asher Date: Tue, 17 May 2022 16:51:33 -0500 Subject: [PATCH 1/2] Fix not being able to specify agent when connecting to terminal The `workspace.agent` syntax was only used when fetching the agent and not the workspace so it would try to fetch a workspace called `workspace.agent` instead of just `workspace`. --- .../pages/TerminalPage/TerminalPage.test.tsx | 20 +++++++++++++++++-- site/src/pages/TerminalPage/TerminalPage.tsx | 6 +++++- site/src/testHelpers/handlers.ts | 11 +++++++++- .../xServices/terminal/terminalXService.ts | 17 ++++++++-------- 4 files changed, 41 insertions(+), 13 deletions(-) diff --git a/site/src/pages/TerminalPage/TerminalPage.test.tsx b/site/src/pages/TerminalPage/TerminalPage.test.tsx index 2ece384d7e41d..c29560ba5dbce 100644 --- a/site/src/pages/TerminalPage/TerminalPage.test.tsx +++ b/site/src/pages/TerminalPage/TerminalPage.test.tsx @@ -6,7 +6,7 @@ import React from "react" import { Route, Routes } from "react-router-dom" import { TextDecoder, TextEncoder } from "util" import { ReconnectingPTYRequest } from "../../api/types" -import { history, MockWorkspaceAgent, render } from "../../testHelpers/renderHelpers" +import { history, MockWorkspace, MockWorkspaceAgent, render } from "../../testHelpers/renderHelpers" import { server } from "../../testHelpers/server" import TerminalPage, { Language } from "./TerminalPage" @@ -52,7 +52,7 @@ const expectTerminalText = (container: HTMLElement, text: string) => { describe("TerminalPage", () => { beforeEach(() => { - history.push("/some-user/my-workspace/terminal") + history.push(`/some-user/${MockWorkspace.name}/terminal`) }) it("shows an error if fetching organizations fails", async () => { @@ -146,4 +146,20 @@ describe("TerminalPage", () => { expect(req.width).toBeGreaterThan(0) server.close() }) + + it("supports workspace.agent syntax", async () => { + // Given + const server = new WS("ws://localhost/api/v2/workspaceagents/" + MockWorkspaceAgent.id + "/pty") + const text = "something to render" + + // When + history.push(`/some-user/${MockWorkspace.name}.${MockWorkspaceAgent.name}/terminal`) + const { container } = renderTerminal() + + // Then + await server.connected + server.send(text) + await expectTerminalText(container, text) + server.close() + }) }) diff --git a/site/src/pages/TerminalPage/TerminalPage.tsx b/site/src/pages/TerminalPage/TerminalPage.tsx index 0beaf3017d5ec..093cc7781fc07 100644 --- a/site/src/pages/TerminalPage/TerminalPage.tsx +++ b/site/src/pages/TerminalPage/TerminalPage.tsx @@ -34,10 +34,14 @@ const TerminalPage: React.FC<{ const search = new URLSearchParams(location.search) return search.get("reconnect") ?? uuidv4() }) + // The workspace name is in the format: + // [.] + const workspaceNameParts = workspace?.split(".") const [terminalState, sendEvent] = useMachine(terminalMachine, { context: { + agentName: workspaceNameParts?.[1], reconnection: reconnectionToken, - workspaceName: workspace, + workspaceName: workspaceNameParts?.[0], username: username, }, actions: { diff --git a/site/src/testHelpers/handlers.ts b/site/src/testHelpers/handlers.ts index 1f65874616dc1..e43e36057053a 100644 --- a/site/src/testHelpers/handlers.ts +++ b/site/src/testHelpers/handlers.ts @@ -77,7 +77,16 @@ export const handlers = [ // workspaces rest.get("/api/v2/organizations/:organizationId/workspaces/:userName/:workspaceName", (req, res, ctx) => { - return res(ctx.status(200), ctx.json(M.MockWorkspace)) + if (req.params.workspaceName !== M.MockWorkspace.name) { + return res( + ctx.status(404), + ctx.json({ + message: "workspace not found", + }), + ) + } else { + return res(ctx.status(200), ctx.json(M.MockWorkspace)) + } }), rest.get("/api/v2/workspaces/:workspaceId", async (req, res, ctx) => { return res(ctx.status(200), ctx.json(M.MockWorkspace)) diff --git a/site/src/xServices/terminal/terminalXService.ts b/site/src/xServices/terminal/terminalXService.ts index 3a17618f6d486..056a7ddf7cafe 100644 --- a/site/src/xServices/terminal/terminalXService.ts +++ b/site/src/xServices/terminal/terminalXService.ts @@ -14,13 +14,16 @@ export interface TerminalContext { websocketError?: Error | unknown // Assigned by connecting! + // The workspace agent is entirely optional. If the agent is omitted the + // first agent will be used. + agentName?: string username?: string workspaceName?: string reconnection?: string } export type TerminalEvent = - | { type: "CONNECT"; reconnection?: string; workspaceName?: string; username?: string } + | { type: "CONNECT"; agentName?: string; reconnection?: string; workspaceName?: string; username?: string } | { type: "WRITE"; request: Types.ReconnectingPTYRequest } | { type: "READ"; data: ArrayBuffer } | { type: "DISCONNECT" } @@ -153,7 +156,7 @@ export const terminalMachine = getOrganizations: API.getOrganizations, getWorkspace: async (context) => { if (!context.organizations || !context.workspaceName) { - throw new Error("organizations or workspace not set") + throw new Error("organizations or workspace name not set") } return API.getWorkspaceByOwnerAndName(context.organizations[0].id, context.username, context.workspaceName) }, @@ -161,11 +164,6 @@ export const terminalMachine = if (!context.workspace || !context.workspaceName) { throw new Error("workspace or workspace name is not set") } - // The workspace name is in the format: - // [.] - // The workspace agent is entirely optional. - const workspaceNameParts = context.workspaceName.split(".") - const agentName = workspaceNameParts[1] const resources = await API.getWorkspaceResources(context.workspace.latest_build.id) @@ -174,10 +172,10 @@ export const terminalMachine = if (!resource.agents || resource.agents.length < 1) { return } - if (!agentName) { + if (!context.agentName) { return resource.agents[0] } - return resource.agents.find((agent) => agent.name === agentName) + return resource.agents.find((agent) => agent.name === context.agentName) }) .filter((a) => a)[0] if (!agent) { @@ -218,6 +216,7 @@ export const terminalMachine = actions: { assignConnection: assign((context, event) => ({ ...context, + agentName: event.agentName ?? context.agentName, reconnection: event.reconnection ?? context.reconnection, workspaceName: event.workspaceName ?? context.workspaceName, })), From 200a60896626776b2630fd0421e8607cfae7cb48 Mon Sep 17 00:00:00 2001 From: Asher Date: Tue, 17 May 2022 17:16:45 -0500 Subject: [PATCH 2/2] Add terminal link component Currently it does not show anywhere but we can drop it into the resources card later. --- .../TerminalLink/TerminalLink.stories.tsx | 16 +++++++++++ .../components/TerminalLink/TerminalLink.tsx | 28 +++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 site/src/components/TerminalLink/TerminalLink.stories.tsx create mode 100644 site/src/components/TerminalLink/TerminalLink.tsx diff --git a/site/src/components/TerminalLink/TerminalLink.stories.tsx b/site/src/components/TerminalLink/TerminalLink.stories.tsx new file mode 100644 index 0000000000000..417821f53d30b --- /dev/null +++ b/site/src/components/TerminalLink/TerminalLink.stories.tsx @@ -0,0 +1,16 @@ +import { Story } from "@storybook/react" +import React from "react" +import { MockWorkspace } from "../../testHelpers/renderHelpers" +import { TerminalLink, TerminalLinkProps } from "./TerminalLink" + +export default { + title: "components/TerminalLink", + component: TerminalLink, +} + +const Template: Story = (args) => + +export const Example = Template.bind({}) +Example.args = { + workspaceName: MockWorkspace.name, +} diff --git a/site/src/components/TerminalLink/TerminalLink.tsx b/site/src/components/TerminalLink/TerminalLink.tsx new file mode 100644 index 0000000000000..f8c93212103c7 --- /dev/null +++ b/site/src/components/TerminalLink/TerminalLink.tsx @@ -0,0 +1,28 @@ +import Link from "@material-ui/core/Link" +import React from "react" +import * as TypesGen from "../../api/typesGenerated" + +export const Language = { + linkText: "Open in terminal", +} + +export interface TerminalLinkProps { + agentName?: TypesGen.WorkspaceAgent["name"] + userName?: TypesGen.User["username"] + workspaceName: TypesGen.Workspace["name"] +} + +/** + * Generate a link to a terminal connected to the provided workspace agent. If + * no agent is provided connect to the first agent. + * + * If no user name is provided "me" is used however it makes the link not + * shareable. + */ +export const TerminalLink: React.FC = ({ agentName, userName = "me", workspaceName }) => { + return ( + + {Language.linkText} + + ) +}