Skip to content

feat(cli/ssh): simplify log file flags #7863

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Jun 12, 2023
Merged

feat(cli/ssh): simplify log file flags #7863

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
4b3b2bd
feat(cli/ssh): simplify log file flags
ammario Jun 5, 2023
82ce7a2
Fix race
ammario Jun 6, 2023
ad2c946
Fix race the right way
ammario Jun 6, 2023
1ca5930
Restore directory flag
ammario Jun 6, 2023
d7b26ad
Update tests
ammario Jun 6, 2023
48cb9b2
make
ammario Jun 6, 2023
3c43cfc
Merge remote-tracking branch 'origin/main' into log-race
ammario Jun 7, 2023
a1b68e5
mess around
ammario Jun 8, 2023
23a6833
Fix another race
ammario Jun 8, 2023
f17e7ce
fixup! Fix another race
ammario Jun 8, 2023
5a311b3
Update cli/ssh.go
ammario Jun 8, 2023
65cdbcc
Move CloseStdin into clibase
ammario Jun 9, 2023
d1cb325
Merge remote-tracking branch 'origin/main' into log-race
ammario Jun 9, 2023
8471555
Revert "Move CloseStdin into clibase"
ammario Jun 10, 2023
6593dfe
Merge remote-tracking branch 'origin/main' into log-race
ammario Jun 10, 2023
c5a1968
Spike-ify
ammario Jun 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Restore directory flag
  • Loading branch information
@ammario
ammario committed Jun 6, 2023
commit 1ca5930c4a19c8bdbacb3af349bc92ae3f69ed35
38 changes: 30 additions & 8 deletions cli/ssh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (r *RootCmd) ssh() *clibase.Cmd {
identityAgent string
wsPollInterval time.Duration
noWait bool
logFilePath string
logDirPath string
)
client := new(codersdk.Client)
cmd := &clibase.Cmd{
Expand All @@ -73,11 +73,33 @@ func (r *RootCmd) ssh() *clibase.Cmd {
logger.Error(ctx, "command exit", slog.Error(retErr))
}
}()
if logFilePath != "" {
logFile, err := os.OpenFile(logFilePath, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o600)
if logDirPath != "" {
nonce, err := cryptorand.StringCharset(cryptorand.Lower, 5)
if err != nil {
return xerrors.Errorf("error opening %s for logging: %w", logFilePath, err)
panic(err)
}
logFilePath := filepath.Join(
logDirPath,
fmt.Sprintf(
"coder-ssh-%s-%s.log",
// The time portion makes it easier to find the right
// log file.
time.Now().Format("20060102-150405"),
// The nonce prevents collisions, as SSH invocations
// frequently happen in parallel.
nonce,
),
)
logFile, err := os.OpenFile(
logFilePath,
os.O_CREATE|os.O_APPEND|os.O_WRONLY|os.O_EXCL,
0o600,
)
if err != nil {
return xerrors.Errorf("error opening %s for logging: %w", logDirPath, err)
}
defer logFile.Close()

logger = slog.Make(sloghuman.Sink(logFile))
if r.verbose {
logger = logger.Leveled(slog.LevelDebug)
Expand Down Expand Up @@ -359,11 +381,11 @@ func (r *RootCmd) ssh() *clibase.Cmd {
Value: clibase.BoolOf(&noWait),
},
{
Flag: "log-file",
Description: "Specify the location of an SSH diagnostic log file.",
Env: "CODER_SSH_LOG_FILE",
Flag: "log-dir",
Description: "Specify the directory containing SSH diagnostic log files.",
Env: "CODER_SSH_LOG_DIR",
FlagShorthand: "l",
Value: clibase.StringOf(&logFilePath),
Value: clibase.StringOf(&logDirPath),
},
}
return cmd
Expand Down
12 changes: 5 additions & 7 deletions cli/ssh_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,10 +411,10 @@ func TestSSH(t *testing.T) {
t.Run("FileLogging", func(t *testing.T) {
t.Parallel()

logFile := filepath.Join(t.TempDir(), "coder-ssh.log")
logDir := t.TempDir()

client, workspace, agentToken := setupWorkspaceForAgent(t, nil)
inv, root := clitest.New(t, "ssh", workspace.Name, "-l", logFile)
inv, root := clitest.New(t, "ssh", workspace.Name, "-l", logDir)
clitest.SetupConfig(t, client, root)
pty := ptytest.New(t).Attach(inv)

Expand All @@ -441,12 +441,10 @@ func TestSSH(t *testing.T) {
pty.WriteLine("exit")
<-cmdDone

info, err := os.Stat(logFile)
if err != nil {
t.Fatalf("failed to find ssh logfile: %v", err)
}
ents, err := os.ReadDir(t.TempDir())
require.NoError(t, err)

require.Greater(t, info.Size(), int64(0), "ssh logfile is empty")
require.Len(t, ents, 1, "expected one file in logdir")
})
}

Expand Down