STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #4915 from znull/znull/stdio

Browse source 
`gh cs ssh` cli integration with openssh config
This commit is contained in:
Nate Smith 2022-01-11 14:43:39 -06:00 • committed by GitHub
commit 2deb0ec908
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 308 additions and 98 deletions
Download patch file Download diff file Expand all files Collapse all files

3
pkg/cmd/auth/login/login_test.go
View file

@ -147,8 +147,7 @@ func Test_NewCmdLogin(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
io, stdin, _, _ := iostreams.Test()
f := &cmdutil.Factory{
IOStreams: io,
Executable: func() string { return "/path/to/gh" },
IOStreams: io,
}
io.SetStdoutTTY(true)

3
pkg/cmd/auth/refresh/refresh_test.go
View file

@ -91,8 +91,7 @@ func Test_NewCmdRefresh(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
io, _, _, _ := iostreams.Test()
f := &cmdutil.Factory{
IOStreams: io,
Executable: func() string { return "/path/to/gh" },
IOStreams: io,
}
io.SetStdinTTY(tt.tty)
io.SetStdoutTTY(tt.tty)

31
pkg/cmd/codespace/common.go
View file

@ -21,19 +21,25 @@ import (
"golang.org/x/term"
)
type App struct {
io *iostreams.IOStreams
apiClient apiClient
errLogger *log.Logger
type executable interface {
Executable() string
}
func NewApp(io *iostreams.IOStreams, apiClient apiClient) *App {
type App struct {
io *iostreams.IOStreams
apiClient apiClient
errLogger *log.Logger
executable executable
}
func NewApp(io *iostreams.IOStreams, exe executable, apiClient apiClient) *App {
errLogger := log.New(io.ErrOut, "", 0)
return &App{
io: io,
apiClient: apiClient,
errLogger: errLogger,
io: io,
apiClient: apiClient,
errLogger: errLogger,
executable: exe,
}
}
@ -209,8 +215,13 @@ func ask(qs []*survey.Question, response interface{}) error {
// checkAuthorizedKeys reports an error if the user has not registered any SSH keys;
// see https://github.com/cli/cli/v2/issues/166#issuecomment-921769703.
// The check is not required for security but it improves the error message.
func checkAuthorizedKeys(ctx context.Context, client apiClient, user string) error {
keys, err := client.AuthorizedKeys(ctx, user)
func checkAuthorizedKeys(ctx context.Context, client apiClient) error {
user, err := client.GetUser(ctx)
if err != nil {
return fmt.Errorf("error getting user: %w", err)
}
keys, err := client.AuthorizedKeys(ctx, user.Login)
if err != nil {
return fmt.Errorf("failed to read GitHub-authorized SSH keys for %s: %w", user, err)
}

2
pkg/cmd/codespace/delete_test.go
View file

@ -190,7 +190,7 @@ func TestDelete(t *testing.T) {
io, _, stdout, stderr := iostreams.Test()
io.SetStdinTTY(true)
io.SetStdoutTTY(true)
app := NewApp(io, apiMock)
app := NewApp(io, nil, apiMock)
err := app.Delete(context.Background(), opts)
if (err != nil) != tt.wantErr {
t.Errorf("delete() error = %v, wantErr %v", err, tt.wantErr)

7
pkg/cmd/codespace/logs.go
View file

@ -41,14 +41,9 @@ func (a *App) Logs(ctx context.Context, codespaceName string, follow bool) (err
return fmt.Errorf("get or choose codespace: %w", err)
}
user, err := a.apiClient.GetUser(ctx)
if err != nil {
return fmt.Errorf("getting user: %w", err)
}
authkeys := make(chan error, 1)
go func() {
authkeys <- checkAuthorizedKeys(ctx, a.apiClient, user.Login)
authkeys <- checkAuthorizedKeys(ctx, a.apiClient)
}()
session, err := codespaces.ConnectToLiveshare(ctx, a, noopLogger(), a.apiClient, codespace)

238
pkg/cmd/codespace/ssh.go
View file

@ -4,16 +4,21 @@ package codespace
import (
"context"
"errors"
"fmt"
"io"
"io/ioutil"
"log"
"net"
"os"
"path/filepath"
"strings"
"sync"
"text/template"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/codespaces"
"github.com/cli/cli/v2/internal/codespaces/api"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/liveshare"
"github.com/spf13/cobra"
@ -25,6 +30,8 @@ type sshOptions struct {
serverPort int
debug bool
debugFile string
stdio bool
config bool
scpArgs []string // scp arguments, for 'cs cp' (nil for 'cs ssh')
}
@ -34,8 +41,57 @@ func newSSHCmd(app *App) *cobra.Command {
sshCmd := &cobra.Command{
Use: "ssh [<flags>...] [-- <ssh-flags>...] [<command>]",
Short: "SSH into a codespace",
Long: heredoc.Doc(`
The 'ssh' command is used to SSH into a codespace. In its simplest form, you can
run 'gh cs ssh', select a codespace interactively, and connect.
The 'ssh' command also supports deeper integration with OpenSSH using a
'--config' option that generates per-codespace ssh configuration in OpenSSH
format. Including this configuration in your ~/.ssh/config improves the user
experience of tools that integrate with OpenSSH, such as bash/zsh completion of
ssh hostnames, remote path completion for scp/rsync/sshfs, git ssh remotes, and
so on.
Once that is set up (see the second example below), you can ssh to codespaces as
if they were ordinary remote hosts (using 'ssh', not 'gh cs ssh').
`),
Example: heredoc.Doc(`
$ gh codespace ssh
$ gh codespace ssh --config > ~/.ssh/codespaces
$ echo 'include ~/.ssh/codespaces' >> ~/.ssh/config'
`),
PreRunE: func(c *cobra.Command, args []string) error {
if opts.stdio {
if opts.codespace == "" {
return errors.New("`--stdio` requires explicit `--codespace`")
}
if opts.config {
return errors.New("cannot use `--stdio` with `--config`")
}
if opts.serverPort != 0 {
return errors.New("cannot use `--stdio` with `--server-port`")
}
if opts.profile != "" {
return errors.New("cannot use `--stdio` with `--profile`")
}
}
if opts.config {
if opts.profile != "" {
return errors.New("cannot use `--config` with `--profile`")
}
if opts.serverPort != 0 {
return errors.New("cannot use `--config` with `--server-port`")
}
}
return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
return app.SSH(cmd.Context(), args, opts)
if opts.config {
return app.printOpenSSHConfig(cmd.Context(), opts)
} else {
return app.SSH(cmd.Context(), args, opts)
}
},
DisableFlagsInUseLine: true,
}
@ -45,6 +101,11 @@ func newSSHCmd(app *App) *cobra.Command {
sshCmd.Flags().StringVarP(&opts.codespace, "codespace", "c", "", "Name of the codespace")
sshCmd.Flags().BoolVarP(&opts.debug, "debug", "d", false, "Log debug data to a file")
sshCmd.Flags().StringVarP(&opts.debugFile, "debug-file", "", "", "Path of the file log to")
sshCmd.Flags().BoolVarP(&opts.config, "config", "", false, "Write OpenSSH configuration to stdout")
sshCmd.Flags().BoolVar(&opts.stdio, "stdio", false, "Proxy sshd connection to stdio")
if err := sshCmd.Flags().MarkHidden("stdio"); err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
}
return sshCmd
}
@ -55,23 +116,18 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e
ctx, cancel := context.WithCancel(ctx)
defer cancel()
// While connecting, ensure in the background that the user has keys installed.
// That lets us report a more useful error message if they don't.
authkeys := make(chan error, 1)
go func() {
authkeys <- checkAuthorizedKeys(ctx, a.apiClient)
}()
codespace, err := getOrChooseCodespace(ctx, a.apiClient, opts.codespace)
if err != nil {
return fmt.Errorf("get or choose codespace: %w", err)
}
// TODO(josebalius): We can fetch the user in parallel to everything else
// we should convert this call and others to happen async
user, err := a.apiClient.GetUser(ctx)
if err != nil {
return fmt.Errorf("error getting user: %w", err)
}
authkeys := make(chan error, 1)
go func() {
authkeys <- checkAuthorizedKeys(ctx, a.apiClient, user.Login)
}()
liveshareLogger := noopLogger()
if opts.debug {
debugLogger, err := newFileLogger(opts.debugFile)
@ -86,14 +142,13 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e
session, err := codespaces.ConnectToLiveshare(ctx, a, liveshareLogger, a.apiClient, codespace)
if err != nil {
if authErr := <-authkeys; authErr != nil {
return authErr
}
return fmt.Errorf("error connecting to codespace: %w", err)
}
defer safeClose(session, &err)
if err := <-authkeys; err != nil {
return err
}
a.StartProgressIndicatorWithLabel("Fetching SSH Details")
remoteSSHServerPort, sshUser, err := session.StartSSHServer(ctx)
a.StopProgressIndicator()
@ -101,6 +156,13 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e
return fmt.Errorf("error getting ssh server details: %w", err)
}
if opts.stdio {
fwd := liveshare.NewPortForwarder(session, "sshd", remoteSSHServerPort, true)
stdio := newReadWriteCloser(os.Stdin, os.Stdout)
err := fwd.Forward(ctx, stdio) // always non-nil
return fmt.Errorf("tunnel closed: %w", err)
}
localSSHServerPort := opts.serverPort
usingCustomPort := localSSHServerPort != 0 // suppress log of command line in Shell
@ -147,6 +209,130 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e
}
}
func (a *App) printOpenSSHConfig(ctx context.Context, opts sshOptions) error {
ctx, cancel := context.WithCancel(ctx)
defer cancel()
var err error
var csList []*api.Codespace
if opts.codespace == "" {
a.StartProgressIndicatorWithLabel("Fetching codespaces")
csList, err = a.apiClient.ListCodespaces(ctx, -1)
a.StopProgressIndicator()
} else {
var codespace *api.Codespace
codespace, err = getOrChooseCodespace(ctx, a.apiClient, opts.codespace)
csList = []*api.Codespace{codespace}
}
if err != nil {
return fmt.Errorf("error getting codespace info: %w", err)
}
type sshResult struct {
codespace *api.Codespace
user string // on success, the remote ssh username; else nil
err error
}
sshUsers := make(chan sshResult, len(csList))
var wg sync.WaitGroup
var status error
for _, cs := range csList {
if cs.State != "Available" && opts.codespace == "" {
fmt.Fprintf(os.Stderr, "skipping unavailable codespace %s: %s\n", cs.Name, cs.State)
status = cmdutil.SilentError
continue
}
cs := cs
wg.Add(1)
go func() {
result := sshResult{}
defer wg.Done()
session, err := codespaces.ConnectToLiveshare(ctx, a, noopLogger(), a.apiClient, cs)
if err != nil {
result.err = fmt.Errorf("error connecting to codespace: %w", err)
} else {
defer session.Close()
_, result.user, err = session.StartSSHServer(ctx)
if err != nil {
result.err = fmt.Errorf("error getting ssh server details: %w", err)
} else {
result.codespace = cs
}
}
sshUsers <- result
}()
}
go func() {
wg.Wait()
close(sshUsers)
}()
// While the above fetches are running, ensure that the user has keys installed.
// That lets us report a more useful error message if they don't.
if err = checkAuthorizedKeys(ctx, a.apiClient); err != nil {
return err
}
t, err := template.New("ssh_config").Parse(heredoc.Doc(`
Host cs.{{.Name}}.{{.EscapedRef}}
User {{.SSHUser}}
ProxyCommand {{.GHExec}} cs ssh -c {{.Name}} --stdio
UserKnownHostsFile=/dev/null
StrictHostKeyChecking no
LogLevel quiet
ControlMaster auto
`))
if err != nil {
return fmt.Errorf("error formatting template: %w", err)
}
ghExec := a.executable.Executable()
for result := range sshUsers {
if result.err != nil {
fmt.Fprintf(os.Stderr, "%v\n", result.err)
status = cmdutil.SilentError
continue
}
// codespaceSSHConfig contains values needed to write an OpenSSH host
// configuration for a single codespace. For example:
//
// Host {{Name}}.{{EscapedRef}
// User {{SSHUser}
// ProxyCommand {{GHExec}} cs ssh -c {{Name}} --stdio
//
// EscapedRef is included in the name to help distinguish between codespaces
// when tab-completing ssh hostnames. '/' characters in EscapedRef are
// flattened to '-' to prevent problems with tab completion or when the
// hostname appears in ControlMaster socket paths.
type codespaceSSHConfig struct {
Name string // the codespace name, passed to `ssh -c`
EscapedRef string // the currently checked-out branch
SSHUser string // the remote ssh username
GHExec string // path used for invoking the current `gh` binary
}
conf := codespaceSSHConfig{
Name: result.codespace.Name,
EscapedRef: strings.ReplaceAll(result.codespace.GitStatus.Ref, "/", "-"),
SSHUser: result.user,
GHExec: ghExec,
}
if err := t.Execute(a.io.Out, conf); err != nil {
return err
}
}
return status
}
type cpOptions struct {
sshOptions
recursive bool // -r
@ -277,3 +463,21 @@ func (fl *fileLogger) Name() string {
func (fl *fileLogger) Close() error {
return fl.f.Close()
}
type combinedReadWriteCloser struct {
io.ReadCloser
io.WriteCloser
}
func newReadWriteCloser(reader io.ReadCloser, writer io.WriteCloser) io.ReadWriteCloser {
return &combinedReadWriteCloser{reader, writer}
}
func (crwc *combinedReadWriteCloser) Close() error {
werr := crwc.WriteCloser.Close()
rerr := crwc.ReadCloser.Close()
if werr != nil {
return werr
}
return rerr
}

60
pkg/cmd/factory/default.go
View file

@ -5,7 +5,6 @@ import (
"fmt"
"net/http"
"os"
"path/filepath"
"time"
"github.com/cli/cli/v2/api"
@ -19,17 +18,10 @@ import (
)
func New(appVersion string) *cmdutil.Factory {
var exe string
f := &cmdutil.Factory{
Config: configFunc(), // No factory dependencies
Branch: branchFunc(), // No factory dependencies
Executable: func() string {
if exe != "" {
return exe
}
exe = executable("gh")
return exe
},
Config: configFunc(), // No factory dependencies
Branch: branchFunc(), // No factory dependencies
ExecutableName: "gh",
}
f.IOStreams = ioStreams(f) // Depends on Config
@ -121,52 +113,6 @@ func browserLauncher(f *cmdutil.Factory) string {
return os.Getenv("BROWSER")
}
// Finds the location of the executable for the current process as it's found in PATH, respecting symlinks.
// If the process couldn't determine its location, return fallbackName. If the executable wasn't found in
// PATH, return the absolute location to the program.
//
// The idea is that the result of this function is callable in the future and refers to the same
// installation of gh, even across upgrades. This is needed primarily for Homebrew, which installs software
// under a location such as `/usr/local/Cellar/gh/1.13.1/bin/gh` and symlinks it from `/usr/local/bin/gh`.
// When the version is upgraded, Homebrew will often delete older versions, but keep the symlink. Because of
// this, we want to refer to the `gh` binary as `/usr/local/bin/gh` and not as its internal Homebrew
// location.
//
// None of this would be needed if we could just refer to GitHub CLI as `gh`, i.e. without using an absolute
// path. However, for some reason Homebrew does not include `/usr/local/bin` in PATH when it invokes git
// commands to update its taps. If `gh` (no path) is being used as git credential helper, as set up by `gh
// auth login`, running `brew update` will print out authentication errors as git is unable to locate
// Homebrew-installed `gh`.
func executable(fallbackName string) string {
exe, err := os.Executable()
if err != nil {
return fallbackName
}
base := filepath.Base(exe)
path := os.Getenv("PATH")
for _, dir := range filepath.SplitList(path) {
p, err := filepath.Abs(filepath.Join(dir, base))
if err != nil {
continue
}
f, err := os.Stat(p)
if err != nil {
continue
}
if p == exe {
return p
} else if f.Mode()&os.ModeSymlink != 0 {
if t, err := os.Readlink(p); err == nil && t == exe {
return p
}
}
}
return exe
}
func configFunc() func() (config.Config, error) {
var cachedConfig config.Config
var configError error

1
pkg/cmd/root/root.go
View file

@ -135,6 +135,7 @@ func newCodespaceCmd(f *cmdutil.Factory) *cobra.Command {
vscsURL := os.Getenv("INTERNAL_VSCS_TARGET_URL")
app := codespaceCmd.NewApp(
f.IOStreams,
f,
codespacesAPI.New(
serverURL,
apiURL,

61
pkg/cmdutil/factory.go
View file

@ -2,6 +2,9 @@ package cmdutil
import (
"net/http"
"os"
"path/filepath"
"strings"
"github.com/cli/cli/v2/context"
"github.com/cli/cli/v2/internal/config"
@ -25,7 +28,59 @@ type Factory struct {
Branch func() (string, error)
ExtensionManager extensions.ExtensionManager
// Executable is the path to the currently invoked gh binary
Executable func() string
ExecutableName string
}
// Executable is the path to the currently invoked binary
func (f *Factory) Executable() string {
if !strings.ContainsRune(f.ExecutableName, os.PathSeparator) {
f.ExecutableName = executable(f.ExecutableName)
}
return f.ExecutableName
}
// Finds the location of the executable for the current process as it's found in PATH, respecting symlinks.
// If the process couldn't determine its location, return fallbackName. If the executable wasn't found in
// PATH, return the absolute location to the program.
//
// The idea is that the result of this function is callable in the future and refers to the same
// installation of gh, even across upgrades. This is needed primarily for Homebrew, which installs software
// under a location such as `/usr/local/Cellar/gh/1.13.1/bin/gh` and symlinks it from `/usr/local/bin/gh`.
// When the version is upgraded, Homebrew will often delete older versions, but keep the symlink. Because of
// this, we want to refer to the `gh` binary as `/usr/local/bin/gh` and not as its internal Homebrew
// location.
//
// None of this would be needed if we could just refer to GitHub CLI as `gh`, i.e. without using an absolute
// path. However, for some reason Homebrew does not include `/usr/local/bin` in PATH when it invokes git
// commands to update its taps. If `gh` (no path) is being used as git credential helper, as set up by `gh
// auth login`, running `brew update` will print out authentication errors as git is unable to locate
// Homebrew-installed `gh`.
func executable(fallbackName string) string {
exe, err := os.Executable()
if err != nil {
return fallbackName
}
base := filepath.Base(exe)
path := os.Getenv("PATH")
for _, dir := range filepath.SplitList(path) {
p, err := filepath.Abs(filepath.Join(dir, base))
if err != nil {
continue
}
f, err := os.Stat(p)
if err != nil {
continue
}
if p == exe {
return p
} else if f.Mode()&os.ModeSymlink != 0 {
if t, err := os.Readlink(p); err == nil && t == exe {
return p
}
}
}
return exe
}