Skip to content

Commit cb5ae98

Browse files
committed
Add the webpage for accessing a web terminal
1 parent 15d843e commit cb5ae98

File tree

17 files changed

+431
-76
lines changed

17 files changed

+431
-76
lines changed

.vscode/settings.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
{
22
"cSpell.words": [
3+
"circbuf",
34
"cliflag",
45
"cliui",
56
"coderd",
@@ -61,8 +62,10 @@
6162
"unconvert",
6263
"Untar",
6364
"VMID",
65+
"weblinks",
6466
"webrtc",
6567
"xerrors",
68+
"xstate",
6669
"yamux"
6770
],
6871
"emeraldwalk.runonsave": {

agent/agent.go

Lines changed: 70 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ import (
1818
"sync"
1919
"time"
2020

21+
"github.com/armon/circbuf"
2122
"github.com/google/uuid"
22-
"github.com/smallnest/ringbuffer"
2323

2424
gsyslog "github.com/hashicorp/go-syslog"
2525
"go.uber.org/atomic"
@@ -417,6 +417,8 @@ func (a *agent) handleSSHSession(session ssh.Session) error {
417417
func (a *agent) handleReconnectingPTY(ctx context.Context, rawID string, conn net.Conn) {
418418
defer conn.Close()
419419

420+
// The ID format is referenced in conn.go.
421+
// <uuid>:<height>:<width>
420422
idParts := strings.Split(rawID, ":")
421423
if len(idParts) != 3 {
422424
a.logger.Warn(ctx, "client sent invalid id format", slog.F("raw-id", rawID))
@@ -429,6 +431,7 @@ func (a *agent) handleReconnectingPTY(ctx context.Context, rawID string, conn ne
429431
a.logger.Warn(ctx, "client sent reconnection token that isn't a uuid", slog.F("id", id), slog.Error(err))
430432
return
431433
}
434+
// Parse the initial terminal dimensions.
432435
height, err := strconv.Atoi(idParts[1])
433436
if err != nil {
434437
a.logger.Warn(ctx, "client sent invalid height", slog.F("id", id), slog.F("height", idParts[1]))
@@ -454,41 +457,55 @@ func (a *agent) handleReconnectingPTY(ctx context.Context, rawID string, conn ne
454457
a.logger.Warn(ctx, "create reconnecting pty command", slog.Error(err))
455458
return
456459
}
457-
ptty, _, err := pty.Start(cmd)
460+
cmd.Env = append(cmd.Env, "TERM=xterm-256color")
461+
462+
ptty, process, err := pty.Start(cmd)
458463
if err != nil {
459464
a.logger.Warn(ctx, "start reconnecting pty command", slog.F("id", id))
460465
}
461466

467+
// Default to buffer 64KB.
468+
circularBuffer, err := circbuf.NewBuffer(64 * 1024)
469+
if err != nil {
470+
a.logger.Warn(ctx, "create circular buffer", slog.Error(err))
471+
return
472+
}
473+
462474
a.closeMutex.Lock()
463475
a.connCloseWait.Add(1)
464476
a.closeMutex.Unlock()
477+
ctx, cancelFunc := context.WithCancel(ctx)
465478
rpty = &reconnectingPTY{
466479
activeConns: make(map[string]net.Conn),
467480
ptty: ptty,
468-
timeout: time.NewTimer(a.reconnectingPTYTimeout),
469-
// Default to buffer 1MB.
470-
ringBuffer: ringbuffer.New(1 << 20),
481+
// Timeouts created with an after func can be reset!
482+
timeout: time.AfterFunc(a.reconnectingPTYTimeout, cancelFunc),
483+
circularBuffer: circularBuffer,
471484
}
472485
a.reconnectingPTYs.Store(id, rpty)
473486
go func() {
474-
// Close if the inactive timeout occurs, or the context ends.
475-
select {
476-
case <-rpty.timeout.C:
477-
a.logger.Info(ctx, "killing reconnecting pty due to inactivity", slog.F("id", id))
478-
case <-ctx.Done():
479-
}
487+
// When the context has been completed either:
488+
// 1. The timeout completed.
489+
// 2. The parent context was cancelled.
490+
<-ctx.Done()
491+
_ = process.Kill()
492+
}()
493+
go func() {
494+
// If the process dies randomly, we should
495+
// close the pty.
496+
_, _ = process.Wait()
480497
rpty.Close()
481498
}()
482499
go func() {
483-
buffer := make([]byte, 32*1024)
500+
buffer := make([]byte, 1024)
484501
for {
485502
read, err := rpty.ptty.Output().Read(buffer)
486503
if err != nil {
487-
rpty.Close()
504+
// When the PTY is closed, this is triggered.
488505
break
489506
}
490507
part := buffer[:read]
491-
_, err = rpty.ringBuffer.Write(part)
508+
_, err = rpty.circularBuffer.Write(part)
492509
if err != nil {
493510
a.logger.Error(ctx, "reconnecting pty write buffer", slog.Error(err), slog.F("id", id))
494511
break
@@ -499,27 +516,56 @@ func (a *agent) handleReconnectingPTY(ctx context.Context, rawID string, conn ne
499516
}
500517
rpty.activeConnsMutex.Unlock()
501518
}
502-
// If we break from the loop, the reconnecting PTY ended.
519+
520+
// Cleanup the process, PTY, and delete it's
521+
// ID from memory.
522+
_ = process.Kill()
523+
rpty.Close()
503524
a.reconnectingPTYs.Delete(id)
504525
a.connCloseWait.Done()
505526
}()
506527
}
528+
// Resize the PTY to initial height + width.
507529
err = rpty.ptty.Resize(uint16(height), uint16(width))
508530
if err != nil {
509531
// We can continue after this, it's not fatal!
510532
a.logger.Error(ctx, "resize reconnecting pty", slog.F("id", id), slog.Error(err))
511533
}
512-
513-
_, err = conn.Write(rpty.ringBuffer.Bytes())
534+
// Write any previously stored data for the TTY.
535+
_, err = conn.Write(rpty.circularBuffer.Bytes())
514536
if err != nil {
515537
a.logger.Warn(ctx, "write reconnecting pty buffer", slog.F("id", id), slog.Error(err))
516538
return
517539
}
518540
connectionID := uuid.NewString()
541+
// Multiple connections to the same TTY are permitted.
542+
// This could easily be used for terminal sharing, but
543+
// we do it because it's a nice user experience to
544+
// copy/paste a terminal URL and have it _just work_.
519545
rpty.activeConnsMutex.Lock()
520546
rpty.activeConns[connectionID] = conn
521547
rpty.activeConnsMutex.Unlock()
548+
// Resetting this timeout prevents the PTY from exiting.
549+
rpty.timeout.Reset(a.reconnectingPTYTimeout)
550+
551+
heartbeat := time.NewTimer(a.reconnectingPTYTimeout / 2)
552+
defer heartbeat.Stop()
553+
go func() {
554+
// Keep updating the activity while this
555+
// connection is alive!
556+
for {
557+
select {
558+
case <-ctx.Done():
559+
return
560+
case <-heartbeat.C:
561+
}
562+
rpty.timeout.Reset(a.reconnectingPTYTimeout)
563+
}
564+
}()
522565
defer func() {
566+
// After this connection ends, remove it from
567+
// the PTYs active connections. If it isn't
568+
// removed, all PTY data will be sent to it.
523569
rpty.activeConnsMutex.Lock()
524570
delete(rpty.activeConns, connectionID)
525571
rpty.activeConnsMutex.Unlock()
@@ -579,17 +625,20 @@ type reconnectingPTY struct {
579625
activeConnsMutex sync.Mutex
580626
activeConns map[string]net.Conn
581627

582-
ringBuffer *ringbuffer.RingBuffer
583-
timeout *time.Timer
584-
ptty pty.PTY
628+
circularBuffer *circbuf.Buffer
629+
timeout *time.Timer
630+
ptty pty.PTY
585631
}
586632

633+
// Close ends all connections to the reconnecting
634+
// PTY and clear the circular buffer.
587635
func (r *reconnectingPTY) Close() {
588636
r.activeConnsMutex.Lock()
589637
defer r.activeConnsMutex.Unlock()
590638
for _, conn := range r.activeConns {
591639
_ = conn.Close()
592640
}
593641
_ = r.ptty.Close()
594-
r.ringBuffer.Reset()
642+
r.circularBuffer.Reset()
643+
r.timeout.Stop()
595644
}

agent/agent_test.go

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func TestAgent(t *testing.T) {
133133

134134
t.Run("SFTP", func(t *testing.T) {
135135
t.Parallel()
136-
sshClient, err := setupAgent(t, agent.Metadata{}).SSHClient()
136+
sshClient, err := setupAgent(t, agent.Metadata{}, 0).SSHClient()
137137
require.NoError(t, err)
138138
client, err := sftp.NewClient(sshClient)
139139
require.NoError(t, err)
@@ -170,7 +170,7 @@ func TestAgent(t *testing.T) {
170170
content := "somethingnice"
171171
setupAgent(t, agent.Metadata{
172172
StartupScript: "echo " + content + " > " + tempPath,
173-
})
173+
}, 0)
174174
var gotContent string
175175
require.Eventually(t, func() bool {
176176
content, err := os.ReadFile(tempPath)
@@ -193,7 +193,13 @@ func TestAgent(t *testing.T) {
193193

194194
t.Run("ReconnectingPTY", func(t *testing.T) {
195195
t.Parallel()
196-
conn := setupAgent(t, agent.Metadata{})
196+
if runtime.GOOS == "windows" {
197+
// This might be our implementation, or ConPTY itself.
198+
// It's difficult to find extensive tests for it, so
199+
// it seems like it could be either.
200+
t.Skip("ConPTY appears to be inconsistent on Windows.")
201+
}
202+
conn := setupAgent(t, agent.Metadata{}, 0)
197203
id := uuid.NewString()
198204
netConn, err := conn.ReconnectingPTY(id, 100, 100)
199205
require.NoError(t, err)
@@ -231,7 +237,7 @@ func TestAgent(t *testing.T) {
231237
}
232238

233239
func setupSSHCommand(t *testing.T, beforeArgs []string, afterArgs []string) *exec.Cmd {
234-
agentConn := setupAgent(t, agent.Metadata{})
240+
agentConn := setupAgent(t, agent.Metadata{}, 0)
235241
listener, err := net.Listen("tcp", "127.0.0.1:0")
236242
require.NoError(t, err)
237243
go func() {
@@ -260,20 +266,21 @@ func setupSSHCommand(t *testing.T, beforeArgs []string, afterArgs []string) *exe
260266
}
261267

262268
func setupSSHSession(t *testing.T, options agent.Metadata) *ssh.Session {
263-
sshClient, err := setupAgent(t, options).SSHClient()
269+
sshClient, err := setupAgent(t, options, 0).SSHClient()
264270
require.NoError(t, err)
265271
session, err := sshClient.NewSession()
266272
require.NoError(t, err)
267273
return session
268274
}
269275

270-
func setupAgent(t *testing.T, metadata agent.Metadata) *agent.Conn {
276+
func setupAgent(t *testing.T, metadata agent.Metadata, ptyTimeout time.Duration) *agent.Conn {
271277
client, server := provisionersdk.TransportPipe()
272278
closer := agent.New(func(ctx context.Context, logger slog.Logger) (agent.Metadata, *peerbroker.Listener, error) {
273279
listener, err := peerbroker.Listen(server, nil)
274280
return metadata, listener, err
275281
}, &agent.Options{
276-
Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug),
282+
Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug),
283+
ReconnectingPTYTimeout: ptyTimeout,
277284
})
278285
t.Cleanup(func() {
279286
_ = client.Close()

coderd/workspaceagents.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ func (api *api) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) {
375375
_ = conn.Close(websocket.StatusNormalClosure, "ended")
376376
}()
377377
// Accept text connections, because it's more developer friendly.
378-
wsNetConn := websocket.NetConn(r.Context(), conn, websocket.MessageText)
378+
wsNetConn := websocket.NetConn(r.Context(), conn, websocket.MessageBinary)
379379
agentConn, err := api.dialWorkspaceAgent(r, workspaceAgent.ID)
380380
if err != nil {
381381
return

codersdk/workspaceagents.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ func (c *Client) WorkspaceAgentReconnectingPTY(ctx context.Context, agentID, rec
368368
}
369369
return nil, readBodyAsError(res)
370370
}
371-
return websocket.NetConn(ctx, conn, websocket.MessageText), nil
371+
return websocket.NetConn(ctx, conn, websocket.MessageBinary), nil
372372
}
373373

374374
func (c *Client) turnProxyDialer(ctx context.Context, httpClient *http.Client, path string) proxy.Dialer {

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ require (
4343
cdr.dev/slog v1.4.1
4444
cloud.google.com/go/compute v1.6.1
4545
github.com/AlecAivazis/survey/v2 v2.3.4
46+
github.com/armon/circbuf v0.0.0-20190214190532-5111143e8da2
4647
github.com/awalterschulze/gographviz v2.0.3+incompatible
4748
github.com/bgentry/speakeasy v0.1.0
4849
github.com/briandowns/spinner v1.18.1
@@ -92,7 +93,6 @@ require (
9293
github.com/pkg/sftp v1.13.4
9394
github.com/quasilyte/go-ruleguard/dsl v0.3.19
9495
github.com/robfig/cron/v3 v3.0.1
95-
github.com/smallnest/ringbuffer v0.0.0-20210227121335-0a58434b36f2
9696
github.com/spf13/cobra v1.4.0
9797
github.com/spf13/pflag v1.0.5
9898
github.com/stretchr/testify v1.7.1

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,8 @@ github.com/apparentlymart/go-textseg v1.0.0/go.mod h1:z96Txxhf3xSFMPmb5X/1W05FF/
182182
github.com/apparentlymart/go-textseg/v13 v13.0.0 h1:Y+KvPE1NYz0xl601PVImeQfFyEy6iT90AvPUL1NNfNw=
183183
github.com/apparentlymart/go-textseg/v13 v13.0.0/go.mod h1:ZK2fH7c4NqDTLtiYLvIkEghdlcqw7yxLeM89kiTRPUo=
184184
github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e/go.mod h1:3U/XgcO3hCbHZ8TKRvWD2dDTCfh9M9ya+I9JpbB7O8o=
185+
github.com/armon/circbuf v0.0.0-20190214190532-5111143e8da2 h1:7Ip0wMmLHLRJdrloDxZfhMm0xrLXZS8+COSu2bXmEQs=
186+
github.com/armon/circbuf v0.0.0-20190214190532-5111143e8da2/go.mod h1:3U/XgcO3hCbHZ8TKRvWD2dDTCfh9M9ya+I9JpbB7O8o=
185187
github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
186188
github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmVTwzkszR9V5SSuryQ31EELlFMUz1kKyl939pY=
187189
github.com/armon/go-metrics v0.3.0/go.mod h1:zXjbSimjXTd7vOpY8B0/2LpvNvDoXBuplAD+gJD3GYs=
@@ -1493,8 +1495,6 @@ github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrf
14931495
github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
14941496
github.com/sirupsen/logrus v1.8.1 h1:dJKuHgqk1NNQlqoA6BTlM1Wf9DOH3NBjQyu0h9+AZZE=
14951497
github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
1496-
github.com/smallnest/ringbuffer v0.0.0-20210227121335-0a58434b36f2 h1:co1YnJJ6rDvcodJzcXObchJMfHclIROMulsWObuNfTY=
1497-
github.com/smallnest/ringbuffer v0.0.0-20210227121335-0a58434b36f2/go.mod h1:mXcZNMJHswhQDDJZIjdtJoG97JIwIa/HdcHNM3w15T0=
14981498
github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d h1:zE9ykElWQ6/NYmHa3jpm/yHnI4xSofP+UP6SpjHcSeM=
14991499
github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc=
15001500
github.com/smartystreets/go-aws-auth v0.0.0-20180515143844-0c1422d1fdb9/go.mod h1:SnhjPscd9TpLiy1LpzGSKh3bXCfxxXuqd9xmQJy3slM=

pty/pty_other.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ func (p *otherPty) Resize(height uint16, width uint16) error {
4646
p.mutex.Lock()
4747
defer p.mutex.Unlock()
4848
return pty.Setsize(p.pty, &pty.Winsize{
49-
Rows: width,
50-
Cols: height,
49+
Rows: height,
50+
Cols: width,
5151
})
5252
}
5353

site/package.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@
4242
"react-router-dom": "6.3.0",
4343
"swr": "1.2.2",
4444
"xstate": "4.31.0",
45+
"xterm-addon-fit": "^0.5.0",
46+
"xterm-addon-web-links": "^0.5.1",
47+
"xterm-addon-webgl": "^0.11.4",
48+
"xterm-for-react": "^1.0.4",
4549
"yup": "0.32.11"
4650
},
4751
"devDependencies": {

site/src/AppRouter.tsx

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { SettingsPage } from "./pages/SettingsPage/SettingsPage"
1717
import { CreateWorkspacePage } from "./pages/TemplatesPages/OrganizationPage/TemplatePage/CreateWorkspacePage"
1818
import { TemplatePage } from "./pages/TemplatesPages/OrganizationPage/TemplatePage/TemplatePage"
1919
import { TemplatesPage } from "./pages/TemplatesPages/TemplatesPage"
20+
import { TerminalPage } from "./pages/TerminalPage/TerminalPage"
2021
import { UsersPage } from "./pages/UsersPage/UsersPage"
2122
import { WorkspacePage } from "./pages/WorkspacesPage/WorkspacesPage"
2223

@@ -115,6 +116,19 @@ export const AppRouter: React.FC = () => (
115116
<Route path="linked-accounts" element={<LinkedAccountsPage />} />
116117
</Route>
117118

119+
<Route path=":username">
120+
<Route path=":workspace">
121+
<Route
122+
path="terminal"
123+
element={
124+
<RequireAuth>
125+
<TerminalPage />
126+
</RequireAuth>
127+
}
128+
/>
129+
</Route>
130+
</Route>
131+
118132
{/* Using path="*"" means "match anything", so this route
119133
acts like a catch-all for URLs that we don't have explicit
120134
routes for. */}

site/src/api/index.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,9 @@ export const getOrganizations = async (): Promise<Types.Organization[]> => {
9191
}
9292

9393
export const getWorkspace = async (organizationID: string, workspaceName: string): Promise<Types.Workspace> => {
94-
const response = await axios.get<Types.Workspace>(`/api/v2/organizations/${organizationID}/workspaces/me/${workspaceName}`)
94+
const response = await axios.get<Types.Workspace>(
95+
`/api/v2/organizations/${organizationID}/workspaces/me/${workspaceName}`,
96+
)
9597
return response.data
9698
}
9799

site/src/api/types.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ export interface Workspace {
7373

7474
export interface WorkspaceResource {
7575
id: string
76-
agents: WorkspaceAgent[]
76+
agents?: WorkspaceAgent[]
7777
}
7878

7979
export interface WorkspaceAgent {
@@ -116,7 +116,7 @@ export interface UpdateProfileRequest {
116116
}
117117

118118
export interface ReconnectingPTYRequest {
119-
readonly data: string
120-
readonly height: number
121-
readonly width: number
119+
readonly data?: string
120+
readonly height?: number
121+
readonly width?: number
122122
}

0 commit comments

Comments
 (0)