- Notifications
You must be signed in to change notification settings - Fork907
feat: modify config-ssh to check for Coder Connect#17419
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Uh oh!
There was an error while loading.Please reload this page.
Merged
Changes fromall commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
283 changes: 155 additions & 128 deletionscli/configssh.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -48,13 +48,17 @@ const ( | ||
| type sshConfigOptions struct { | ||
| waitEnum string | ||
| // Deprecated: moving away from prefix to hostnameSuffix | ||
| userHostPrefix string | ||
| hostnameSuffix string | ||
| sshOptions []string | ||
| disableAutostart bool | ||
| header []string | ||
| headerCommand string | ||
| removedKeys map[string]bool | ||
| globalConfigPath string | ||
| coderBinaryPath string | ||
| skipProxyCommand bool | ||
| forceUnixSeparators bool | ||
| } | ||
| // addOptions expects options in the form of "option=value" or "option value". | ||
| @@ -107,6 +111,80 @@ func (o sshConfigOptions) equal(other sshConfigOptions) bool { | ||
| o.hostnameSuffix == other.hostnameSuffix | ||
| } | ||
| func (o sshConfigOptions) writeToBuffer(buf *bytes.Buffer) error { | ||
spikecurtis marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| escapedCoderBinary, err := sshConfigExecEscape(o.coderBinaryPath, o.forceUnixSeparators) | ||
| if err != nil { | ||
| return xerrors.Errorf("escape coder binary for ssh failed: %w", err) | ||
| } | ||
| escapedGlobalConfig, err := sshConfigExecEscape(o.globalConfigPath, o.forceUnixSeparators) | ||
| if err != nil { | ||
| return xerrors.Errorf("escape global config for ssh failed: %w", err) | ||
| } | ||
| rootFlags := fmt.Sprintf("--global-config %s", escapedGlobalConfig) | ||
| for _, h := range o.header { | ||
| rootFlags += fmt.Sprintf(" --header %q", h) | ||
| } | ||
| if o.headerCommand != "" { | ||
| rootFlags += fmt.Sprintf(" --header-command %q", o.headerCommand) | ||
| } | ||
| flags := "" | ||
| if o.waitEnum != "auto" { | ||
| flags += " --wait=" + o.waitEnum | ||
| } | ||
| if o.disableAutostart { | ||
| flags += " --disable-autostart=true" | ||
| } | ||
| // Prefix block: | ||
| if o.userHostPrefix != "" { | ||
| _, _ = buf.WriteString("Host") | ||
| _, _ = buf.WriteString(" ") | ||
| _, _ = buf.WriteString(o.userHostPrefix) | ||
| _, _ = buf.WriteString("*\n") | ||
| for _, v := range o.sshOptions { | ||
| _, _ = buf.WriteString("\t") | ||
| _, _ = buf.WriteString(v) | ||
| _, _ = buf.WriteString("\n") | ||
| } | ||
| if !o.skipProxyCommand && o.userHostPrefix != "" { | ||
| _, _ = buf.WriteString("\t") | ||
| _, _ = fmt.Fprintf(buf, | ||
| "ProxyCommand %s %s ssh --stdio%s --ssh-host-prefix %s %%h", | ||
| escapedCoderBinary, rootFlags, flags, o.userHostPrefix, | ||
| ) | ||
| _, _ = buf.WriteString("\n") | ||
| } | ||
| } | ||
| // Suffix block | ||
| if o.hostnameSuffix == "" { | ||
| return nil | ||
| } | ||
| _, _ = fmt.Fprintf(buf, "\nHost *.%s\n", o.hostnameSuffix) | ||
| for _, v := range o.sshOptions { | ||
| _, _ = buf.WriteString("\t") | ||
| _, _ = buf.WriteString(v) | ||
| _, _ = buf.WriteString("\n") | ||
| } | ||
| // the ^^ options should always apply, but we only want to use the proxy command if Coder Connect is not running. | ||
| if !o.skipProxyCommand { | ||
| _, _ = fmt.Fprintf(buf, "\nMatch host *.%s !exec \"%s connect exists %%h\"\n", | ||
| o.hostnameSuffix, escapedCoderBinary) | ||
| _, _ = buf.WriteString("\t") | ||
| _, _ = fmt.Fprintf(buf, | ||
| "ProxyCommand %s %s ssh --stdio%s --hostname-suffix %s %%h", | ||
| escapedCoderBinary, rootFlags, flags, o.hostnameSuffix, | ||
| ) | ||
| _, _ = buf.WriteString("\n") | ||
| } | ||
| return nil | ||
| } | ||
| // slicesSortedEqual compares two slices without side-effects or regard to order. | ||
| func slicesSortedEqual[S ~[]E, E constraints.Ordered](a, b S) bool { | ||
| if len(a) != len(b) { | ||
| @@ -147,13 +225,11 @@ func (o sshConfigOptions) asList() (list []string) { | ||
| func (r *RootCmd) configSSH() *serpent.Command { | ||
| var ( | ||
| sshConfigFile string | ||
| sshConfigOpts sshConfigOptions | ||
| usePreviousOpts bool | ||
| dryRun bool | ||
| coderCliPath string | ||
| ) | ||
| client := new(codersdk.Client) | ||
| cmd := &serpent.Command{ | ||
| @@ -177,7 +253,7 @@ func (r *RootCmd) configSSH() *serpent.Command { | ||
| Handler: func(inv *serpent.Invocation) error { | ||
| ctx := inv.Context() | ||
| if sshConfigOpts.waitEnum != "auto" &&sshConfigOpts.skipProxyCommand { | ||
| // The wait option is applied to the ProxyCommand. If the user | ||
| // specifies skip-proxy-command, then wait cannot be applied. | ||
| return xerrors.Errorf("cannot specify both --skip-proxy-command and --wait") | ||
| @@ -207,18 +283,7 @@ func (r *RootCmd) configSSH() *serpent.Command { | ||
| return err | ||
| } | ||
| } | ||
| root := r.createConfig() | ||
| homedir, err := os.UserHomeDir() | ||
| if err != nil { | ||
| return xerrors.Errorf("user home dir failed: %w", err) | ||
| @@ -320,94 +385,15 @@ func (r *RootCmd) configSSH() *serpent.Command { | ||
| coderdConfig.HostnamePrefix = "coder." | ||
| } | ||
| configOptions, err := mergeSSHOptions(sshConfigOpts, coderdConfig, string(root), coderBinary) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| err = configOptions.writeToBuffer(buf) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| sshConfigWriteSectionEnd(buf) | ||
| // Write the remainder of the users config file to buf. | ||
| @@ -523,7 +509,7 @@ func (r *RootCmd) configSSH() *serpent.Command { | ||
| Flag: "skip-proxy-command", | ||
| Env: "CODER_SSH_SKIP_PROXY_COMMAND", | ||
| Description: "Specifies whether the ProxyCommand option should be skipped. Useful for testing.", | ||
| Value: serpent.BoolOf(&sshConfigOpts.skipProxyCommand), | ||
| Hidden: true, | ||
| }, | ||
| { | ||
| @@ -564,7 +550,7 @@ func (r *RootCmd) configSSH() *serpent.Command { | ||
| 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: serpent.BoolOf(&sshConfigOpts.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 | ||
| @@ -577,6 +563,63 @@ func (r *RootCmd) configSSH() *serpent.Command { | ||
| return cmd | ||
| } | ||
| func mergeSSHOptions( | ||
| user sshConfigOptions, coderd codersdk.SSHConfigResponse, globalConfigPath, coderBinaryPath string, | ||
| ) ( | ||
| sshConfigOptions, error, | ||
| ) { | ||
| // Write agent configuration. | ||
| defaultOptions := []string{ | ||
| "ConnectTimeout=0", | ||
| "StrictHostKeyChecking=no", | ||
| // Without this, the "REMOTE HOST IDENTITY CHANGED" | ||
| // message will appear. | ||
| "UserKnownHostsFile=/dev/null", | ||
| // This disables the "Warning: Permanently added 'hostname' (RSA) to the list of known hosts." | ||
| // message from appearing on every SSH. This happens because we ignore the known hosts. | ||
| "LogLevel ERROR", | ||
| } | ||
| // Create a copy of the options so we can modify them. | ||
| configOptions := user | ||
| configOptions.sshOptions = nil | ||
| configOptions.globalConfigPath = globalConfigPath | ||
| configOptions.coderBinaryPath = coderBinaryPath | ||
| // user config takes precedence | ||
| if user.userHostPrefix == "" { | ||
| configOptions.userHostPrefix = coderd.HostnamePrefix | ||
| } | ||
| if user.hostnameSuffix == "" { | ||
| configOptions.hostnameSuffix = coderd.HostnameSuffix | ||
| } | ||
| // User options first (SSH only uses the first | ||
| // option unless it can be given multiple times) | ||
| for _, opt := range user.sshOptions { | ||
| err := configOptions.addOptions(opt) | ||
| if err != nil { | ||
| return sshConfigOptions{}, xerrors.Errorf("add flag config option %q: %w", opt, err) | ||
| } | ||
| } | ||
| // Deployment options second, allow them to | ||
| // override standard options. | ||
| for k, v := range coderd.SSHConfigOptions { | ||
| opt := fmt.Sprintf("%s %s", k, v) | ||
| err := configOptions.addOptions(opt) | ||
| if err != nil { | ||
| return sshConfigOptions{}, xerrors.Errorf("add coderd config option %q: %w", opt, err) | ||
| } | ||
| } | ||
| // Finally, add the standard options. | ||
| if err := configOptions.addOptions(defaultOptions...); err != nil { | ||
| return sshConfigOptions{}, err | ||
| } | ||
| return configOptions, nil | ||
| } | ||
| //nolint:revive | ||
| func sshConfigWriteSectionHeader(w io.Writer, addNewline bool, o sshConfigOptions) { | ||
| nl := "\n" | ||
| @@ -844,19 +887,3 @@ func diffBytes(name string, b1, b2 []byte, color bool) ([]byte, error) { | ||
| } | ||
| return b, nil | ||
| } | ||
15 changes: 11 additions & 4 deletionscli/configssh_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
Oops, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.