From f4ec444e54c964f12488384aa5ddc994b1b27066 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 7 Jun 2023 12:39:42 +0000 Subject: [PATCH 1/2] feat(cli): Add wait and no-wait support to config-ssh Refs #7768 --- cli/configssh.go | 90 +++++++++++++++++---- cli/configssh_test.go | 24 +++++- cli/testdata/coder_config-ssh_--help.golden | 11 +++ docs/cli/config-ssh.md | 18 +++++ 4 files changed, 127 insertions(+), 16 deletions(-) diff --git a/cli/configssh.go b/cli/configssh.go index 92da6cb5f8c0b..e913e90919b88 100644 --- a/cli/configssh.go +++ b/cli/configssh.go @@ -45,7 +45,10 @@ const ( // sshConfigOptions represents options that can be stored and read // from the coder config in ~/.ssh/coder. type sshConfigOptions struct { - sshOptions []string + wait bool + noWait bool + userHostPrefix string + sshOptions []string } // addOptions expects options in the form of "option=value" or "option value". @@ -100,10 +103,22 @@ func (o sshConfigOptions) equal(other sshConfigOptions) bool { sort.Strings(opt1) opt2 := slices.Clone(other.sshOptions) sort.Strings(opt2) - return slices.Equal(opt1, opt2) + if !slices.Equal(opt1, opt2) { + return false + } + return o.wait == other.wait && o.noWait == other.noWait && o.userHostPrefix == other.userHostPrefix } func (o sshConfigOptions) asList() (list []string) { + if o.wait { + list = append(list, "wait") + } + if o.noWait { + list = append(list, "no-wait") + } + if o.userHostPrefix != "" { + list = append(list, fmt.Sprintf("ssh-host-prefix: %s", o.userHostPrefix)) + } for _, opt := range o.sshOptions { list = append(list, fmt.Sprintf("ssh-option: %s", opt)) } @@ -178,6 +193,7 @@ func sshPrepareWorkspaceConfigs(ctx context.Context, client *codersdk.Client) (r } } +//nolint:gocyclo func (r *RootCmd) configSSH() *clibase.Cmd { var ( sshConfigFile string @@ -185,7 +201,6 @@ func (r *RootCmd) configSSH() *clibase.Cmd { usePreviousOpts bool dryRun bool skipProxyCommand bool - userHostPrefix string ) client := new(codersdk.Client) cmd := &clibase.Cmd{ @@ -207,6 +222,13 @@ func (r *RootCmd) configSSH() *clibase.Cmd { r.InitClient(client), ), Handler: func(inv *clibase.Invocation) error { + if sshConfigOpts.wait && sshConfigOpts.noWait { + return xerrors.Errorf("cannot specify both --wait and --no-wait") + } + if skipProxyCommand && (sshConfigOpts.wait || sshConfigOpts.noWait) { + return xerrors.Errorf("cannot specify --skip-proxy-command with --wait or --no-wait") + } + recvWorkspaceConfigs := sshPrepareWorkspaceConfigs(inv.Context(), client) out := inv.Stdout @@ -295,7 +317,7 @@ func (r *RootCmd) configSSH() *clibase.Cmd { // Selecting "no" will use the last config. sshConfigOpts = *lastConfig } else { - changes = append(changes, "Use new SSH options") + changes = append(changes, "Use new options") } // Only print when prompts are shown. if yes, _ := inv.ParsedFlags().GetBool("yes"); !yes { @@ -336,9 +358,9 @@ func (r *RootCmd) configSSH() *clibase.Cmd { coderdConfig.HostnamePrefix = "coder." } - if userHostPrefix != "" { + if sshConfigOpts.userHostPrefix != "" { // Override with user flag. - coderdConfig.HostnamePrefix = userHostPrefix + coderdConfig.HostnamePrefix = sshConfigOpts.userHostPrefix } // Ensure stable sorting of output. @@ -363,13 +385,22 @@ func (r *RootCmd) configSSH() *clibase.Cmd { } if !skipProxyCommand { + flags := "" + if sshConfigOpts.wait { + flags += " --wait" + } else if sshConfigOpts.noWait { + flags += " --no-wait" + } defaultOptions = append(defaultOptions, fmt.Sprintf( - "ProxyCommand %s --global-config %s ssh --stdio %s", - escapedCoderBinary, escapedGlobalConfig, workspaceHostname, + "ProxyCommand %s --global-config %s ssh --stdio%s %s", + escapedCoderBinary, escapedGlobalConfig, flags, workspaceHostname, )) } - var configOptions sshConfigOptions + // Create a copy of the options so we can modify them. + configOptions := sshConfigOpts + configOptions.sshOptions = nil + // Add standard options. err := configOptions.addOptions(defaultOptions...) if err != nil { @@ -507,7 +538,19 @@ func (r *RootCmd) configSSH() *clibase.Cmd { Flag: "ssh-host-prefix", Env: "", Description: "Override the default host prefix.", - Value: clibase.StringOf(&userHostPrefix), + Value: clibase.StringOf(&sshConfigOpts.userHostPrefix), + }, + { + Flag: "wait", + Env: "CODER_CONFIGSSH_WAIT", // Not to be mixed with CODER_SSH_WAIT. + Description: "Set the option to wait for the the startup script to finish executing. This is the default if the template has configured the agent startup script behavior as blocking. Can not be used together with --no-wait.", + Value: clibase.BoolOf(&sshConfigOpts.wait), + }, + { + Flag: "no-wait", + Env: "CODER_CONFIGSSH_NO_WAIT", // Not to be mixed with CODER_SSH_NO_WAIT. + Description: "Set the option to enter workspace immediately after the agent has connected. This is the default if the template has configured the agent startup script behavior as non-blocking. Can not be used together with --wait.", + Value: clibase.BoolOf(&sshConfigOpts.noWait), }, cliui.SkipPromptOption(), } @@ -524,12 +567,25 @@ func sshConfigWriteSectionHeader(w io.Writer, addNewline bool, o sshConfigOption _, _ = fmt.Fprint(w, nl+sshStartToken+"\n") _, _ = fmt.Fprint(w, sshConfigSectionHeader) _, _ = fmt.Fprint(w, sshConfigDocsHeader) - if len(o.sshOptions) > 0 { + + var ow strings.Builder + if o.wait { + _, _ = fmt.Fprintf(&ow, "# :%s\n", "wait") + } + if o.noWait { + _, _ = fmt.Fprintf(&ow, "# :%s\n", "no-wait") + } + if o.userHostPrefix != "" { + _, _ = fmt.Fprintf(&ow, "# :%s=%s\n", "ssh-host-prefix", o.userHostPrefix) + } + for _, opt := range o.sshOptions { + _, _ = fmt.Fprintf(&ow, "# :%s=%s\n", "ssh-option", opt) + } + if ow.Len() > 0 { _, _ = fmt.Fprint(w, sshConfigOptionsHeader) - for _, opt := range o.sshOptions { - _, _ = fmt.Fprintf(w, "# :%s=%s\n", "ssh-option", opt) - } + _, _ = fmt.Fprint(w, ow.String()) } + _, _ = fmt.Fprint(w, "#\n") } @@ -545,6 +601,12 @@ func sshConfigParseLastOptions(r io.Reader) (o sshConfigOptions) { line = strings.TrimPrefix(line, "# :") parts := strings.SplitN(line, "=", 2) switch parts[0] { + case "wait": + o.wait = true + case "no-wait": + o.noWait = true + case "user-host-prefix": + o.userHostPrefix = parts[1] case "ssh-option": o.sshOptions = append(o.sshOptions, parts[1]) default: diff --git a/cli/configssh_test.go b/cli/configssh_test.go index 1d1ab44de86cd..c71440210183d 100644 --- a/cli/configssh_test.go +++ b/cli/configssh_test.go @@ -481,12 +481,32 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) { }, args: []string{"--yes"}, }, + { + name: "Serialize supported flags", + wantConfig: wantConfig{ + ssh: strings.Join([]string{ + headerStart, + "# Last config-ssh options:", + "# :wait", + "# :ssh-host-prefix=coder-test.", + "#", + headerEnd, + "", + }, "\n"), + }, + args: []string{ + "--yes", + "--wait", + "--ssh-host-prefix", "coder-test.", + }, + }, { name: "Do not prompt for new options when prev opts flag is set", writeConfig: writeConfig{ ssh: strings.Join([]string{ headerStart, "# Last config-ssh options:", + "# :no-wait", "# :ssh-option=ForwardAgent=yes", "#", headerEnd, @@ -497,6 +517,7 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) { ssh: strings.Join([]string{ headerStart, "# Last config-ssh options:", + "# :no-wait", "# :ssh-option=ForwardAgent=yes", "#", headerEnd, @@ -589,8 +610,7 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) { clitest.SetupConfig(t, client, root) pty := ptytest.New(t) - inv.Stdin = pty.Input() - inv.Stdout = pty.Output() + pty.Attach(inv) done := tGo(t, func() { err := inv.Run() if !tt.wantErr { diff --git a/cli/testdata/coder_config-ssh_--help.golden b/cli/testdata/coder_config-ssh_--help.golden index 299eeb39ddaa6..051a1ca324c03 100644 --- a/cli/testdata/coder_config-ssh_--help.golden +++ b/cli/testdata/coder_config-ssh_--help.golden @@ -15,6 +15,12 @@ Add an SSH Host entry for your workspaces "ssh coder.workspace" -n, --dry-run bool, $CODER_SSH_DRY_RUN Perform a trial run with no changes made, showing a diff at the end. + --no-wait bool, $CODER_CONFIGSSH_NO_WAIT + Set the option to enter workspace immediately after the agent has + connected. This is the default if the template has configured the + agent startup script behavior as non-blocking. Can not be used + together with --wait. + --ssh-config-file string, $CODER_SSH_CONFIG_FILE (default: ~/.ssh/config) Specifies the path to an SSH config. @@ -28,6 +34,11 @@ Add an SSH Host entry for your workspaces "ssh coder.workspace" Specifies whether or not to keep options from previous run of config-ssh. + --wait bool, $CODER_CONFIGSSH_WAIT + Set the option to wait for the the startup script to finish executing. + This is the default if the template has configured the agent startup + script behavior as blocking. Can not be used together with --no-wait. + -y, --yes bool Bypass prompts. diff --git a/docs/cli/config-ssh.md b/docs/cli/config-ssh.md index 9339e9c0d49ff..e6f352839ffdc 100644 --- a/docs/cli/config-ssh.md +++ b/docs/cli/config-ssh.md @@ -34,6 +34,15 @@ coder config-ssh [flags] Perform a trial run with no changes made, showing a diff at the end. +### --no-wait + +| | | +| ----------- | ------------------------------------- | +| Type | bool | +| Environment | $CODER_CONFIGSSH_NO_WAIT | + +Set the option to enter workspace immediately after the agent has connected. This is the default if the template has configured the agent startup script behavior as non-blocking. Can not be used together with --wait. + ### --ssh-config-file | | | @@ -70,6 +79,15 @@ Specifies additional SSH options to embed in each host stanza. Specifies whether or not to keep options from previous run of config-ssh. +### --wait + +| | | +| ----------- | ---------------------------------- | +| Type | bool | +| Environment | $CODER_CONFIGSSH_WAIT | + +Set the option to wait for the the startup script to finish executing. This is the default if the template has configured the agent startup script behavior as blocking. Can not be used together with --no-wait. + ### -y, --yes | | | From 38ea959bde6f5caf14740efec2dbd302340389c3 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 7 Jun 2023 16:39:21 +0000 Subject: [PATCH 2/2] Replace --wait/--no-wait with --wait=yes|no|auto --- cli/configssh.go | 54 ++++++++------------- cli/configssh_test.go | 8 +-- cli/testdata/coder_config-ssh_--help.golden | 16 ++---- docs/cli/config-ssh.md | 23 +++------ 4 files changed, 36 insertions(+), 65 deletions(-) diff --git a/cli/configssh.go b/cli/configssh.go index e913e90919b88..6d20205e865bb 100644 --- a/cli/configssh.go +++ b/cli/configssh.go @@ -45,8 +45,7 @@ const ( // sshConfigOptions represents options that can be stored and read // from the coder config in ~/.ssh/coder. type sshConfigOptions struct { - wait bool - noWait bool + waitEnum string userHostPrefix string sshOptions []string } @@ -106,15 +105,12 @@ func (o sshConfigOptions) equal(other sshConfigOptions) bool { if !slices.Equal(opt1, opt2) { return false } - return o.wait == other.wait && o.noWait == other.noWait && o.userHostPrefix == other.userHostPrefix + return o.waitEnum == other.waitEnum && o.userHostPrefix == other.userHostPrefix } func (o sshConfigOptions) asList() (list []string) { - if o.wait { - list = append(list, "wait") - } - if o.noWait { - list = append(list, "no-wait") + if o.waitEnum != "auto" { + list = append(list, fmt.Sprintf("wait: %s", o.waitEnum)) } if o.userHostPrefix != "" { list = append(list, fmt.Sprintf("ssh-host-prefix: %s", o.userHostPrefix)) @@ -222,11 +218,8 @@ func (r *RootCmd) configSSH() *clibase.Cmd { r.InitClient(client), ), Handler: func(inv *clibase.Invocation) error { - if sshConfigOpts.wait && sshConfigOpts.noWait { - return xerrors.Errorf("cannot specify both --wait and --no-wait") - } - if skipProxyCommand && (sshConfigOpts.wait || sshConfigOpts.noWait) { - return xerrors.Errorf("cannot specify --skip-proxy-command with --wait or --no-wait") + if sshConfigOpts.waitEnum != "auto" && skipProxyCommand { + return xerrors.Errorf("cannot specify both --skip-proxy-command and --wait") } recvWorkspaceConfigs := sshPrepareWorkspaceConfigs(inv.Context(), client) @@ -386,10 +379,8 @@ func (r *RootCmd) configSSH() *clibase.Cmd { if !skipProxyCommand { flags := "" - if sshConfigOpts.wait { - flags += " --wait" - } else if sshConfigOpts.noWait { - flags += " --no-wait" + if sshConfigOpts.waitEnum != "auto" { + flags += " --wait=" + sshConfigOpts.waitEnum } defaultOptions = append(defaultOptions, fmt.Sprintf( "ProxyCommand %s --global-config %s ssh --stdio%s %s", @@ -536,21 +527,16 @@ func (r *RootCmd) configSSH() *clibase.Cmd { }, { Flag: "ssh-host-prefix", - Env: "", + Env: "CODER_CONFIGSSH_SSH_HOST_PREFIX", Description: "Override the default host prefix.", Value: clibase.StringOf(&sshConfigOpts.userHostPrefix), }, { Flag: "wait", Env: "CODER_CONFIGSSH_WAIT", // Not to be mixed with CODER_SSH_WAIT. - Description: "Set the option to wait for the the startup script to finish executing. This is the default if the template has configured the agent startup script behavior as blocking. Can not be used together with --no-wait.", - Value: clibase.BoolOf(&sshConfigOpts.wait), - }, - { - Flag: "no-wait", - Env: "CODER_CONFIGSSH_NO_WAIT", // Not to be mixed with CODER_SSH_NO_WAIT. - Description: "Set the option to enter workspace immediately after the agent has connected. This is the default if the template has configured the agent startup script behavior as non-blocking. Can not be used together with --wait.", - Value: clibase.BoolOf(&sshConfigOpts.noWait), + Description: "Specifies whether or not to wait for the startup script to finish executing. Auto means that the agent startup script behavior configured in the workspace template is used.", + Default: "auto", + Value: clibase.EnumOf(&sshConfigOpts.waitEnum, "yes", "no", "auto"), }, cliui.SkipPromptOption(), } @@ -569,11 +555,8 @@ func sshConfigWriteSectionHeader(w io.Writer, addNewline bool, o sshConfigOption _, _ = fmt.Fprint(w, sshConfigDocsHeader) var ow strings.Builder - if o.wait { - _, _ = fmt.Fprintf(&ow, "# :%s\n", "wait") - } - if o.noWait { - _, _ = fmt.Fprintf(&ow, "# :%s\n", "no-wait") + if o.waitEnum != "auto" { + _, _ = fmt.Fprintf(&ow, "# :%s=%s\n", "wait", o.waitEnum) } if o.userHostPrefix != "" { _, _ = fmt.Fprintf(&ow, "# :%s=%s\n", "ssh-host-prefix", o.userHostPrefix) @@ -594,6 +577,9 @@ func sshConfigWriteSectionEnd(w io.Writer) { } func sshConfigParseLastOptions(r io.Reader) (o sshConfigOptions) { + // Default values. + o.waitEnum = "auto" + s := bufio.NewScanner(r) for s.Scan() { line := s.Text() @@ -602,10 +588,8 @@ func sshConfigParseLastOptions(r io.Reader) (o sshConfigOptions) { parts := strings.SplitN(line, "=", 2) switch parts[0] { case "wait": - o.wait = true - case "no-wait": - o.noWait = true - case "user-host-prefix": + o.waitEnum = parts[1] + case "ssh-host-prefix": o.userHostPrefix = parts[1] case "ssh-option": o.sshOptions = append(o.sshOptions, parts[1]) diff --git a/cli/configssh_test.go b/cli/configssh_test.go index c71440210183d..f502304373f80 100644 --- a/cli/configssh_test.go +++ b/cli/configssh_test.go @@ -487,7 +487,7 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) { ssh: strings.Join([]string{ headerStart, "# Last config-ssh options:", - "# :wait", + "# :wait=yes", "# :ssh-host-prefix=coder-test.", "#", headerEnd, @@ -496,7 +496,7 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) { }, args: []string{ "--yes", - "--wait", + "--wait=yes", "--ssh-host-prefix", "coder-test.", }, }, @@ -506,7 +506,7 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) { ssh: strings.Join([]string{ headerStart, "# Last config-ssh options:", - "# :no-wait", + "# :wait=no", "# :ssh-option=ForwardAgent=yes", "#", headerEnd, @@ -517,7 +517,7 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) { ssh: strings.Join([]string{ headerStart, "# Last config-ssh options:", - "# :no-wait", + "# :wait=no", "# :ssh-option=ForwardAgent=yes", "#", headerEnd, diff --git a/cli/testdata/coder_config-ssh_--help.golden b/cli/testdata/coder_config-ssh_--help.golden index 051a1ca324c03..712c958ad3b88 100644 --- a/cli/testdata/coder_config-ssh_--help.golden +++ b/cli/testdata/coder_config-ssh_--help.golden @@ -15,16 +15,10 @@ Add an SSH Host entry for your workspaces "ssh coder.workspace" -n, --dry-run bool, $CODER_SSH_DRY_RUN Perform a trial run with no changes made, showing a diff at the end. - --no-wait bool, $CODER_CONFIGSSH_NO_WAIT - Set the option to enter workspace immediately after the agent has - connected. This is the default if the template has configured the - agent startup script behavior as non-blocking. Can not be used - together with --wait. - --ssh-config-file string, $CODER_SSH_CONFIG_FILE (default: ~/.ssh/config) Specifies the path to an SSH config. - --ssh-host-prefix string + --ssh-host-prefix string, $CODER_CONFIGSSH_SSH_HOST_PREFIX Override the default host prefix. -o, --ssh-option string-array, $CODER_SSH_CONFIG_OPTS @@ -34,10 +28,10 @@ Add an SSH Host entry for your workspaces "ssh coder.workspace" Specifies whether or not to keep options from previous run of config-ssh. - --wait bool, $CODER_CONFIGSSH_WAIT - Set the option to wait for the the startup script to finish executing. - This is the default if the template has configured the agent startup - script behavior as blocking. Can not be used together with --no-wait. + --wait yes|no|auto, $CODER_CONFIGSSH_WAIT (default: auto) + Specifies whether or not to wait for the startup script to finish + executing. Auto means that the agent startup script behavior + configured in the workspace template is used. -y, --yes bool Bypass prompts. diff --git a/docs/cli/config-ssh.md b/docs/cli/config-ssh.md index e6f352839ffdc..6178e207e1d30 100644 --- a/docs/cli/config-ssh.md +++ b/docs/cli/config-ssh.md @@ -34,15 +34,6 @@ coder config-ssh [flags] Perform a trial run with no changes made, showing a diff at the end. -### --no-wait - -| | | -| ----------- | ------------------------------------- | -| Type | bool | -| Environment | $CODER_CONFIGSSH_NO_WAIT | - -Set the option to enter workspace immediately after the agent has connected. This is the default if the template has configured the agent startup script behavior as non-blocking. Can not be used together with --wait. - ### --ssh-config-file | | | @@ -55,9 +46,10 @@ Specifies the path to an SSH config. ### --ssh-host-prefix -| | | -| ---- | ------------------- | -| Type | string | +| | | +| ----------- | --------------------------------------------- | +| Type | string | +| Environment | $CODER_CONFIGSSH_SSH_HOST_PREFIX | Override the default host prefix. @@ -82,11 +74,12 @@ Specifies whether or not to keep options from previous run of config-ssh. ### --wait | | | -| ----------- | ---------------------------------- | -| Type | bool | +| ----------- | ---------------------------------- | --- | ------------ | +| Type | enum[yes | no | auto] | | Environment | $CODER_CONFIGSSH_WAIT | +| Default | auto | -Set the option to wait for the the startup script to finish executing. This is the default if the template has configured the agent startup script behavior as blocking. Can not be used together with --no-wait. +Specifies whether or not to wait for the startup script to finish executing. Auto means that the agent startup script behavior configured in the workspace template is used. ### -y, --yes