diff --git a/cmd/ghcs/code.go b/cmd/ghcs/code.go index 3bd67053d..0ec363a6d 100644 --- a/cmd/ghcs/code.go +++ b/cmd/ghcs/code.go @@ -6,7 +6,7 @@ import ( "net/url" "os" - "github.com/github/ghcs/api" + "github.com/github/ghcs/internal/api" "github.com/skratchdot/open-golang/open" "github.com/spf13/cobra" ) diff --git a/cmd/ghcs/common.go b/cmd/ghcs/common.go index bfcb67496..133e6d1de 100644 --- a/cmd/ghcs/common.go +++ b/cmd/ghcs/common.go @@ -11,7 +11,7 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/AlecAivazis/survey/v2/terminal" - "github.com/github/ghcs/api" + "github.com/github/ghcs/internal/api" "golang.org/x/term" ) diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index 093450e7d..b5238a6f7 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -9,8 +9,8 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/fatih/camelcase" - "github.com/github/ghcs/api" "github.com/github/ghcs/cmd/ghcs/output" + "github.com/github/ghcs/internal/api" "github.com/github/ghcs/internal/codespaces" "github.com/spf13/cobra" ) @@ -105,12 +105,16 @@ func create(opts *createOptions) error { return nil } +// showStatus polls the codespace for a list of post create states and their status. It will keep polling +// until all states have finished. Once all states have finished, we poll once more to check if any new +// states have been introduced and stop polling otherwise. func showStatus(ctx context.Context, log *output.Logger, apiClient *api.API, user *api.User, codespace *api.Codespace) error { var lastState codespaces.PostCreateState var breakNextState bool finishedStates := make(map[string]bool) ctx, stopPolling := context.WithCancel(ctx) + defer stopPolling() poller := func(states []codespaces.PostCreateState) { var inProgress bool @@ -153,7 +157,12 @@ func showStatus(ctx context.Context, log *output.Logger, apiClient *api.API, use } } - if err := codespaces.PollPostCreateStates(ctx, log, apiClient, user, codespace, poller); err != nil { + err := codespaces.PollPostCreateStates(ctx, log, apiClient, user, codespace, poller) + if err != nil { + if errors.Is(err, context.Canceled) && breakNextState { + return nil // we cancelled the context to stop polling, we can ignore the error + } + return fmt.Errorf("failed to poll state changes from codespace: %v", err) } diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index 7800a13c0..3f5d68684 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -7,8 +7,8 @@ import ( "os" "strings" - "github.com/github/ghcs/api" "github.com/github/ghcs/cmd/ghcs/output" + "github.com/github/ghcs/internal/api" "github.com/spf13/cobra" ) diff --git a/cmd/ghcs/list.go b/cmd/ghcs/list.go index ee26e3013..dee1b0875 100644 --- a/cmd/ghcs/list.go +++ b/cmd/ghcs/list.go @@ -5,8 +5,8 @@ import ( "fmt" "os" - "github.com/github/ghcs/api" "github.com/github/ghcs/cmd/ghcs/output" + "github.com/github/ghcs/internal/api" "github.com/spf13/cobra" ) diff --git a/cmd/ghcs/logs.go b/cmd/ghcs/logs.go index a685a364d..ccfb46236 100644 --- a/cmd/ghcs/logs.go +++ b/cmd/ghcs/logs.go @@ -6,8 +6,8 @@ import ( "net" "os" - "github.com/github/ghcs/api" "github.com/github/ghcs/cmd/ghcs/output" + "github.com/github/ghcs/internal/api" "github.com/github/ghcs/internal/codespaces" "github.com/github/go-liveshare" "github.com/spf13/cobra" @@ -70,7 +70,8 @@ func logs(ctx context.Context, log *output.Logger, codespaceName string, follow defer listen.Close() localPort := listen.Addr().(*net.TCPAddr).Port - remoteSSHServerPort, sshUser, err := codespaces.StartSSHServer(ctx, session, log) + log.Println("Fetching SSH Details...") + remoteSSHServerPort, sshUser, err := session.StartSSHServer(ctx) if err != nil { return fmt.Errorf("error getting ssh server details: %v", err) } diff --git a/cmd/ghcs/ports.go b/cmd/ghcs/ports.go index f6d1f6e2a..8256c13e2 100644 --- a/cmd/ghcs/ports.go +++ b/cmd/ghcs/ports.go @@ -11,8 +11,8 @@ import ( "strconv" "strings" - "github.com/github/ghcs/api" "github.com/github/ghcs/cmd/ghcs/output" + "github.com/github/ghcs/internal/api" "github.com/github/ghcs/internal/codespaces" "github.com/github/go-liveshare" "github.com/muhammadmuzzammil1998/jsonc" diff --git a/cmd/ghcs/ssh.go b/cmd/ghcs/ssh.go index 3bc2110a0..cdfcdbcbe 100644 --- a/cmd/ghcs/ssh.go +++ b/cmd/ghcs/ssh.go @@ -6,8 +6,8 @@ import ( "net" "os" - "github.com/github/ghcs/api" "github.com/github/ghcs/cmd/ghcs/output" + "github.com/github/ghcs/internal/api" "github.com/github/ghcs/internal/codespaces" "github.com/github/go-liveshare" "github.com/spf13/cobra" @@ -59,7 +59,8 @@ func ssh(ctx context.Context, sshArgs []string, sshProfile, codespaceName string return fmt.Errorf("error connecting to Live Share: %v", err) } - remoteSSHServerPort, sshUser, err := codespaces.StartSSHServer(ctx, session, log) + log.Println("Fetching SSH Details...") + remoteSSHServerPort, sshUser, err := session.StartSSHServer(ctx) if err != nil { return fmt.Errorf("error getting ssh server details: %v", err) } diff --git a/api/api.go b/internal/api/api.go similarity index 94% rename from api/api.go rename to internal/api/api.go index ad69f23fc..8277e903a 100644 --- a/api/api.go +++ b/internal/api/api.go @@ -1,4 +1,3 @@ -// TODO(adonovan): rename to package codespaces, and codespaces.Client. package api // For descriptions of service interfaces, see: @@ -7,6 +6,24 @@ package api // - https://github.com/github/github/blob/master/app/api/codespaces.rb (for vscs_internal) // TODO(adonovan): replace the last link with a public doc URL when available. +// TODO(adonovan): a possible reorganization would be to split this +// file into three internal packages, one per backend service, and to +// rename api.API to github.Client: +// +// - github.GetUser(github.Client) +// - github.GetRepository(Client) +// - github.ReadFile(Client, nwo, branch, path) // was GetCodespaceRepositoryContents +// - codespaces.Create(Client, user, repo, sku, branch, location) +// - codespaces.Delete(Client, user, token, name) +// - codespaces.Get(Client, token, owner, name) +// - codespaces.GetMachineTypes(Client, user, repo, branch, location) +// - codespaces.GetToken(Client, login, name) +// - codespaces.List(Client, user) +// - codespaces.Start(Client, token, codespace) +// - visualstudio.GetRegionLocation(http.Client) // no dependency on github +// +// This would make the meaning of each operation clearer. + import ( "bytes" "context" diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index 9aee3564c..804c2dab5 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -6,7 +6,7 @@ import ( "fmt" "time" - "github.com/github/ghcs/api" + "github.com/github/ghcs/internal/api" "github.com/github/go-liveshare" ) diff --git a/internal/codespaces/ssh.go b/internal/codespaces/ssh.go index 717371286..661caecdc 100644 --- a/internal/codespaces/ssh.go +++ b/internal/codespaces/ssh.go @@ -2,40 +2,12 @@ package codespaces import ( "context" - "errors" - "fmt" "os" "os/exec" "strconv" "strings" - - "github.com/github/go-liveshare" ) -// StartSSHServer installs (if necessary) and starts the SSH in the codespace. -// It returns the remote port where it is running, the user to log in with, or an error if something failed. -func StartSSHServer(ctx context.Context, session *liveshare.Session, log logger) (serverPort int, user string, err error) { - log.Println("Fetching SSH details...") - - sshServer := session.SSHServer() - - sshServerStartResult, err := sshServer.StartRemoteServer(ctx) - if err != nil { - return 0, "", fmt.Errorf("error starting live share: %v", err) - } - - if !sshServerStartResult.Result { - return 0, "", errors.New(sshServerStartResult.Message) - } - - portInt, err := strconv.Atoi(sshServerStartResult.ServerPort) - if err != nil { - return 0, "", fmt.Errorf("error parsing port: %v", err) - } - - return portInt, sshServerStartResult.User, nil -} - // Shell runs an interactive secure shell over an existing // port-forwarding session. It runs until the shell is terminated // (including by cancellation of the context). @@ -60,9 +32,14 @@ func NewRemoteCommand(ctx context.Context, tunnelPort int, destination, command // an interactive shell) over ssh. func newSSHCommand(ctx context.Context, port int, dst, command string) (*exec.Cmd, []string) { connArgs := []string{"-p", strconv.Itoa(port), "-o", "NoHostAuthenticationForLocalhost=yes"} - // TODO(adonovan): eliminate X11 and X11Trust flags where unneeded. - cmdArgs := append([]string{dst, "-X", "-Y", "-C"}, connArgs...) // X11, X11Trust, Compression + cmdArgs := []string{dst, "-C"} // Always use Compression + if command == "" { + // if we are in a shell send X11 and X11Trust + cmdArgs = append(cmdArgs, "-X", "-Y") + } + + cmdArgs = append(cmdArgs, connArgs...) if command != "" { cmdArgs = append(cmdArgs, command) } diff --git a/internal/codespaces/states.go b/internal/codespaces/states.go index 492ce3964..99683b51d 100644 --- a/internal/codespaces/states.go +++ b/internal/codespaces/states.go @@ -9,7 +9,7 @@ import ( "strings" "time" - "github.com/github/ghcs/api" + "github.com/github/ghcs/internal/api" "github.com/github/go-liveshare" ) @@ -39,12 +39,12 @@ type PostCreateState struct { func PollPostCreateStates(ctx context.Context, log logger, apiClient *api.API, user *api.User, codespace *api.Codespace, poller func([]PostCreateState)) error { token, err := apiClient.GetCodespaceToken(ctx, user.Login, codespace.Name) if err != nil { - return fmt.Errorf("getting codespace token: %v", err) + return fmt.Errorf("getting codespace token: %w", err) } session, err := ConnectToLiveshare(ctx, log, apiClient, user.Login, token, codespace) if err != nil { - return fmt.Errorf("connect to Live Share: %v", err) + return fmt.Errorf("connect to Live Share: %w", err) } // Ensure local port is listening before client (getPostCreateOutput) connects. @@ -54,9 +54,10 @@ func PollPostCreateStates(ctx context.Context, log logger, apiClient *api.API, u } localPort := listen.Addr().(*net.TCPAddr).Port - remoteSSHServerPort, sshUser, err := StartSSHServer(ctx, session, log) + log.Println("Fetching SSH Details...") + remoteSSHServerPort, sshUser, err := session.StartSSHServer(ctx) if err != nil { - return fmt.Errorf("error getting ssh server details: %v", err) + return fmt.Errorf("error getting ssh server details: %w", err) } tunnelClosed := make(chan error, 1) // buffered to avoid sender stuckness @@ -74,12 +75,12 @@ func PollPostCreateStates(ctx context.Context, log logger, apiClient *api.API, u return ctx.Err() case err := <-tunnelClosed: - return fmt.Errorf("connection failed: %v", err) + return fmt.Errorf("connection failed: %w", err) case <-t.C: states, err := getPostCreateOutput(ctx, localPort, codespace, sshUser) if err != nil { - return fmt.Errorf("get post create output: %v", err) + return fmt.Errorf("get post create output: %w", err) } poller(states) @@ -95,13 +96,13 @@ func getPostCreateOutput(ctx context.Context, tunnelPort int, codespace *api.Cod stdout := new(bytes.Buffer) cmd.Stdout = stdout if err := cmd.Run(); err != nil { - return nil, fmt.Errorf("run command: %v", err) + return nil, fmt.Errorf("run command: %w", err) } var output struct { Steps []PostCreateState `json:"steps"` } if err := json.Unmarshal(stdout.Bytes(), &output); err != nil { - return nil, fmt.Errorf("unmarshal output: %v", err) + return nil, fmt.Errorf("unmarshal output: %w", err) } return output.Steps, nil