gh cs ssh cli integration with openssh config#4915
Conversation
mislav
left a comment
There was a problem hiding this comment.
Thanks for this! The direction generally looks good, but I'm wondering about whether this overloads the gh cs ssh command too much.
I agree. What makes sense as an alternative? |
We could also move this to a toplevel command, but I don't want to pollute that namespace too much. Open to suggestions.
This reverts commit 456f438.
When running `gh cs ssh config` without a `-c` option, we skip codespaces that aren't available. This change suppresses that behavior when a single codespace is explicitly requested, starting the codespace if it's not running.
mislav
left a comment
There was a problem hiding this comment.
Looking pretty good! My final notes are mostly about error/context handling.
pkg/cmd/codespace/ssh.go
Outdated
| sshCmd.Flags().BoolVar(&opts.stdio, "stdio", false, "Proxy sshd connection to stdio") | ||
| sshCmd.Flags().MarkHidden("stdio") | ||
|
|
||
| sshCmd.AddCommand(newConfigCmd(app)) |
There was a problem hiding this comment.
Good point about disambiguation. Let's decide on final naming at the start of January.
cmdutil.Factory.Executable() accounts for things like package managers and symlinks to the actual executable. An alternative to passing the *cmdutil.Factory down the stack would be stashing the executable string in the codespace.App, which works (and the diff is smaller), but it produced some odd non-local test failures. This way seems less mysterious and more like other uses of Factory in the codebase.
Also, other than that, restore the original ordering of this function
This way, factory can satisfy an interface that requires `Executable()`.
This is to avoid having to explicitly pass it to each subcommand that needs it. Each codespaces command runs in the context of App, so that's a point of shared context that we can store state in.
mislav
left a comment
There was a problem hiding this comment.
This looks good to me. Thank you for all the hard work! I have pushed the changes to iron out how Executable() is being passed around and consumed. Basically, this information is now stored on codespace.App level rather than being passed around to subcommands.
…command" This reverts commit c9d0085.
This PR allows the creation of virtual ssh "hosts" for codespaces that look and behave like real ssh hosts at the CLI. This enables codespaces to integrate seamlessly with the rich ecosystem of existing openssh features and add-ons, like:
ControlMasterssh/scp/sshfs(or anything else that supports ssh completion)gitpush/pull to/from the codespace using standard ssh urlsRepeated ssh to a codespace is also much faster this way.
It does this by adding two features:
gh cs ssh --stdiooption that starts the remote sshd and proxies a connection to it togh's stdin/stdoutgh cs ssh configsubcommand that generates an ssh configuration for use with #1For example, a user would set this up by doing:
With that in place, codespaces can now be treated like ordinary ssh remote hosts at the command line:
Note that hostname completion for the codespace works here, as well as remote pathname completion for
scp.sshfsworks as well - bothsshfsitself, and shell hostname and remote path completion:The codespace can also be used as a git remote, again with hostname and remote path completion:
Notes / Enhancements
gh cs ssh configwhen codespaces are added/removed; this could potentially be automated.cs.to group them together for tab completion purposes, but we could use other schemes or make this configurable.fixes #4713