Skip to content

Commit b0854aa

Browse files
spikecurtisspikecurtis
authored
feat: modify config-ssh to check for Coder Connect (coder#17419)
relates to coder#16828 Changes SSH config so that suffixes only match if Coder Connect is not running / available. This means that we will use the existing Coder Connect tunnel if it is available, rather than creating a new tunnel via `coder ssh --stdio`.
1 parent 3b54254 commit b0854aa

File tree

2 files changed

+166
-132
lines changed

2 files changed

+166
-132
lines changed

‎cli/configssh.go

Lines changed: 155 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,17 @@ const (
4848
type sshConfigOptions struct {
4949
waitEnum string
5050
// Deprecated: moving away from prefix to hostnameSuffix
51-
userHostPrefix string
52-
hostnameSuffix string
53-
sshOptions []string
54-
disableAutostart bool
55-
header []string
56-
headerCommand string
57-
removedKeys map[string]bool
51+
userHostPrefix string
52+
hostnameSuffix string
53+
sshOptions []string
54+
disableAutostart bool
55+
header []string
56+
headerCommand string
57+
removedKeys map[string]bool
58+
globalConfigPath string
59+
coderBinaryPath string
60+
skipProxyCommand bool
61+
forceUnixSeparators bool
5862
}
5963

6064
// addOptions expects options in the form of "option=value" or "option value".
@@ -107,6 +111,80 @@ func (o sshConfigOptions) equal(other sshConfigOptions) bool {
107111
o.hostnameSuffix == other.hostnameSuffix
108112
}
109113

114+
func (o sshConfigOptions) writeToBuffer(buf *bytes.Buffer) error {
115+
escapedCoderBinary, err := sshConfigExecEscape(o.coderBinaryPath, o.forceUnixSeparators)
116+
if err != nil {
117+
return xerrors.Errorf("escape coder binary for ssh failed: %w", err)
118+
}
119+
120+
escapedGlobalConfig, err := sshConfigExecEscape(o.globalConfigPath, o.forceUnixSeparators)
121+
if err != nil {
122+
return xerrors.Errorf("escape global config for ssh failed: %w", err)
123+
}
124+
125+
rootFlags := fmt.Sprintf("--global-config %s", escapedGlobalConfig)
126+
for _, h := range o.header {
127+
rootFlags += fmt.Sprintf(" --header %q", h)
128+
}
129+
if o.headerCommand != "" {
130+
rootFlags += fmt.Sprintf(" --header-command %q", o.headerCommand)
131+
}
132+
133+
flags := ""
134+
if o.waitEnum != "auto" {
135+
flags += " --wait=" + o.waitEnum
136+
}
137+
if o.disableAutostart {
138+
flags += " --disable-autostart=true"
139+
}
140+
141+
// Prefix block:
142+
if o.userHostPrefix != "" {
143+
_, _ = buf.WriteString("Host")
144+
145+
_, _ = buf.WriteString(" ")
146+
_, _ = buf.WriteString(o.userHostPrefix)
147+
_, _ = buf.WriteString("*\n")
148+
149+
for _, v := range o.sshOptions {
150+
_, _ = buf.WriteString("\t")
151+
_, _ = buf.WriteString(v)
152+
_, _ = buf.WriteString("\n")
153+
}
154+
if !o.skipProxyCommand && o.userHostPrefix != "" {
155+
_, _ = buf.WriteString("\t")
156+
_, _ = fmt.Fprintf(buf,
157+
"ProxyCommand %s %s ssh --stdio%s --ssh-host-prefix %s %%h",
158+
escapedCoderBinary, rootFlags, flags, o.userHostPrefix,
159+
)
160+
_, _ = buf.WriteString("\n")
161+
}
162+
}
163+
164+
// Suffix block
165+
if o.hostnameSuffix == "" {
166+
return nil
167+
}
168+
_, _ = fmt.Fprintf(buf, "\nHost *.%s\n", o.hostnameSuffix)
169+
for _, v := range o.sshOptions {
170+
_, _ = buf.WriteString("\t")
171+
_, _ = buf.WriteString(v)
172+
_, _ = buf.WriteString("\n")
173+
}
174+
// the ^^ options should always apply, but we only want to use the proxy command if Coder Connect is not running.
175+
if !o.skipProxyCommand {
176+
_, _ = fmt.Fprintf(buf, "\nMatch host *.%s !exec \"%s connect exists %%h\"\n",
177+
o.hostnameSuffix, escapedCoderBinary)
178+
_, _ = buf.WriteString("\t")
179+
_, _ = fmt.Fprintf(buf,
180+
"ProxyCommand %s %s ssh --stdio%s --hostname-suffix %s %%h",
181+
escapedCoderBinary, rootFlags, flags, o.hostnameSuffix,
182+
)
183+
_, _ = buf.WriteString("\n")
184+
}
185+
return nil
186+
}
187+
110188
// slicesSortedEqual compares two slices without side-effects or regard to order.
111189
func slicesSortedEqual[S ~[]E, E constraints.Ordered](a, b S) bool {
112190
if len(a) != len(b) {
@@ -147,13 +225,11 @@ func (o sshConfigOptions) asList() (list []string) {
147225

148226
func (r *RootCmd) configSSH() *serpent.Command {
149227
var (
150-
sshConfigFile string
151-
sshConfigOpts sshConfigOptions
152-
usePreviousOpts bool
153-
dryRun bool
154-
skipProxyCommand bool
155-
forceUnixSeparators bool
156-
coderCliPath string
228+
sshConfigFile string
229+
sshConfigOpts sshConfigOptions
230+
usePreviousOpts bool
231+
dryRun bool
232+
coderCliPath string
157233
)
158234
client := new(codersdk.Client)
159235
cmd := &serpent.Command{
@@ -177,7 +253,7 @@ func (r *RootCmd) configSSH() *serpent.Command {
177253
Handler: func(inv *serpent.Invocation) error {
178254
ctx := inv.Context()
179255

180-
if sshConfigOpts.waitEnum != "auto" && skipProxyCommand {
256+
if sshConfigOpts.waitEnum != "auto" && sshConfigOpts.skipProxyCommand {
181257
// The wait option is applied to the ProxyCommand. If the user
182258
// specifies skip-proxy-command, then wait cannot be applied.
183259
return xerrors.Errorf("cannot specify both --skip-proxy-command and --wait")
@@ -207,18 +283,7 @@ func (r *RootCmd) configSSH() *serpent.Command {
207283
return err
208284
}
209285
}
210-
211-
escapedCoderBinary, err := sshConfigExecEscape(coderBinary, forceUnixSeparators)
212-
if err != nil {
213-
return xerrors.Errorf("escape coder binary for ssh failed: %w", err)
214-
}
215-
216286
root := r.createConfig()
217-
escapedGlobalConfig, err := sshConfigExecEscape(string(root), forceUnixSeparators)
218-
if err != nil {
219-
return xerrors.Errorf("escape global config for ssh failed: %w", err)
220-
}
221-
222287
homedir, err := os.UserHomeDir()
223288
if err != nil {
224289
return xerrors.Errorf("user home dir failed: %w", err)
@@ -320,94 +385,15 @@ func (r *RootCmd) configSSH() *serpent.Command {
320385
coderdConfig.HostnamePrefix = "coder."
321386
}
322387

323-
if sshConfigOpts.userHostPrefix != "" {
324-
// Override with user flag.
325-
coderdConfig.HostnamePrefix = sshConfigOpts.userHostPrefix
326-
}
327-
if sshConfigOpts.hostnameSuffix != "" {
328-
// Override with user flag.
329-
coderdConfig.HostnameSuffix = sshConfigOpts.hostnameSuffix
330-
}
331-
332-
// Write agent configuration.
333-
defaultOptions := []string{
334-
"ConnectTimeout=0",
335-
"StrictHostKeyChecking=no",
336-
// Without this, the "REMOTE HOST IDENTITY CHANGED"
337-
// message will appear.
338-
"UserKnownHostsFile=/dev/null",
339-
// This disables the "Warning: Permanently added 'hostname' (RSA) to the list of known hosts."
340-
// message from appearing on every SSH. This happens because we ignore the known hosts.
341-
"LogLevel ERROR",
342-
}
343-
344-
if !skipProxyCommand {
345-
rootFlags := fmt.Sprintf("--global-config %s", escapedGlobalConfig)
346-
for _, h := range sshConfigOpts.header {
347-
rootFlags += fmt.Sprintf(" --header %q", h)
348-
}
349-
if sshConfigOpts.headerCommand != "" {
350-
rootFlags += fmt.Sprintf(" --header-command %q", sshConfigOpts.headerCommand)
351-
}
352-
353-
flags := ""
354-
if sshConfigOpts.waitEnum != "auto" {
355-
flags += " --wait=" + sshConfigOpts.waitEnum
356-
}
357-
if sshConfigOpts.disableAutostart {
358-
flags += " --disable-autostart=true"
359-
}
360-
if coderdConfig.HostnamePrefix != "" {
361-
flags += " --ssh-host-prefix " + coderdConfig.HostnamePrefix
362-
}
363-
if coderdConfig.HostnameSuffix != "" {
364-
flags += " --hostname-suffix " + coderdConfig.HostnameSuffix
365-
}
366-
defaultOptions = append(defaultOptions, fmt.Sprintf(
367-
"ProxyCommand %s %s ssh --stdio%s %%h",
368-
escapedCoderBinary, rootFlags, flags,
369-
))
370-
}
371-
372-
// Create a copy of the options so we can modify them.
373-
configOptions := sshConfigOpts
374-
configOptions.sshOptions = nil
375-
376-
// User options first (SSH only uses the first
377-
// option unless it can be given multiple times)
378-
for _, opt := range sshConfigOpts.sshOptions {
379-
err := configOptions.addOptions(opt)
380-
if err != nil {
381-
return xerrors.Errorf("add flag config option %q: %w", opt, err)
382-
}
383-
}
384-
385-
// Deployment options second, allow them to
386-
// override standard options.
387-
for k, v := range coderdConfig.SSHConfigOptions {
388-
opt := fmt.Sprintf("%s %s", k, v)
389-
err := configOptions.addOptions(opt)
390-
if err != nil {
391-
return xerrors.Errorf("add coderd config option %q: %w", opt, err)
392-
}
393-
}
394-
395-
// Finally, add the standard options.
396-
if err := configOptions.addOptions(defaultOptions...); err != nil {
388+
configOptions, err := mergeSSHOptions(sshConfigOpts, coderdConfig, string(root), coderBinary)
389+
if err != nil {
397390
return err
398391
}
399-
400-
hostBlock := []string{
401-
sshConfigHostLinePatterns(coderdConfig),
402-
}
403-
// Prefix with '\t'
404-
for _, v := range configOptions.sshOptions {
405-
hostBlock = append(hostBlock, "\t"+v)
392+
err = configOptions.writeToBuffer(buf)
393+
if err != nil {
394+
return err
406395
}
407396

408-
_, _ = buf.WriteString(strings.Join(hostBlock, "\n"))
409-
_ = buf.WriteByte('\n')
410-
411397
sshConfigWriteSectionEnd(buf)
412398

413399
// Write the remainder of the users config file to buf.
@@ -523,7 +509,7 @@ func (r *RootCmd) configSSH() *serpent.Command {
523509
Flag: "skip-proxy-command",
524510
Env: "CODER_SSH_SKIP_PROXY_COMMAND",
525511
Description: "Specifies whether the ProxyCommand option should be skipped. Useful for testing.",
526-
Value: serpent.BoolOf(&skipProxyCommand),
512+
Value: serpent.BoolOf(&sshConfigOpts.skipProxyCommand),
527513
Hidden: true,
528514
},
529515
{
@@ -564,7 +550,7 @@ func (r *RootCmd) configSSH() *serpent.Command {
564550
Description: "By default, 'config-ssh' uses the os path separator when writing the ssh config. " +
565551
"This might be an issue in Windows machine that use a unix-like shell. " +
566552
"This flag forces the use of unix file paths (the forward slash '/').",
567-
Value: serpent.BoolOf(&forceUnixSeparators),
553+
Value: serpent.BoolOf(&sshConfigOpts.forceUnixSeparators),
568554
// On non-windows showing this command is useless because it is a noop.
569555
// Hide vs disable it though so if a command is copied from a Windows
570556
// machine to a unix machine it will still work and not throw an
@@ -577,6 +563,63 @@ func (r *RootCmd) configSSH() *serpent.Command {
577563
return cmd
578564
}
579565

566+
func mergeSSHOptions(
567+
user sshConfigOptions, coderd codersdk.SSHConfigResponse, globalConfigPath, coderBinaryPath string,
568+
) (
569+
sshConfigOptions, error,
570+
) {
571+
// Write agent configuration.
572+
defaultOptions := []string{
573+
"ConnectTimeout=0",
574+
"StrictHostKeyChecking=no",
575+
// Without this, the "REMOTE HOST IDENTITY CHANGED"
576+
// message will appear.
577+
"UserKnownHostsFile=/dev/null",
578+
// This disables the "Warning: Permanently added 'hostname' (RSA) to the list of known hosts."
579+
// message from appearing on every SSH. This happens because we ignore the known hosts.
580+
"LogLevel ERROR",
581+
}
582+
583+
// Create a copy of the options so we can modify them.
584+
configOptions := user
585+
configOptions.sshOptions = nil
586+
587+
configOptions.globalConfigPath = globalConfigPath
588+
configOptions.coderBinaryPath = coderBinaryPath
589+
// user config takes precedence
590+
if user.userHostPrefix == "" {
591+
configOptions.userHostPrefix = coderd.HostnamePrefix
592+
}
593+
if user.hostnameSuffix == "" {
594+
configOptions.hostnameSuffix = coderd.HostnameSuffix
595+
}
596+
597+
// User options first (SSH only uses the first
598+
// option unless it can be given multiple times)
599+
for _, opt := range user.sshOptions {
600+
err := configOptions.addOptions(opt)
601+
if err != nil {
602+
return sshConfigOptions{}, xerrors.Errorf("add flag config option %q: %w", opt, err)
603+
}
604+
}
605+
606+
// Deployment options second, allow them to
607+
// override standard options.
608+
for k, v := range coderd.SSHConfigOptions {
609+
opt := fmt.Sprintf("%s %s", k, v)
610+
err := configOptions.addOptions(opt)
611+
if err != nil {
612+
return sshConfigOptions{}, xerrors.Errorf("add coderd config option %q: %w", opt, err)
613+
}
614+
}
615+
616+
// Finally, add the standard options.
617+
if err := configOptions.addOptions(defaultOptions...); err != nil {
618+
return sshConfigOptions{}, err
619+
}
620+
return configOptions, nil
621+
}
622+
580623
//nolint:revive
581624
func sshConfigWriteSectionHeader(w io.Writer, addNewline bool, o sshConfigOptions) {
582625
nl := "\n"
@@ -844,19 +887,3 @@ func diffBytes(name string, b1, b2 []byte, color bool) ([]byte, error) {
844887
}
845888
return b, nil
846889
}
847-
848-
func sshConfigHostLinePatterns(config codersdk.SSHConfigResponse) string {
849-
builder := strings.Builder{}
850-
// by inspection, WriteString always returns nil error
851-
_, _ = builder.WriteString("Host")
852-
if config.HostnamePrefix != "" {
853-
_, _ = builder.WriteString(" ")
854-
_, _ = builder.WriteString(config.HostnamePrefix)
855-
_, _ = builder.WriteString("*")
856-
}
857-
if config.HostnameSuffix != "" {
858-
_, _ = builder.WriteString(" *.")
859-
_, _ = builder.WriteString(config.HostnameSuffix)
860-
}
861-
return builder.String()
862-
}

‎cli/configssh_test.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -615,13 +615,21 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) {
615615
name: "Hostname Suffix",
616616
args: []string{
617617
"--yes",
618+
"--ssh-option", "Foo=bar",
618619
"--hostname-suffix", "testy",
619620
},
620621
wantErr: false,
621622
hasAgent: true,
622623
wantConfig: wantConfig{
623-
ssh: []string{"Host coder.* *.testy"},
624-
regexMatch: `ProxyCommand .* ssh .* --hostname-suffix testy %h`,
624+
ssh: []string{
625+
"Host *.testy",
626+
"Foo=bar",
627+
"ConnectTimeout=0",
628+
"StrictHostKeyChecking=no",
629+
"UserKnownHostsFile=/dev/null",
630+
"LogLevel ERROR",
631+
},
632+
regexMatch: `Match host \*\.testy !exec ".* connect exists %h"\n\tProxyCommand .* ssh .* --hostname-suffix testy %h`,
625633
},
626634
},
627635
{
@@ -634,8 +642,7 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) {
634642
wantErr: false,
635643
hasAgent: true,
636644
wantConfig: wantConfig{
637-
ssh: []string{"Host presto.* *.testy"},
638-
regexMatch: `ProxyCommand .* ssh .* --ssh-host-prefix presto\. --hostname-suffix testy %h`,
645+
ssh: []string{"Host presto.*", "Match host *.testy !exec"},
639646
},
640647
},
641648
}

0 commit comments

Comments
 (0)