diff --git a/cli/configssh.go b/cli/configssh.go index 26465bf75fe83..76b9332a69659 100644 --- a/cli/configssh.go +++ b/cli/configssh.go @@ -54,6 +54,7 @@ type sshConfigOptions struct { disableAutostart bool header []string headerCommand string + removedKeys map[string]bool } // addOptions expects options in the form of "option=value" or "option value". @@ -74,30 +75,20 @@ func (o *sshConfigOptions) addOption(option string) error { if err != nil { return err } - for i, existing := range o.sshOptions { - // Override existing option if they share the same key. - // This is case-insensitive. Parsing each time might be a little slow, - // but it is ok. - existingKey, _, err := codersdk.ParseSSHConfigOption(existing) - if err != nil { - // Don't mess with original values if there is an error. - // This could have come from the user's manual edits. - continue - } - if strings.EqualFold(existingKey, key) { - if value == "" { - // Delete existing option. - o.sshOptions = append(o.sshOptions[:i], o.sshOptions[i+1:]...) - } else { - // Override existing option. - o.sshOptions[i] = option - } - return nil - } + lowerKey := strings.ToLower(key) + if o.removedKeys != nil && o.removedKeys[lowerKey] { + // Key marked as removed, skip. + return nil } - // Only append the option if it is not empty. + // Only append the option if it is not empty + // (we interpret empty as removal). if value != "" { o.sshOptions = append(o.sshOptions, option) + } else { + if o.removedKeys == nil { + o.removedKeys = make(map[string]bool) + } + o.removedKeys[lowerKey] = true } return nil } @@ -440,13 +431,17 @@ func (r *RootCmd) configSSH() *serpent.Command { configOptions := sshConfigOpts configOptions.sshOptions = nil - // Add standard options. - err := configOptions.addOptions(defaultOptions...) - if err != nil { - return err + // User options first (SSH only uses the first + // option unless it can be given multiple times) + for _, opt := range sshConfigOpts.sshOptions { + err := configOptions.addOptions(opt) + if err != nil { + return xerrors.Errorf("add flag config option %q: %w", opt, err) + } } - // Override with deployment options + // Deployment options second, allow them to + // override standard options. for k, v := range coderdConfig.SSHConfigOptions { opt := fmt.Sprintf("%s %s", k, v) err := configOptions.addOptions(opt) @@ -454,12 +449,11 @@ func (r *RootCmd) configSSH() *serpent.Command { return xerrors.Errorf("add coderd config option %q: %w", opt, err) } } - // Override with flag options - for _, opt := range sshConfigOpts.sshOptions { - err := configOptions.addOptions(opt) - if err != nil { - return xerrors.Errorf("add flag config option %q: %w", opt, err) - } + + // Finally, add the standard options. + err := configOptions.addOptions(defaultOptions...) + if err != nil { + return err } hostBlock := []string{ diff --git a/cli/configssh_internal_test.go b/cli/configssh_internal_test.go index 732452a761447..16c950af0fd02 100644 --- a/cli/configssh_internal_test.go +++ b/cli/configssh_internal_test.go @@ -272,24 +272,25 @@ func Test_sshConfigOptions_addOption(t *testing.T) { }, }, { - Name: "Replace", + Name: "AddTwo", Start: []string{ "foo bar", }, Add: []string{"Foo baz"}, Expect: []string{ + "foo bar", "Foo baz", }, }, { - Name: "AddAndReplace", + Name: "AddAndRemove", Start: []string{ - "a b", "foo bar", "buzz bazz", }, Add: []string{ "b c", + "a ", // Empty value, means remove all following entries that start with "a", i.e. next line. "A hello", "hello world", }, @@ -297,7 +298,6 @@ func Test_sshConfigOptions_addOption(t *testing.T) { "foo bar", "buzz bazz", "b c", - "A hello", "hello world", }, }, diff --git a/cli/configssh_test.go b/cli/configssh_test.go index f1be8abe8b4b9..81eceb1b8c971 100644 --- a/cli/configssh_test.go +++ b/cli/configssh_test.go @@ -65,7 +65,7 @@ func TestConfigSSH(t *testing.T) { const hostname = "test-coder." const expectedKey = "ConnectionAttempts" - const removeKey = "ConnectionTimeout" + const removeKey = "ConnectTimeout" client, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{ ConfigSSH: codersdk.SSHConfigResponse{ HostnamePrefix: hostname, @@ -620,6 +620,19 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) { regexMatch: `ProxyCommand .* --header-command "printf h1=v1 h2='v2'" ssh`, }, }, + { + name: "Multiple remote forwards", + args: []string{ + "--yes", + "--ssh-option", "RemoteForward 2222 192.168.11.1:2222", + "--ssh-option", "RemoteForward 2223 192.168.11.1:2223", + }, + wantErr: false, + hasAgent: true, + wantConfig: wantConfig{ + regexMatch: "RemoteForward 2222 192.168.11.1:2222.*\n.*RemoteForward 2223 192.168.11.1:2223", + }, + }, } for _, tt := range tests { tt := tt