Skip to content

Commit 3f058f2

Browse files
mafredrimafredri
authored
test(agent): use afero for motd tests to allow parallel execution (#8329)
1 parent c6fcd7e commit 3f058f2

File tree

3 files changed

+59
-53
lines changed

3 files changed

+59
-53
lines changed

‎agent/agent.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ func (a *agent) setLifecycle(ctx context.Context, state codersdk.WorkspaceAgentL
503503
// not be fetched immediately; the expectation is that it is primed elsewhere
504504
// (and must be done before the session actually starts).
505505
func (a *agent) fetchServiceBannerLoop(ctx context.Context) {
506-
ticker := time.NewTicker(adjustIntervalForTests(2*time.Minute, time.Millisecond*100))
506+
ticker := time.NewTicker(adjustIntervalForTests(2*time.Minute, time.Millisecond*5))
507507
defer ticker.Stop()
508508
for {
509509
select {

‎agent/agent_test.go

Lines changed: 55 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ func TestAgent_Stats_Magic(t *testing.T) {
192192

193193
func TestAgent_SessionExec(t *testing.T) {
194194
t.Parallel()
195-
session := setupSSHSession(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{})
195+
session := setupSSHSession(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{}, nil)
196196

197197
command := "echo test"
198198
if runtime.GOOS == "windows" {
@@ -205,7 +205,7 @@ func TestAgent_SessionExec(t *testing.T) {
205205

206206
func TestAgent_GitSSH(t *testing.T) {
207207
t.Parallel()
208-
session := setupSSHSession(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{})
208+
session := setupSSHSession(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{}, nil)
209209
command := "sh -c 'echo $GIT_SSH_COMMAND'"
210210
if runtime.GOOS == "windows" {
211211
command = "cmd.exe /c echo %GIT_SSH_COMMAND%"
@@ -225,7 +225,7 @@ func TestAgent_SessionTTYShell(t *testing.T) {
225225
// it seems like it could be either.
226226
t.Skip("ConPTY appears to be inconsistent on Windows.")
227227
}
228-
session := setupSSHSession(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{})
228+
session := setupSSHSession(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{}, nil)
229229
command := "sh"
230230
if runtime.GOOS == "windows" {
231231
command = "cmd.exe"
@@ -248,7 +248,7 @@ func TestAgent_SessionTTYShell(t *testing.T) {
248248

249249
func TestAgent_SessionTTYExitCode(t *testing.T) {
250250
t.Parallel()
251-
session := setupSSHSession(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{})
251+
session := setupSSHSession(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{}, nil)
252252
command := "areallynotrealcommand"
253253
err := session.RequestPty("xterm", 128, 128, ssh.TerminalModes{})
254254
require.NoError(t, err)
@@ -268,25 +268,22 @@ func TestAgent_SessionTTYExitCode(t *testing.T) {
268268
}
269269
}
270270

271-
//nolint:paralleltest // This test sets an environment variable.
272271
func TestAgent_Session_TTY_MOTD(t *testing.T) {
272+
t.Parallel()
273273
if runtime.GOOS == "windows" {
274274
// This might be our implementation, or ConPTY itself.
275275
// It's difficult to find extensive tests for it, so
276276
// it seems like it could be either.
277277
t.Skip("ConPTY appears to be inconsistent on Windows.")
278278
}
279279

280-
wantMOTD := "Welcome to your Coder workspace!"
281-
wantServiceBanner := "Service banner text goes here"
280+
u, err := user.Current()
281+
require.NoError(t, err, "get current user")
282282

283-
tmpdir := t.TempDir()
284-
name := filepath.Join(tmpdir, "motd")
285-
err := os.WriteFile(name, []byte(wantMOTD), 0o600)
286-
require.NoError(t, err, "write motd file")
283+
name := filepath.Join(u.HomeDir, "motd")
287284

288-
// Set HOME so we can ensure no ~/.hushlogin is present.
289-
t.Setenv("HOME", tmpdir)
285+
wantMOTD := "Welcome to your Coder workspace!"
286+
wantServiceBanner := "Service banner text goes here"
290287

291288
tests := []struct {
292289
name string
@@ -362,14 +359,20 @@ func TestAgent_Session_TTY_MOTD(t *testing.T) {
362359
for _, test := range tests {
363360
test := test
364361
t.Run(test.name, func(t *testing.T) {
365-
session := setupSSHSession(t, test.manifest, test.banner)
362+
t.Parallel()
363+
session := setupSSHSession(t, test.manifest, test.banner, func(fs afero.Fs) {
364+
err := fs.MkdirAll(filepath.Dir(name), 0o700)
365+
require.NoError(t, err)
366+
err = afero.WriteFile(fs, name, []byte(wantMOTD), 0o600)
367+
require.NoError(t, err)
368+
})
366369
testSessionOutput(t, session, test.expected, test.unexpected, test.expectedRe)
367370
})
368371
}
369372
}
370373

371-
//nolint:paralleltest // This test sets an environment variable.
372374
func TestAgent_Session_TTY_MOTD_Update(t *testing.T) {
375+
t.Parallel()
373376
if runtime.GOOS == "windows" {
374377
// This might be our implementation, or ConPTY itself.
375378
// It's difficult to find extensive tests for it, so
@@ -380,11 +383,6 @@ func TestAgent_Session_TTY_MOTD_Update(t *testing.T) {
380383
// Only the banner updates dynamically; the MOTD file does not.
381384
wantServiceBanner := "Service banner text goes here"
382385

383-
tmpdir := t.TempDir()
384-
385-
// Set HOME so we can ensure no ~/.hushlogin is present.
386-
t.Setenv("HOME", tmpdir)
387-
388386
tests := []struct {
389387
banner codersdk.ServiceBannerConfig
390388
expected []string
@@ -424,22 +422,25 @@ func TestAgent_Session_TTY_MOTD_Update(t *testing.T) {
424422
},
425423
}
426424

427-
const updateInterval = 100 * time.Millisecond
428-
429425
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
430426
defer cancel()
431427
//nolint:dogsled // Allow the blank identifiers.
432428
conn, client, _, _, _ := setupAgent(t, &client{}, 0)
433429
for _, test := range tests {
434430
test := test
435-
// Set new banner func and wait for the agent to call it to update the
436-
// banner.
431+
432+
ready := make(chan struct{}, 2)
437433
client.mu.Lock()
438434
client.getServiceBanner = func() (codersdk.ServiceBannerConfig, error) {
435+
select {
436+
case ready <- struct{}{}:
437+
default:
438+
}
439439
return test.banner, nil
440440
}
441441
client.mu.Unlock()
442-
time.Sleep(updateInterval)
442+
<-ready
443+
<-ready // Wait for two updates to ensure the value has propagated.
443444

444445
sshClient, err := conn.SSHClient(ctx)
445446
require.NoError(t, err)
@@ -466,50 +467,51 @@ func TestAgent_Session_TTY_QuietLogin(t *testing.T) {
466467
}
467468

468469
wantNotMOTD := "Welcome to your Coder workspace!"
469-
wantServiceBanner := "Service banner text goes here"
470+
wantMaybeServiceBanner := "Service banner text goes here"
470471

471-
tmpdir := t.TempDir()
472-
name := filepath.Join(tmpdir, "motd")
473-
err := os.WriteFile(name, []byte(wantNotMOTD), 0o600)
474-
require.NoError(t, err, "write motd file")
472+
u, err := user.Current()
473+
require.NoError(t, err, "get current user")
475474

476-
// Set HOME so we can ensure ~/.hushlogin is present.
477-
t.Setenv("HOME", tmpdir)
475+
name := filepath.Join(u.HomeDir, "motd")
478476

479477
// Neither banner nor MOTD should show if not a login shell.
480478
t.Run("NotLogin", func(t *testing.T) {
481-
wantEcho := "foobar"
482479
session := setupSSHSession(t, agentsdk.Manifest{
483480
MOTDFile: name,
484481
}, codersdk.ServiceBannerConfig{
485482
Enabled: true,
486-
Message: wantServiceBanner,
483+
Message: wantMaybeServiceBanner,
484+
}, func(fs afero.Fs) {
485+
err := afero.WriteFile(fs, name, []byte(wantNotMOTD), 0o600)
486+
require.NoError(t, err, "write motd file")
487487
})
488488
err = session.RequestPty("xterm", 128, 128, ssh.TerminalModes{})
489489
require.NoError(t, err)
490490

491+
wantEcho := "foobar"
491492
command := "echo " + wantEcho
492493
output, err := session.Output(command)
493494
require.NoError(t, err)
494495

495496
require.Contains(t, string(output), wantEcho, "should show echo")
496497
require.NotContains(t, string(output), wantNotMOTD, "should not show motd")
497-
require.NotContains(t, string(output), wantServiceBanner, "should not show service banner")
498+
require.NotContains(t, string(output), wantMaybeServiceBanner, "should not show service banner")
498499
})
499500

500-
// Only the MOTD should be silenced.
501+
// Only the MOTD should be silenced when hushlogin is present.
501502
t.Run("Hushlogin", func(t *testing.T) {
502-
// Create hushlogin to silence motd.
503-
f, err := os.Create(filepath.Join(tmpdir, ".hushlogin"))
504-
require.NoError(t, err, "create .hushlogin file")
505-
err = f.Close()
506-
require.NoError(t, err, "close .hushlogin file")
507-
508503
session := setupSSHSession(t, agentsdk.Manifest{
509504
MOTDFile: name,
510505
}, codersdk.ServiceBannerConfig{
511506
Enabled: true,
512-
Message: wantServiceBanner,
507+
Message: wantMaybeServiceBanner,
508+
}, func(fs afero.Fs) {
509+
err := afero.WriteFile(fs, name, []byte(wantNotMOTD), 0o600)
510+
require.NoError(t, err, "write motd file")
511+
512+
// Create hushlogin to silence motd.
513+
err = afero.WriteFile(fs, name, []byte{}, 0o600)
514+
require.NoError(t, err, "write hushlogin file")
513515
})
514516
err = session.RequestPty("xterm", 128, 128, ssh.TerminalModes{})
515517
require.NoError(t, err)
@@ -527,7 +529,7 @@ func TestAgent_Session_TTY_QuietLogin(t *testing.T) {
527529
require.NoError(t, err)
528530

529531
require.NotContains(t, stdout.String(), wantNotMOTD, "should not show motd")
530-
require.Contains(t, stdout.String(), wantServiceBanner, "should show service banner")
532+
require.Contains(t, stdout.String(), wantMaybeServiceBanner, "should show service banner")
531533
})
532534
}
533535

@@ -975,7 +977,7 @@ func TestAgent_EnvironmentVariables(t *testing.T) {
975977
EnvironmentVariables: map[string]string{
976978
key: value,
977979
},
978-
}, codersdk.ServiceBannerConfig{})
980+
}, codersdk.ServiceBannerConfig{}, nil)
979981
command := "sh -c 'echo $" + key + "'"
980982
if runtime.GOOS == "windows" {
981983
command = "cmd.exe /c echo %" + key + "%"
@@ -992,7 +994,7 @@ func TestAgent_EnvironmentVariableExpansion(t *testing.T) {
992994
EnvironmentVariables: map[string]string{
993995
key: "$SOMETHINGNOTSET",
994996
},
995-
}, codersdk.ServiceBannerConfig{})
997+
}, codersdk.ServiceBannerConfig{}, nil)
996998
command := "sh -c 'echo $" + key + "'"
997999
if runtime.GOOS == "windows" {
9981000
command = "cmd.exe /c echo %" + key + "%"
@@ -1015,7 +1017,7 @@ func TestAgent_CoderEnvVars(t *testing.T) {
10151017
t.Run(key, func(t *testing.T) {
10161018
t.Parallel()
10171019

1018-
session := setupSSHSession(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{})
1020+
session := setupSSHSession(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{}, nil)
10191021
command := "sh -c 'echo $" + key + "'"
10201022
if runtime.GOOS == "windows" {
10211023
command = "cmd.exe /c echo %" + key + "%"
@@ -1038,7 +1040,7 @@ func TestAgent_SSHConnectionEnvVars(t *testing.T) {
10381040
t.Run(key, func(t *testing.T) {
10391041
t.Parallel()
10401042

1041-
session := setupSSHSession(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{})
1043+
session := setupSSHSession(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{}, nil)
10421044
command := "sh -c 'echo $" + key + "'"
10431045
if runtime.GOOS == "windows" {
10441046
command = "cmd.exe /c echo %" + key + "%"
@@ -1876,16 +1878,20 @@ func setupSSHSession(
18761878
t *testing.T,
18771879
options agentsdk.Manifest,
18781880
serviceBanner codersdk.ServiceBannerConfig,
1881+
prepareFS func(fs afero.Fs),
18791882
) *ssh.Session {
18801883
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
18811884
defer cancel()
18821885
//nolint:dogsled
1883-
conn, _, _, _, _ := setupAgent(t, &client{
1886+
conn, _, _, fs, _ := setupAgent(t, &client{
18841887
manifest: options,
18851888
getServiceBanner: func() (codersdk.ServiceBannerConfig, error) {
18861889
return serviceBanner, nil
18871890
},
18881891
}, 0)
1892+
if prepareFS != nil {
1893+
prepareFS(fs)
1894+
}
18891895
sshClient, err := conn.SSHClient(ctx)
18901896
require.NoError(t, err)
18911897
t.Cleanup(func() {

‎agent/agentssh/agentssh.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ func (s *Server) startPTYSession(session ptySession, magicTypeLabel string, cmd
361361
if !isQuietLogin(session.RawCommand()) {
362362
manifest := s.Manifest.Load()
363363
if manifest != nil {
364-
err := showMOTD(session, manifest.MOTDFile)
364+
err := showMOTD(s.fs, session, manifest.MOTDFile)
365365
if err != nil {
366366
s.logger.Error(ctx, "agent failed to show MOTD", slog.Error(err))
367367
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "yes", "motd").Add(1)
@@ -796,12 +796,12 @@ func showServiceBanner(session io.Writer, banner *codersdk.ServiceBannerConfig)
796796
// the given filename to dest, if the file exists.
797797
//
798798
// https://github.com/openssh/openssh-portable/blob/25bd659cc72268f2858c5415740c442ee950049f/session.c#L784
799-
func showMOTD(dest io.Writer, filename string) error {
799+
func showMOTD(fs afero.Fs, dest io.Writer, filename string) error {
800800
if filename == "" {
801801
return nil
802802
}
803803

804-
f, err := os.Open(filename)
804+
f, err := fs.Open(filename)
805805
if err != nil {
806806
if xerrors.Is(err, os.ErrNotExist) {
807807
// This is not an error, there simply isn't a MOTD to show.

0 commit comments

Comments
 (0)