mirror of https://github.com/coder/coder.git
feat: add flag for Windows to create unix compatible filepaths (#8164)
* feat: add flag for Windows to create unix compatible filepaths
This commit is contained in:
parent
5d45218a5d
commit
797e91d4c6
|
@ -192,11 +192,12 @@ func sshPrepareWorkspaceConfigs(ctx context.Context, client *codersdk.Client) (r
|
||||||
//nolint:gocyclo
|
//nolint:gocyclo
|
||||||
func (r *RootCmd) configSSH() *clibase.Cmd {
|
func (r *RootCmd) configSSH() *clibase.Cmd {
|
||||||
var (
|
var (
|
||||||
sshConfigFile string
|
sshConfigFile string
|
||||||
sshConfigOpts sshConfigOptions
|
sshConfigOpts sshConfigOptions
|
||||||
usePreviousOpts bool
|
usePreviousOpts bool
|
||||||
dryRun bool
|
dryRun bool
|
||||||
skipProxyCommand bool
|
skipProxyCommand bool
|
||||||
|
forceUnixSeparators bool
|
||||||
)
|
)
|
||||||
client := new(codersdk.Client)
|
client := new(codersdk.Client)
|
||||||
cmd := &clibase.Cmd{
|
cmd := &clibase.Cmd{
|
||||||
|
@ -236,13 +237,13 @@ func (r *RootCmd) configSSH() *clibase.Cmd {
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
escapedCoderBinary, err := sshConfigExecEscape(coderBinary)
|
escapedCoderBinary, err := sshConfigExecEscape(coderBinary, forceUnixSeparators)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return xerrors.Errorf("escape coder binary for ssh failed: %w", err)
|
return xerrors.Errorf("escape coder binary for ssh failed: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
root := r.createConfig()
|
root := r.createConfig()
|
||||||
escapedGlobalConfig, err := sshConfigExecEscape(string(root))
|
escapedGlobalConfig, err := sshConfigExecEscape(string(root), forceUnixSeparators)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return xerrors.Errorf("escape global config for ssh failed: %w", err)
|
return xerrors.Errorf("escape global config for ssh failed: %w", err)
|
||||||
}
|
}
|
||||||
|
@ -540,6 +541,19 @@ func (r *RootCmd) configSSH() *clibase.Cmd {
|
||||||
Default: "auto",
|
Default: "auto",
|
||||||
Value: clibase.EnumOf(&sshConfigOpts.waitEnum, "yes", "no", "auto"),
|
Value: clibase.EnumOf(&sshConfigOpts.waitEnum, "yes", "no", "auto"),
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
Flag: "force-unix-filepaths",
|
||||||
|
Env: "CODER_CONFIGSSH_UNIX_FILEPATHS",
|
||||||
|
Description: "By default, 'config-ssh' uses the os path separator when writing the ssh config. " +
|
||||||
|
"This might be an issue in Windows machine that use a unix-like shell. " +
|
||||||
|
"This flag forces the use of unix file paths (the forward slash '/').",
|
||||||
|
Value: clibase.BoolOf(&forceUnixSeparators),
|
||||||
|
// On non-windows showing this command is useless because it is a noop.
|
||||||
|
// Hide vs disable it though so if a command is copied from a Windows
|
||||||
|
// machine to a unix machine it will still work and not throw an
|
||||||
|
// "unknown flag" error.
|
||||||
|
Hidden: hideForceUnixSlashes,
|
||||||
|
},
|
||||||
cliui.SkipPromptOption(),
|
cliui.SkipPromptOption(),
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -727,7 +741,31 @@ func writeWithTempFileAndMove(path string, r io.Reader) (err error) {
|
||||||
// - https://github.com/openssh/openssh-portable/blob/V_9_0_P1/sshconnect.c#L158-L167
|
// - https://github.com/openssh/openssh-portable/blob/V_9_0_P1/sshconnect.c#L158-L167
|
||||||
// - https://github.com/PowerShell/openssh-portable/blob/v8.1.0.0/sshconnect.c#L231-L293
|
// - https://github.com/PowerShell/openssh-portable/blob/v8.1.0.0/sshconnect.c#L231-L293
|
||||||
// - https://github.com/PowerShell/openssh-portable/blob/v8.1.0.0/contrib/win32/win32compat/w32fd.c#L1075-L1100
|
// - https://github.com/PowerShell/openssh-portable/blob/v8.1.0.0/contrib/win32/win32compat/w32fd.c#L1075-L1100
|
||||||
func sshConfigExecEscape(path string) (string, error) {
|
//
|
||||||
|
// Additional Windows-specific notes:
|
||||||
|
//
|
||||||
|
// In some situations a Windows user could be using a unix-like shell such as
|
||||||
|
// git bash. In these situations the coder.exe is using the windows filepath
|
||||||
|
// separator (\), but the shell wants the unix filepath separator (/).
|
||||||
|
// Trying to determine if the shell is unix-like is difficult, so this function
|
||||||
|
// takes the argument 'forceUnixPath' to force the filepath to be unix-like.
|
||||||
|
//
|
||||||
|
// On actual unix machines, this is **always** a noop. Even if a windows
|
||||||
|
// path is provided.
|
||||||
|
//
|
||||||
|
// Passing a "false" for forceUnixPath will result in the filepath separator
|
||||||
|
// untouched from the original input.
|
||||||
|
// ---
|
||||||
|
// This is a control flag, and that is ok. It is a control flag
|
||||||
|
// based on the OS of the user. Making this a different file is excessive.
|
||||||
|
// nolint:revive
|
||||||
|
func sshConfigExecEscape(path string, forceUnixPath bool) (string, error) {
|
||||||
|
if forceUnixPath {
|
||||||
|
// This is a workaround for #7639, where the filepath separator is
|
||||||
|
// incorrectly the Windows separator (\) instead of the unix separator (/).
|
||||||
|
path = filepath.ToSlash(path)
|
||||||
|
}
|
||||||
|
|
||||||
// This is unlikely to ever happen, but newlines are allowed on
|
// This is unlikely to ever happen, but newlines are allowed on
|
||||||
// certain filesystems, but cannot be used inside ssh config.
|
// certain filesystems, but cannot be used inside ssh config.
|
||||||
if strings.ContainsAny(path, "\n") {
|
if strings.ContainsAny(path, "\n") {
|
||||||
|
|
|
@ -12,6 +12,11 @@ import (
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
func init() {
|
||||||
|
// For golden files, always show the flag.
|
||||||
|
hideForceUnixSlashes = false
|
||||||
|
}
|
||||||
|
|
||||||
func Test_sshConfigSplitOnCoderSection(t *testing.T) {
|
func Test_sshConfigSplitOnCoderSection(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
@ -140,14 +145,14 @@ func Test_sshConfigExecEscape(t *testing.T) {
|
||||||
name string
|
name string
|
||||||
path string
|
path string
|
||||||
wantErr bool
|
wantErr bool
|
||||||
windows bool
|
|
||||||
}{
|
}{
|
||||||
{"no spaces", "simple", false, true},
|
{"windows path", `C:\Program Files\Coder\bin\coder.exe`, false},
|
||||||
{"spaces", "path with spaces", false, true},
|
{"no spaces", "simple", false},
|
||||||
{"quotes", "path with \"quotes\"", false, false},
|
{"spaces", "path with spaces", false},
|
||||||
{"backslashes", "path with \\backslashes", false, false},
|
{"quotes", "path with \"quotes\"", false},
|
||||||
{"tabs", "path with \ttabs", false, false},
|
{"backslashes", "path with \\backslashes", false},
|
||||||
{"newline fails", "path with \nnewline", true, false},
|
{"tabs", "path with \ttabs", false},
|
||||||
|
{"newline fails", "path with \nnewline", true},
|
||||||
}
|
}
|
||||||
for _, tt := range tests {
|
for _, tt := range tests {
|
||||||
tt := tt
|
tt := tt
|
||||||
|
@ -166,7 +171,7 @@ func Test_sshConfigExecEscape(t *testing.T) {
|
||||||
err = os.WriteFile(bin, contents, 0o755) //nolint:gosec
|
err = os.WriteFile(bin, contents, 0o755) //nolint:gosec
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|
||||||
escaped, err := sshConfigExecEscape(bin)
|
escaped, err := sshConfigExecEscape(bin, false)
|
||||||
if tt.wantErr {
|
if tt.wantErr {
|
||||||
require.Error(t, err)
|
require.Error(t, err)
|
||||||
return
|
return
|
||||||
|
@ -181,6 +186,72 @@ func Test_sshConfigExecEscape(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func Test_sshConfigExecEscapeSeparatorForce(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
path string
|
||||||
|
// Behavior is different on Windows
|
||||||
|
expWindowsPath string
|
||||||
|
expOtherPath string
|
||||||
|
forceUnix bool
|
||||||
|
wantErr bool
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "windows_keep_forward_slashes_with_spaces",
|
||||||
|
// Has a space, expect quotes
|
||||||
|
path: `C:\Program Files\Coder\bin\coder.exe`,
|
||||||
|
expWindowsPath: `"C:\Program Files\Coder\bin\coder.exe"`,
|
||||||
|
expOtherPath: `"C:\Program Files\Coder\bin\coder.exe"`,
|
||||||
|
forceUnix: false,
|
||||||
|
wantErr: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "windows_keep_forward_slashes",
|
||||||
|
path: `C:\ProgramFiles\Coder\bin\coder.exe`,
|
||||||
|
expWindowsPath: `C:\ProgramFiles\Coder\bin\coder.exe`,
|
||||||
|
expOtherPath: `C:\ProgramFiles\Coder\bin\coder.exe`,
|
||||||
|
forceUnix: false,
|
||||||
|
wantErr: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "windows_force_unix_with_spaces",
|
||||||
|
path: `C:\Program Files\Coder\bin\coder.exe`,
|
||||||
|
expWindowsPath: `"C:/Program Files/Coder/bin/coder.exe"`,
|
||||||
|
expOtherPath: `"C:\Program Files\Coder\bin\coder.exe"`,
|
||||||
|
forceUnix: true,
|
||||||
|
wantErr: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "windows_force_unix",
|
||||||
|
path: `C:\ProgramFiles\Coder\bin\coder.exe`,
|
||||||
|
expWindowsPath: `C:/ProgramFiles/Coder/bin/coder.exe`,
|
||||||
|
expOtherPath: `C:\ProgramFiles\Coder\bin\coder.exe`,
|
||||||
|
forceUnix: true,
|
||||||
|
wantErr: false,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
for _, tt := range tests {
|
||||||
|
tt := tt
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
found, err := sshConfigExecEscape(tt.path, tt.forceUnix)
|
||||||
|
if tt.wantErr {
|
||||||
|
require.Error(t, err)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
require.NoError(t, err)
|
||||||
|
if runtime.GOOS == "windows" {
|
||||||
|
require.Equal(t, tt.expWindowsPath, found, "(Windows) expected path")
|
||||||
|
} else {
|
||||||
|
// this is a noop on non-windows!
|
||||||
|
require.Equal(t, tt.expOtherPath, found, "(Non-Windows) expected path")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func Test_sshConfigOptions_addOption(t *testing.T) {
|
func Test_sshConfigOptions_addOption(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
testCases := []struct {
|
testCases := []struct {
|
||||||
|
|
|
@ -0,0 +1,5 @@
|
||||||
|
//go:build !windows
|
||||||
|
|
||||||
|
package cli
|
||||||
|
|
||||||
|
var hideForceUnixSlashes = true
|
|
@ -0,0 +1,6 @@
|
||||||
|
//go:build windows
|
||||||
|
|
||||||
|
package cli
|
||||||
|
|
||||||
|
// Must be a var for unit tests to conform behavior
|
||||||
|
var hideForceUnixSlashes = false
|
|
@ -15,6 +15,12 @@ Add an SSH Host entry for your workspaces "ssh coder.workspace"
|
||||||
-n, --dry-run bool, $CODER_SSH_DRY_RUN
|
-n, --dry-run bool, $CODER_SSH_DRY_RUN
|
||||||
Perform a trial run with no changes made, showing a diff at the end.
|
Perform a trial run with no changes made, showing a diff at the end.
|
||||||
|
|
||||||
|
--force-unix-filepaths bool, $CODER_CONFIGSSH_UNIX_FILEPATHS
|
||||||
|
By default, 'config-ssh' uses the os path separator when writing the
|
||||||
|
ssh config. This might be an issue in Windows machine that use a
|
||||||
|
unix-like shell. This flag forces the use of unix file paths (the
|
||||||
|
forward slash '/').
|
||||||
|
|
||||||
--ssh-config-file string, $CODER_SSH_CONFIG_FILE (default: ~/.ssh/config)
|
--ssh-config-file string, $CODER_SSH_CONFIG_FILE (default: ~/.ssh/config)
|
||||||
Specifies the path to an SSH config.
|
Specifies the path to an SSH config.
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue