Skip to content

Commit 44f86d5

Browse files
ParkreinerParkreiner
committed
merge main
2 parents 91ce668 + 65c726e commit 44f86d5

File tree

262 files changed

+6450
-4191
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

262 files changed

+6450
-4191
lines changed

‎.vscode/settings.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@
170170
"wsconncache",
171171
"wsjson",
172172
"xerrors",
173+
"xlarge",
173174
"yamux"
174175
],
175176
"cSpell.ignorePaths": ["site/package.json", ".vscode/settings.json"],
@@ -205,8 +206,6 @@
205206
"files.insertFinalNewline": true,
206207
"go.lintTool": "golangci-lint",
207208
"go.lintFlags": ["--fast"],
208-
"go.lintOnSave": "package",
209-
"go.coverOnSave": true,
210209
"go.coverageDecorator": {
211210
"type": "gutter",
212211
"coveredGutterStyle": "blockgreen",

‎agent/agent.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ type Options struct {
6868
EnvironmentVariables map[string]string
6969
Logger slog.Logger
7070
IgnorePorts map[int]string
71+
PortCacheDuration time.Duration
7172
SSHMaxTimeout time.Duration
7273
TailnetListenPort uint16
7374
Subsystems []codersdk.AgentSubsystem
@@ -126,6 +127,9 @@ func New(options Options) Agent {
126127
if options.ServiceBannerRefreshInterval == 0 {
127128
options.ServiceBannerRefreshInterval = 2 * time.Minute
128129
}
130+
if options.PortCacheDuration == 0 {
131+
options.PortCacheDuration = 1 * time.Second
132+
}
129133

130134
prometheusRegistry := options.PrometheusRegistry
131135
if prometheusRegistry == nil {
@@ -153,6 +157,7 @@ func New(options Options) Agent {
153157
lifecycleReported: make(chan codersdk.WorkspaceAgentLifecycle, 1),
154158
lifecycleStates: []agentsdk.PostLifecycleRequest{{State: codersdk.WorkspaceAgentLifecycleCreated}},
155159
ignorePorts: options.IgnorePorts,
160+
portCacheDuration: options.PortCacheDuration,
156161
connStatsChan: make(chan *agentsdk.Stats, 1),
157162
reportMetadataInterval: options.ReportMetadataInterval,
158163
serviceBannerRefreshInterval: options.ServiceBannerRefreshInterval,
@@ -181,8 +186,9 @@ type agent struct {
181186
// ignorePorts tells the api handler which ports to ignore when
182187
// listing all listening ports. This is helpful to hide ports that
183188
// are used by the agent, that the user does not care about.
184-
ignorePorts map[int]string
185-
subsystems []codersdk.AgentSubsystem
189+
ignorePorts map[int]string
190+
portCacheDuration time.Duration
191+
subsystems []codersdk.AgentSubsystem
186192

187193
reconnectingPTYs sync.Map
188194
reconnectingPTYTimeout time.Duration

‎agent/agentssh/agentssh.go

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"time"
2020

2121
"github.com/gliderlabs/ssh"
22+
"github.com/google/uuid"
2223
"github.com/kballard/go-shellquote"
2324
"github.com/pkg/sftp"
2425
"github.com/prometheus/client_golang/prometheus"
@@ -192,9 +193,16 @@ func (s *Server) ConnStats() ConnStats {
192193
}
193194

194195
func (s *Server) sessionHandler(session ssh.Session) {
195-
logger := s.logger.With(slog.F("remote_addr", session.RemoteAddr()), slog.F("local_addr", session.LocalAddr()))
196-
logger.Info(session.Context(), "handling ssh session")
197196
ctx := session.Context()
197+
logger := s.logger.With(
198+
slog.F("remote_addr", session.RemoteAddr()),
199+
slog.F("local_addr", session.LocalAddr()),
200+
// Assigning a random uuid for each session is useful for tracking
201+
// logs for the same ssh session.
202+
slog.F("id", uuid.NewString()),
203+
)
204+
logger.Info(ctx, "handling ssh session")
205+
198206
if !s.trackSession(session, true) {
199207
// See (*Server).Close() for why we call Close instead of Exit.
200208
_ = session.Close()
@@ -218,15 +226,15 @@ func (s *Server) sessionHandler(session ssh.Session) {
218226
switch ss := session.Subsystem(); ss {
219227
case "":
220228
case "sftp":
221-
s.sftpHandler(session)
229+
s.sftpHandler(logger, session)
222230
return
223231
default:
224232
logger.Warn(ctx, "unsupported subsystem", slog.F("subsystem", ss))
225233
_ = session.Exit(1)
226234
return
227235
}
228236

229-
err := s.sessionStart(session, extraEnv)
237+
err := s.sessionStart(logger, session, extraEnv)
230238
var exitError *exec.ExitError
231239
if xerrors.As(err, &exitError) {
232240
logger.Info(ctx, "ssh session returned", slog.Error(exitError))
@@ -244,7 +252,7 @@ func (s *Server) sessionHandler(session ssh.Session) {
244252
_ = session.Exit(0)
245253
}
246254

247-
func (s *Server) sessionStart(session ssh.Session, extraEnv []string) (retErr error) {
255+
func (s *Server) sessionStart(logger slog.Logger, session ssh.Session, extraEnv []string) (retErr error) {
248256
ctx := session.Context()
249257
env := append(session.Environ(), extraEnv...)
250258
var magicType string
@@ -268,7 +276,7 @@ func (s *Server) sessionStart(session ssh.Session, extraEnv []string) (retErr er
268276
s.connCountSSHSession.Add(1)
269277
defer s.connCountSSHSession.Add(-1)
270278
default:
271-
s.logger.Warn(ctx, "invalid magic ssh session type specified", slog.F("type", magicType))
279+
logger.Warn(ctx, "invalid magic ssh session type specified", slog.F("type", magicType))
272280
}
273281

274282
magicTypeLabel := magicTypeMetricLabel(magicType)
@@ -301,7 +309,7 @@ func (s *Server) sessionStart(session ssh.Session, extraEnv []string) (retErr er
301309
}
302310

303311
if isPty {
304-
return s.startPTYSession(session, magicTypeLabel, cmd, sshPty, windowSize)
312+
return s.startPTYSession(logger, session, magicTypeLabel, cmd, sshPty, windowSize)
305313
}
306314
return s.startNonPTYSession(session, magicTypeLabel, cmd.AsExec())
307315
}
@@ -342,7 +350,7 @@ type ptySession interface {
342350
RawCommand() string
343351
}
344352

345-
func (s *Server) startPTYSession(session ptySession, magicTypeLabel string, cmd *pty.Cmd, sshPty ssh.Pty, windowSize <-chan ssh.Window) (retErr error) {
353+
func (s *Server) startPTYSession(logger slog.Logger, session ptySession, magicTypeLabel string, cmd *pty.Cmd, sshPty ssh.Pty, windowSize <-chan ssh.Window) (retErr error) {
346354
s.metrics.sessionsTotal.WithLabelValues(magicTypeLabel, "yes").Add(1)
347355

348356
ctx := session.Context()
@@ -355,7 +363,7 @@ func (s *Server) startPTYSession(session ptySession, magicTypeLabel string, cmd
355363
if serviceBanner != nil {
356364
err := showServiceBanner(session, serviceBanner)
357365
if err != nil {
358-
s.logger.Error(ctx, "agent failed to show service banner", slog.Error(err))
366+
logger.Error(ctx, "agent failed to show service banner", slog.Error(err))
359367
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "yes", "service_banner").Add(1)
360368
}
361369
}
@@ -366,11 +374,11 @@ func (s *Server) startPTYSession(session ptySession, magicTypeLabel string, cmd
366374
if manifest != nil {
367375
err := showMOTD(s.fs, session, manifest.MOTDFile)
368376
if err != nil {
369-
s.logger.Error(ctx, "agent failed to show MOTD", slog.Error(err))
377+
logger.Error(ctx, "agent failed to show MOTD", slog.Error(err))
370378
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "yes", "motd").Add(1)
371379
}
372380
} else {
373-
s.logger.Warn(ctx, "metadata lookup failed, unable to show MOTD")
381+
logger.Warn(ctx, "metadata lookup failed, unable to show MOTD")
374382
}
375383
}
376384

@@ -379,7 +387,7 @@ func (s *Server) startPTYSession(session ptySession, magicTypeLabel string, cmd
379387
// The pty package sets `SSH_TTY` on supported platforms.
380388
ptty, process, err := pty.Start(cmd, pty.WithPTYOption(
381389
pty.WithSSHRequest(sshPty),
382-
pty.WithLogger(slog.Stdlib(ctx, s.logger, slog.LevelInfo)),
390+
pty.WithLogger(slog.Stdlib(ctx, logger, slog.LevelInfo)),
383391
))
384392
if err != nil {
385393
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "yes", "start_command").Add(1)
@@ -388,7 +396,7 @@ func (s *Server) startPTYSession(session ptySession, magicTypeLabel string, cmd
388396
defer func() {
389397
closeErr := ptty.Close()
390398
if closeErr != nil {
391-
s.logger.Warn(ctx, "failed to close tty", slog.Error(closeErr))
399+
logger.Warn(ctx, "failed to close tty", slog.Error(closeErr))
392400
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "yes", "close").Add(1)
393401
if retErr == nil {
394402
retErr = closeErr
@@ -400,7 +408,7 @@ func (s *Server) startPTYSession(session ptySession, magicTypeLabel string, cmd
400408
resizeErr := ptty.Resize(uint16(win.Height), uint16(win.Width))
401409
// If the pty is closed, then command has exited, no need to log.
402410
if resizeErr != nil && !errors.Is(resizeErr, pty.ErrClosed) {
403-
s.logger.Warn(ctx, "failed to resize tty", slog.Error(resizeErr))
411+
logger.Warn(ctx, "failed to resize tty", slog.Error(resizeErr))
404412
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "yes", "resize").Add(1)
405413
}
406414
}
@@ -422,7 +430,7 @@ func (s *Server) startPTYSession(session ptySession, magicTypeLabel string, cmd
422430
// 2. The client hangs up, which cancels the command's Context, and go will
423431
// kill the command's process. This then has the same effect as (1).
424432
n, err := io.Copy(session, ptty.OutputReader())
425-
s.logger.Debug(ctx, "copy output done", slog.F("bytes", n), slog.Error(err))
433+
logger.Debug(ctx, "copy output done", slog.F("bytes", n), slog.Error(err))
426434
if err != nil {
427435
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "yes", "output_io_copy").Add(1)
428436
return xerrors.Errorf("copy error: %w", err)
@@ -435,7 +443,7 @@ func (s *Server) startPTYSession(session ptySession, magicTypeLabel string, cmd
435443
// ExitErrors just mean the command we run returned a non-zero exit code, which is normal
436444
// and not something to be concerned about. But, if it's something else, we should log it.
437445
if err != nil && !xerrors.As(err, &exitErr) {
438-
s.logger.Warn(ctx, "process wait exited with error", slog.Error(err))
446+
logger.Warn(ctx, "process wait exited with error", slog.Error(err))
439447
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "yes", "wait").Add(1)
440448
}
441449
if err != nil {
@@ -444,7 +452,7 @@ func (s *Server) startPTYSession(session ptySession, magicTypeLabel string, cmd
444452
return nil
445453
}
446454

447-
func (s *Server) sftpHandler(session ssh.Session) {
455+
func (s *Server) sftpHandler(logger slog.Logger, session ssh.Session) {
448456
s.metrics.sftpConnectionsTotal.Add(1)
449457

450458
ctx := session.Context()
@@ -460,14 +468,14 @@ func (s *Server) sftpHandler(session ssh.Session) {
460468
// directory so that SFTP connections land there.
461469
homedir, err := userHomeDir()
462470
if err != nil {
463-
s.logger.Warn(ctx, "get sftp working directory failed, unable to get home dir", slog.Error(err))
471+
logger.Warn(ctx, "get sftp working directory failed, unable to get home dir", slog.Error(err))
464472
} else {
465473
opts = append(opts, sftp.WithServerWorkingDirectory(homedir))
466474
}
467475

468476
server, err := sftp.NewServer(session, opts...)
469477
if err != nil {
470-
s.logger.Debug(ctx, "initialize sftp server", slog.Error(err))
478+
logger.Debug(ctx, "initialize sftp server", slog.Error(err))
471479
return
472480
}
473481
defer server.Close()
@@ -485,7 +493,7 @@ func (s *Server) sftpHandler(session ssh.Session) {
485493
_ = session.Exit(0)
486494
return
487495
}
488-
s.logger.Warn(ctx, "sftp server closed with error", slog.Error(err))
496+
logger.Warn(ctx, "sftp server closed with error", slog.Error(err))
489497
s.metrics.sftpServerErrors.Add(1)
490498
_ = session.Exit(1)
491499
}

‎agent/agentssh/agentssh_internal_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func Test_sessionStart_orphan(t *testing.T) {
6363
// we don't really care what the error is here. In the larger scenario,
6464
// the client has disconnected, so we can't return any error information
6565
// to them.
66-
_ = s.startPTYSession(sess, "ssh", cmd, ptyInfo, windowSize)
66+
_ = s.startPTYSession(logger, sess, "ssh", cmd, ptyInfo, windowSize)
6767
}()
6868

6969
readDone := make(chan struct{})

‎agent/api.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,27 @@ func (a *agent) apiHandler() http.Handler {
2626
cpy[k] = b
2727
}
2828

29-
lp := &listeningPortsHandler{ignorePorts: cpy}
29+
cacheDuration := 1 * time.Second
30+
if a.portCacheDuration > 0 {
31+
cacheDuration = a.portCacheDuration
32+
}
33+
34+
lp := &listeningPortsHandler{
35+
ignorePorts: cpy,
36+
cacheDuration: cacheDuration,
37+
}
3038
r.Get("/api/v0/listening-ports", lp.handler)
3139

3240
return r
3341
}
3442

3543
type listeningPortsHandler struct {
36-
mut sync.Mutex
37-
ports []codersdk.WorkspaceAgentListeningPort
38-
mtime time.Time
39-
ignorePorts map[int]string
44+
ignorePorts map[int]string
45+
cacheDuration time.Duration
46+
47+
mut sync.Mutex
48+
ports []codersdk.WorkspaceAgentListeningPort
49+
mtime time.Time
4050
}
4151

4252
// handler returns a list of listening ports. This is tested by coderd's

‎agent/ports_supported.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ func (lp *listeningPortsHandler) getListeningPorts() ([]codersdk.WorkspaceAgentL
1515
lp.mut.Lock()
1616
defer lp.mut.Unlock()
1717

18-
if time.Since(lp.mtime) < time.Second {
18+
if time.Since(lp.mtime) < lp.cacheDuration {
1919
// copy
2020
ports := make([]codersdk.WorkspaceAgentListeningPort, len(lp.ports))
2121
copy(ports, lp.ports)

‎cli/agent.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,9 @@ func (r *RootCmd) workspaceAgent() *clibase.Cmd {
153153
logWriter := &lumberjackWriteCloseFixer{w: &lumberjack.Logger{
154154
Filename: filepath.Join(logDir, "coder-agent.log"),
155155
MaxSize: 5, // MB
156-
// Without this, rotated logs will never be deleted.
157-
MaxBackups: 1,
156+
// Per customer incident on November 17th, 2023, its helpful
157+
// to have the log of the last few restarts to debug a failing agent.
158+
MaxBackups: 10,
158159
}}
159160
defer logWriter.Close()
160161

‎cli/agent_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,15 @@ func TestWorkspaceAgent(t *testing.T) {
6969
OrganizationID: user.OrganizationID,
7070
OwnerID: user.UserID,
7171
})
72-
dbfake.WorkspaceBuild(t, db, ws, database.WorkspaceBuild{}, &proto.Resource{
72+
dbfake.NewWorkspaceBuildBuilder(t, db, ws).Resource(&proto.Resource{
7373
Name: "somename",
7474
Type: "someinstance",
7575
Agents: []*proto.Agent{{
7676
Auth: &proto.Agent_InstanceId{
7777
InstanceId: instanceID,
7878
},
7979
}},
80-
})
80+
}).Do()
8181

8282
inv, _ := clitest.New(t, "agent", "--auth", "azure-instance-identity", "--agent-url", client.URL.String())
8383
inv = inv.WithContext(
@@ -112,15 +112,15 @@ func TestWorkspaceAgent(t *testing.T) {
112112
OrganizationID: user.OrganizationID,
113113
OwnerID: user.UserID,
114114
})
115-
dbfake.WorkspaceBuild(t, db, ws, database.WorkspaceBuild{}, &proto.Resource{
115+
dbfake.NewWorkspaceBuildBuilder(t, db, ws).Resource(&proto.Resource{
116116
Name: "somename",
117117
Type: "someinstance",
118118
Agents: []*proto.Agent{{
119119
Auth: &proto.Agent_InstanceId{
120120
InstanceId: instanceID,
121121
},
122122
}},
123-
})
123+
}).Do()
124124

125125
inv, _ := clitest.New(t, "agent", "--auth", "aws-instance-identity", "--agent-url", client.URL.String())
126126
inv = inv.WithContext(
@@ -156,15 +156,15 @@ func TestWorkspaceAgent(t *testing.T) {
156156
OrganizationID: owner.OrganizationID,
157157
OwnerID: memberUser.ID,
158158
})
159-
dbfake.WorkspaceBuild(t, db, ws, database.WorkspaceBuild{}, &proto.Resource{
159+
dbfake.NewWorkspaceBuildBuilder(t, db, ws).Resource(&proto.Resource{
160160
Name: "somename",
161161
Type: "someinstance",
162162
Agents: []*proto.Agent{{
163163
Auth: &proto.Agent_InstanceId{
164164
InstanceId: instanceID,
165165
},
166166
}},
167-
})
167+
}).Do()
168168
inv, cfg := clitest.New(t, "agent", "--auth", "google-instance-identity", "--agent-url", client.URL.String())
169169
clitest.SetupConfig(t, member, cfg)
170170

‎cli/clibase/cmd.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,15 @@ func (inv *Invocation) SignalNotifyContext(parent context.Context, signals ...os
226226
return inv.signalNotifyContext(parent, signals...)
227227
}
228228

229+
func (inv *Invocation) WithTestParsedFlags(
230+
_ testing.TB, // ensure we only call this from tests
231+
parsedFlags *pflag.FlagSet,
232+
) *Invocation {
233+
return inv.with(func(i *Invocation) {
234+
i.parsedFlags = parsedFlags
235+
})
236+
}
237+
229238
func (inv *Invocation) Context() context.Context {
230239
if inv.ctx == nil {
231240
return context.Background()

‎cli/cliutil/sink_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
package cliutil_test
22

33
import (
4-
"errors"
54
"testing"
65

76
"github.com/stretchr/testify/require"
7+
"golang.org/x/xerrors"
88

99
"github.com/coder/coder/v2/cli/cliutil"
1010
)
1111

1212
func TestDiscardAfterClose(t *testing.T) {
1313
t.Parallel()
14-
exErr := errors.New("test")
14+
exErr := xerrors.New("test")
1515
fwc := &fakeWriteCloser{err: exErr}
1616
uut := cliutil.DiscardAfterClose(fwc)
1717

0 commit comments

Comments
 (0)