From 9416056da18bb1cc731917a32d6a3c3e4fe613d5 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Mon, 13 Dec 2021 15:18:32 -0700 Subject: [PATCH 01/45] allow combining os.Stdin and os.Stdout as an io.ReadWriteCloser --- pkg/cmd/codespace/ssh.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 840386d5c..0a1425a87 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -277,3 +277,32 @@ func (fl *fileLogger) Name() string { func (fl *fileLogger) Close() error { return fl.f.Close() } + +type combinedReadWriteCloser struct { + reader *os.File + writer *os.File +} + +func newCombinedReadWriteCloser(reader *os.File, writer *os.File) (crwc *combinedReadWriteCloser) { + return &combinedReadWriteCloser{ + reader: reader, + writer: writer, + } +} + +func (crwc *combinedReadWriteCloser) Read(p []byte) (n int, err error) { + return crwc.reader.Read(p) +} + +func (crwc *combinedReadWriteCloser) Write(p []byte) (n int, err error) { + return crwc.writer.Write(p) +} + +func (crwc *combinedReadWriteCloser) Close() error { + werr := crwc.writer.Close() + rerr := crwc.reader.Close() + if werr != nil { + return werr + } + return rerr +} From 81b658ea750ff05733f3bc6bf51bdcfe1e81be6c Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Mon, 13 Dec 2021 15:19:25 -0700 Subject: [PATCH 02/45] use go-multierror to combine read/write close errors --- go.mod | 1 + go.sum | 2 ++ pkg/cmd/codespace/ssh.go | 13 ++++++++----- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index fe082e3a3..e79aff389 100644 --- a/go.mod +++ b/go.mod @@ -17,6 +17,7 @@ require ( github.com/google/go-cmp v0.5.6 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/gorilla/websocket v1.4.2 + github.com/hashicorp/go-multierror v1.0.0 github.com/hashicorp/go-version v1.3.0 github.com/henvic/httpretty v0.0.6 github.com/itchyny/gojq v0.12.6 diff --git a/go.sum b/go.sum index fd9b7ae9d..bd9272641 100644 --- a/go.sum +++ b/go.sum @@ -192,10 +192,12 @@ github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/ad github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw= github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q= github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8= +github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA= github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80= github.com/hashicorp/go-immutable-radix v1.0.0/go.mod h1:0y9vanUI8NX6FsYoO3zeMjhV/C5i9g4Q3DwcSNZ4P60= github.com/hashicorp/go-msgpack v0.5.3/go.mod h1:ahLV/dePpqEmjfWmKiqvPkv/twdG7iPBM1vqhUKIvfM= +github.com/hashicorp/go-multierror v1.0.0 h1:iVjPR7a6H0tWELX5NxNe7bYopibicUzc7uPribsnS6o= github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk= github.com/hashicorp/go-rootcerts v1.0.0/go.mod h1:K6zTfqpRlCUIjkwsN4Z+hiSfzSTQa6eBIzfwKfwNnHU= github.com/hashicorp/go-sockaddr v1.0.0/go.mod h1:7Xibr9yA9JjQq1JpNB2Vw7kxv8xerXegt+ozgdvDeDU= diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 0a1425a87..738b352ba 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -16,6 +16,7 @@ import ( "github.com/cli/cli/v2/internal/codespaces" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/liveshare" + "github.com/hashicorp/go-multierror" "github.com/spf13/cobra" ) @@ -299,10 +300,12 @@ func (crwc *combinedReadWriteCloser) Write(p []byte) (n int, err error) { } func (crwc *combinedReadWriteCloser) Close() error { - werr := crwc.writer.Close() - rerr := crwc.reader.Close() - if werr != nil { - return werr + var errs error + if err := crwc.writer.Close(); err != nil { + errs = multierror.Append(errs, err) } - return rerr + if err := crwc.reader.Close(); err != nil { + errs = multierror.Append(errs, err) + } + return errs } From c5e553e4001e5ea90d6f3a7db33dea3b754d43c3 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Mon, 13 Dec 2021 15:27:44 -0700 Subject: [PATCH 03/45] implement `gh cs ssh --stdio` --- pkg/cmd/codespace/ssh.go | 91 +++++++++++++++++++++++----------------- 1 file changed, 52 insertions(+), 39 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 738b352ba..a397961fc 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -26,6 +26,7 @@ type sshOptions struct { serverPort int debug bool debugFile string + stdio bool scpArgs []string // scp arguments, for 'cs cp' (nil for 'cs ssh') } @@ -47,6 +48,11 @@ func newSSHCmd(app *App) *cobra.Command { 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") + // TODO: alternate name: "--proxy"? something else? + // also need to make this mutually exclusive with profile/server-port + // should probably require --codespace as well + sshCmd.Flags().BoolVarP(&opts.stdio, "stdio", "", false, "Proxy sshd connection to stdio") + return sshCmd } @@ -102,49 +108,56 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e return fmt.Errorf("error getting ssh server details: %w", err) } - localSSHServerPort := opts.serverPort - usingCustomPort := localSSHServerPort != 0 // suppress log of command line in Shell - - // Ensure local port is listening before client (Shell) connects. - // Unless the user specifies a server port, localSSHServerPort is 0 - // and thus the client will pick a random port. - listen, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", localSSHServerPort)) - if err != nil { - return err - } - defer listen.Close() - localSSHServerPort = listen.Addr().(*net.TCPAddr).Port - - connectDestination := opts.profile - if connectDestination == "" { - connectDestination = fmt.Sprintf("%s@localhost", sshUser) - } - - tunnelClosed := make(chan error, 1) - go func() { + if opts.stdio { fwd := liveshare.NewPortForwarder(session, "sshd", remoteSSHServerPort, true) - tunnelClosed <- fwd.ForwardToListener(ctx, listen) // always non-nil - }() - - shellClosed := make(chan error, 1) - go func() { - var err error - if opts.scpArgs != nil { - err = codespaces.Copy(ctx, opts.scpArgs, localSSHServerPort, connectDestination) - } else { - err = codespaces.Shell(ctx, a.errLogger, sshArgs, localSSHServerPort, connectDestination, usingCustomPort) - } - shellClosed <- err - }() - - select { - case err := <-tunnelClosed: + stdio := newCombinedReadWriteCloser(os.Stdin, os.Stdout) + err := fwd.Forward(ctx, stdio) // always non-nil return fmt.Errorf("tunnel closed: %w", err) - case err := <-shellClosed: + } else { + localSSHServerPort := opts.serverPort + usingCustomPort := localSSHServerPort != 0 // suppress log of command line in Shell + + // Ensure local port is listening before client (Shell) connects. + // Unless the user specifies a server port, localSSHServerPort is 0 + // and thus the client will pick a random port. + listen, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", localSSHServerPort)) if err != nil { - return fmt.Errorf("shell closed: %w", err) + return err + } + defer listen.Close() + localSSHServerPort = listen.Addr().(*net.TCPAddr).Port + + connectDestination := opts.profile + if connectDestination == "" { + connectDestination = fmt.Sprintf("%s@localhost", sshUser) + } + + tunnelClosed := make(chan error, 1) + go func() { + fwd := liveshare.NewPortForwarder(session, "sshd", remoteSSHServerPort, true) + tunnelClosed <- fwd.ForwardToListener(ctx, listen) // always non-nil + }() + + shellClosed := make(chan error, 1) + go func() { + var err error + if opts.scpArgs != nil { + err = codespaces.Copy(ctx, opts.scpArgs, localSSHServerPort, connectDestination) + } else { + err = codespaces.Shell(ctx, a.errLogger, sshArgs, localSSHServerPort, connectDestination, usingCustomPort) + } + shellClosed <- err + }() + + select { + case err := <-tunnelClosed: + return fmt.Errorf("tunnel closed: %w", err) + case err := <-shellClosed: + if err != nil { + return fmt.Errorf("shell closed: %w", err) + } + return nil // success } - return nil // success } } From 4306762f8bb4ac9038b888054859e7ec9b37f0f3 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Mon, 13 Dec 2021 18:43:56 -0700 Subject: [PATCH 04/45] factor out openSshSession() helper function --- pkg/cmd/codespace/ssh.go | 57 ++++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 22 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index a397961fc..133483ebb 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -62,23 +62,6 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e ctx, cancel := context.WithCancel(ctx) defer cancel() - 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) @@ -91,16 +74,12 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e a.errLogger.Printf("Debug file located at: %s", debugLogger.Name()) } - session, err := codespaces.ConnectToLiveshare(ctx, a, liveshareLogger, a.apiClient, codespace) + session, err := openSshSession(ctx, a, opts.codespace, liveshareLogger) if err != nil { 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() @@ -161,6 +140,40 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e } } +func openSshSession(ctx context.Context, a *App, csName string, liveshareLogger *log.Logger) (*liveshare.Session, error) { + codespace, err := getOrChooseCodespace(ctx, a.apiClient, csName) + if err != nil { + return nil, 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 nil, fmt.Errorf("error getting user: %w", err) + } + + authkeys := make(chan error, 1) + go func() { + authkeys <- checkAuthorizedKeys(ctx, a.apiClient, user.Login) + }() + + if liveshareLogger == nil { + liveshareLogger = noopLogger() + } + + session, err := codespaces.ConnectToLiveshare(ctx, a, liveshareLogger, a.apiClient, codespace) + if err != nil { + return nil, fmt.Errorf("error connecting to codespace: %w", err) + } + + if err := <-authkeys; err != nil { + return nil, err + } + + return session, nil +} + type cpOptions struct { sshOptions recursive bool // -r From 77650006012724a3a0d6c770402af813d798b69c Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Tue, 14 Dec 2021 16:26:25 -0700 Subject: [PATCH 05/45] add `gh cs ssh` openssh config file generator --- pkg/cmd/codespace/ssh.go | 166 +++++++++++++++++++++++++++++---------- 1 file changed, 125 insertions(+), 41 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 133483ebb..7ae079272 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -11,6 +11,7 @@ import ( "os" "path/filepath" "strings" + "text/template" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/codespaces" @@ -27,6 +28,7 @@ type sshOptions struct { debug bool debugFile string stdio bool + config bool scpArgs []string // scp arguments, for 'cs cp' (nil for 'cs ssh') } @@ -52,6 +54,7 @@ func newSSHCmd(app *App) *cobra.Command { // also need to make this mutually exclusive with profile/server-port // should probably require --codespace as well sshCmd.Flags().BoolVarP(&opts.stdio, "stdio", "", false, "Proxy sshd connection to stdio") + sshCmd.Flags().BoolVarP(&opts.config, "config", "", false, "Write OpenSSH configuration to stdout") return sshCmd } @@ -62,6 +65,10 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e ctx, cancel := context.WithCancel(ctx) defer cancel() + if opts.config { + return a.ListOpensshConfig(ctx) + } + liveshareLogger := noopLogger() if opts.debug { debugLogger, err := newFileLogger(opts.debugFile) @@ -92,52 +99,129 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e stdio := newCombinedReadWriteCloser(os.Stdin, os.Stdout) err := fwd.Forward(ctx, stdio) // always non-nil return fmt.Errorf("tunnel closed: %w", err) - } else { - localSSHServerPort := opts.serverPort - usingCustomPort := localSSHServerPort != 0 // suppress log of command line in Shell + } - // Ensure local port is listening before client (Shell) connects. - // Unless the user specifies a server port, localSSHServerPort is 0 - // and thus the client will pick a random port. - listen, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", localSSHServerPort)) + localSSHServerPort := opts.serverPort + usingCustomPort := localSSHServerPort != 0 // suppress log of command line in Shell + + // Ensure local port is listening before client (Shell) connects. + // Unless the user specifies a server port, localSSHServerPort is 0 + // and thus the client will pick a random port. + listen, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", localSSHServerPort)) + if err != nil { + return err + } + defer listen.Close() + localSSHServerPort = listen.Addr().(*net.TCPAddr).Port + + connectDestination := opts.profile + if connectDestination == "" { + connectDestination = fmt.Sprintf("%s@localhost", sshUser) + } + + tunnelClosed := make(chan error, 1) + go func() { + fwd := liveshare.NewPortForwarder(session, "sshd", remoteSSHServerPort, true) + tunnelClosed <- fwd.ForwardToListener(ctx, listen) // always non-nil + }() + + shellClosed := make(chan error, 1) + go func() { + var err error + if opts.scpArgs != nil { + err = codespaces.Copy(ctx, opts.scpArgs, localSSHServerPort, connectDestination) + } else { + err = codespaces.Shell(ctx, a.errLogger, sshArgs, localSSHServerPort, connectDestination, usingCustomPort) + } + shellClosed <- err + }() + + select { + case err := <-tunnelClosed: + return fmt.Errorf("tunnel closed: %w", err) + case err := <-shellClosed: if err != nil { + return fmt.Errorf("shell closed: %w", err) + } + return nil // success + } +} + +func (a *App) ListOpensshConfig(ctx context.Context) error { + a.StartProgressIndicatorWithLabel("Fetching codespaces") + codespaces, err := a.apiClient.ListCodespaces(ctx, -1) + a.StopProgressIndicator() + if err != nil { + return fmt.Errorf("error getting codespaces: %w", err) + } + + t, err := template.New("ssh_config").Parse(`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, err := os.Executable() + if err != nil { + return err + } + + sshUsers := map[string]string{} + + for _, cs := range codespaces { + + if cs.State != "Available" { + fmt.Fprintf(os.Stderr, "skipping unavailable codespace %s: %s\n", cs.Name, cs.State) + continue + } + + var sshUser string + var ok bool + + if sshUser, ok = sshUsers[cs.Repository.FullName]; !ok { + session, err := openSshSession(ctx, a, cs.Name, nil) + if err != nil { + fmt.Fprintf(os.Stderr, "error connecting to codespace: %v\n", err) + continue + } + defer session.Close() + + a.StartProgressIndicatorWithLabel(fmt.Sprintf("Fetching SSH Details for %s", cs.Name)) + _, sshUser, err = session.StartSSHServer(ctx) + a.StopProgressIndicator() + if err != nil { + fmt.Fprintf(os.Stderr, "error getting ssh server details: %v", err) + continue + } + sshUsers[cs.Repository.FullName] = sshUser + } + + conf := codespaceSshConfig{ + Name: cs.Name, + EscapedRef: strings.ReplaceAll(cs.GitStatus.Ref, "/", "-"), + SshUser: sshUser, + GHExec: ghexec, + } + if err := t.Execute(a.io.Out, conf); err != nil { return err } - defer listen.Close() - localSSHServerPort = listen.Addr().(*net.TCPAddr).Port - - connectDestination := opts.profile - if connectDestination == "" { - connectDestination = fmt.Sprintf("%s@localhost", sshUser) - } - - tunnelClosed := make(chan error, 1) - go func() { - fwd := liveshare.NewPortForwarder(session, "sshd", remoteSSHServerPort, true) - tunnelClosed <- fwd.ForwardToListener(ctx, listen) // always non-nil - }() - - shellClosed := make(chan error, 1) - go func() { - var err error - if opts.scpArgs != nil { - err = codespaces.Copy(ctx, opts.scpArgs, localSSHServerPort, connectDestination) - } else { - err = codespaces.Shell(ctx, a.errLogger, sshArgs, localSSHServerPort, connectDestination, usingCustomPort) - } - shellClosed <- err - }() - - select { - case err := <-tunnelClosed: - return fmt.Errorf("tunnel closed: %w", err) - case err := <-shellClosed: - if err != nil { - return fmt.Errorf("shell closed: %w", err) - } - return nil // success - } } + + return nil +} + +type codespaceSshConfig struct { + Name string + EscapedRef string + SshUser string + GHExec string } func openSshSession(ctx context.Context, a *App, csName string, liveshareLogger *log.Logger) (*liveshare.Session, error) { From 92403f3a2fbef66d9ba5a835b73c6a7e8952c169 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Tue, 14 Dec 2021 17:54:09 -0700 Subject: [PATCH 06/45] check for incompatible command line options --- pkg/cmd/codespace/ssh.go | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 7ae079272..68be66187 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -4,6 +4,7 @@ package codespace import ( "context" + "errors" "fmt" "io/ioutil" "log" @@ -38,6 +39,38 @@ func newSSHCmd(app *App) *cobra.Command { sshCmd := &cobra.Command{ Use: "ssh [...] [-- ...] []", Short: "SSH into a codespace", + PreRunE: func(c *cobra.Command, args []string) error { + f := c.Flags() + codespaceFlag := f.Lookup("codespace") + configFlag := f.Lookup("config") + portFlag := f.Lookup("server-port") + profileFlag := f.Lookup("profile") + stdioFlag := f.Lookup("stdio") + + if stdioFlag.Changed { + if !codespaceFlag.Changed { + return errors.New("`--stdio` requires explicit `--codespace`") + } + if configFlag.Changed { + return errors.New("cannot use `--stdio` with `--config`") + } + if portFlag.Changed { + return errors.New("cannot use `--stdio` with `--server-port`") + } + if profileFlag.Changed { + return errors.New("cannot use `--stdio` with `--profile`") + } + } + if configFlag.Changed { + if profileFlag.Changed { + return errors.New("cannot use `--config` with `--profile`") + } + if portFlag.Changed { + 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) }, @@ -49,10 +82,6 @@ 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") - - // TODO: alternate name: "--proxy"? something else? - // also need to make this mutually exclusive with profile/server-port - // should probably require --codespace as well sshCmd.Flags().BoolVarP(&opts.stdio, "stdio", "", false, "Proxy sshd connection to stdio") sshCmd.Flags().BoolVarP(&opts.config, "config", "", false, "Write OpenSSH configuration to stdout") From 0e6abda73b1186f02c769bc1a65ce47f8b6b2d35 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Tue, 14 Dec 2021 17:54:25 -0700 Subject: [PATCH 07/45] allow generating ssh config for a single codespace --- pkg/cmd/codespace/ssh.go | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 68be66187..5c551eee0 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -16,6 +16,7 @@ import ( "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/hashicorp/go-multierror" @@ -72,7 +73,11 @@ func newSSHCmd(app *App) *cobra.Command { return nil }, RunE: func(cmd *cobra.Command, args []string) error { - return app.SSH(cmd.Context(), args, opts) + if opts.config { + return app.ListOpensshConfig(cmd.Context(), opts) + } else { + return app.SSH(cmd.Context(), args, opts) + } }, DisableFlagsInUseLine: true, } @@ -94,10 +99,6 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e ctx, cancel := context.WithCancel(ctx) defer cancel() - if opts.config { - return a.ListOpensshConfig(ctx) - } - liveshareLogger := noopLogger() if opts.debug { debugLogger, err := newFileLogger(opts.debugFile) @@ -176,12 +177,24 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e } } -func (a *App) ListOpensshConfig(ctx context.Context) error { - a.StartProgressIndicatorWithLabel("Fetching codespaces") - codespaces, err := a.apiClient.ListCodespaces(ctx, -1) - a.StopProgressIndicator() +func (a *App) ListOpensshConfig(ctx context.Context, opts sshOptions) error { + // Ensure all child tasks (e.g. port forwarding) terminate before return. + ctx, cancel := context.WithCancel(ctx) + defer cancel() + + var err error + var codespaces []*api.Codespace + if opts.codespace == "" { + a.StartProgressIndicatorWithLabel("Fetching codespaces") + codespaces, err = a.apiClient.ListCodespaces(ctx, -1) + a.StopProgressIndicator() + } else { + var codespace *api.Codespace + codespace, err = getOrChooseCodespace(ctx, a.apiClient, opts.codespace) + codespaces = []*api.Codespace{codespace} + } if err != nil { - return fmt.Errorf("error getting codespaces: %w", err) + return fmt.Errorf("error getting codespace info: %w", err) } t, err := template.New("ssh_config").Parse(`Host cs.{{.Name}}.{{.EscapedRef}} From c9d0085e57a153d30722f4f99d431d7078db9da7 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Thu, 16 Dec 2021 15:09:52 -0700 Subject: [PATCH 08/45] move `gh cs ssh --config` into a separate `gh cs ssh config` command We could also move this to a toplevel command, but I don't want to pollute that namespace too much. Open to suggestions. --- pkg/cmd/codespace/ssh.go | 55 +++++++++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 5c551eee0..b6c2d6596 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -30,7 +30,6 @@ type sshOptions struct { debug bool debugFile string stdio bool - config bool scpArgs []string // scp arguments, for 'cs cp' (nil for 'cs ssh') } @@ -43,7 +42,6 @@ func newSSHCmd(app *App) *cobra.Command { PreRunE: func(c *cobra.Command, args []string) error { f := c.Flags() codespaceFlag := f.Lookup("codespace") - configFlag := f.Lookup("config") portFlag := f.Lookup("server-port") profileFlag := f.Lookup("profile") stdioFlag := f.Lookup("stdio") @@ -52,9 +50,6 @@ func newSSHCmd(app *App) *cobra.Command { if !codespaceFlag.Changed { return errors.New("`--stdio` requires explicit `--codespace`") } - if configFlag.Changed { - return errors.New("cannot use `--stdio` with `--config`") - } if portFlag.Changed { return errors.New("cannot use `--stdio` with `--server-port`") } @@ -62,22 +57,10 @@ func newSSHCmd(app *App) *cobra.Command { return errors.New("cannot use `--stdio` with `--profile`") } } - if configFlag.Changed { - if profileFlag.Changed { - return errors.New("cannot use `--config` with `--profile`") - } - if portFlag.Changed { - return errors.New("cannot use `--config` with `--server-port`") - } - } return nil }, RunE: func(cmd *cobra.Command, args []string) error { - if opts.config { - return app.ListOpensshConfig(cmd.Context(), opts) - } else { - return app.SSH(cmd.Context(), args, opts) - } + return app.SSH(cmd.Context(), args, opts) }, DisableFlagsInUseLine: true, } @@ -88,7 +71,8 @@ func newSSHCmd(app *App) *cobra.Command { 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.stdio, "stdio", "", false, "Proxy sshd connection to stdio") - sshCmd.Flags().BoolVarP(&opts.config, "config", "", false, "Write OpenSSH configuration to stdout") + + sshCmd.AddCommand(newConfigCmd(app)) return sshCmd } @@ -177,7 +161,7 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e } } -func (a *App) ListOpensshConfig(ctx context.Context, opts sshOptions) error { +func (a *App) ListOpensshConfig(ctx context.Context, opts configOptions) error { // Ensure all child tasks (e.g. port forwarding) terminate before return. ctx, cancel := context.WithCancel(ctx) defer cancel() @@ -391,6 +375,37 @@ func (a *App) Copy(ctx context.Context, args []string, opts cpOptions) error { return a.SSH(ctx, nil, opts.sshOptions) } +type configOptions struct { + codespace string +} + +func newConfigCmd(app *App) *cobra.Command { + var opts configOptions + + configCmd := &cobra.Command{ + Use: "config [-c codespace]", + Short: "Write OpenSSH configuration to stdout", + Long: heredoc.Docf(` + The config command generates ssh connection configuration in OpenSSH format. + + If -c/--codespace is specified, configuration is generated for that codespace + only. Otherwise configuration is emitted for all available codespaces. + `, "`"), + Example: heredoc.Doc(` + $ gh codespace config > ~/.ssh/codespaces + $ echo 'include ~/.ssh/codespaces' >> ~/.ssh/config' + `), + RunE: func(cmd *cobra.Command, args []string) error { + return app.ListOpensshConfig(cmd.Context(), opts) + }, + DisableFlagsInUseLine: true, + } + + configCmd.Flags().StringVarP(&opts.codespace, "codespace", "c", "", "Name of the codespace") + + return configCmd +} + // fileLogger is a wrapper around an log.Logger configured to write // to a file. It exports two additional methods to get the log file name // and close the file handle when the operation is finished. From 8687fcb2a040bf1992cd8e74f76bd634cd57185f Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Thu, 16 Dec 2021 15:18:59 -0700 Subject: [PATCH 09/45] clean up `gh cs ssh` option parsing/validation --- pkg/cmd/codespace/ssh.go | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index b6c2d6596..031651f80 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -40,20 +40,14 @@ func newSSHCmd(app *App) *cobra.Command { Use: "ssh [...] [-- ...] []", Short: "SSH into a codespace", PreRunE: func(c *cobra.Command, args []string) error { - f := c.Flags() - codespaceFlag := f.Lookup("codespace") - portFlag := f.Lookup("server-port") - profileFlag := f.Lookup("profile") - stdioFlag := f.Lookup("stdio") - - if stdioFlag.Changed { - if !codespaceFlag.Changed { + if opts.stdio { + if opts.codespace == "" { return errors.New("`--stdio` requires explicit `--codespace`") } - if portFlag.Changed { + if opts.serverPort != 0 { return errors.New("cannot use `--stdio` with `--server-port`") } - if profileFlag.Changed { + if opts.profile != "" { return errors.New("cannot use `--stdio` with `--profile`") } } From 3ca5cbab84ece3f415b1d15f9425fe46e7a75c87 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Thu, 16 Dec 2021 15:19:13 -0700 Subject: [PATCH 10/45] hide the `gh cs ssh --stdio` option in the --help message --- pkg/cmd/codespace/ssh.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 031651f80..68de85c23 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -64,7 +64,8 @@ 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.stdio, "stdio", "", false, "Proxy sshd connection to stdio") + sshCmd.Flags().BoolVar(&opts.stdio, "stdio", false, "Proxy sshd connection to stdio") + sshCmd.Flags().MarkHidden("stdio") sshCmd.AddCommand(newConfigCmd(app)) From 92609bd1ec29e8b70d4b010d02aea5b9ed264591 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Thu, 16 Dec 2021 15:23:36 -0700 Subject: [PATCH 11/45] Revert "use go-multierror to combine read/write close errors" This reverts commit 456f4381e02aa843ddb3cc1b9628cb81487ba895. --- go.mod | 1 - go.sum | 2 -- pkg/cmd/codespace/ssh.go | 13 +++++-------- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/go.mod b/go.mod index e79aff389..fe082e3a3 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,6 @@ require ( github.com/google/go-cmp v0.5.6 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/gorilla/websocket v1.4.2 - github.com/hashicorp/go-multierror v1.0.0 github.com/hashicorp/go-version v1.3.0 github.com/henvic/httpretty v0.0.6 github.com/itchyny/gojq v0.12.6 diff --git a/go.sum b/go.sum index bd9272641..fd9b7ae9d 100644 --- a/go.sum +++ b/go.sum @@ -192,12 +192,10 @@ github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/ad github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw= github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q= github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8= -github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA= github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80= github.com/hashicorp/go-immutable-radix v1.0.0/go.mod h1:0y9vanUI8NX6FsYoO3zeMjhV/C5i9g4Q3DwcSNZ4P60= github.com/hashicorp/go-msgpack v0.5.3/go.mod h1:ahLV/dePpqEmjfWmKiqvPkv/twdG7iPBM1vqhUKIvfM= -github.com/hashicorp/go-multierror v1.0.0 h1:iVjPR7a6H0tWELX5NxNe7bYopibicUzc7uPribsnS6o= github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk= github.com/hashicorp/go-rootcerts v1.0.0/go.mod h1:K6zTfqpRlCUIjkwsN4Z+hiSfzSTQa6eBIzfwKfwNnHU= github.com/hashicorp/go-sockaddr v1.0.0/go.mod h1:7Xibr9yA9JjQq1JpNB2Vw7kxv8xerXegt+ozgdvDeDU= diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 68de85c23..27fa21149 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -19,7 +19,6 @@ import ( "github.com/cli/cli/v2/internal/codespaces/api" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/liveshare" - "github.com/hashicorp/go-multierror" "github.com/spf13/cobra" ) @@ -462,12 +461,10 @@ func (crwc *combinedReadWriteCloser) Write(p []byte) (n int, err error) { } func (crwc *combinedReadWriteCloser) Close() error { - var errs error - if err := crwc.writer.Close(); err != nil { - errs = multierror.Append(errs, err) + werr := crwc.writer.Close() + rerr := crwc.reader.Close() + if werr != nil { + return werr } - if err := crwc.reader.Close(); err != nil { - errs = multierror.Append(errs, err) - } - return errs + return rerr } From ac3b0c50e33ac1f327843af3c4a5694372428e29 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Thu, 16 Dec 2021 16:07:26 -0700 Subject: [PATCH 12/45] Ssh -> SSH --- pkg/cmd/codespace/ssh.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 27fa21149..2b74290bd 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -89,7 +89,7 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e a.errLogger.Printf("Debug file located at: %s", debugLogger.Name()) } - session, err := openSshSession(ctx, a, opts.codespace, liveshareLogger) + session, err := openSSHSession(ctx, a, opts.codespace, liveshareLogger) if err != nil { return fmt.Errorf("error connecting to codespace: %w", err) } @@ -176,7 +176,7 @@ func (a *App) ListOpensshConfig(ctx context.Context, opts configOptions) error { } t, err := template.New("ssh_config").Parse(`Host cs.{{.Name}}.{{.EscapedRef}} - User {{.SshUser}} + User {{.SSHUser}} ProxyCommand {{.GHExec}} cs ssh -c {{.Name}} --stdio UserKnownHostsFile=/dev/null StrictHostKeyChecking no @@ -206,7 +206,7 @@ func (a *App) ListOpensshConfig(ctx context.Context, opts configOptions) error { var ok bool if sshUser, ok = sshUsers[cs.Repository.FullName]; !ok { - session, err := openSshSession(ctx, a, cs.Name, nil) + session, err := openSSHSession(ctx, a, cs.Name, nil) if err != nil { fmt.Fprintf(os.Stderr, "error connecting to codespace: %v\n", err) continue @@ -223,10 +223,10 @@ func (a *App) ListOpensshConfig(ctx context.Context, opts configOptions) error { sshUsers[cs.Repository.FullName] = sshUser } - conf := codespaceSshConfig{ + conf := codespaceSSHConfig{ Name: cs.Name, EscapedRef: strings.ReplaceAll(cs.GitStatus.Ref, "/", "-"), - SshUser: sshUser, + SSHUser: sshUser, GHExec: ghexec, } if err := t.Execute(a.io.Out, conf); err != nil { @@ -237,14 +237,14 @@ func (a *App) ListOpensshConfig(ctx context.Context, opts configOptions) error { return nil } -type codespaceSshConfig struct { +type codespaceSSHConfig struct { Name string EscapedRef string - SshUser string + SSHUser string GHExec string } -func openSshSession(ctx context.Context, a *App, csName string, liveshareLogger *log.Logger) (*liveshare.Session, error) { +func openSSHSession(ctx context.Context, a *App, csName string, liveshareLogger *log.Logger) (*liveshare.Session, error) { codespace, err := getOrChooseCodespace(ctx, a.apiClient, csName) if err != nil { return nil, fmt.Errorf("get or choose codespace: %w", err) From ca3b59dd351a1508d7f230a64db3e7240878fec3 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Thu, 16 Dec 2021 16:07:58 -0700 Subject: [PATCH 13/45] use struct embedding to express this less verbosely --- pkg/cmd/codespace/ssh.go | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 2b74290bd..3c729740d 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -6,6 +6,7 @@ import ( "context" "errors" "fmt" + "io" "io/ioutil" "log" "net" @@ -441,28 +442,17 @@ func (fl *fileLogger) Close() error { } type combinedReadWriteCloser struct { - reader *os.File - writer *os.File + io.ReadCloser + io.WriteCloser } func newCombinedReadWriteCloser(reader *os.File, writer *os.File) (crwc *combinedReadWriteCloser) { - return &combinedReadWriteCloser{ - reader: reader, - writer: writer, - } -} - -func (crwc *combinedReadWriteCloser) Read(p []byte) (n int, err error) { - return crwc.reader.Read(p) -} - -func (crwc *combinedReadWriteCloser) Write(p []byte) (n int, err error) { - return crwc.writer.Write(p) + return &combinedReadWriteCloser{reader, writer} } func (crwc *combinedReadWriteCloser) Close() error { - werr := crwc.writer.Close() - rerr := crwc.reader.Close() + werr := crwc.WriteCloser.Close() + rerr := crwc.ReadCloser.Close() if werr != nil { return werr } From 96a2e125e6b9e64ed48e6423e309ba22ea02e93e Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Thu, 16 Dec 2021 16:37:26 -0700 Subject: [PATCH 14/45] document the codespaceSSHConfig struct --- pkg/cmd/codespace/ssh.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 3c729740d..153cb884c 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -238,11 +238,22 @@ func (a *App) ListOpensshConfig(ctx context.Context, opts configOptions) error { return nil } +// 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 - EscapedRef string - SSHUser string - GHExec string + 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 } func openSSHSession(ctx context.Context, a *App, csName string, liveshareLogger *log.Logger) (*liveshare.Session, error) { From 3cb3cf7ca5ec7c10fa5b1e5552e7014b828a7ab6 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Thu, 16 Dec 2021 16:38:47 -0700 Subject: [PATCH 15/45] make ListOpensshConfig private --- pkg/cmd/codespace/ssh.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 153cb884c..5e9b99ef8 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -156,7 +156,7 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e } } -func (a *App) ListOpensshConfig(ctx context.Context, opts configOptions) error { +func (a *App) listOpensshConfig(ctx context.Context, opts configOptions) error { // Ensure all child tasks (e.g. port forwarding) terminate before return. ctx, cancel := context.WithCancel(ctx) defer cancel() @@ -402,7 +402,7 @@ func newConfigCmd(app *App) *cobra.Command { $ echo 'include ~/.ssh/codespaces' >> ~/.ssh/config' `), RunE: func(cmd *cobra.Command, args []string) error { - return app.ListOpensshConfig(cmd.Context(), opts) + return app.listOpensshConfig(cmd.Context(), opts) }, DisableFlagsInUseLine: true, } From b7cd2bbc72464e9e1ed87853ef2c9046c9ed5032 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Thu, 16 Dec 2021 16:39:42 -0700 Subject: [PATCH 16/45] no need to separately declare vars here --- pkg/cmd/codespace/ssh.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 5e9b99ef8..abc95f318 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -203,10 +203,8 @@ func (a *App) listOpensshConfig(ctx context.Context, opts configOptions) error { continue } - var sshUser string - var ok bool - - if sshUser, ok = sshUsers[cs.Repository.FullName]; !ok { + sshUser, ok := sshUsers[cs.Repository.FullName] + if !ok { session, err := openSSHSession(ctx, a, cs.Name, nil) if err != nil { fmt.Fprintf(os.Stderr, "error connecting to codespace: %v\n", err) From 6f8635a17e69e879ea730e32296a719a1c0c2f58 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Thu, 16 Dec 2021 16:43:55 -0700 Subject: [PATCH 17/45] listOpensshConfig -> printOpenSSHConfig --- pkg/cmd/codespace/ssh.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index abc95f318..301f9b49e 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -156,7 +156,7 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e } } -func (a *App) listOpensshConfig(ctx context.Context, opts configOptions) error { +func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error { // Ensure all child tasks (e.g. port forwarding) terminate before return. ctx, cancel := context.WithCancel(ctx) defer cancel() @@ -400,7 +400,7 @@ func newConfigCmd(app *App) *cobra.Command { $ echo 'include ~/.ssh/codespaces' >> ~/.ssh/config' `), RunE: func(cmd *cobra.Command, args []string) error { - return app.listOpensshConfig(cmd.Context(), opts) + return app.printOpenSSHConfig(cmd.Context(), opts) }, DisableFlagsInUseLine: true, } From 4a6887f99e0be2df3deb5e94656ca649c11cb838 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Thu, 16 Dec 2021 16:47:16 -0700 Subject: [PATCH 18/45] add a comment explaining why we build an sshUsers map --- pkg/cmd/codespace/ssh.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 301f9b49e..7dfce6ec4 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -194,6 +194,10 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error return err } + // store a mapping of repository -> remote ssh username. This is + // necessary because the username can vary between codespaces, but + // since fetching it is slow, we store it here so we at least only do + // it once per repository. sshUsers := map[string]string{} for _, cs := range codespaces { From 369ea534b67add703502849909db3c7f2767e277 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Thu, 16 Dec 2021 16:52:07 -0700 Subject: [PATCH 19/45] explain why failure to connect to a codespace doesn't fail the entire command --- pkg/cmd/codespace/ssh.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 7dfce6ec4..9e35bc544 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -212,6 +212,10 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error session, err := openSSHSession(ctx, a, cs.Name, nil) if err != nil { fmt.Fprintf(os.Stderr, "error connecting to codespace: %v\n", err) + + // Move on to the next codespace. We don't want to bail here - just because we're not + // able to set up connectivity to one doesn't mean we shouldn't make a best effort to + // generate configs for the rest of them. continue } defer session.Close() @@ -221,7 +225,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error a.StopProgressIndicator() if err != nil { fmt.Fprintf(os.Stderr, "error getting ssh server details: %v", err) - continue + continue // see above } sshUsers[cs.Repository.FullName] = sshUser } From 71c9fa11f06034ceaa9dce3971f6942e6c94ebb2 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Thu, 16 Dec 2021 16:57:51 -0700 Subject: [PATCH 20/45] add more `ssh config` --help documentation --- pkg/cmd/codespace/ssh.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 9e35bc544..bfa7e0391 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -398,10 +398,17 @@ func newConfigCmd(app *App) *cobra.Command { Use: "config [-c codespace]", Short: "Write OpenSSH configuration to stdout", Long: heredoc.Docf(` - The config command generates ssh connection configuration in OpenSSH format. + The config command generates per-codespace ssh configuration in OpenSSH format. + + Including this configuration in ~/.ssh/config simplifies integration with other tools + that integrate with OpenSSH, such as bash/zsh ssh hostname completion, remote path + completion for scp/rsync/sshfs, git ssh remotes, and so on. If -c/--codespace is specified, configuration is generated for that codespace only. Otherwise configuration is emitted for all available codespaces. + + Codespaces that aren't in "Available" state are skipped because it's necessary to + connect to the running codespace to determine the required remote ssh username. `, "`"), Example: heredoc.Doc(` $ gh codespace config > ~/.ssh/codespaces From ac685611b117451163f1df2874433b8eef35ed62 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Thu, 16 Dec 2021 16:58:16 -0700 Subject: [PATCH 21/45] openssh -> OpenSSH --- pkg/cmd/codespace/ssh.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index bfa7e0391..5654f3432 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -244,7 +244,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error return nil } -// codespaceSSHConfig contains values needed to write an openssh host +// codespaceSSHConfig contains values needed to write an OpenSSH host // configuration for a single codespace. For example: // // Host {{Name}}.{{EscapedRef} From a9d02a746baa3b3cfa17d7c9cd1f80e030cb4f65 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Thu, 16 Dec 2021 17:19:34 -0700 Subject: [PATCH 22/45] push fetching codespace details into openSSHSession callers --- pkg/cmd/codespace/ssh.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 5654f3432..33f837bbc 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -90,7 +90,12 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e a.errLogger.Printf("Debug file located at: %s", debugLogger.Name()) } - session, err := openSSHSession(ctx, a, opts.codespace, liveshareLogger) + codespace, err := getOrChooseCodespace(ctx, a.apiClient, opts.codespace) + if err != nil { + return fmt.Errorf("get or choose codespace: %w", err) + } + + session, err := openSSHSession(ctx, a, codespace, liveshareLogger) if err != nil { return fmt.Errorf("error connecting to codespace: %w", err) } @@ -209,7 +214,12 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error sshUser, ok := sshUsers[cs.Repository.FullName] if !ok { - session, err := openSSHSession(ctx, a, cs.Name, nil) + codespace, err := a.apiClient.GetCodespace(ctx, cs.Name, true) + if err != nil { + return fmt.Errorf("getting full codespace details: %w", err) + } + + session, err := openSSHSession(ctx, a, codespace, nil) if err != nil { fmt.Fprintf(os.Stderr, "error connecting to codespace: %v\n", err) @@ -262,12 +272,7 @@ type codespaceSSHConfig struct { GHExec string // path used for invoking the current `gh` binary } -func openSSHSession(ctx context.Context, a *App, csName string, liveshareLogger *log.Logger) (*liveshare.Session, error) { - codespace, err := getOrChooseCodespace(ctx, a.apiClient, csName) - if err != nil { - return nil, fmt.Errorf("get or choose codespace: %w", err) - } - +func openSSHSession(ctx context.Context, a *App, codespace *api.Codespace, liveshareLogger *log.Logger) (*liveshare.Session, error) { // 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) From 206b6379c32d85b29768eff3fffea013c590995b Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Fri, 17 Dec 2021 10:47:22 -0700 Subject: [PATCH 23/45] return non-zero if `ssh config` skips any codespaces --- pkg/cmd/codespace/ssh.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 33f837bbc..eb4bae7a2 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -205,10 +205,12 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error // it once per repository. sshUsers := map[string]string{} + var status error for _, cs := range codespaces { if cs.State != "Available" { fmt.Fprintf(os.Stderr, "skipping unavailable codespace %s: %s\n", cs.Name, cs.State) + status = cmdutil.SilentError continue } @@ -226,6 +228,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error // Move on to the next codespace. We don't want to bail here - just because we're not // able to set up connectivity to one doesn't mean we shouldn't make a best effort to // generate configs for the rest of them. + status = cmdutil.SilentError continue } defer session.Close() @@ -235,6 +238,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error a.StopProgressIndicator() if err != nil { fmt.Fprintf(os.Stderr, "error getting ssh server details: %v", err) + status = cmdutil.SilentError continue // see above } sshUsers[cs.Repository.FullName] = sshUser @@ -251,7 +255,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error } } - return nil + return status } // codespaceSSHConfig contains values needed to write an OpenSSH host From 811d6505d28c82e4c03b2778be8a2a71794fe888 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Fri, 17 Dec 2021 11:03:11 -0700 Subject: [PATCH 24/45] tighten up `ssh config` help wording --- pkg/cmd/codespace/ssh.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index eb4bae7a2..510a65502 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -409,9 +409,9 @@ func newConfigCmd(app *App) *cobra.Command { Long: heredoc.Docf(` The config command generates per-codespace ssh configuration in OpenSSH format. - Including this configuration in ~/.ssh/config simplifies integration with other tools - that integrate with OpenSSH, such as bash/zsh ssh hostname completion, remote path - completion for scp/rsync/sshfs, git ssh remotes, and so on. + Including this configuration in ~/.ssh/config improves the user experience of other + 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. If -c/--codespace is specified, configuration is generated for that codespace only. Otherwise configuration is emitted for all available codespaces. @@ -429,7 +429,7 @@ func newConfigCmd(app *App) *cobra.Command { DisableFlagsInUseLine: true, } - configCmd.Flags().StringVarP(&opts.codespace, "codespace", "c", "", "Name of the codespace") + configCmd.Flags().StringVarP(&opts.codespace, "codespace", "c", "", "Name of a codespace") return configCmd } From 0670a7758b1eb17f91e10480fe7a61143f91afba Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Fri, 17 Dec 2021 11:18:59 -0700 Subject: [PATCH 25/45] use generic io interfaces --- pkg/cmd/codespace/ssh.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 510a65502..24729ecca 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -110,7 +110,7 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e if opts.stdio { fwd := liveshare.NewPortForwarder(session, "sshd", remoteSSHServerPort, true) - stdio := newCombinedReadWriteCloser(os.Stdin, os.Stdout) + stdio := newReadWriteCloser(os.Stdin, os.Stdout) err := fwd.Forward(ctx, stdio) // always non-nil return fmt.Errorf("tunnel closed: %w", err) } @@ -479,7 +479,7 @@ type combinedReadWriteCloser struct { io.WriteCloser } -func newCombinedReadWriteCloser(reader *os.File, writer *os.File) (crwc *combinedReadWriteCloser) { +func newReadWriteCloser(reader io.ReadCloser, writer io.WriteCloser) io.ReadWriteCloser { return &combinedReadWriteCloser{reader, writer} } From a05541f4ed924e9732144959790405bb1a63b39e Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Fri, 17 Dec 2021 11:39:14 -0700 Subject: [PATCH 26/45] drop redundant API call --- pkg/cmd/codespace/ssh.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 24729ecca..8275b0634 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -216,12 +216,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error sshUser, ok := sshUsers[cs.Repository.FullName] if !ok { - codespace, err := a.apiClient.GetCodespace(ctx, cs.Name, true) - if err != nil { - return fmt.Errorf("getting full codespace details: %w", err) - } - - session, err := openSSHSession(ctx, a, codespace, nil) + session, err := openSSHSession(ctx, a, cs, nil) if err != nil { fmt.Fprintf(os.Stderr, "error connecting to codespace: %v\n", err) From 2ee88da647c85f957de710519ed628ce59a14687 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Fri, 17 Dec 2021 14:42:54 -0700 Subject: [PATCH 27/45] close session on error --- pkg/cmd/codespace/ssh.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 8275b0634..dfca3ec03 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -294,6 +294,7 @@ func openSSHSession(ctx context.Context, a *App, codespace *api.Codespace, lives } if err := <-authkeys; err != nil { + session.Close() return nil, err } From 06eb5ad47fbb055046bb3c86ee12587d7a714c93 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Fri, 17 Dec 2021 16:10:44 -0700 Subject: [PATCH 28/45] fetch remote ssh usernames in parallel --- pkg/cmd/codespace/ssh.go | 103 ++++++++++++++++++++++----------------- 1 file changed, 58 insertions(+), 45 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index dfca3ec03..72b5611d5 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -162,7 +162,6 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e } func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error { - // Ensure all child tasks (e.g. port forwarding) terminate before return. ctx, cancel := context.WithCancel(ctx) defer cancel() @@ -181,6 +180,51 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error return fmt.Errorf("error getting codespace info: %w", err) } + sshUsers := make(chan sshResult) + fetches := 0 + var status error + for _, cs := range codespaces { + if cs.State != "Available" { + fmt.Fprintf(os.Stderr, "skipping unavailable codespace %s: %s\n", cs.Name, cs.State) + status = cmdutil.SilentError + continue + } + + cs := cs + fetches += 1 + go func() { + result := sshResult{} + defer func() { + select { + case sshUsers <- result: + case <-ctx.Done(): + } + }() + + session, err := openSSHSession(ctx, a, cs, nil) + if err != nil { + result.err = fmt.Errorf("error connecting to codespace: %w", err) + return + } + defer session.Close() + + //a.StartProgressIndicatorWithLabel(fmt.Sprintf("Fetching SSH Details for %s", cs.Name)) + _, result.user, err = session.StartSSHServer(ctx) + //a.StopProgressIndicator() + if err != nil { + result.err = fmt.Errorf("error getting ssh server details: %w", err) + return + } + + result.codespace = cs + }() + } + + ghexec, err := os.Executable() + if err != nil { + return err + } + t, err := template.New("ssh_config").Parse(`Host cs.{{.Name}}.{{.EscapedRef}} User {{.SSHUser}} ProxyCommand {{.GHExec}} cs ssh -c {{.Name}} --stdio @@ -194,55 +238,18 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error return fmt.Errorf("error formatting template: %w", err) } - ghexec, err := os.Executable() - if err != nil { - return err - } - - // store a mapping of repository -> remote ssh username. This is - // necessary because the username can vary between codespaces, but - // since fetching it is slow, we store it here so we at least only do - // it once per repository. - sshUsers := map[string]string{} - - var status error - for _, cs := range codespaces { - - if cs.State != "Available" { - fmt.Fprintf(os.Stderr, "skipping unavailable codespace %s: %s\n", cs.Name, cs.State) + for i := 0; i < fetches; i++ { + result := <-sshUsers + if result.err != nil { + fmt.Fprintf(os.Stderr, "%v\n", result.err) status = cmdutil.SilentError continue } - sshUser, ok := sshUsers[cs.Repository.FullName] - if !ok { - session, err := openSSHSession(ctx, a, cs, nil) - if err != nil { - fmt.Fprintf(os.Stderr, "error connecting to codespace: %v\n", err) - - // Move on to the next codespace. We don't want to bail here - just because we're not - // able to set up connectivity to one doesn't mean we shouldn't make a best effort to - // generate configs for the rest of them. - status = cmdutil.SilentError - continue - } - defer session.Close() - - a.StartProgressIndicatorWithLabel(fmt.Sprintf("Fetching SSH Details for %s", cs.Name)) - _, sshUser, err = session.StartSSHServer(ctx) - a.StopProgressIndicator() - if err != nil { - fmt.Fprintf(os.Stderr, "error getting ssh server details: %v", err) - status = cmdutil.SilentError - continue // see above - } - sshUsers[cs.Repository.FullName] = sshUser - } - conf := codespaceSSHConfig{ - Name: cs.Name, - EscapedRef: strings.ReplaceAll(cs.GitStatus.Ref, "/", "-"), - SSHUser: sshUser, + 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 { @@ -253,6 +260,12 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error return status } +type sshResult struct { + codespace *api.Codespace + user string // on success, the remote ssh username; else nil + err error +} + // codespaceSSHConfig contains values needed to write an OpenSSH host // configuration for a single codespace. For example: // From 0af268da4edb6b67f29704391ed0f4fdf8aabdbc Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Fri, 17 Dec 2021 16:11:55 -0700 Subject: [PATCH 29/45] properly indent ssh config example --- pkg/cmd/codespace/ssh.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 72b5611d5..36aef65fd 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -270,8 +270,8 @@ type sshResult struct { // configuration for a single codespace. For example: // // Host {{Name}}.{{EscapedRef} -// User {{SSHUser} -// ProxyCommand {{GHExec}} cs ssh -c {{Name}} --stdio +// 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 From 61823997c228ecf8b52e2c1c9fcfdff8f1589549 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Fri, 17 Dec 2021 16:46:53 -0700 Subject: [PATCH 30/45] always verify authorized keys in parallel with other work, and at most once --- pkg/cmd/codespace/ssh.go | 46 +++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 36aef65fd..5dc4050a6 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -95,8 +95,18 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e return fmt.Errorf("get or choose codespace: %w", err) } - session, err := openSSHSession(ctx, a, codespace, liveshareLogger) + // 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 <- a.ensureAuthorizedKeys(ctx) + }() + + session, err := a.openSSHSession(ctx, codespace, liveshareLogger) if err != nil { + if authErr := <-authkeys; authErr != nil { + return authErr + } return fmt.Errorf("error connecting to codespace: %w", err) } defer safeClose(session, &err) @@ -201,7 +211,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error } }() - session, err := openSSHSession(ctx, a, cs, nil) + session, err := a.openSSHSession(ctx, cs, nil) if err != nil { result.err = fmt.Errorf("error connecting to codespace: %w", err) return @@ -220,6 +230,12 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error }() } + // 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 = a.ensureAuthorizedKeys(ctx); err != nil { + return err + } + ghexec, err := os.Executable() if err != nil { return err @@ -284,19 +300,7 @@ type codespaceSSHConfig struct { GHExec string // path used for invoking the current `gh` binary } -func openSSHSession(ctx context.Context, a *App, codespace *api.Codespace, liveshareLogger *log.Logger) (*liveshare.Session, error) { - // 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 nil, fmt.Errorf("error getting user: %w", err) - } - - authkeys := make(chan error, 1) - go func() { - authkeys <- checkAuthorizedKeys(ctx, a.apiClient, user.Login) - }() - +func (a *App) openSSHSession(ctx context.Context, codespace *api.Codespace, liveshareLogger *log.Logger) (*liveshare.Session, error) { if liveshareLogger == nil { liveshareLogger = noopLogger() } @@ -306,12 +310,16 @@ func openSSHSession(ctx context.Context, a *App, codespace *api.Codespace, lives return nil, fmt.Errorf("error connecting to codespace: %w", err) } - if err := <-authkeys; err != nil { - session.Close() - return nil, err + return session, nil +} + +func (a *App) ensureAuthorizedKeys(ctx context.Context) error { + user, err := a.apiClient.GetUser(ctx) + if err != nil { + return fmt.Errorf("error getting user: %w", err) } - return session, nil + return checkAuthorizedKeys(ctx, a.apiClient, user.Login) } type cpOptions struct { From b2598d64f97a59558e06cd58ea21ebdf50e78e9d Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Fri, 17 Dec 2021 16:57:29 -0700 Subject: [PATCH 31/45] start codespace to fetch config if it's explicitly requested 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. --- pkg/cmd/codespace/ssh.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 5dc4050a6..6349db5fc 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -194,7 +194,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error fetches := 0 var status error for _, cs := range codespaces { - if cs.State != "Available" { + if cs.State != "Available" && opts.codespace == "" { fmt.Fprintf(os.Stderr, "skipping unavailable codespace %s: %s\n", cs.Name, cs.State) status = cmdutil.SilentError continue @@ -423,7 +423,7 @@ func newConfigCmd(app *App) *cobra.Command { configCmd := &cobra.Command{ Use: "config [-c codespace]", Short: "Write OpenSSH configuration to stdout", - Long: heredoc.Docf(` + Long: heredoc.Doc(` The config command generates per-codespace ssh configuration in OpenSSH format. Including this configuration in ~/.ssh/config improves the user experience of other @@ -433,9 +433,10 @@ func newConfigCmd(app *App) *cobra.Command { If -c/--codespace is specified, configuration is generated for that codespace only. Otherwise configuration is emitted for all available codespaces. - Codespaces that aren't in "Available" state are skipped because it's necessary to - connect to the running codespace to determine the required remote ssh username. - `, "`"), + When generating configuration for all codespaces, ones that aren't in "Available" + state are skipped because it's necessary to start the codespace to determine its + remote ssh username. When generating configuration for a single codespace with '-c', + `), Example: heredoc.Doc(` $ gh codespace config > ~/.ssh/codespaces $ echo 'include ~/.ssh/codespaces' >> ~/.ssh/config' From 7b432de5c2af8862350f40a7a89fd4fd32f6ed27 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Fri, 17 Dec 2021 17:09:06 -0700 Subject: [PATCH 32/45] use heredoc helper for config template --- pkg/cmd/codespace/ssh.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 6349db5fc..86ed5d068 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -241,15 +241,16 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error return err } - t, err := template.New("ssh_config").Parse(`Host cs.{{.Name}}.{{.EscapedRef}} - User {{.SSHUser}} - ProxyCommand {{.GHExec}} cs ssh -c {{.Name}} --stdio - UserKnownHostsFile=/dev/null - StrictHostKeyChecking no - LogLevel quiet - ControlMaster auto + 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) } From f22be4a03d489c35f3f57ce96a5449ebc3ad4249 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Mon, 20 Dec 2021 11:27:42 -0700 Subject: [PATCH 33/45] use a more robust method to get the executable path 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. --- pkg/cmd/codespace/root.go | 5 +++-- pkg/cmd/codespace/ssh.go | 17 ++++++----------- pkg/cmd/root/root.go | 2 +- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/pkg/cmd/codespace/root.go b/pkg/cmd/codespace/root.go index 5b2c0d8fc..5a3499dee 100644 --- a/pkg/cmd/codespace/root.go +++ b/pkg/cmd/codespace/root.go @@ -1,10 +1,11 @@ package codespace import ( + "github.com/cli/cli/v2/pkg/cmdutil" "github.com/spf13/cobra" ) -func NewRootCmd(app *App) *cobra.Command { +func NewRootCmd(app *App, f *cmdutil.Factory) *cobra.Command { root := &cobra.Command{ Use: "codespace", Short: "Connect to and manage your codespaces", @@ -16,7 +17,7 @@ func NewRootCmd(app *App) *cobra.Command { root.AddCommand(newListCmd(app)) root.AddCommand(newLogsCmd(app)) root.AddCommand(newPortsCmd(app)) - root.AddCommand(newSSHCmd(app)) + root.AddCommand(newSSHCmd(app, f)) root.AddCommand(newCpCmd(app)) root.AddCommand(newStopCmd(app)) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 86ed5d068..be19eb3f9 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -33,7 +33,7 @@ type sshOptions struct { scpArgs []string // scp arguments, for 'cs cp' (nil for 'cs ssh') } -func newSSHCmd(app *App) *cobra.Command { +func newSSHCmd(app *App, f *cmdutil.Factory) *cobra.Command { var opts sshOptions sshCmd := &cobra.Command{ @@ -67,7 +67,7 @@ func newSSHCmd(app *App) *cobra.Command { sshCmd.Flags().BoolVar(&opts.stdio, "stdio", false, "Proxy sshd connection to stdio") sshCmd.Flags().MarkHidden("stdio") - sshCmd.AddCommand(newConfigCmd(app)) + sshCmd.AddCommand(newConfigCmd(app, f)) return sshCmd } @@ -171,7 +171,7 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e } } -func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error { +func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions, executable string) error { ctx, cancel := context.WithCancel(ctx) defer cancel() @@ -236,11 +236,6 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error return err } - ghexec, err := os.Executable() - if err != nil { - return err - } - t, err := template.New("ssh_config").Parse(heredoc.Doc(` Host cs.{{.Name}}.{{.EscapedRef}} User {{.SSHUser}} @@ -267,7 +262,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error Name: result.codespace.Name, EscapedRef: strings.ReplaceAll(result.codespace.GitStatus.Ref, "/", "-"), SSHUser: result.user, - GHExec: ghexec, + GHExec: executable, } if err := t.Execute(a.io.Out, conf); err != nil { return err @@ -418,7 +413,7 @@ type configOptions struct { codespace string } -func newConfigCmd(app *App) *cobra.Command { +func newConfigCmd(app *App, f *cmdutil.Factory) *cobra.Command { var opts configOptions configCmd := &cobra.Command{ @@ -443,7 +438,7 @@ func newConfigCmd(app *App) *cobra.Command { $ echo 'include ~/.ssh/codespaces' >> ~/.ssh/config' `), RunE: func(cmd *cobra.Command, args []string) error { - return app.printOpenSSHConfig(cmd.Context(), opts) + return app.printOpenSSHConfig(cmd.Context(), opts, f.Executable()) }, DisableFlagsInUseLine: true, } diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index d393d1a7f..58272306c 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -142,7 +142,7 @@ func newCodespaceCmd(f *cmdutil.Factory) *cobra.Command { &lazyLoadedHTTPClient{factory: f}, ), ) - cmd := codespaceCmd.NewRootCmd(app) + cmd := codespaceCmd.NewRootCmd(app, f) cmd.Use = "codespace" cmd.Aliases = []string{"cs"} cmd.Annotations = map[string]string{"IsCore": "true"} From 6b34fa2a274dda76349d3fab84c11297a2b40446 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Mon, 20 Dec 2021 11:38:51 -0700 Subject: [PATCH 34/45] oh look, struct definitions can be scoped! --- pkg/cmd/codespace/ssh.go | 48 ++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index be19eb3f9..66dae59d9 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -190,6 +190,12 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions, execut 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) fetches := 0 var status error @@ -258,6 +264,24 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions, execut 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, "/", "-"), @@ -272,30 +296,6 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions, execut return status } -type sshResult struct { - codespace *api.Codespace - user string // on success, the remote ssh username; else nil - err error -} - -// 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 -} - func (a *App) openSSHSession(ctx context.Context, codespace *api.Codespace, liveshareLogger *log.Logger) (*liveshare.Session, error) { if liveshareLogger == nil { liveshareLogger = noopLogger() From ae3aacb9641c630fa4e4edcb1fb8742083472d69 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Mon, 20 Dec 2021 11:43:55 -0700 Subject: [PATCH 35/45] fix `errcheck` linter warning --- pkg/cmd/codespace/ssh.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 66dae59d9..e16bcb2a0 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -65,7 +65,9 @@ func newSSHCmd(app *App, f *cmdutil.Factory) *cobra.Command { 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().BoolVar(&opts.stdio, "stdio", false, "Proxy sshd connection to stdio") - sshCmd.Flags().MarkHidden("stdio") + if err := sshCmd.Flags().MarkHidden("stdio"); err != nil { + fmt.Fprintf(os.Stderr, "%v\n", err) + } sshCmd.AddCommand(newConfigCmd(app, f)) From 37f8039f76f112dc9ca827f0e2fa48e22e84dbb8 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Mon, 20 Dec 2021 11:49:58 -0700 Subject: [PATCH 36/45] merge ensureAuthorizedKeys into checkAuthorizedKeys --- pkg/cmd/codespace/common.go | 9 +++++++-- pkg/cmd/codespace/logs.go | 7 +------ pkg/cmd/codespace/ssh.go | 13 ++----------- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index 0f6f9cf4e..09b9308aa 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -209,8 +209,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) } diff --git a/pkg/cmd/codespace/logs.go b/pkg/cmd/codespace/logs.go index 9fc6010b4..d0a0c233b 100644 --- a/pkg/cmd/codespace/logs.go +++ b/pkg/cmd/codespace/logs.go @@ -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) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index e16bcb2a0..cce05f131 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -101,7 +101,7 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e // That lets us report a more useful error message if they don't. authkeys := make(chan error, 1) go func() { - authkeys <- a.ensureAuthorizedKeys(ctx) + authkeys <- checkAuthorizedKeys(ctx, a.apiClient) }() session, err := a.openSSHSession(ctx, codespace, liveshareLogger) @@ -240,7 +240,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions, execut // 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 = a.ensureAuthorizedKeys(ctx); err != nil { + if err = checkAuthorizedKeys(ctx, a.apiClient); err != nil { return err } @@ -311,15 +311,6 @@ func (a *App) openSSHSession(ctx context.Context, codespace *api.Codespace, live return session, nil } -func (a *App) ensureAuthorizedKeys(ctx context.Context) error { - user, err := a.apiClient.GetUser(ctx) - if err != nil { - return fmt.Errorf("error getting user: %w", err) - } - - return checkAuthorizedKeys(ctx, a.apiClient, user.Login) -} - type cpOptions struct { sshOptions recursive bool // -r From 28dd73ffdf1d5defab21e4b46d21a60b58fbc28c Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Mon, 20 Dec 2021 11:52:39 -0700 Subject: [PATCH 37/45] always pass a non-nil logger to openSSHSession --- pkg/cmd/codespace/ssh.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index cce05f131..ce7e7be1d 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -219,7 +219,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions, execut } }() - session, err := a.openSSHSession(ctx, cs, nil) + session, err := a.openSSHSession(ctx, cs, noopLogger()) if err != nil { result.err = fmt.Errorf("error connecting to codespace: %w", err) return @@ -299,10 +299,6 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions, execut } func (a *App) openSSHSession(ctx context.Context, codespace *api.Codespace, liveshareLogger *log.Logger) (*liveshare.Session, error) { - if liveshareLogger == nil { - liveshareLogger = noopLogger() - } - session, err := codespaces.ConnectToLiveshare(ctx, a, liveshareLogger, a.apiClient, codespace) if err != nil { return nil, fmt.Errorf("error connecting to codespace: %w", err) From 81b34d272ce262495b8fd3446763310fa7c55d63 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Mon, 20 Dec 2021 11:58:52 -0700 Subject: [PATCH 38/45] inline openSSHSession --- pkg/cmd/codespace/ssh.go | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index ce7e7be1d..af1c3cc06 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -104,7 +104,7 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e authkeys <- checkAuthorizedKeys(ctx, a.apiClient) }() - session, err := a.openSSHSession(ctx, codespace, liveshareLogger) + session, err := codespaces.ConnectToLiveshare(ctx, a, liveshareLogger, a.apiClient, codespace) if err != nil { if authErr := <-authkeys; authErr != nil { return authErr @@ -178,15 +178,15 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions, execut defer cancel() var err error - var codespaces []*api.Codespace + var csList []*api.Codespace if opts.codespace == "" { a.StartProgressIndicatorWithLabel("Fetching codespaces") - codespaces, err = a.apiClient.ListCodespaces(ctx, -1) + csList, err = a.apiClient.ListCodespaces(ctx, -1) a.StopProgressIndicator() } else { var codespace *api.Codespace codespace, err = getOrChooseCodespace(ctx, a.apiClient, opts.codespace) - codespaces = []*api.Codespace{codespace} + csList = []*api.Codespace{codespace} } if err != nil { return fmt.Errorf("error getting codespace info: %w", err) @@ -201,7 +201,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions, execut sshUsers := make(chan sshResult) fetches := 0 var status error - for _, cs := range codespaces { + 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 @@ -219,7 +219,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions, execut } }() - session, err := a.openSSHSession(ctx, cs, noopLogger()) + session, err := codespaces.ConnectToLiveshare(ctx, a, noopLogger(), a.apiClient, cs) if err != nil { result.err = fmt.Errorf("error connecting to codespace: %w", err) return @@ -298,15 +298,6 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions, execut return status } -func (a *App) openSSHSession(ctx context.Context, codespace *api.Codespace, liveshareLogger *log.Logger) (*liveshare.Session, error) { - session, err := codespaces.ConnectToLiveshare(ctx, a, liveshareLogger, a.apiClient, codespace) - if err != nil { - return nil, fmt.Errorf("error connecting to codespace: %w", err) - } - - return session, nil -} - type cpOptions struct { sshOptions recursive bool // -r From a864985f0a007de2f880f4e3a8ca066251dcb2ca Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Mon, 20 Dec 2021 12:12:22 -0700 Subject: [PATCH 39/45] use WaitGroup for a more idiomatic concurrency pattern --- pkg/cmd/codespace/ssh.go | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index af1c3cc06..df19c9c83 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -13,6 +13,7 @@ import ( "os" "path/filepath" "strings" + "sync" "text/template" "github.com/MakeNowJust/heredoc" @@ -199,7 +200,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions, execut } sshUsers := make(chan sshResult) - fetches := 0 + var wg sync.WaitGroup var status error for _, cs := range csList { if cs.State != "Available" && opts.codespace == "" { @@ -209,35 +210,34 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions, execut } cs := cs - fetches += 1 + wg.Add(1) go func() { result := sshResult{} - defer func() { - select { - case sshUsers <- result: - case <-ctx.Done(): - } - }() + 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) - return - } - defer session.Close() + } else { + defer session.Close() - //a.StartProgressIndicatorWithLabel(fmt.Sprintf("Fetching SSH Details for %s", cs.Name)) - _, result.user, err = session.StartSSHServer(ctx) - //a.StopProgressIndicator() - if err != nil { - result.err = fmt.Errorf("error getting ssh server details: %w", err) - return + _, result.user, err = session.StartSSHServer(ctx) + if err != nil { + result.err = fmt.Errorf("error getting ssh server details: %w", err) + } else { + result.codespace = cs + } } - 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 { @@ -258,8 +258,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions, execut return fmt.Errorf("error formatting template: %w", err) } - for i := 0; i < fetches; i++ { - result := <-sshUsers + for result := range sshUsers { if result.err != nil { fmt.Fprintf(os.Stderr, "%v\n", result.err) status = cmdutil.SilentError From 7bd6fe9af8f4c139546c213d3521f2cbc6f82972 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Mon, 20 Dec 2021 12:28:25 -0700 Subject: [PATCH 40/45] maximize the time checkAuthorizedKeys has to run concurrently Also, other than that, restore the original ordering of this function --- pkg/cmd/codespace/ssh.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index df19c9c83..2c214668f 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -81,6 +81,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) + } + liveshareLogger := noopLogger() if opts.debug { debugLogger, err := newFileLogger(opts.debugFile) @@ -93,18 +105,6 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e a.errLogger.Printf("Debug file located at: %s", debugLogger.Name()) } - codespace, err := getOrChooseCodespace(ctx, a.apiClient, opts.codespace) - if err != nil { - return fmt.Errorf("get or choose codespace: %w", err) - } - - // 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) - }() - session, err := codespaces.ConnectToLiveshare(ctx, a, liveshareLogger, a.apiClient, codespace) if err != nil { if authErr := <-authkeys; authErr != nil { From 932c9da4736089180083cccc4e830512eb7982a7 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Mon, 20 Dec 2021 12:39:00 -0700 Subject: [PATCH 41/45] clean up inadvertently truncated help message --- pkg/cmd/codespace/ssh.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 2c214668f..d8ee955f1 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -401,16 +401,17 @@ func newConfigCmd(app *App, f *cmdutil.Factory) *cobra.Command { Long: heredoc.Doc(` The config command generates per-codespace ssh configuration in OpenSSH format. - Including this configuration in ~/.ssh/config improves the user experience of other + Including this configuration in ~/.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. If -c/--codespace is specified, configuration is generated for that codespace only. Otherwise configuration is emitted for all available codespaces. - When generating configuration for all codespaces, ones that aren't in "Available" - state are skipped because it's necessary to start the codespace to determine its - remote ssh username. When generating configuration for a single codespace with '-c', + When generating configuration for all codespaces, ones that aren't in + "Available" state are skipped because it's necessary to start the codespace to + determine its remote ssh username. However, when using '-c' to generate + configuration for a single codespace, it will be started if necessary. `), Example: heredoc.Doc(` $ gh codespace config > ~/.ssh/codespaces From 38eb894d73132dfc3d3c8ec57d42f57a49cef54b Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Mon, 20 Dec 2021 13:33:13 -0700 Subject: [PATCH 42/45] prevent leaking any blocked goroutines on error --- pkg/cmd/codespace/ssh.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index d8ee955f1..18730a176 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -199,7 +199,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions, execut err error } - sshUsers := make(chan sshResult) + sshUsers := make(chan sshResult, len(csList)) var wg sync.WaitGroup var status error for _, cs := range csList { From 3cce16e72d76b2c3981fe27ff8a8864e565b2a5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 21 Dec 2021 13:50:55 +0100 Subject: [PATCH 43/45] Refactor `factory.Executable()` to be a method rather than a func This way, factory can satisfy an interface that requires `Executable()`. --- pkg/cmd/auth/login/login_test.go | 3 +- pkg/cmd/auth/refresh/refresh_test.go | 3 +- pkg/cmd/factory/default.go | 60 ++------------------------- pkg/cmdutil/factory.go | 61 ++++++++++++++++++++++++++-- 4 files changed, 63 insertions(+), 64 deletions(-) diff --git a/pkg/cmd/auth/login/login_test.go b/pkg/cmd/auth/login/login_test.go index f6fbcabd5..c3cde03cf 100644 --- a/pkg/cmd/auth/login/login_test.go +++ b/pkg/cmd/auth/login/login_test.go @@ -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) diff --git a/pkg/cmd/auth/refresh/refresh_test.go b/pkg/cmd/auth/refresh/refresh_test.go index dbdae26af..c8dbebbe5 100644 --- a/pkg/cmd/auth/refresh/refresh_test.go +++ b/pkg/cmd/auth/refresh/refresh_test.go @@ -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) diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go index 08d93c2be..110dfb4e9 100644 --- a/pkg/cmd/factory/default.go +++ b/pkg/cmd/factory/default.go @@ -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 diff --git a/pkg/cmdutil/factory.go b/pkg/cmdutil/factory.go index 83d4a638f..73cee489a 100644 --- a/pkg/cmdutil/factory.go +++ b/pkg/cmdutil/factory.go @@ -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 } From 3b7e5fc24678c8e6558ddd42c565f8280d73f8e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 21 Dec 2021 14:03:10 +0100 Subject: [PATCH 44/45] Store Executable() information on codespaces App 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. --- pkg/cmd/codespace/common.go | 22 ++++++++++++++-------- pkg/cmd/codespace/delete_test.go | 2 +- pkg/cmd/codespace/root.go | 5 ++--- pkg/cmd/codespace/ssh.go | 13 +++++++------ pkg/cmd/root/root.go | 3 ++- 5 files changed, 26 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index 09b9308aa..1107ae6a5 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -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, } } diff --git a/pkg/cmd/codespace/delete_test.go b/pkg/cmd/codespace/delete_test.go index 58090c809..89318b5e3 100644 --- a/pkg/cmd/codespace/delete_test.go +++ b/pkg/cmd/codespace/delete_test.go @@ -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) diff --git a/pkg/cmd/codespace/root.go b/pkg/cmd/codespace/root.go index 5a3499dee..5b2c0d8fc 100644 --- a/pkg/cmd/codespace/root.go +++ b/pkg/cmd/codespace/root.go @@ -1,11 +1,10 @@ package codespace import ( - "github.com/cli/cli/v2/pkg/cmdutil" "github.com/spf13/cobra" ) -func NewRootCmd(app *App, f *cmdutil.Factory) *cobra.Command { +func NewRootCmd(app *App) *cobra.Command { root := &cobra.Command{ Use: "codespace", Short: "Connect to and manage your codespaces", @@ -17,7 +16,7 @@ func NewRootCmd(app *App, f *cmdutil.Factory) *cobra.Command { root.AddCommand(newListCmd(app)) root.AddCommand(newLogsCmd(app)) root.AddCommand(newPortsCmd(app)) - root.AddCommand(newSSHCmd(app, f)) + root.AddCommand(newSSHCmd(app)) root.AddCommand(newCpCmd(app)) root.AddCommand(newStopCmd(app)) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 18730a176..19c4930f5 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -34,7 +34,7 @@ type sshOptions struct { scpArgs []string // scp arguments, for 'cs cp' (nil for 'cs ssh') } -func newSSHCmd(app *App, f *cmdutil.Factory) *cobra.Command { +func newSSHCmd(app *App) *cobra.Command { var opts sshOptions sshCmd := &cobra.Command{ @@ -70,7 +70,7 @@ func newSSHCmd(app *App, f *cmdutil.Factory) *cobra.Command { fmt.Fprintf(os.Stderr, "%v\n", err) } - sshCmd.AddCommand(newConfigCmd(app, f)) + sshCmd.AddCommand(newConfigCmd(app)) return sshCmd } @@ -174,7 +174,7 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e } } -func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions, executable string) error { +func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error { ctx, cancel := context.WithCancel(ctx) defer cancel() @@ -258,6 +258,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions, execut 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) @@ -287,7 +288,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions, execut Name: result.codespace.Name, EscapedRef: strings.ReplaceAll(result.codespace.GitStatus.Ref, "/", "-"), SSHUser: result.user, - GHExec: executable, + GHExec: ghExec, } if err := t.Execute(a.io.Out, conf); err != nil { return err @@ -392,7 +393,7 @@ type configOptions struct { codespace string } -func newConfigCmd(app *App, f *cmdutil.Factory) *cobra.Command { +func newConfigCmd(app *App) *cobra.Command { var opts configOptions configCmd := &cobra.Command{ @@ -418,7 +419,7 @@ func newConfigCmd(app *App, f *cmdutil.Factory) *cobra.Command { $ echo 'include ~/.ssh/codespaces' >> ~/.ssh/config' `), RunE: func(cmd *cobra.Command, args []string) error { - return app.printOpenSSHConfig(cmd.Context(), opts, f.Executable()) + return app.printOpenSSHConfig(cmd.Context(), opts) }, DisableFlagsInUseLine: true, } diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 58272306c..cff797e93 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -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, @@ -142,7 +143,7 @@ func newCodespaceCmd(f *cmdutil.Factory) *cobra.Command { &lazyLoadedHTTPClient{factory: f}, ), ) - cmd := codespaceCmd.NewRootCmd(app, f) + cmd := codespaceCmd.NewRootCmd(app) cmd.Use = "codespace" cmd.Aliases = []string{"cs"} cmd.Annotations = map[string]string{"IsCore": "true"} From 61e5fbb00764d9ed9b9b8d1c443487eb131edda4 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Thu, 6 Jan 2022 10:01:58 -0700 Subject: [PATCH 45/45] Revert "move `gh cs ssh --config` into a separate `gh cs ssh config` command" This reverts commit c9d0085e57a153d30722f4f99d431d7078db9da7. --- pkg/cmd/codespace/ssh.go | 83 +++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 44 deletions(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 19c4930f5..726f2152f 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -31,6 +31,7 @@ type sshOptions struct { debug bool debugFile string stdio bool + config bool scpArgs []string // scp arguments, for 'cs cp' (nil for 'cs ssh') } @@ -40,11 +41,34 @@ func newSSHCmd(app *App) *cobra.Command { sshCmd := &cobra.Command{ Use: "ssh [...] [-- ...] []", 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`") } @@ -52,10 +76,22 @@ func newSSHCmd(app *App) *cobra.Command { 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, } @@ -65,13 +101,12 @@ 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) } - sshCmd.AddCommand(newConfigCmd(app)) - return sshCmd } @@ -174,7 +209,7 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e } } -func (a *App) printOpenSSHConfig(ctx context.Context, opts configOptions) error { +func (a *App) printOpenSSHConfig(ctx context.Context, opts sshOptions) error { ctx, cancel := context.WithCancel(ctx) defer cancel() @@ -389,46 +424,6 @@ func (a *App) Copy(ctx context.Context, args []string, opts cpOptions) error { return a.SSH(ctx, nil, opts.sshOptions) } -type configOptions struct { - codespace string -} - -func newConfigCmd(app *App) *cobra.Command { - var opts configOptions - - configCmd := &cobra.Command{ - Use: "config [-c codespace]", - Short: "Write OpenSSH configuration to stdout", - Long: heredoc.Doc(` - The config command generates per-codespace ssh configuration in OpenSSH format. - - Including this configuration in ~/.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. - - If -c/--codespace is specified, configuration is generated for that codespace - only. Otherwise configuration is emitted for all available codespaces. - - When generating configuration for all codespaces, ones that aren't in - "Available" state are skipped because it's necessary to start the codespace to - determine its remote ssh username. However, when using '-c' to generate - configuration for a single codespace, it will be started if necessary. - `), - Example: heredoc.Doc(` - $ gh codespace config > ~/.ssh/codespaces - $ echo 'include ~/.ssh/codespaces' >> ~/.ssh/config' - `), - RunE: func(cmd *cobra.Command, args []string) error { - return app.printOpenSSHConfig(cmd.Context(), opts) - }, - DisableFlagsInUseLine: true, - } - - configCmd.Flags().StringVarP(&opts.codespace, "codespace", "c", "", "Name of a codespace") - - return configCmd -} - // fileLogger is a wrapper around an log.Logger configured to write // to a file. It exports two additional methods to get the log file name // and close the file handle when the operation is finished.