feat(cli): support header and header-command in config-ssh #10413
Conversation
|
Hey @JoshVee, thanks for the PR and sorry for the long wait-time. I think this PR looks good, but there's a change I'd like to see before we merge it, and that's support for serializing the header command into the options. For instance, compare these two: ❯ ./coder config-ssh --header-command 'echo foo=bar'
> The following changes will be made to your SSH configuration:
* Update the coder section in /home/coder/.ssh/coder
Continue? (yes/no)yesVersus: ❯ ./coder config-ssh --header-command 'echo foo=bar' -o ForwardAgent=yes
> New options differ from previous options:
New options:
* ssh-option: ForwardAgent=yes
Previous options: none (<- should list header command)
Use new options? (yes/no)yes
> The following changes will be made to your SSH configuration:
* Use new options
* Update the coder section in /home/coder/.ssh/coder
Continue? (yes/no)yesNow let's say we forget about the header command next run, there's only going to be a cryptic change to the SSH config, and users may be left wondering why SSH isn't working afterwards (the update removes the header command, but doesn't say that the options/settings have changed): ❯ ./coder config-ssh -o ForwardAgent=yes
> The following changes will be made to your SSH configuration:
* Update the coder section in /home/coder/.ssh/coder
Continue? (yes/no)The options are serialized under # ------------START-CODER-----------
# This section is managed by coder. DO NOT EDIT.
#
# You should not hand-edit this section unless you are removing it, all
# changes will be lost when running "coder config-ssh".
#
# Last config-ssh options:
# :ssh-option=ForwardAgent=yes
#I would suggest something like |
|
@JoshVee are you still interested in working on this PR? If not, we can see if someone on the team has the bandwidth to tackle it. |
| rootFlags += fmt.Sprintf(" --header %q", h) | ||
| } | ||
| if sshConfigOpts.headerCommand != "" { | ||
| rootFlags += fmt.Sprintf(" --header-command %q", sshConfigOpts.headerCommand) |
There was a problem hiding this comment.
Review: We shouldn't need to do special escaping here like we do for the coder binary, that's because we're simply passing a string argument to coder.
|
Updated the PR with changes to support serialization, while I was at it, I included support for ❯ ./coder config-ssh --header-command 'echo "foo=bar"' -o ForwardAgent=yes
> New options differ from previous options:
New options:
* ssh-option: ForwardAgent=yes
* header-command: echo "foo=bar"
Previous options:
* ssh-option: ForwardAgent=yes
* header: foo=barReady for review @Emyrk. |
| a = slices.Clone(a) | ||
| slices.Sort(a) | ||
| b = slices.Clone(b) | ||
| slices.Sort(b) |
There was a problem hiding this comment.
nit: curious, can we replace slices.Clone with a simple copy to omit cloning and just operate on same objects?
There was a problem hiding this comment.
// Clone returns a copy of the slice.
// The elements are copied using assignment, so this is a shallow clone.
It's actually just a simple copy, so I think it's fine.
|
Thanks for your contribution @JoshVee, this finally went in! |
This PR modifies the
config-sshcommand so that it includes any provided--header-command\CODER_HEADER_COMMANDin the generated SSHProxyCommand.This ensures that any custom headers are also available when running
ssh coder.workspace.The avoids the need to always rely on having the
CODER_HEADER_COMMANDenvironment variable set globally.