Skip to content

Commit 7c804c9

Browse files
code-ashercode-asher
committed
Remove backend type from SDK/API
Still need to figure out a way to test both, but we would rather not add the ability to select the backend through the API.
1 parent 0150f51 commit 7c804c9

File tree

16 files changed

+115
-194
lines changed

16 files changed

+115
-194
lines changed

‎agent/agent.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1108,7 +1108,7 @@ func (a *agent) handleReconnectingPTY(ctx context.Context, logger slog.Logger, m
11081108
}
11091109
c <- rpty // Put it back for the next reconnect.
11101110
} else {
1111-
logger.Debug(ctx, "creating new reconnecting pty", slog.F("backend", msg.BackendType))
1111+
logger.Debug(ctx, "creating new reconnecting pty")
11121112

11131113
connected := false
11141114
defer func() {
@@ -1126,9 +1126,8 @@ func (a *agent) handleReconnectingPTY(ctx context.Context, logger slog.Logger, m
11261126
}
11271127

11281128
rpty = reconnectingpty.New(ctx, cmd, &reconnectingpty.Options{
1129-
BackendType: msg.BackendType,
1130-
Timeout: a.reconnectingPTYTimeout,
1131-
Metrics: a.metrics.reconnectingPTYErrors,
1129+
Timeout: a.reconnectingPTYTimeout,
1130+
Metrics: a.metrics.reconnectingPTYErrors,
11321131
}, logger.With(slog.F("message_id", msg.ID)))
11331132

11341133
if err = a.trackConnGoroutine(func() {

‎agent/agent_test.go

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func TestAgent_Stats_ReconnectingPTY(t *testing.T) {
103103
//nolint:dogsled
104104
conn, _, stats, _, _ := setupAgent(t, agentsdk.Manifest{}, 0)
105105

106-
ptyConn, err := conn.ReconnectingPTY(ctx, uuid.New(), 128, 128, "bash", codersdk.ReconnectingPTYBackendTypeBuffered)
106+
ptyConn, err := conn.ReconnectingPTY(ctx, uuid.New(), 128, 128, "bash")
107107
require.NoError(t, err)
108108
defer ptyConn.Close()
109109

@@ -1601,18 +1601,15 @@ func TestAgent_ReconnectingPTY(t *testing.T) {
16011601
t.Skip("ConPTY appears to be inconsistent on Windows.")
16021602
}
16031603

1604-
backends := []codersdk.ReconnectingPTYBackendType{
1605-
codersdk.ReconnectingPTYBackendTypeBuffered,
1606-
codersdk.ReconnectingPTYBackendTypeScreen,
1607-
}
1604+
backends := []string{"Buffered", "Screen"}
16081605

16091606
for _, backendType := range backends {
16101607
backendType := backendType
1611-
t.Run(string(backendType), func(t *testing.T) {
1608+
t.Run(backendType, func(t *testing.T) {
16121609
t.Parallel()
16131610
if runtime.GOOS == "darwin" {
16141611
t.Skip("`screen` is flaky on darwin")
1615-
} else if backendType == codersdk.ReconnectingPTYBackendTypeScreen {
1612+
} else if backendType == "Screen" {
16161613
_, err := exec.LookPath("screen")
16171614
if err != nil {
16181615
t.Skip("`screen` not found")
@@ -1625,14 +1622,14 @@ func TestAgent_ReconnectingPTY(t *testing.T) {
16251622
//nolint:dogsled
16261623
conn, _, _, _, _ := setupAgent(t, agentsdk.Manifest{}, 0)
16271624
id := uuid.New()
1628-
netConn1, err := conn.ReconnectingPTY(ctx, id, 100, 100, "bash", backendType)
1625+
netConn1, err := conn.ReconnectingPTY(ctx, id, 100, 100, "bash")
16291626
require.NoError(t, err)
16301627
defer netConn1.Close()
16311628

16321629
scanner1 := bufio.NewScanner(netConn1)
16331630

16341631
// A second simultaneous connection.
1635-
netConn2, err := conn.ReconnectingPTY(ctx, id, 100, 100, "bash", backendType)
1632+
netConn2, err := conn.ReconnectingPTY(ctx, id, 100, 100, "bash")
16361633
require.NoError(t, err)
16371634
defer netConn2.Close()
16381635
scanner2 := bufio.NewScanner(netConn2)
@@ -1683,7 +1680,7 @@ func TestAgent_ReconnectingPTY(t *testing.T) {
16831680

16841681
_ = netConn1.Close()
16851682
_ = netConn2.Close()
1686-
netConn3, err := conn.ReconnectingPTY(ctx, id, 100, 100, "bash", backendType)
1683+
netConn3, err := conn.ReconnectingPTY(ctx, id, 100, 100, "bash")
16871684
require.NoError(t, err)
16881685
defer netConn3.Close()
16891686

‎agent/reconnectingpty/reconnectingpty.go

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@ type Options struct {
3232
Timeout time.Duration
3333
// Metrics tracks various error counters.
3434
Metrics *prometheus.CounterVec
35-
// BackendType indicates which backend to use for reconnections.
36-
BackendType codersdk.ReconnectingPTYBackendType
3735
}
3836

3937
// ReconnectingPTY is a pty that can be reconnected within a timeout and to
@@ -64,23 +62,20 @@ func New(ctx context.Context, cmd *pty.Cmd, options *Options, logger slog.Logger
6462
}
6563
// Screen seems flaky on Darwin. Locally the tests pass 100% of the time (100
6664
// runs) but in CI screen often incorrectly claims the session name does not
67-
// exist even though screen -list shows it.
68-
if runtime.GOOS == "darwin" {
69-
options.BackendType = codersdk.ReconnectingPTYBackendTypeBuffered
70-
} else if options.BackendType == "" || options.BackendType == codersdk.ReconnectingPTYBackendTypeAuto {
65+
// exist even though screen -list shows it. For now, restrict screen to
66+
// Linux.
67+
backendType := "buffered"
68+
if runtime.GOOS == "linux" {
7169
_, err := exec.LookPath("screen")
7270
if err == nil {
73-
options.BackendType = codersdk.ReconnectingPTYBackendTypeScreen
74-
} else {
75-
options.BackendType = codersdk.ReconnectingPTYBackendTypeBuffered
71+
backendType = "screen"
7672
}
77-
logger.Debug(ctx, "auto backend selection", slog.F("backend", options.BackendType))
7873
}
7974

80-
logger.Info(ctx, "start reconnecting pty")
75+
logger.Info(ctx, "start reconnecting pty", slog.F("backend_type", backendType))
8176

82-
switch options.BackendType {
83-
case codersdk.ReconnectingPTYBackendTypeScreen:
77+
switch backendType {
78+
case "screen":
8479
return newScreen(ctx, cmd, options, logger)
8580
default:
8681
return newBuffered(ctx, cmd, options, logger)

‎cli/exp_scaletest.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -676,11 +676,10 @@ func (r *RootCmd) scaletestCreateWorkspaces() *clibase.Cmd {
676676
config.ReconnectingPTY = &reconnectingpty.Config{
677677
// AgentID is set by the test automatically.
678678
Init: codersdk.WorkspaceAgentReconnectingPTYInit{
679-
ID: uuid.Nil,
680-
Height: 24,
681-
Width: 80,
682-
Command: runCommand,
683-
BackendType: codersdk.ReconnectingPTYBackendTypeBuffered,
679+
ID: uuid.Nil,
680+
Height: 24,
681+
Width: 80,
682+
Command: runCommand,
684683
},
685684
Timeout: httpapi.Duration(runTimeout),
686685
ExpectTimeout: runExpectTimeout,

‎coderd/httpapi/queryparams.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ func (p *QueryParamParser) Strings(vals url.Values, def []string, queryParam str
161161
// ValidEnum parses enum query params. Add more to the list as needed.
162162
type ValidEnum interface {
163163
database.ResourceType | database.AuditAction | database.BuildReason | database.UserStatus |
164-
database.WorkspaceStatus | codersdk.ReconnectingPTYBackendType
164+
database.WorkspaceStatus
165165

166166
// Valid is required on the enum type to be used with ParseEnum.
167167
Valid() bool

‎coderd/insights_test.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -348,11 +348,10 @@ func TestTemplateInsights(t *testing.T) {
348348

349349
// Start an rpty session to generate rpty usage stats.
350350
rpty, err := client.WorkspaceAgentReconnectingPTY(ctx, codersdk.WorkspaceAgentReconnectingPTYOpts{
351-
AgentID: resources[0].Agents[0].ID,
352-
Reconnect: uuid.New(),
353-
Width: 80,
354-
Height: 24,
355-
BackendType: codersdk.ReconnectingPTYBackendTypeBuffered,
351+
AgentID: resources[0].Agents[0].ID,
352+
Reconnect: uuid.New(),
353+
Width: 80,
354+
Height: 24,
356355
})
357356
require.NoError(t, err)
358357
defer rpty.Close()

‎coderd/workspaceapps/apptest/apptest.go

Lines changed: 45 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"net/http/cookiejar"
1212
"net/http/httputil"
1313
"net/url"
14-
"os/exec"
1514
"path"
1615
"regexp"
1716
"runtime"
@@ -57,81 +56,59 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
5756
t.Skip("ConPTY appears to be inconsistent on Windows.")
5857
}
5958

60-
backends := []codersdk.ReconnectingPTYBackendType{
61-
codersdk.ReconnectingPTYBackendTypeBuffered,
62-
codersdk.ReconnectingPTYBackendTypeScreen,
63-
}
64-
65-
for _, backendType := range backends {
66-
backendType := backendType
67-
t.Run(string(backendType), func(t *testing.T) {
68-
t.Parallel()
69-
if runtime.GOOS == "darwin" {
70-
t.Skip("`screen` is flaky on darwin")
71-
} else if backendType == codersdk.ReconnectingPTYBackendTypeScreen {
72-
_, err := exec.LookPath("screen")
73-
if err != nil {
74-
t.Skip("`screen` not found")
75-
}
76-
}
77-
78-
t.Run("OK", func(t *testing.T) {
79-
t.Parallel()
80-
appDetails := setupProxyTest(t, nil)
59+
t.Run("OK", func(t *testing.T) {
60+
t.Parallel()
61+
appDetails := setupProxyTest(t, nil)
8162

82-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
83-
defer cancel()
63+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
64+
defer cancel()
8465

85-
// Run the test against the path app hostname since that's where the
86-
// reconnecting-pty proxy server we want to test is mounted.
87-
client := appDetails.AppClient(t)
88-
testReconnectingPTY(ctx, t, client, codersdk.WorkspaceAgentReconnectingPTYOpts{
89-
AgentID: appDetails.Agent.ID,
90-
Reconnect: uuid.New(),
91-
Height: 80,
92-
Width: 80,
93-
Command: "bash",
94-
BackendType: backendType,
95-
})
96-
})
66+
// Run the test against the path app hostname since that's where the
67+
// reconnecting-pty proxy server we want to test is mounted.
68+
client := appDetails.AppClient(t)
69+
testReconnectingPTY(ctx, t, client, codersdk.WorkspaceAgentReconnectingPTYOpts{
70+
AgentID: appDetails.Agent.ID,
71+
Reconnect: uuid.New(),
72+
Height: 80,
73+
Width: 80,
74+
Command: "bash",
75+
})
76+
})
9777

98-
t.Run("SignedTokenQueryParameter", func(t *testing.T) {
99-
t.Parallel()
100-
if appHostIsPrimary {
101-
t.Skip("Tickets are not used for terminal requests on the primary.")
102-
}
78+
t.Run("SignedTokenQueryParameter", func(t *testing.T) {
79+
t.Parallel()
80+
if appHostIsPrimary {
81+
t.Skip("Tickets are not used for terminal requests on the primary.")
82+
}
10383

104-
appDetails := setupProxyTest(t, nil)
84+
appDetails := setupProxyTest(t, nil)
10585

106-
u := *appDetails.PathAppBaseURL
107-
if u.Scheme == "http" {
108-
u.Scheme = "ws"
109-
} else {
110-
u.Scheme = "wss"
111-
}
112-
u.Path = fmt.Sprintf("/api/v2/workspaceagents/%s/pty", appDetails.Agent.ID.String())
86+
u := *appDetails.PathAppBaseURL
87+
if u.Scheme == "http" {
88+
u.Scheme = "ws"
89+
} else {
90+
u.Scheme = "wss"
91+
}
92+
u.Path = fmt.Sprintf("/api/v2/workspaceagents/%s/pty", appDetails.Agent.ID.String())
11393

114-
ctx := testutil.Context(t, testutil.WaitLong)
115-
issueRes, err := appDetails.SDKClient.IssueReconnectingPTYSignedToken(ctx, codersdk.IssueReconnectingPTYSignedTokenRequest{
116-
URL: u.String(),
117-
AgentID: appDetails.Agent.ID,
118-
})
119-
require.NoError(t, err)
94+
ctx := testutil.Context(t, testutil.WaitLong)
95+
issueRes, err := appDetails.SDKClient.IssueReconnectingPTYSignedToken(ctx, codersdk.IssueReconnectingPTYSignedTokenRequest{
96+
URL: u.String(),
97+
AgentID: appDetails.Agent.ID,
98+
})
99+
require.NoError(t, err)
120100

121-
// Make an unauthenticated client.
122-
unauthedAppClient := codersdk.New(appDetails.AppClient(t).URL)
123-
testReconnectingPTY(ctx, t, unauthedAppClient, codersdk.WorkspaceAgentReconnectingPTYOpts{
124-
AgentID: appDetails.Agent.ID,
125-
Reconnect: uuid.New(),
126-
Height: 80,
127-
Width: 80,
128-
Command: "bash",
129-
SignedToken: issueRes.SignedToken,
130-
BackendType: backendType,
131-
})
132-
})
101+
// Make an unauthenticated client.
102+
unauthedAppClient := codersdk.New(appDetails.AppClient(t).URL)
103+
testReconnectingPTY(ctx, t, unauthedAppClient, codersdk.WorkspaceAgentReconnectingPTYOpts{
104+
AgentID: appDetails.Agent.ID,
105+
Reconnect: uuid.New(),
106+
Height: 80,
107+
Width: 80,
108+
Command: "bash",
109+
SignedToken: issueRes.SignedToken,
133110
})
134-
}
111+
})
135112
})
136113

137114
t.Run("WorkspaceAppsProxyPath", func(t *testing.T) {

‎coderd/workspaceapps/proxy.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -632,7 +632,6 @@ func (s *Server) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) {
632632
reconnect := parser.Required("reconnect").UUID(values, uuid.New(), "reconnect")
633633
height := parser.UInt(values, 80, "height")
634634
width := parser.UInt(values, 80, "width")
635-
backendType := httpapi.ParseCustom(parser, values, codersdk.ReconnectingPTYBackendTypeAuto, "backend", httpapi.ParseEnum[codersdk.ReconnectingPTYBackendType])
636635
if len(parser.Errors) > 0 {
637636
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
638637
Message: "Invalid query parameters.",
@@ -671,7 +670,7 @@ func (s *Server) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) {
671670
}
672671
defer release()
673672
log.Debug(ctx, "dialed workspace agent")
674-
ptNetConn, err := agentConn.ReconnectingPTY(ctx, reconnect, uint16(height), uint16(width), r.URL.Query().Get("command"), backendType)
673+
ptNetConn, err := agentConn.ReconnectingPTY(ctx, reconnect, uint16(height), uint16(width), r.URL.Query().Get("command"))
675674
if err != nil {
676675
log.Debug(ctx, "dial reconnecting pty server in workspace agent", slog.Error(err))
677676
_ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("dial: %s", err))

‎codersdk/workspaceagentconn.go

Lines changed: 9 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -193,32 +193,13 @@ func (c *WorkspaceAgentConn) Close() error {
193193
return c.Conn.Close()
194194
}
195195

196-
type ReconnectingPTYBackendType string
197-
198-
const (
199-
ReconnectingPTYBackendTypeAuto ReconnectingPTYBackendType = "auto"
200-
ReconnectingPTYBackendTypeScreen ReconnectingPTYBackendType = "screen"
201-
ReconnectingPTYBackendTypeBuffered ReconnectingPTYBackendType = "buffered"
202-
)
203-
204-
func (s ReconnectingPTYBackendType) Valid() bool {
205-
switch s {
206-
case ReconnectingPTYBackendTypeAuto, ReconnectingPTYBackendTypeBuffered,
207-
ReconnectingPTYBackendTypeScreen:
208-
return true
209-
default:
210-
return false
211-
}
212-
}
213-
214196
// WorkspaceAgentReconnectingPTYInit initializes a new reconnecting PTY session.
215197
// @typescript-ignore WorkspaceAgentReconnectingPTYInit
216198
type WorkspaceAgentReconnectingPTYInit struct {
217-
ID uuid.UUID
218-
Height uint16
219-
Width uint16
220-
Command string
221-
BackendType ReconnectingPTYBackendType
199+
ID uuid.UUID
200+
Height uint16
201+
Width uint16
202+
Command string
222203
}
223204

224205
// ReconnectingPTYRequest is sent from the client to the server
@@ -233,7 +214,7 @@ type ReconnectingPTYRequest struct {
233214
// ReconnectingPTY spawns a new reconnecting terminal session.
234215
// `ReconnectingPTYRequest` should be JSON marshaled and written to the returned net.Conn.
235216
// Raw terminal output will be read from the returned net.Conn.
236-
func (c *WorkspaceAgentConn) ReconnectingPTY(ctx context.Context, id uuid.UUID, height, width uint16, command string, backendType ReconnectingPTYBackendType) (net.Conn, error) {
217+
func (c *WorkspaceAgentConn) ReconnectingPTY(ctx context.Context, id uuid.UUID, height, width uint16, command string) (net.Conn, error) {
237218
ctx, span := tracing.StartSpan(ctx)
238219
defer span.End()
239220

@@ -246,11 +227,10 @@ func (c *WorkspaceAgentConn) ReconnectingPTY(ctx context.Context, id uuid.UUID,
246227
return nil, err
247228
}
248229
data, err := json.Marshal(WorkspaceAgentReconnectingPTYInit{
249-
ID: id,
250-
Height: height,
251-
Width: width,
252-
Command: command,
253-
BackendType: backendType,
230+
ID: id,
231+
Height: height,
232+
Width: width,
233+
Command: command,
254234
})
255235
if err != nil {
256236
_ = conn.Close()

‎codersdk/workspaceagents.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -552,12 +552,11 @@ func (c *Client) IssueReconnectingPTYSignedToken(ctx context.Context, req IssueR
552552

553553
// @typescript-ignore:WorkspaceAgentReconnectingPTYOpts
554554
type WorkspaceAgentReconnectingPTYOpts struct {
555-
AgentID uuid.UUID
556-
Reconnect uuid.UUID
557-
Width uint16
558-
Height uint16
559-
Command string
560-
BackendType ReconnectingPTYBackendType
555+
AgentID uuid.UUID
556+
Reconnect uuid.UUID
557+
Width uint16
558+
Height uint16
559+
Command string
561560

562561
// SignedToken is an optional signed token from the
563562
// issue-reconnecting-pty-signed-token endpoint. If set, the session token
@@ -578,7 +577,6 @@ func (c *Client) WorkspaceAgentReconnectingPTY(ctx context.Context, opts Workspa
578577
q.Set("width", strconv.Itoa(int(opts.Width)))
579578
q.Set("height", strconv.Itoa(int(opts.Height)))
580579
q.Set("command", opts.Command)
581-
q.Set("backend", string(opts.BackendType))
582580
// If we're using a signed token, set the query parameter.
583581
if opts.SignedToken != "" {
584582
q.Set(SignedAppTokenQueryParameter, opts.SignedToken)

0 commit comments

Comments
 (0)