From e57b390d4a75e4b97e304b65a122f5874b1e14c7 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Tue, 3 Aug 2021 13:42:34 +0000 Subject: [PATCH 01/26] dotfiles status spike --- cmd/ghcs/create.go | 33 +++++++++- internal/codespaces/dotfiles.go | 109 ++++++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 internal/codespaces/dotfiles.go diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index 385e5d957..790364c19 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -9,6 +9,7 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/fatih/camelcase" "github.com/github/ghcs/api" + "github.com/github/ghcs/internal/codespaces" "github.com/spf13/cobra" ) @@ -17,7 +18,7 @@ var repo, branch, machine string func newCreateCmd() *cobra.Command { createCmd := &cobra.Command{ Use: "create", - Short: "Create a GitHub Codespace.", + Short: "Create a GitHub Codespace.", RunE: func(cmd *cobra.Command, args []string) error { return Create() }, @@ -80,6 +81,36 @@ func Create() error { return fmt.Errorf("error creating codespace: %v", err) } + states, err := codespaces.PollPostCreateStates(ctx, apiClient, userResult.User, codespace) + if err != nil { + return fmt.Errorf("poll post create states: %v", err) + } + + for { + select { + case stateUpdate := <-states: + if stateUpdate.Err != nil { + return fmt.Errorf("receive state update: %v", err) + } + + var inProgress bool + for _, state := range stateUpdate.PostCreateStates { + fmt.Print(state.Name) + switch state.Status { + case codespaces.PostCreateStateRunning: + inProgress = true + case codespaces.PostCreateStateFailed: + fmt.Print("...Failed") + } + fmt.Print("\n") + } + + if !inProgress { + break + } + } + } + fmt.Println("Codespace created: " + codespace.Name) return nil diff --git a/internal/codespaces/dotfiles.go b/internal/codespaces/dotfiles.go new file mode 100644 index 000000000..75411c35f --- /dev/null +++ b/internal/codespaces/dotfiles.go @@ -0,0 +1,109 @@ +package codespaces + +import ( + "context" + "encoding/json" + "fmt" + "io/ioutil" + "time" + + "github.com/github/ghcs/api" +) + +type PostCreateStateStatus string + +const ( + PostCreateStateRunning PostCreateStateStatus = "running" + PostCreateStateSuccess PostCreateStateStatus = "succeeded" + PostCreateStateFailed PostCreateStateStatus = "failed" +) + +type PostCreateStatesResult struct { + PostCreateStates PostCreateStates + Err error +} + +type PostCreateStates []*PostCreateState + +type PostCreateState struct { + Name string `json:"name"` + Status PostCreateStateStatus `json:"status"` +} + +func PollPostCreateStates(ctx context.Context, apiClient *api.API, user *api.User, codespace *api.Codespace) (<-chan PostCreateStatesResult, error) { + pollch := make(chan PostCreateStatesResult) + + token, err := apiClient.GetCodespaceToken(ctx, user.Login, codespace.Name) + if err != nil { + return nil, fmt.Errorf("getting codespace token: %v", err) + } + + lsclient, err := ConnectToLiveshare(ctx, apiClient, token, codespace) + if err != nil { + return nil, fmt.Errorf("connect to liveshare: %v", err) + } + + tunnelPort, connClosed, err := MakeSSHTunnel(ctx, lsclient, 0) + if err != nil { + return nil, fmt.Errorf("make ssh tunnel: %v", err) + } + + go func() { + t := time.NewTicker(1 * time.Second) + for { + select { + case <-ctx.Done(): + return + case err := <-connClosed: + if err != nil { + pollch <- PostCreateStatesResult{Err: fmt.Errorf("connection closed: %v", err)} + return + } + case <-t.C: + states, err := getPostCreateOutput(ctx, tunnelPort, codespace) + if err != nil { + pollch <- PostCreateStatesResult{Err: fmt.Errorf("get post create output: %v", err)} + return + } + + pollch <- PostCreateStatesResult{ + PostCreateStates: states, + } + } + } + }() + + return pollch, nil +} + +func getPostCreateOutput(ctx context.Context, tunnelPort int, codespace *api.Codespace) (PostCreateStates, error) { + stdout, err := RunCommand( + ctx, tunnelPort, sshDestination(codespace), + "cat /workspaces/.codespaces/shared/postCreateOutput.json", + ) + if err != nil { + return nil, fmt.Errorf("run command: %v", err) + } + + b, err := ioutil.ReadAll(stdout) + if err != nil { + return nil, fmt.Errorf("read output: %v", err) + } + + output := struct { + Steps PostCreateStates `json:"steps"` + }{} + if err := json.Unmarshal(b, &output); err != nil { + return nil, fmt.Errorf("unmarshal output: %v", err) + } + + return output.Steps, nil +} + +func sshDestination(codespace *api.Codespace) string { + user := "codespace" + if codespace.RepositoryNWO == "github/github" { + user = "root" + } + return user + "@localhost" +} From 76aca39f5bc56e01fc07628903e940d75ae5064d Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Wed, 4 Aug 2021 17:35:11 +0000 Subject: [PATCH 02/26] Create status support --- cmd/ghcs/create.go | 31 ++++++++++++++++--- cmd/ghcs/logs.go | 2 +- cmd/ghcs/ports.go | 6 ++-- cmd/ghcs/ssh.go | 7 ++--- internal/codespaces/codespaces.go | 11 ++++--- .../codespaces/{dotfiles.go => states.go} | 4 +-- 6 files changed, 41 insertions(+), 20 deletions(-) rename internal/codespaces/{dotfiles.go => states.go} (95%) diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index 790364c19..236ed0e01 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -86,6 +86,10 @@ func Create() error { return fmt.Errorf("poll post create states: %v", err) } + var lastState codespaces.PostCreateState + var breakNextState bool + +PollStates: for { select { case stateUpdate := <-states: @@ -95,18 +99,35 @@ func Create() error { var inProgress bool for _, state := range stateUpdate.PostCreateStates { - fmt.Print(state.Name) switch state.Status { case codespaces.PostCreateStateRunning: + if lastState != state { + lastState = state + fmt.Print(state.Name) + } else { + fmt.Print(".") + } + inProgress = true + break case codespaces.PostCreateStateFailed: - fmt.Print("...Failed") + if lastState.Name == state.Name && lastState.Status != state.Status { + lastState = state + fmt.Print(".Failed\n") + } + case codespaces.PostCreateStateSuccess: + if lastState.Name == state.Name && lastState.Status != state.Status { + lastState = state + fmt.Print(".Success\n") + } } - fmt.Print("\n") } - if !inProgress { - break + switch { + case !inProgress && !breakNextState: + breakNextState = true + case !inProgress && breakNextState: + break PollStates } } } diff --git a/cmd/ghcs/logs.go b/cmd/ghcs/logs.go index 03a7c963a..c8c95182e 100644 --- a/cmd/ghcs/logs.go +++ b/cmd/ghcs/logs.go @@ -49,7 +49,7 @@ func Logs(tail bool, codespaceName string) error { return fmt.Errorf("get or choose codespace: %v", err) } - lsclient, err := codespaces.ConnectToLiveshare(ctx, apiClient, token, codespace) + lsclient, err := codespaces.ConnectToLiveshare(ctx, apiClient, user.Login, token, codespace) if err != nil { return fmt.Errorf("connecting to liveshare: %v", err) } diff --git a/cmd/ghcs/ports.go b/cmd/ghcs/ports.go index 77d1b00f7..e573df483 100644 --- a/cmd/ghcs/ports.go +++ b/cmd/ghcs/ports.go @@ -61,7 +61,7 @@ func Ports() error { return fmt.Errorf("error getting codespace token: %v", err) } - liveShareClient, err := codespaces.ConnectToLiveshare(ctx, apiClient, token, codespace) + liveShareClient, err := codespaces.ConnectToLiveshare(ctx, apiClient, user.Login, token, codespace) if err != nil { return fmt.Errorf("error connecting to liveshare: %v", err) } @@ -214,7 +214,7 @@ func updatePortVisibility(codespaceName, sourcePort string, public bool) error { return fmt.Errorf("error getting codespace: %v", err) } - lsclient, err := codespaces.ConnectToLiveshare(ctx, apiClient, token, codespace) + lsclient, err := codespaces.ConnectToLiveshare(ctx, apiClient, user.Login, token, codespace) if err != nil { return fmt.Errorf("error connecting to liveshare: %v", err) } @@ -276,7 +276,7 @@ func forwardPort(codespaceName, sourcePort, destPort string) error { return fmt.Errorf("error getting codespace: %v", err) } - lsclient, err := codespaces.ConnectToLiveshare(ctx, apiClient, token, codespace) + lsclient, err := codespaces.ConnectToLiveshare(ctx, apiClient, user.Login, token, codespace) if err != nil { return fmt.Errorf("error connecting to liveshare: %v", err) } diff --git a/cmd/ghcs/ssh.go b/cmd/ghcs/ssh.go index 60fdee498..caf85c576 100644 --- a/cmd/ghcs/ssh.go +++ b/cmd/ghcs/ssh.go @@ -51,7 +51,7 @@ func SSH(sshProfile, codespaceName string, sshServerPort int) error { return fmt.Errorf("get or choose codespace: %v", err) } - lsclient, err := codespaces.ConnectToLiveshare(ctx, apiClient, token, codespace) + lsclient, err := codespaces.ConnectToLiveshare(ctx, apiClient, user.Login, token, codespace) if err != nil { return fmt.Errorf("error connecting to liveshare: %v", err) } @@ -61,7 +61,7 @@ func SSH(sshProfile, codespaceName string, sshServerPort int) error { return fmt.Errorf("error creating liveshare terminal: %v", err) } - fmt.Println("Preparing SSH...") + fmt.Print("Preparing SSH...") if sshProfile == "" { containerID, err := getContainerID(ctx, terminal) if err != nil { @@ -71,9 +71,8 @@ func SSH(sshProfile, codespaceName string, sshServerPort int) error { if err := setupSSH(ctx, terminal, containerID, codespace.RepositoryName); err != nil { return fmt.Errorf("error creating ssh server: %v", err) } - - fmt.Printf("\n") } + fmt.Print("\n") tunnelPort, tunnelClosed, err := codespaces.MakeSSHTunnel(ctx, lsclient, sshServerPort) if err != nil { diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index 4c62d9aff..5c40c9931 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -57,9 +57,11 @@ func ChooseCodespace(ctx context.Context, apiClient *api.API, user *api.User) (* return codespace, nil } -func ConnectToLiveshare(ctx context.Context, apiClient *api.API, token string, codespace *api.Codespace) (client *liveshare.Client, err error) { +func ConnectToLiveshare(ctx context.Context, apiClient *api.API, userLogin, token string, codespace *api.Codespace) (client *liveshare.Client, err error) { + var startedCodespace bool if codespace.Environment.State != api.CodespaceEnvironmentStateAvailable { - fmt.Println("Starting your codespace...") // TODO(josebalius): better way of notifying of events + startedCodespace = true + fmt.Print("Starting your codespace...") // TODO(josebalius): better way of notifying of events if err := apiClient.StartCodespace(ctx, token, codespace); err != nil { return nil, fmt.Errorf("error starting codespace: %v", err) } @@ -79,7 +81,7 @@ func ConnectToLiveshare(ctx context.Context, apiClient *api.API, token string, c return nil, errors.New("timed out while waiting for the codespace to start") } - codespace, err = apiClient.GetCodespace(ctx, token, codespace.OwnerLogin, codespace.Name) + codespace, err = apiClient.GetCodespace(ctx, token, userLogin, codespace.Name) if err != nil { return nil, fmt.Errorf("error getting codespace: %v", err) } @@ -87,10 +89,9 @@ func ConnectToLiveshare(ctx context.Context, apiClient *api.API, token string, c retries += 1 } - if retries >= 2 { + if startedCodespace { fmt.Print("\n") } - fmt.Println("Connecting to your codespace...") lsclient, err := liveshare.NewClient( diff --git a/internal/codespaces/dotfiles.go b/internal/codespaces/states.go similarity index 95% rename from internal/codespaces/dotfiles.go rename to internal/codespaces/states.go index 75411c35f..e16e4fc7d 100644 --- a/internal/codespaces/dotfiles.go +++ b/internal/codespaces/states.go @@ -23,7 +23,7 @@ type PostCreateStatesResult struct { Err error } -type PostCreateStates []*PostCreateState +type PostCreateStates []PostCreateState type PostCreateState struct { Name string `json:"name"` @@ -38,7 +38,7 @@ func PollPostCreateStates(ctx context.Context, apiClient *api.API, user *api.Use return nil, fmt.Errorf("getting codespace token: %v", err) } - lsclient, err := ConnectToLiveshare(ctx, apiClient, token, codespace) + lsclient, err := ConnectToLiveshare(ctx, apiClient, user.Login, token, codespace) if err != nil { return nil, fmt.Errorf("connect to liveshare: %v", err) } From 46ee45bcdd99c54849f76c4484950c7825a85616 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Tue, 24 Aug 2021 17:46:24 -0400 Subject: [PATCH 03/26] simplify the state iteration --- cmd/ghcs/create.go | 64 +++++++++++++++---------------- cmd/ghcs/ssh.go | 2 +- internal/codespaces/codespaces.go | 3 +- 3 files changed, 33 insertions(+), 36 deletions(-) diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index 9c509b851..890e6b424 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -92,46 +92,42 @@ func Create() error { var lastState codespaces.PostCreateState var breakNextState bool -PollStates: for { - select { - case stateUpdate := <-states: - if stateUpdate.Err != nil { - return fmt.Errorf("receive state update: %v", err) - } + stateUpdate := <-states + if stateUpdate.Err != nil { + return fmt.Errorf("receive state update: %v", err) + } - var inProgress bool - for _, state := range stateUpdate.PostCreateStates { - switch state.Status { - case codespaces.PostCreateStateRunning: - if lastState != state { - lastState = state - fmt.Print(state.Name) - } else { - fmt.Print(".") - } + var inProgress bool + for _, state := range stateUpdate.PostCreateStates { + switch state.Status { + case codespaces.PostCreateStateRunning: + if lastState != state { + lastState = state + log.Print(state.Name) + } else { + log.Print(".") + } - inProgress = true - break - case codespaces.PostCreateStateFailed: - if lastState.Name == state.Name && lastState.Status != state.Status { - lastState = state - fmt.Print(".Failed\n") - } - case codespaces.PostCreateStateSuccess: - if lastState.Name == state.Name && lastState.Status != state.Status { - lastState = state - fmt.Print(".Success\n") - } + inProgress = true + break + case codespaces.PostCreateStateFailed: + if lastState.Name == state.Name && lastState.Status != state.Status { + lastState = state + log.Print(".Failed\n") + } + case codespaces.PostCreateStateSuccess: + if lastState.Name == state.Name && lastState.Status != state.Status { + lastState = state + log.Print(".Success\n") } } + } - switch { - case !inProgress && !breakNextState: - breakNextState = true - case !inProgress && breakNextState: - break PollStates - } + if !inProgress && !breakNextState { + breakNextState = true + } else if !inProgress && breakNextState { + break } } diff --git a/cmd/ghcs/ssh.go b/cmd/ghcs/ssh.go index bb1edfeee..a895b6b4d 100644 --- a/cmd/ghcs/ssh.go +++ b/cmd/ghcs/ssh.go @@ -64,7 +64,7 @@ func SSH(sshProfile, codespaceName string, sshServerPort int) error { return fmt.Errorf("error creating liveshare terminal: %v", err) } - log.Println("Preparing SSH...") + log.Print("Preparing SSH...") if sshProfile == "" { containerID, err := getContainerID(ctx, log, terminal) if err != nil { diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index bbf63b709..005ea0fda 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -66,7 +66,7 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient *api.API, use var startedCodespace bool if codespace.Environment.State != api.CodespaceEnvironmentStateAvailable { startedCodespace = true - log.Println("Starting your codespace...") + log.Print("Starting your codespace...") if err := apiClient.StartCodespace(ctx, token, codespace); err != nil { return nil, fmt.Errorf("error starting codespace: %v", err) } @@ -97,6 +97,7 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient *api.API, use if startedCodespace { fmt.Print("\n") } + log.Println("Connecting to your codespace...") lsclient, err := liveshare.NewClient( From 2ef6e95982342bcb0069c810889a853d8857f4a7 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Tue, 24 Aug 2021 20:15:21 -0400 Subject: [PATCH 04/26] show status under a flag --- cmd/ghcs/create.go | 48 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index 890e6b424..9b25b00b5 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -17,19 +17,29 @@ import ( var repo, branch, machine string +type CreateOptions struct { + Repo string + Branch string + Machine string + ShowStatus bool +} + func newCreateCmd() *cobra.Command { + opts := &CreateOptions{} + createCmd := &cobra.Command{ Use: "create", Short: "Create a Codespace", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - return Create() + return Create(opts) }, } - createCmd.Flags().StringVarP(&repo, "repo", "r", "", "repository name with owner: user/repo") - createCmd.Flags().StringVarP(&branch, "branch", "b", "", "repository branch") - createCmd.Flags().StringVarP(&machine, "machine", "m", "", "hardware specifications for the VM") + createCmd.Flags().StringVarP(&opts.Repo, "repo", "r", "", "repository name with owner: user/repo") + createCmd.Flags().StringVarP(&opts.Branch, "branch", "b", "", "repository branch") + createCmd.Flags().StringVarP(&opts.Machine, "machine", "m", "", "hardware specifications for the VM") + createCmd.Flags().BoolVarP(&opts.ShowStatus, "status", "s", false, "show status of post-create command and dotfiles") return createCmd } @@ -38,18 +48,18 @@ func init() { rootCmd.AddCommand(newCreateCmd()) } -func Create() error { +func Create(opts *CreateOptions) error { ctx := context.Background() apiClient := api.New(os.Getenv("GITHUB_TOKEN")) locationCh := getLocation(ctx, apiClient) userCh := getUser(ctx, apiClient) log := output.NewLogger(os.Stdout, os.Stderr, false) - repo, err := getRepoName() + repo, err := getRepoName(opts.Repo) if err != nil { return fmt.Errorf("error getting repository name: %v", err) } - branch, err := getBranchName() + branch, err := getBranchName(opts.Branch) if err != nil { return fmt.Errorf("error getting branch name: %v", err) } @@ -69,7 +79,7 @@ func Create() error { return fmt.Errorf("error getting codespace user: %v", userResult.Err) } - machine, err := getMachineName(ctx, userResult.User, repository, locationResult.Location, apiClient) + machine, err := getMachineName(ctx, opts.Machine, userResult.User, repository, locationResult.Location, apiClient) if err != nil { return fmt.Errorf("error getting machine type: %v", err) } @@ -84,7 +94,19 @@ func Create() error { return fmt.Errorf("error creating codespace: %v", err) } - states, err := codespaces.PollPostCreateStates(ctx, log, apiClient, userResult.User, codespace) + if opts.ShowStatus { + if err := showStatus(ctx, log, apiClient, userResult.User, codespace); err != nil { + return fmt.Errorf("show status: %w", err) + } + } + + log.Printf("Codespace created: %s\n", codespace.Name) + + return nil +} + +func showStatus(ctx context.Context, log *output.Logger, apiClient *api.API, user *api.User, codespace *api.Codespace) error { + states, err := codespaces.PollPostCreateStates(ctx, log, apiClient, user, codespace) if err != nil { return fmt.Errorf("poll post create states: %v", err) } @@ -131,8 +153,6 @@ func Create() error { } } - log.Printf("Codespace created: %s\n", codespace.Name) - return nil } @@ -164,7 +184,7 @@ func getLocation(ctx context.Context, apiClient *api.API) <-chan locationResult return ch } -func getRepoName() (string, error) { +func getRepoName(repo string) (string, error) { if repo != "" { return repo, nil } @@ -180,7 +200,7 @@ func getRepoName() (string, error) { return repo, err } -func getBranchName() (string, error) { +func getBranchName(branch string) (string, error) { if branch != "" { return branch, nil } @@ -196,7 +216,7 @@ func getBranchName() (string, error) { return branch, err } -func getMachineName(ctx context.Context, user *api.User, repo *api.Repository, location string, apiClient *api.API) (string, error) { +func getMachineName(ctx context.Context, machine string, user *api.User, repo *api.Repository, location string, apiClient *api.API) (string, error) { skus, err := apiClient.GetCodespacesSkus(ctx, user, repo, location) if err != nil { return "", fmt.Errorf("error getting codespace skus: %v", err) From 151eb2b656e8ee2b4864653f3f785133f506c47e Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Thu, 26 Aug 2021 08:35:30 -0400 Subject: [PATCH 05/26] fix linter --- cmd/ghcs/create.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index 9b25b00b5..d08c4913d 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -15,8 +15,6 @@ import ( "github.com/spf13/cobra" ) -var repo, branch, machine string - type CreateOptions struct { Repo string Branch string @@ -132,7 +130,6 @@ func showStatus(ctx context.Context, log *output.Logger, apiClient *api.API, use } inProgress = true - break case codespaces.PostCreateStateFailed: if lastState.Name == state.Name && lastState.Status != state.Status { lastState = state From 3dcee5cca72f0a70aed7cdc270cd4a0d6f0d584b Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Fri, 27 Aug 2021 12:41:36 +0000 Subject: [PATCH 06/26] remove dst port column and add docs --- cmd/ghcs/ports.go | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/cmd/ghcs/ports.go b/cmd/ghcs/ports.go index 09397af54..9e2713a5e 100644 --- a/cmd/ghcs/ports.go +++ b/cmd/ghcs/ports.go @@ -19,11 +19,16 @@ import ( "golang.org/x/sync/errgroup" ) +// PortOptions represents the options accepted by the ports command. type PortsOptions struct { + // CodespaceName is the name of the codespace, optional CodespaceName string - AsJSON bool + + // AsJSON dictates whether the command returns a json output or not, optional + AsJSON bool } +// NewPortsCmd returns a new cobra command representing the ports command and sub commands func NewPortsCmd() *cobra.Command { opts := &PortsOptions{} @@ -50,6 +55,7 @@ func init() { rootCmd.AddCommand(NewPortsCmd()) } +// Ports accepts a PortOptions pointer and logs a list the list of available open ports found in a codespace func Ports(opts *PortsOptions) error { apiClient := api.New(os.Getenv("GITHUB_TOKEN")) ctx := context.Background() @@ -87,7 +93,7 @@ func Ports(opts *PortsOptions) error { } table := output.NewTable(os.Stdout, opts.AsJSON) - table.SetHeader([]string{"Label", "Source Port", "Destination Port", "Public", "Browse URL"}) + table.SetHeader([]string{"Label", "Port", "Public", "Browse URL"}) for _, port := range ports { sourcePort := strconv.Itoa(port.SourcePort) var portName string @@ -100,7 +106,6 @@ func Ports(opts *PortsOptions) error { table.Append([]string{ portName, sourcePort, - strconv.Itoa(port.DestinationPort), strings.ToUpper(strconv.FormatBool(port.IsPublic)), fmt.Sprintf("https://%s-%s.githubpreview.dev/", codespace.Name, sourcePort), }) @@ -168,6 +173,8 @@ func getDevContainer(ctx context.Context, apiClient *api.API, codespace *api.Cod return ch } +// NewPortsPublicCmd returns a cobra command representing the ports subcommand used +// to make a given port public func NewPortsPublicCmd() *cobra.Command { return &cobra.Command{ Use: "public ", @@ -180,6 +187,8 @@ func NewPortsPublicCmd() *cobra.Command { } } +// NewPortsPrivateCmd rturns a cobra command representing the ports subcommand used +// to make a given port private func NewPortsPrivateCmd() *cobra.Command { return &cobra.Command{ Use: "private ", @@ -239,6 +248,8 @@ func updatePortVisibility(log *output.Logger, codespaceName, sourcePort string, return nil } +// NewPortsForwardCmd returns a cobra command representing the ports subcommand used to forward +// ports from the codespace to localhost, it supports multiple ports to be forwarded at once func NewPortsForwardCmd() *cobra.Command { return &cobra.Command{ Use: "forward :", From 0392c5017408cac6e63b71dd13b7e318350fd225 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Fri, 27 Aug 2021 11:25:24 -0400 Subject: [PATCH 07/26] api: close HTTP response body on all paths --- api/api.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/api/api.go b/api/api.go index 83510d8c5..25faf5724 100644 --- a/api/api.go +++ b/api/api.go @@ -44,6 +44,7 @@ func (a *API) GetUser(ctx context.Context) (*User, error) { if err != nil { return nil, fmt.Errorf("error making request: %v", err) } + defer resp.Body.Close() b, err := ioutil.ReadAll(resp.Body) if err != nil { @@ -86,6 +87,7 @@ func (a *API) GetRepository(ctx context.Context, nwo string) (*Repository, error if err != nil { return nil, fmt.Errorf("error making request: %v", err) } + defer resp.Body.Close() b, err := ioutil.ReadAll(resp.Body) if err != nil { @@ -152,6 +154,7 @@ func (a *API) ListCodespaces(ctx context.Context, user *User) (Codespaces, error if err != nil { return nil, fmt.Errorf("error making request: %v", err) } + defer resp.Body.Close() b, err := ioutil.ReadAll(resp.Body) if err != nil { @@ -199,6 +202,7 @@ func (a *API) GetCodespaceToken(ctx context.Context, ownerLogin, codespaceName s if err != nil { return "", fmt.Errorf("error making request: %v", err) } + defer resp.Body.Close() b, err := ioutil.ReadAll(resp.Body) if err != nil { @@ -232,6 +236,7 @@ func (a *API) GetCodespace(ctx context.Context, token, owner, codespace string) if err != nil { return nil, fmt.Errorf("error making request: %v", err) } + defer resp.Body.Close() b, err := ioutil.ReadAll(resp.Body) if err != nil { @@ -261,10 +266,13 @@ func (a *API) StartCodespace(ctx context.Context, token string, codespace *Codes } req.Header.Set("Authorization", "Bearer "+token) - _, err = a.client.Do(req) + resp, err := a.client.Do(req) if err != nil { return fmt.Errorf("error making request: %v", err) } + defer resp.Body.Close() + + // TODO: check status code? return nil } @@ -283,12 +291,15 @@ func (a *API) GetCodespaceRegionLocation(ctx context.Context) (string, error) { if err != nil { return "", fmt.Errorf("error making request: %v", err) } + defer resp.Body.Close() b, err := ioutil.ReadAll(resp.Body) if err != nil { return "", fmt.Errorf("error reading response body: %v", err) } + // TODO: check status code? + var response getCodespaceRegionLocationResponse if err := json.Unmarshal(b, &response); err != nil { return "", fmt.Errorf("error unmarshaling response: %v", err) @@ -320,12 +331,15 @@ func (a *API) GetCodespacesSkus(ctx context.Context, user *User, repository *Rep if err != nil { return nil, fmt.Errorf("error making request: %v", err) } + defer resp.Body.Close() b, err := ioutil.ReadAll(resp.Body) if err != nil { return nil, fmt.Errorf("error reading response body: %v", err) } + // TODO: check status code? + response := struct { Skus Skus `json:"skus"` }{} @@ -359,6 +373,7 @@ func (a *API) CreateCodespace(ctx context.Context, user *User, repository *Repos if err != nil { return nil, fmt.Errorf("error making request: %v", err) } + defer resp.Body.Close() b, err := ioutil.ReadAll(resp.Body) if err != nil { @@ -388,6 +403,7 @@ func (a *API) DeleteCodespace(ctx context.Context, user *User, token, codespaceN if err != nil { return fmt.Errorf("error making request: %v", err) } + defer resp.Body.Close() if resp.StatusCode > http.StatusAccepted { b, err := ioutil.ReadAll(resp.Body) @@ -419,6 +435,7 @@ func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *Cod if err != nil { return nil, fmt.Errorf("error making request: %v", err) } + defer resp.Body.Close() if resp.StatusCode == http.StatusNotFound { return nil, nil From 5dc923777be8c5ca232128c2b7924420b49b6bfd Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Fri, 27 Aug 2021 15:32:18 +0000 Subject: [PATCH 08/26] update docs, make ports private to be more consistent --- cmd/ghcs/ports.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/cmd/ghcs/ports.go b/cmd/ghcs/ports.go index 9e2713a5e..2318097aa 100644 --- a/cmd/ghcs/ports.go +++ b/cmd/ghcs/ports.go @@ -21,14 +21,15 @@ import ( // PortOptions represents the options accepted by the ports command. type PortsOptions struct { - // CodespaceName is the name of the codespace, optional + // CodespaceName is the name of the codespace, optional. CodespaceName string - // AsJSON dictates whether the command returns a json output or not, optional + // AsJSON dictates whether the command returns a json output or not, optional. AsJSON bool } -// NewPortsCmd returns a new cobra command representing the ports command and sub commands +// NewPortsCmd returns a Cobra "ports" command that displays a table of available ports, +// according to the specified flags. func NewPortsCmd() *cobra.Command { opts := &PortsOptions{} @@ -37,7 +38,7 @@ func NewPortsCmd() *cobra.Command { Short: "List ports in a Codespace", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - return Ports(opts) + return ports(opts) }, } @@ -55,8 +56,7 @@ func init() { rootCmd.AddCommand(NewPortsCmd()) } -// Ports accepts a PortOptions pointer and logs a list the list of available open ports found in a codespace -func Ports(opts *PortsOptions) error { +func ports(opts *PortsOptions) error { apiClient := api.New(os.Getenv("GITHUB_TOKEN")) ctx := context.Background() log := output.NewLogger(os.Stdout, os.Stderr, opts.AsJSON) @@ -173,7 +173,7 @@ func getDevContainer(ctx context.Context, apiClient *api.API, codespace *api.Cod return ch } -// NewPortsPublicCmd returns a cobra command representing the ports subcommand used +// NewPortsPublicCmd returns a Cobra "ports public" subcommand, which makes a given port public. // to make a given port public func NewPortsPublicCmd() *cobra.Command { return &cobra.Command{ @@ -187,8 +187,7 @@ func NewPortsPublicCmd() *cobra.Command { } } -// NewPortsPrivateCmd rturns a cobra command representing the ports subcommand used -// to make a given port private +// NewPortsPrivateCmd returns a Cobra "ports private" subcommand, which makes a given port private. func NewPortsPrivateCmd() *cobra.Command { return &cobra.Command{ Use: "private ", @@ -248,8 +247,8 @@ func updatePortVisibility(log *output.Logger, codespaceName, sourcePort string, return nil } -// NewPortsForwardCmd returns a cobra command representing the ports subcommand used to forward -// ports from the codespace to localhost, it supports multiple ports to be forwarded at once +// NewPortsForwardCmd returns a Cobra "ports forward" subcommand, which forwards a set of +// port pairs from the codespace to localhost. func NewPortsForwardCmd() *cobra.Command { return &cobra.Command{ Use: "forward :", From 8e95493872f31e953a33a86381e12ab93f7999f5 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Fri, 27 Aug 2021 15:46:40 +0000 Subject: [PATCH 09/26] period --- cmd/ghcs/ports.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/ghcs/ports.go b/cmd/ghcs/ports.go index 2318097aa..ea501b73e 100644 --- a/cmd/ghcs/ports.go +++ b/cmd/ghcs/ports.go @@ -174,7 +174,7 @@ func getDevContainer(ctx context.Context, apiClient *api.API, codespace *api.Cod } // NewPortsPublicCmd returns a Cobra "ports public" subcommand, which makes a given port public. -// to make a given port public +// to make a given port public. func NewPortsPublicCmd() *cobra.Command { return &cobra.Command{ Use: "public ", From 38ff786a7d2b209a74b3ab9bfd1ee3f2106323da Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Fri, 27 Aug 2021 11:25:24 -0400 Subject: [PATCH 10/26] cmd/ghcs: style tweaks --- api/api.go | 40 +++++++------------ cmd/ghcs/code.go | 8 ++-- cmd/ghcs/create.go | 6 +-- cmd/ghcs/delete.go | 22 +++++------ cmd/ghcs/list.go | 18 ++++----- cmd/ghcs/logs.go | 8 ++-- cmd/ghcs/main.go | 9 +++-- cmd/ghcs/ports.go | 64 ++++++++++++++++--------------- cmd/ghcs/ssh.go | 8 ++-- internal/codespaces/codespaces.go | 5 ++- 10 files changed, 92 insertions(+), 96 deletions(-) diff --git a/api/api.go b/api/api.go index 83510d8c5..9d9eae79f 100644 --- a/api/api.go +++ b/api/api.go @@ -1,3 +1,4 @@ +// TODO(adonovan): rename to package codespaces, and codespaces.Client? package api import ( @@ -9,7 +10,6 @@ import ( "fmt" "io/ioutil" "net/http" - "sort" "strconv" "strings" ) @@ -29,10 +29,6 @@ type User struct { Login string `json:"login"` } -type errResponse struct { - Message string `json:"message"` -} - func (a *API) GetUser(ctx context.Context) (*User, error) { req, err := http.NewRequest(http.MethodGet, githubAPI+"/user", nil) if err != nil { @@ -63,7 +59,9 @@ func (a *API) GetUser(ctx context.Context) (*User, error) { } func (a *API) errorResponse(b []byte) error { - var response errResponse + var response struct { + Message string `json:"message"` + } if err := json.Unmarshal(b, &response); err != nil { return fmt.Errorf("error unmarshaling error response: %v", err) } @@ -104,14 +102,6 @@ func (a *API) GetRepository(ctx context.Context, nwo string) (*Repository, error return &response, nil } -type Codespaces []*Codespace - -func (c Codespaces) SortByCreatedAt() { - sort.Slice(c, func(i, j int) bool { - return c[i].CreatedAt > c[j].CreatedAt - }) -} - type Codespace struct { Name string `json:"name"` GUID string `json:"guid"` @@ -139,7 +129,7 @@ type CodespaceEnvironmentConnection struct { RelaySAS string `json:"relaySas"` } -func (a *API) ListCodespaces(ctx context.Context, user *User) (Codespaces, error) { +func (a *API) ListCodespaces(ctx context.Context, user *User) ([]*Codespace, error) { req, err := http.NewRequest( http.MethodGet, githubAPI+"/vscs_internal/user/"+user.Login+"/codespaces", nil, ) @@ -162,9 +152,9 @@ func (a *API) ListCodespaces(ctx context.Context, user *User) (Codespaces, error return nil, a.errorResponse(b) } - response := struct { - Codespaces Codespaces `json:"codespaces"` - }{} + var response struct { + Codespaces []*Codespace `json:"codespaces"` + } if err := json.Unmarshal(b, &response); err != nil { return nil, fmt.Errorf("error unmarshaling response: %v", err) } @@ -297,14 +287,12 @@ func (a *API) GetCodespaceRegionLocation(ctx context.Context) (string, error) { return response.Current, nil } -type Skus []*Sku - -type Sku struct { +type SKU struct { Name string `json:"name"` DisplayName string `json:"display_name"` } -func (a *API) GetCodespacesSkus(ctx context.Context, user *User, repository *Repository, location string) (Skus, error) { +func (a *API) GetCodespacesSkus(ctx context.Context, user *User, repository *Repository, location string) ([]*SKU, error) { req, err := http.NewRequest(http.MethodGet, githubAPI+"/vscs_internal/user/"+user.Login+"/skus", nil) if err != nil { return nil, fmt.Errorf("err creating request: %v", err) @@ -326,14 +314,14 @@ func (a *API) GetCodespacesSkus(ctx context.Context, user *User, repository *Rep return nil, fmt.Errorf("error reading response body: %v", err) } - response := struct { - Skus Skus `json:"skus"` - }{} + var response struct { + SKUs []*SKU `json:"skus"` + } if err := json.Unmarshal(b, &response); err != nil { return nil, fmt.Errorf("error unmarshaling response: %v", err) } - return response.Skus, nil + return response.SKUs, nil } type createCodespaceRequest struct { diff --git a/cmd/ghcs/code.go b/cmd/ghcs/code.go index 81dbdbb2c..9bd4db634 100644 --- a/cmd/ghcs/code.go +++ b/cmd/ghcs/code.go @@ -12,7 +12,7 @@ import ( "github.com/spf13/cobra" ) -func NewCodeCmd() *cobra.Command { +func newCodeCmd() *cobra.Command { useInsiders := false codeCmd := &cobra.Command{ @@ -24,7 +24,7 @@ func NewCodeCmd() *cobra.Command { if len(args) > 0 { codespaceName = args[0] } - return Code(codespaceName, useInsiders) + return code(codespaceName, useInsiders) }, } @@ -34,10 +34,10 @@ func NewCodeCmd() *cobra.Command { } func init() { - rootCmd.AddCommand(NewCodeCmd()) + rootCmd.AddCommand(newCodeCmd()) } -func Code(codespaceName string, useInsiders bool) error { +func code(codespaceName string, useInsiders bool) error { apiClient := api.New(os.Getenv("GITHUB_TOKEN")) ctx := context.Background() diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index c3e8a24a1..8b4e1a743 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -22,7 +22,7 @@ func newCreateCmd() *cobra.Command { Short: "Create a Codespace", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - return Create() + return create() }, } @@ -37,7 +37,7 @@ func init() { rootCmd.AddCommand(newCreateCmd()) } -func Create() error { +func create() error { ctx := context.Background() apiClient := api.New(os.Getenv("GITHUB_TOKEN")) locationCh := getLocation(ctx, apiClient) @@ -176,7 +176,7 @@ func getMachineName(ctx context.Context, user *api.User, repo *api.Repository, l } skuNames := make([]string, 0, len(skus)) - skuByName := make(map[string]*api.Sku) + skuByName := make(map[string]*api.SKU) for _, sku := range skus { nameParts := camelcase.Split(sku.Name) machineName := strings.Title(strings.ToLower(nameParts[0])) diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index c357171d1..d37029753 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -12,7 +12,7 @@ import ( "github.com/spf13/cobra" ) -func NewDeleteCmd() *cobra.Command { +func newDeleteCmd() *cobra.Command { deleteCmd := &cobra.Command{ Use: "delete []", Short: "Delete a Codespace", @@ -22,7 +22,7 @@ func NewDeleteCmd() *cobra.Command { if len(args) > 0 { codespaceName = args[0] } - return Delete(codespaceName) + return delete_(codespaceName) }, } @@ -31,7 +31,7 @@ func NewDeleteCmd() *cobra.Command { Short: "Delete all Codespaces for the current user", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - return DeleteAll() + return deleteAll() }, } @@ -40,7 +40,7 @@ func NewDeleteCmd() *cobra.Command { Short: "Delete all Codespaces for a repository", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - return DeleteByRepo(args[0]) + return deleteByRepo(args[0]) }, } @@ -50,10 +50,10 @@ func NewDeleteCmd() *cobra.Command { } func init() { - rootCmd.AddCommand(NewDeleteCmd()) + rootCmd.AddCommand(newDeleteCmd()) } -func Delete(codespaceName string) error { +func delete_(codespaceName string) error { apiClient := api.New(os.Getenv("GITHUB_TOKEN")) ctx := context.Background() log := output.NewLogger(os.Stdout, os.Stderr, false) @@ -74,10 +74,10 @@ func Delete(codespaceName string) error { log.Println("Codespace deleted.") - return List(&ListOptions{}) + return list(&listOptions{}) } -func DeleteAll() error { +func deleteAll() error { apiClient := api.New(os.Getenv("GITHUB_TOKEN")) ctx := context.Background() log := output.NewLogger(os.Stdout, os.Stderr, false) @@ -105,10 +105,10 @@ func DeleteAll() error { log.Printf("Codespace deleted: %s\n", c.Name) } - return List(&ListOptions{}) + return list(&listOptions{}) } -func DeleteByRepo(repo string) error { +func deleteByRepo(repo string) error { apiClient := api.New(os.Getenv("GITHUB_TOKEN")) ctx := context.Background() log := output.NewLogger(os.Stdout, os.Stderr, false) @@ -146,5 +146,5 @@ func DeleteByRepo(repo string) error { return fmt.Errorf("No codespace was found for repository: %s", repo) } - return List(&ListOptions{}) + return list(&listOptions{}) } diff --git a/cmd/ghcs/list.go b/cmd/ghcs/list.go index 27b11d4fd..a19439296 100644 --- a/cmd/ghcs/list.go +++ b/cmd/ghcs/list.go @@ -10,32 +10,32 @@ import ( "github.com/spf13/cobra" ) -type ListOptions struct { - AsJSON bool +type listOptions struct { + asJSON bool } -func NewListCmd() *cobra.Command { - opts := &ListOptions{} +func newListCmd() *cobra.Command { + opts := &listOptions{} listCmd := &cobra.Command{ Use: "list", Short: "List your Codespaces", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - return List(opts) + return list(opts) }, } - listCmd.Flags().BoolVar(&opts.AsJSON, "json", false, "Output as JSON") + listCmd.Flags().BoolVar(&opts.asJSON, "json", false, "Output as JSON") return listCmd } func init() { - rootCmd.AddCommand(NewListCmd()) + rootCmd.AddCommand(newListCmd()) } -func List(opts *ListOptions) error { +func list(opts *listOptions) error { apiClient := api.New(os.Getenv("GITHUB_TOKEN")) ctx := context.Background() @@ -49,7 +49,7 @@ func List(opts *ListOptions) error { return fmt.Errorf("error getting codespaces: %v", err) } - table := output.NewTable(os.Stdout, opts.AsJSON) + table := output.NewTable(os.Stdout, opts.asJSON) table.SetHeader([]string{"Name", "Repository", "Branch", "State", "Created At"}) for _, codespace := range codespaces { table.Append([]string{ diff --git a/cmd/ghcs/logs.go b/cmd/ghcs/logs.go index dd8664597..006f9c477 100644 --- a/cmd/ghcs/logs.go +++ b/cmd/ghcs/logs.go @@ -12,7 +12,7 @@ import ( "github.com/spf13/cobra" ) -func NewLogsCmd() *cobra.Command { +func newLogsCmd() *cobra.Command { var tail bool logsCmd := &cobra.Command{ @@ -24,7 +24,7 @@ func NewLogsCmd() *cobra.Command { if len(args) > 0 { codespaceName = args[0] } - return Logs(tail, codespaceName) + return logs(tail, codespaceName) }, } @@ -34,10 +34,10 @@ func NewLogsCmd() *cobra.Command { } func init() { - rootCmd.AddCommand(NewLogsCmd()) + rootCmd.AddCommand(newLogsCmd()) } -func Logs(tail bool, codespaceName string) error { +func logs(tail bool, codespaceName string) error { apiClient := api.New(os.Getenv("GITHUB_TOKEN")) ctx := context.Background() log := output.NewLogger(os.Stdout, os.Stderr, false) diff --git a/cmd/ghcs/main.go b/cmd/ghcs/main.go index 58037437a..dbf1dc714 100644 --- a/cmd/ghcs/main.go +++ b/cmd/ghcs/main.go @@ -1,5 +1,8 @@ package main +// TODO(adonovan): write 'help' commands, in manner of the 'go' tool. +// Document GITHUB_TOKEN. + import ( "errors" "fmt" @@ -16,7 +19,7 @@ func main() { } } -var Version = "DEV" +var version = "DEV" var rootCmd = &cobra.Command{ Use: "ghcs", @@ -24,7 +27,7 @@ var rootCmd = &cobra.Command{ Running commands requires the GITHUB_TOKEN environment variable to be set to a token to access the GitHub API with.`, - Version: Version, + Version: version, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { if os.Getenv("GITHUB_TOKEN") == "" { @@ -42,5 +45,5 @@ func explainError(w io.Writer, err error) { fmt.Fprintln(w, "Make sure to enable SSO for your organizations after creating the token.") return } - // fmt.Fprintf(w, "%v\n", err) + fmt.Fprintf(w, "%v\n", err) } diff --git a/cmd/ghcs/ports.go b/cmd/ghcs/ports.go index 09397af54..c5d127892 100644 --- a/cmd/ghcs/ports.go +++ b/cmd/ghcs/ports.go @@ -19,48 +19,48 @@ import ( "golang.org/x/sync/errgroup" ) -type PortsOptions struct { - CodespaceName string - AsJSON bool +type portsOptions struct { + codespaceName string + asJSON bool } -func NewPortsCmd() *cobra.Command { - opts := &PortsOptions{} +func newPortsCmd() *cobra.Command { + opts := &portsOptions{} portsCmd := &cobra.Command{ Use: "ports", Short: "List ports in a Codespace", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - return Ports(opts) + return ports(opts) }, } - portsCmd.Flags().StringVarP(&opts.CodespaceName, "codespace", "c", "", "The `name` of the Codespace to use") - portsCmd.Flags().BoolVar(&opts.AsJSON, "json", false, "Output as JSON") + portsCmd.Flags().StringVarP(&opts.codespaceName, "codespace", "c", "", "The `name` of the Codespace to use") + portsCmd.Flags().BoolVar(&opts.asJSON, "json", false, "Output as JSON") - portsCmd.AddCommand(NewPortsPublicCmd()) - portsCmd.AddCommand(NewPortsPrivateCmd()) - portsCmd.AddCommand(NewPortsForwardCmd()) + portsCmd.AddCommand(newPortsPublicCmd()) + portsCmd.AddCommand(newPortsPrivateCmd()) + portsCmd.AddCommand(newPortsForwardCmd()) return portsCmd } func init() { - rootCmd.AddCommand(NewPortsCmd()) + rootCmd.AddCommand(newPortsCmd()) } -func Ports(opts *PortsOptions) error { +func ports(opts *portsOptions) error { apiClient := api.New(os.Getenv("GITHUB_TOKEN")) ctx := context.Background() - log := output.NewLogger(os.Stdout, os.Stderr, opts.AsJSON) + log := output.NewLogger(os.Stdout, os.Stderr, opts.asJSON) user, err := apiClient.GetUser(ctx) if err != nil { return fmt.Errorf("error getting user: %v", err) } - codespace, token, err := codespaces.GetOrChooseCodespace(ctx, apiClient, user, opts.CodespaceName) + codespace, token, err := codespaces.GetOrChooseCodespace(ctx, apiClient, user, opts.codespaceName) if err != nil { if err == codespaces.ErrNoCodespaces { return err @@ -82,17 +82,18 @@ func Ports(opts *PortsOptions) error { } devContainerResult := <-devContainerCh - if devContainerResult.Err != nil { - _, _ = log.Errorf("Failed to get port names: %v\n", devContainerResult.Err.Error()) + if devContainerResult.err != nil { + _, _ = log.Errorf("Failed to get port names: %v\n", devContainerResult.err.Error()) + // TODO(adonovan): should this cause non-zero exit? } - table := output.NewTable(os.Stdout, opts.AsJSON) + table := output.NewTable(os.Stdout, opts.asJSON) table.SetHeader([]string{"Label", "Source Port", "Destination Port", "Public", "Browse URL"}) for _, port := range ports { sourcePort := strconv.Itoa(port.SourcePort) var portName string - if devContainerResult.DevContainer != nil { - if attributes, ok := devContainerResult.DevContainer.PortAttributes[sourcePort]; ok { + if devContainerResult.devContainer != nil { + if attributes, ok := devContainerResult.devContainer.PortAttributes[sourcePort]; ok { portName = attributes.Label } } @@ -125,8 +126,8 @@ func getPorts(ctx context.Context, lsclient *liveshare.Client) (liveshare.Ports, } type devContainerResult struct { - DevContainer *devContainer - Err error + devContainer *devContainer + err error } type devContainer struct { @@ -168,7 +169,7 @@ func getDevContainer(ctx context.Context, apiClient *api.API, codespace *api.Cod return ch } -func NewPortsPublicCmd() *cobra.Command { +func newPortsPublicCmd() *cobra.Command { return &cobra.Command{ Use: "public ", Short: "Mark port as public", @@ -180,7 +181,7 @@ func NewPortsPublicCmd() *cobra.Command { } } -func NewPortsPrivateCmd() *cobra.Command { +func newPortsPrivateCmd() *cobra.Command { return &cobra.Command{ Use: "private ", Short: "Mark port as private", @@ -239,7 +240,7 @@ func updatePortVisibility(log *output.Logger, codespaceName, sourcePort string, return nil } -func NewPortsForwardCmd() *cobra.Command { +func newPortsForwardCmd() *cobra.Command { return &cobra.Command{ Use: "forward :", Short: "Forward ports", @@ -289,14 +290,14 @@ func forwardPorts(log *output.Logger, codespaceName string, ports []string) erro for _, portPair := range portPairs { pp := portPair - srcstr := strconv.Itoa(portPair.Src) - if err := server.StartSharing(gctx, "share-"+srcstr, pp.Src); err != nil { + srcstr := strconv.Itoa(portPair.src) + if err := server.StartSharing(gctx, "share-"+srcstr, pp.src); err != nil { return fmt.Errorf("start sharing port: %v", err) } g.Go(func() error { - log.Println("Forwarding port: " + srcstr + " ==> " + strconv.Itoa(pp.Dst)) - portForwarder := liveshare.NewPortForwarder(lsclient, server, pp.Dst) + log.Println("Forwarding port: " + srcstr + " ==> " + strconv.Itoa(pp.dst)) + portForwarder := liveshare.NewPortForwarder(lsclient, server, pp.dst) if err := portForwarder.Start(gctx); err != nil { return fmt.Errorf("error forwarding port: %v", err) } @@ -313,16 +314,17 @@ func forwardPorts(log *output.Logger, codespaceName string, ports []string) erro } type portPair struct { - Src, Dst int + src, dst int } +// getPortPairs parses a list of strings of form "%d:%d" into pairs of numbers. func getPortPairs(ports []string) ([]portPair, error) { pp := make([]portPair, 0, len(ports)) for _, portString := range ports { parts := strings.Split(portString, ":") if len(parts) < 2 { - return pp, fmt.Errorf("port pair: '%v' is not valid", portString) + return nil, fmt.Errorf("port pair: '%v' is not valid", portString) } srcp, err := strconv.Atoi(parts[0]) diff --git a/cmd/ghcs/ssh.go b/cmd/ghcs/ssh.go index 1754f968a..a02dd557e 100644 --- a/cmd/ghcs/ssh.go +++ b/cmd/ghcs/ssh.go @@ -15,7 +15,7 @@ import ( "github.com/spf13/cobra" ) -func NewSSHCmd() *cobra.Command { +func newSSHCmd() *cobra.Command { var sshProfile, codespaceName string var sshServerPort int @@ -24,7 +24,7 @@ func NewSSHCmd() *cobra.Command { Short: "SSH into a Codespace", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - return SSH(sshProfile, codespaceName, sshServerPort) + return ssh(sshProfile, codespaceName, sshServerPort) }, } @@ -36,10 +36,10 @@ func NewSSHCmd() *cobra.Command { } func init() { - rootCmd.AddCommand(NewSSHCmd()) + rootCmd.AddCommand(newSSHCmd()) } -func SSH(sshProfile, codespaceName string, sshServerPort int) error { +func ssh(sshProfile, codespaceName string, sshServerPort int) error { apiClient := api.New(os.Getenv("GITHUB_TOKEN")) ctx := context.Background() log := output.NewLogger(os.Stdout, os.Stderr, false) diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index 48369cfa0..a346c06d7 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "sort" "time" "github.com/AlecAivazis/survey/v2" @@ -25,7 +26,9 @@ func ChooseCodespace(ctx context.Context, apiClient *api.API, user *api.User) (* return nil, ErrNoCodespaces } - codespaces.SortByCreatedAt() + sort.Slice(codespaces, func(i, j int) bool { + return codespaces[i].CreatedAt > codespaces[j].CreatedAt + }) codespacesByName := make(map[string]*api.Codespace) codespacesNames := make([]string, 0, len(codespaces)) From cb6552f4cae0d5255471319455f24254a5bcfae0 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Fri, 27 Aug 2021 12:50:32 -0400 Subject: [PATCH 11/26] more efficient impl for processing states --- cmd/ghcs/create.go | 39 ++++++++++++++++++------------- internal/codespaces/codespaces.go | 11 ++++++++- internal/codespaces/states.go | 5 ++++ 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index fb9834351..9ffcc58d1 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -112,6 +112,7 @@ func showStatus(ctx context.Context, log *output.Logger, apiClient *api.API, use } var lastState codespaces.PostCreateState + finishedStates := make(map[string]bool) var breakNextState bool for { @@ -122,25 +123,31 @@ func showStatus(ctx context.Context, log *output.Logger, apiClient *api.API, use var inProgress bool for _, state := range stateUpdate.PostCreateStates { - switch state.Status { - case codespaces.PostCreateStateRunning: - if lastState != state { - lastState = state - log.Print(state.Name) - } else { - log.Print(".") - } + if _, found := finishedStates[state.Name]; found { + continue // skip this state as we've processed it already + } - inProgress = true - case codespaces.PostCreateStateFailed: - if lastState.Name == state.Name && lastState.Status != state.Status { + if state.Name != lastState.Name { + log.Print(state.Name) + + if state.Status == codespaces.PostCreateStateRunning { + inProgress = true lastState = state - log.Print(".Failed\n") + log.Print("...") + break + } else { + finishedStates[state.Name] = true + log.Println("..." + state.Status) } - case codespaces.PostCreateStateSuccess: - if lastState.Name == state.Name && lastState.Status != state.Status { - lastState = state - log.Print(".Success\n") + } else { + if state.Status == codespaces.PostCreateStateRunning { + inProgress = true + log.Print(".") + break + } else { + finishedStates[state.Name] = true + log.Println(state.Status) + lastState = codespaces.PostCreateState{} // reset the value } } } diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index 005ea0fda..11e0a8902 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -62,6 +62,15 @@ type logger interface { Println(v ...interface{}) (int, error) } +func connectionReady(codespace *api.Codespace) bool { + ready := codespace.Environment.Connection.SessionID != "" + ready = ready && codespace.Environment.Connection.SessionToken != "" + ready = ready && codespace.Environment.Connection.RelayEndpoint != "" + ready = ready && codespace.Environment.Connection.RelaySAS != "" + ready = ready && codespace.Environment.State == api.CodespaceEnvironmentStateAvailable + return ready +} + func ConnectToLiveshare(ctx context.Context, log logger, apiClient *api.API, userLogin, token string, codespace *api.Codespace) (client *liveshare.Client, err error) { var startedCodespace bool if codespace.Environment.State != api.CodespaceEnvironmentStateAvailable { @@ -73,7 +82,7 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient *api.API, use } retries := 0 - for codespace.Environment.Connection.SessionID == "" || codespace.Environment.State != api.CodespaceEnvironmentStateAvailable { + for !connectionReady(codespace) { if retries > 1 { if retries%2 == 0 { log.Print(".") diff --git a/internal/codespaces/states.go b/internal/codespaces/states.go index 078dc546c..3ad6c9a4c 100644 --- a/internal/codespaces/states.go +++ b/internal/codespaces/states.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "io/ioutil" + "strings" "time" "github.com/github/ghcs/api" @@ -12,6 +13,10 @@ import ( type PostCreateStateStatus string +func (p PostCreateStateStatus) String() string { + return strings.Title(string(p)) +} + const ( PostCreateStateRunning PostCreateStateStatus = "running" PostCreateStateSuccess PostCreateStateStatus = "succeeded" From da8655209b59fe4a5791e00d26fbbe63f9dc1db4 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Fri, 27 Aug 2021 12:52:30 -0400 Subject: [PATCH 12/26] make things private --- cmd/ghcs/create.go | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index 9ffcc58d1..bb416d53f 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -15,29 +15,29 @@ import ( "github.com/spf13/cobra" ) -type CreateOptions struct { - Repo string - Branch string - Machine string - ShowStatus bool +type createOptions struct { + repo string + branch string + machine string + showStatus bool } func newCreateCmd() *cobra.Command { - opts := &CreateOptions{} + opts := &createOptions{} createCmd := &cobra.Command{ Use: "create", Short: "Create a Codespace", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - return Create(opts) + return create(opts) }, } - createCmd.Flags().StringVarP(&opts.Repo, "repo", "r", "", "repository name with owner: user/repo") - createCmd.Flags().StringVarP(&opts.Branch, "branch", "b", "", "repository branch") - createCmd.Flags().StringVarP(&opts.Machine, "machine", "m", "", "hardware specifications for the VM") - createCmd.Flags().BoolVarP(&opts.ShowStatus, "status", "s", false, "show status of post-create command and dotfiles") + createCmd.Flags().StringVarP(&opts.repo, "repo", "r", "", "repository name with owner: user/repo") + createCmd.Flags().StringVarP(&opts.branch, "branch", "b", "", "repository branch") + createCmd.Flags().StringVarP(&opts.machine, "machine", "m", "", "hardware specifications for the VM") + createCmd.Flags().BoolVarP(&opts.showStatus, "status", "s", false, "show status of post-create command and dotfiles") return createCmd } @@ -46,18 +46,18 @@ func init() { rootCmd.AddCommand(newCreateCmd()) } -func Create(opts *CreateOptions) error { +func create(opts *createOptions) error { ctx := context.Background() apiClient := api.New(os.Getenv("GITHUB_TOKEN")) locationCh := getLocation(ctx, apiClient) userCh := getUser(ctx, apiClient) log := output.NewLogger(os.Stdout, os.Stderr, false) - repo, err := getRepoName(opts.Repo) + repo, err := getRepoName(opts.repo) if err != nil { return fmt.Errorf("error getting repository name: %v", err) } - branch, err := getBranchName(opts.Branch) + branch, err := getBranchName(opts.branch) if err != nil { return fmt.Errorf("error getting branch name: %v", err) } @@ -77,7 +77,7 @@ func Create(opts *CreateOptions) error { return fmt.Errorf("error getting codespace user: %v", userResult.Err) } - machine, err := getMachineName(ctx, opts.Machine, userResult.User, repository, locationResult.Location, apiClient) + machine, err := getMachineName(ctx, opts.machine, userResult.User, repository, locationResult.Location, apiClient) if err != nil { return fmt.Errorf("error getting machine type: %v", err) } @@ -92,7 +92,7 @@ func Create(opts *CreateOptions) error { return fmt.Errorf("error creating codespace: %v", err) } - if opts.ShowStatus { + if opts.showStatus { if err := showStatus(ctx, log, apiClient, userResult.User, codespace); err != nil { return fmt.Errorf("show status: %w", err) } From 2cc91c224edcbc8a266c8b82c9b898364f399cc3 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Fri, 27 Aug 2021 12:53:27 -0400 Subject: [PATCH 13/26] add comment for improvements --- internal/codespaces/codespaces.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index 11e0a8902..8897c0839 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -62,6 +62,7 @@ type logger interface { Println(v ...interface{}) (int, error) } +// TODO(josebalius): we should move some of this to the liveshare.Connection struct func connectionReady(codespace *api.Codespace) bool { ready := codespace.Environment.Connection.SessionID != "" ready = ready && codespace.Environment.Connection.SessionToken != "" From 90f3ac6f56a5b44fc65da1dd3003e97b2f4b378b Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Fri, 27 Aug 2021 14:23:33 -0400 Subject: [PATCH 14/26] check status codes --- api/api.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/api/api.go b/api/api.go index 25faf5724..fb386a57b 100644 --- a/api/api.go +++ b/api/api.go @@ -272,7 +272,17 @@ func (a *API) StartCodespace(ctx context.Context, token string, codespace *Codes } defer resp.Body.Close() - // TODO: check status code? + b, err := ioutil.ReadAll(resp.Body) + if err != nil { + return fmt.Errorf("error reading response body: %v", err) + } + + // TODO(adonovan): the status code proxied from VSCS may distinguish + // "already running" from "fresh start". Find out what code it uses + // and allow it too. + if resp.StatusCode != http.StatusOK { + return a.errorResponse(b) + } return nil } @@ -298,7 +308,9 @@ func (a *API) GetCodespaceRegionLocation(ctx context.Context) (string, error) { return "", fmt.Errorf("error reading response body: %v", err) } - // TODO: check status code? + if resp.StatusCode != http.StatusOK { + return "", a.errorResponse(b) + } var response getCodespaceRegionLocationResponse if err := json.Unmarshal(b, &response); err != nil { @@ -338,7 +350,9 @@ func (a *API) GetCodespacesSkus(ctx context.Context, user *User, repository *Rep return nil, fmt.Errorf("error reading response body: %v", err) } - // TODO: check status code? + if resp.StatusCode != http.StatusOK { + return nil, a.errorResponse(b) + } response := struct { Skus Skus `json:"skus"` From da34d12abb099b9ea3e1093c6c8af6c0fecae4ad Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Fri, 27 Aug 2021 15:26:34 -0400 Subject: [PATCH 15/26] respond to review --- api/api.go | 2 +- cmd/ghcs/main.go | 3 --- cmd/ghcs/ports.go | 4 ++-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/api/api.go b/api/api.go index 9d9eae79f..f534c4587 100644 --- a/api/api.go +++ b/api/api.go @@ -1,4 +1,4 @@ -// TODO(adonovan): rename to package codespaces, and codespaces.Client? +// TODO(adonovan): rename to package codespaces, and codespaces.Client. package api import ( diff --git a/cmd/ghcs/main.go b/cmd/ghcs/main.go index dbf1dc714..bc9bc2c6b 100644 --- a/cmd/ghcs/main.go +++ b/cmd/ghcs/main.go @@ -1,8 +1,5 @@ package main -// TODO(adonovan): write 'help' commands, in manner of the 'go' tool. -// Document GITHUB_TOKEN. - import ( "errors" "fmt" diff --git a/cmd/ghcs/ports.go b/cmd/ghcs/ports.go index c5d127892..c5088bfe6 100644 --- a/cmd/ghcs/ports.go +++ b/cmd/ghcs/ports.go @@ -83,8 +83,8 @@ func ports(opts *portsOptions) error { devContainerResult := <-devContainerCh if devContainerResult.err != nil { - _, _ = log.Errorf("Failed to get port names: %v\n", devContainerResult.err.Error()) - // TODO(adonovan): should this cause non-zero exit? + // Warn about failure to read the devcontainer file. Not a ghcs command error. + log.Errorf("Failed to get port names: %v\n", devContainerResult.err.Error()) } table := output.NewTable(os.Stdout, opts.asJSON) From d8f1baa519e1daa0441664956882f433a6f91df1 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Fri, 27 Aug 2021 15:36:45 -0400 Subject: [PATCH 16/26] more SKU renames. --- api/api.go | 2 +- cmd/ghcs/create.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api/api.go b/api/api.go index f534c4587..3e405dd5c 100644 --- a/api/api.go +++ b/api/api.go @@ -292,7 +292,7 @@ type SKU struct { DisplayName string `json:"display_name"` } -func (a *API) GetCodespacesSkus(ctx context.Context, user *User, repository *Repository, location string) ([]*SKU, error) { +func (a *API) GetCodespacesSKUs(ctx context.Context, user *User, repository *Repository, location string) ([]*SKU, error) { req, err := http.NewRequest(http.MethodGet, githubAPI+"/vscs_internal/user/"+user.Login+"/skus", nil) if err != nil { return nil, fmt.Errorf("err creating request: %v", err) diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index 8b4e1a743..ef2209856 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -151,9 +151,9 @@ func getBranchName() (string, error) { } func getMachineName(ctx context.Context, user *api.User, repo *api.Repository, location string, apiClient *api.API) (string, error) { - skus, err := apiClient.GetCodespacesSkus(ctx, user, repo, location) + skus, err := apiClient.GetCodespacesSKUs(ctx, user, repo, location) if err != nil { - return "", fmt.Errorf("error getting codespace skus: %v", err) + return "", fmt.Errorf("error getting codespace SKUs: %v", err) } // if user supplied a machine type, it must be valid @@ -165,12 +165,12 @@ func getMachineName(ctx context.Context, user *api.User, repo *api.Repository, l } } - availableSkus := make([]string, len(skus)) + availableSKUs := make([]string, len(skus)) for i := 0; i < len(skus); i++ { - availableSkus[i] = skus[i].Name + availableSKUs[i] = skus[i].Name } - return "", fmt.Errorf("there are is no such machine for the repository: %s\nAvailable machines: %v", machine, availableSkus) + return "", fmt.Errorf("there is no such machine for the repository: %s\nAvailable machines: %v", machine, availableSKUs) } else if len(skus) == 0 { return "", nil } From a5ae72cb26fe05bb5d1ac031af14631183fe23d9 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Fri, 27 Aug 2021 15:38:41 -0400 Subject: [PATCH 17/26] revert removal of _ = f() to pacify linter --- cmd/ghcs/ports.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/ghcs/ports.go b/cmd/ghcs/ports.go index c5088bfe6..492107bfc 100644 --- a/cmd/ghcs/ports.go +++ b/cmd/ghcs/ports.go @@ -84,7 +84,7 @@ func ports(opts *portsOptions) error { devContainerResult := <-devContainerCh if devContainerResult.err != nil { // Warn about failure to read the devcontainer file. Not a ghcs command error. - log.Errorf("Failed to get port names: %v\n", devContainerResult.err.Error()) + _, _ = log.Errorf("Failed to get port names: %v\n", devContainerResult.err.Error()) } table := output.NewTable(os.Stdout, opts.asJSON) From 8b395b5ab5b86366ca6e2c6b5a46133cac703028 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Fri, 27 Aug 2021 15:53:55 -0400 Subject: [PATCH 18/26] ghcs code: improve vscode error --- cmd/ghcs/code.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/ghcs/code.go b/cmd/ghcs/code.go index 81dbdbb2c..fd71da74f 100644 --- a/cmd/ghcs/code.go +++ b/cmd/ghcs/code.go @@ -57,8 +57,9 @@ func Code(codespaceName string, useInsiders bool) error { codespaceName = codespace.Name } - if err := open.Run(vscodeProtocolURL(codespaceName, useInsiders)); err != nil { - return fmt.Errorf("error opening vscode URL") + url := vscodeProtocolURL(codespaceName, useInsiders) + if err := open.Run(url); err != nil { + return fmt.Errorf("error opening vscode URL %s: %s. (Is VSCode installed?)", url, err) } return nil From e423cb0ef953f4d6e02707547f1c02615f9a7cff Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Fri, 27 Aug 2021 16:09:02 -0400 Subject: [PATCH 19/26] display colon and cursor in survey prompts --- cmd/ghcs/create.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index c3e8a24a1..7b33322be 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -126,11 +126,11 @@ func getRepoName() (string, error) { repoSurvey := []*survey.Question{ { Name: "repository", - Prompt: &survey.Input{Message: "Repository"}, + Prompt: &survey.Input{Message: "Repository:"}, Validate: survey.Required, }, } - err := survey.Ask(repoSurvey, &repo) + err := ask(repoSurvey, &repo) return repo, err } @@ -142,11 +142,11 @@ func getBranchName() (string, error) { branchSurvey := []*survey.Question{ { Name: "branch", - Prompt: &survey.Input{Message: "Branch"}, + Prompt: &survey.Input{Message: "Branch:"}, Validate: survey.Required, }, } - err := survey.Ask(branchSurvey, &branch) + err := ask(branchSurvey, &branch) return branch, err } @@ -198,7 +198,7 @@ func getMachineName(ctx context.Context, user *api.User, repo *api.Repository, l } skuAnswers := struct{ SKU string }{} - if err := survey.Ask(skuSurvey, &skuAnswers); err != nil { + if err := ask(skuSurvey, &skuAnswers); err != nil { return "", fmt.Errorf("error getting SKU: %v", err) } @@ -207,3 +207,8 @@ func getMachineName(ctx context.Context, user *api.User, repo *api.Repository, l return machine, nil } + +// ask asks survery questions using standard options. +func ask(qs []*survey.Question, response interface{}) error { + return survey.Ask(qs, response, survey.WithShowCursor(true)) +} From 1e8a8370fee4988be9ab089473d8d7ad07438761 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Fri, 27 Aug 2021 16:29:02 -0400 Subject: [PATCH 20/26] initial round of PR feedback --- cmd/ghcs/create.go | 23 ++++++++++++----------- internal/codespaces/codespaces.go | 17 ++++++----------- internal/codespaces/states.go | 12 +++++------- 3 files changed, 23 insertions(+), 29 deletions(-) diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index bb416d53f..c3cfbce22 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -108,7 +108,7 @@ func create(opts *createOptions) error { func showStatus(ctx context.Context, log *output.Logger, apiClient *api.API, user *api.User, codespace *api.Codespace) error { states, err := codespaces.PollPostCreateStates(ctx, log, apiClient, user, codespace) if err != nil { - return fmt.Errorf("poll post create states: %v", err) + return fmt.Errorf("failed to subscribe to state changes from codespace: %v", err) } var lastState codespaces.PostCreateState @@ -135,27 +135,28 @@ func showStatus(ctx context.Context, log *output.Logger, apiClient *api.API, use lastState = state log.Print("...") break - } else { - finishedStates[state.Name] = true - log.Println("..." + state.Status) } + + finishedStates[state.Name] = true + log.Println("..." + state.Status) } else { if state.Status == codespaces.PostCreateStateRunning { inProgress = true log.Print(".") break - } else { - finishedStates[state.Name] = true - log.Println(state.Status) - lastState = codespaces.PostCreateState{} // reset the value } + + finishedStates[state.Name] = true + log.Println(state.Status) + lastState = codespaces.PostCreateState{} // reset the value } } - if !inProgress && !breakNextState { + if !inProgress { + if breakNextState { + break + } breakNextState = true - } else if !inProgress && breakNextState { - break } } diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index 8897c0839..05254c9a7 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -62,14 +62,12 @@ type logger interface { Println(v ...interface{}) (int, error) } -// TODO(josebalius): we should move some of this to the liveshare.Connection struct func connectionReady(codespace *api.Codespace) bool { - ready := codespace.Environment.Connection.SessionID != "" - ready = ready && codespace.Environment.Connection.SessionToken != "" - ready = ready && codespace.Environment.Connection.RelayEndpoint != "" - ready = ready && codespace.Environment.Connection.RelaySAS != "" - ready = ready && codespace.Environment.State == api.CodespaceEnvironmentStateAvailable - return ready + return codespace.Environment.Connection.SessionID != "" && + codespace.Environment.Connection.SessionToken != "" && + codespace.Environment.Connection.RelayEndpoint != "" && + codespace.Environment.Connection.RelaySAS != "" && + codespace.Environment.State == api.CodespaceEnvironmentStateAvailable } func ConnectToLiveshare(ctx context.Context, log logger, apiClient *api.API, userLogin, token string, codespace *api.Codespace) (client *liveshare.Client, err error) { @@ -82,8 +80,7 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient *api.API, use } } - retries := 0 - for !connectionReady(codespace) { + for retries := 0; !connectionReady(codespace); retries++ { if retries > 1 { if retries%2 == 0 { log.Print(".") @@ -100,8 +97,6 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient *api.API, use if err != nil { return nil, fmt.Errorf("error getting codespace: %v", err) } - - retries += 1 } if startedCodespace { diff --git a/internal/codespaces/states.go b/internal/codespaces/states.go index 3ad6c9a4c..9ace150d6 100644 --- a/internal/codespaces/states.go +++ b/internal/codespaces/states.go @@ -24,12 +24,10 @@ const ( ) type PostCreateStatesResult struct { - PostCreateStates PostCreateStates + PostCreateStates []PostCreateState Err error } -type PostCreateStates []PostCreateState - type PostCreateState struct { Name string `json:"name"` Status PostCreateStateStatus `json:"status"` @@ -81,7 +79,7 @@ func PollPostCreateStates(ctx context.Context, log logger, apiClient *api.API, u return pollch, nil } -func getPostCreateOutput(ctx context.Context, tunnelPort int, codespace *api.Codespace) (PostCreateStates, error) { +func getPostCreateOutput(ctx context.Context, tunnelPort int, codespace *api.Codespace) ([]PostCreateState, error) { stdout, err := RunCommand( ctx, tunnelPort, sshDestination(codespace), "cat /workspaces/.codespaces/shared/postCreateOutput.json", @@ -95,9 +93,9 @@ func getPostCreateOutput(ctx context.Context, tunnelPort int, codespace *api.Cod return nil, fmt.Errorf("read output: %v", err) } - output := struct { - Steps PostCreateStates `json:"steps"` - }{} + var output struct { + Steps []PostCreateState `json:"steps"` + } if err := json.Unmarshal(b, &output); err != nil { return nil, fmt.Errorf("unmarshal output: %v", err) } From e5f45d4bfab826871cebabda66da6f46ad0be504 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Fri, 27 Aug 2021 16:41:22 -0400 Subject: [PATCH 21/26] docs and improvement to the showStatus implementation --- cmd/ghcs/create.go | 79 ++++++++++++++++++++++++++-------------------- 1 file changed, 45 insertions(+), 34 deletions(-) diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index c3cfbce22..42e8f11be 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -115,48 +115,54 @@ func showStatus(ctx context.Context, log *output.Logger, apiClient *api.API, use finishedStates := make(map[string]bool) var breakNextState bool +PollStates: for { - stateUpdate := <-states - if stateUpdate.Err != nil { - return fmt.Errorf("receive state update: %v", err) - } + select { + case <-ctx.Done(): + return nil - var inProgress bool - for _, state := range stateUpdate.PostCreateStates { - if _, found := finishedStates[state.Name]; found { - continue // skip this state as we've processed it already + case stateUpdate := <-states: + if stateUpdate.Err != nil { + return fmt.Errorf("receive state update: %v", err) } - if state.Name != lastState.Name { - log.Print(state.Name) - - if state.Status == codespaces.PostCreateStateRunning { - inProgress = true - lastState = state - log.Print("...") - break + var inProgress bool + for _, state := range stateUpdate.PostCreateStates { + if _, found := finishedStates[state.Name]; found { + continue // skip this state as we've processed it already } - finishedStates[state.Name] = true - log.Println("..." + state.Status) - } else { - if state.Status == codespaces.PostCreateStateRunning { - inProgress = true - log.Print(".") - break + if state.Name != lastState.Name { + log.Print(state.Name) + + if state.Status == codespaces.PostCreateStateRunning { + inProgress = true + lastState = state + log.Print("...") + break + } + + finishedStates[state.Name] = true + log.Println("..." + state.Status) + } else { + if state.Status == codespaces.PostCreateStateRunning { + inProgress = true + log.Print(".") + break + } + + finishedStates[state.Name] = true + log.Println(state.Status) + lastState = codespaces.PostCreateState{} // reset the value } - - finishedStates[state.Name] = true - log.Println(state.Status) - lastState = codespaces.PostCreateState{} // reset the value } - } - if !inProgress { - if breakNextState { - break + if !inProgress { + if breakNextState { + break PollStates + } + breakNextState = true } - breakNextState = true } } @@ -168,6 +174,7 @@ type getUserResult struct { Err error } +// getUser fetches the user record associated with the GITHUB_TOKEN func getUser(ctx context.Context, apiClient *api.API) <-chan getUserResult { ch := make(chan getUserResult) go func() { @@ -182,6 +189,7 @@ type locationResult struct { Err error } +// getLocation fetches the closest Codespace datacenter region/location to the user. func getLocation(ctx context.Context, apiClient *api.API) <-chan locationResult { ch := make(chan locationResult) go func() { @@ -191,6 +199,7 @@ func getLocation(ctx context.Context, apiClient *api.API) <-chan locationResult return ch } +// getRepoName prompts the user for the name of the repository, or returns the repository if non-empty. func getRepoName(repo string) (string, error) { if repo != "" { return repo, nil @@ -207,6 +216,7 @@ func getRepoName(repo string) (string, error) { return repo, err } +// getBranchName prompts the user for the name of the branch, or returns the branch if non-empty. func getBranchName(branch string) (string, error) { if branch != "" { return branch, nil @@ -223,6 +233,7 @@ func getBranchName(branch string) (string, error) { return branch, err } +// getMachineName prompts the user to select the machine type, or validates the machine if non-empty. func getMachineName(ctx context.Context, machine string, user *api.User, repo *api.Repository, location string, apiClient *api.API) (string, error) { skus, err := apiClient.GetCodespacesSkus(ctx, user, repo, location) if err != nil { @@ -243,7 +254,7 @@ func getMachineName(ctx context.Context, machine string, user *api.User, repo *a availableSkus[i] = skus[i].Name } - return "", fmt.Errorf("there are is no such machine for the repository: %s\nAvailable machines: %v", machine, availableSkus) + return "", fmt.Errorf("there is no such machine for the repository: %s\nAvailable machines: %v", machine, availableSkus) } else if len(skus) == 0 { return "", nil } @@ -270,7 +281,7 @@ func getMachineName(ctx context.Context, machine string, user *api.User, repo *a }, } - skuAnswers := struct{ SKU string }{} + var skuAnswers struct{ SKU string } if err := survey.Ask(skuSurvey, &skuAnswers); err != nil { return "", fmt.Errorf("error getting SKU: %v", err) } From 368e8c61105f7beafcdedd3d6c3376293db345ca Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Fri, 27 Aug 2021 17:34:06 -0400 Subject: [PATCH 22/26] simplify contract for state polling --- cmd/ghcs/create.go | 83 ++++++++++++++++------------------- internal/codespaces/states.go | 51 +++++++++------------ 2 files changed, 58 insertions(+), 76 deletions(-) diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index 42e8f11be..6b6d10511 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -106,64 +106,55 @@ func create(opts *createOptions) error { } func showStatus(ctx context.Context, log *output.Logger, apiClient *api.API, user *api.User, codespace *api.Codespace) error { - states, err := codespaces.PollPostCreateStates(ctx, log, apiClient, user, codespace) - if err != nil { - return fmt.Errorf("failed to subscribe to state changes from codespace: %v", err) - } - var lastState codespaces.PostCreateState - finishedStates := make(map[string]bool) var breakNextState bool -PollStates: - for { - select { - case <-ctx.Done(): - return nil + finishedStates := make(map[string]bool) + ctx, stopPolling := context.WithCancel(ctx) - case stateUpdate := <-states: - if stateUpdate.Err != nil { - return fmt.Errorf("receive state update: %v", err) + poller := func(states []codespaces.PostCreateState) { + var inProgress bool + for _, state := range states { + if _, found := finishedStates[state.Name]; found { + continue // skip this state as we've processed it already } - var inProgress bool - for _, state := range stateUpdate.PostCreateStates { - if _, found := finishedStates[state.Name]; found { - continue // skip this state as we've processed it already + if state.Name != lastState.Name { + log.Print(state.Name) + + if state.Status == codespaces.PostCreateStateRunning { + inProgress = true + lastState = state + log.Print("...") + break } - if state.Name != lastState.Name { - log.Print(state.Name) - - if state.Status == codespaces.PostCreateStateRunning { - inProgress = true - lastState = state - log.Print("...") - break - } - - finishedStates[state.Name] = true - log.Println("..." + state.Status) - } else { - if state.Status == codespaces.PostCreateStateRunning { - inProgress = true - log.Print(".") - break - } - - finishedStates[state.Name] = true - log.Println(state.Status) - lastState = codespaces.PostCreateState{} // reset the value + finishedStates[state.Name] = true + log.Println("..." + state.Status) + } else { + if state.Status == codespaces.PostCreateStateRunning { + inProgress = true + log.Print(".") + break } - } - if !inProgress { - if breakNextState { - break PollStates - } - breakNextState = true + finishedStates[state.Name] = true + log.Println(state.Status) + lastState = codespaces.PostCreateState{} // reset the value } } + + if !inProgress { + if breakNextState { + stopPolling() + return + } + breakNextState = true + } + } + + if err := codespaces.PollPostCreateStates(ctx, log, apiClient, user, codespace, poller); err != nil { + return fmt.Errorf("failed to poll state changes from codespace: %v", err) } return nil diff --git a/internal/codespaces/states.go b/internal/codespaces/states.go index 9ace150d6..427726a46 100644 --- a/internal/codespaces/states.go +++ b/internal/codespaces/states.go @@ -33,50 +33,40 @@ type PostCreateState struct { Status PostCreateStateStatus `json:"status"` } -func PollPostCreateStates(ctx context.Context, log logger, apiClient *api.API, user *api.User, codespace *api.Codespace) (<-chan PostCreateStatesResult, error) { - pollch := make(chan PostCreateStatesResult) - +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 nil, fmt.Errorf("getting codespace token: %v", err) + return fmt.Errorf("getting codespace token: %v", err) } lsclient, err := ConnectToLiveshare(ctx, log, apiClient, user.Login, token, codespace) if err != nil { - return nil, fmt.Errorf("connect to liveshare: %v", err) + return fmt.Errorf("connect to liveshare: %v", err) } tunnelPort, connClosed, err := MakeSSHTunnel(ctx, lsclient, 0) if err != nil { - return nil, fmt.Errorf("make ssh tunnel: %v", err) + return fmt.Errorf("make ssh tunnel: %v", err) } - go func() { - t := time.NewTicker(1 * time.Second) - for { - select { - case <-ctx.Done(): - return - case err := <-connClosed: - if err != nil { - pollch <- PostCreateStatesResult{Err: fmt.Errorf("connection closed: %v", err)} - return - } - case <-t.C: - states, err := getPostCreateOutput(ctx, tunnelPort, codespace) - if err != nil { - pollch <- PostCreateStatesResult{Err: fmt.Errorf("get post create output: %v", err)} - return - } - - pollch <- PostCreateStatesResult{ - PostCreateStates: states, - } + t := time.NewTicker(1 * time.Second) + for { + select { + case <-ctx.Done(): + return nil + case err := <-connClosed: + return fmt.Errorf("connection closed: %v", err) + case <-t.C: + states, err := getPostCreateOutput(ctx, tunnelPort, codespace) + if err != nil { + return fmt.Errorf("get post create output: %v", err) } - } - }() - return pollch, nil + poller(states) + } + } + + return nil } func getPostCreateOutput(ctx context.Context, tunnelPort int, codespace *api.Codespace) ([]PostCreateState, error) { @@ -87,6 +77,7 @@ func getPostCreateOutput(ctx context.Context, tunnelPort int, codespace *api.Cod if err != nil { return nil, fmt.Errorf("run command: %v", err) } + defer stdout.Close() b, err := ioutil.ReadAll(stdout) if err != nil { From a5a18026cc34382da6b7c7bf3a6d950f7a8f0678 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Fri, 27 Aug 2021 17:39:10 -0400 Subject: [PATCH 23/26] fix linter --- internal/codespaces/states.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/codespaces/states.go b/internal/codespaces/states.go index 427726a46..36cf5e5e3 100644 --- a/internal/codespaces/states.go +++ b/internal/codespaces/states.go @@ -65,8 +65,6 @@ func PollPostCreateStates(ctx context.Context, log logger, apiClient *api.API, u poller(states) } } - - return nil } func getPostCreateOutput(ctx context.Context, tunnelPort int, codespace *api.Codespace) ([]PostCreateState, error) { From dcf4f041e9817e7aed0f2e584f7c00884c85f258 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Fri, 27 Aug 2021 18:01:52 -0400 Subject: [PATCH 24/26] deal with Start errors, non-JSON --- api/api.go | 45 +++++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/api/api.go b/api/api.go index fb386a57b..c86de79f7 100644 --- a/api/api.go +++ b/api/api.go @@ -1,5 +1,11 @@ package api +// For descriptions of service interfaces, see: +// - https://online.visualstudio.com/api/swagger (for visualstudio.com) +// - https://docs.github.com/en/rest/reference/repos (for api.github.com) +// - 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. + import ( "bytes" "context" @@ -29,10 +35,6 @@ type User struct { Login string `json:"login"` } -type errResponse struct { - Message string `json:"message"` -} - func (a *API) GetUser(ctx context.Context) (*User, error) { req, err := http.NewRequest(http.MethodGet, githubAPI+"/user", nil) if err != nil { @@ -52,7 +54,7 @@ func (a *API) GetUser(ctx context.Context) (*User, error) { } if resp.StatusCode != http.StatusOK { - return nil, a.errorResponse(b) + return nil, jsonErrorResponse(b) } var response User @@ -63,8 +65,10 @@ func (a *API) GetUser(ctx context.Context) (*User, error) { return &response, nil } -func (a *API) errorResponse(b []byte) error { - var response errResponse +func jsonErrorResponse(b []byte) error { + var response struct { + Message string `json:"message"` + } if err := json.Unmarshal(b, &response); err != nil { return fmt.Errorf("error unmarshaling error response: %v", err) } @@ -95,7 +99,7 @@ func (a *API) GetRepository(ctx context.Context, nwo string) (*Repository, error } if resp.StatusCode != http.StatusOK { - return nil, a.errorResponse(b) + return nil, jsonErrorResponse(b) } var response Repository @@ -162,7 +166,7 @@ func (a *API) ListCodespaces(ctx context.Context, user *User) (Codespaces, error } if resp.StatusCode != http.StatusOK { - return nil, a.errorResponse(b) + return nil, jsonErrorResponse(b) } response := struct { @@ -210,7 +214,7 @@ func (a *API) GetCodespaceToken(ctx context.Context, ownerLogin, codespaceName s } if resp.StatusCode != http.StatusOK { - return "", a.errorResponse(b) + return "", jsonErrorResponse(b) } var response getCodespaceTokenResponse @@ -244,7 +248,7 @@ func (a *API) GetCodespace(ctx context.Context, token, owner, codespace string) } if resp.StatusCode != http.StatusOK { - return nil, a.errorResponse(b) + return nil, jsonErrorResponse(b) } var response Codespace @@ -277,11 +281,12 @@ func (a *API) StartCodespace(ctx context.Context, token string, codespace *Codes return fmt.Errorf("error reading response body: %v", err) } - // TODO(adonovan): the status code proxied from VSCS may distinguish - // "already running" from "fresh start". Find out what code it uses - // and allow it too. if resp.StatusCode != http.StatusOK { - return a.errorResponse(b) + // Error response is numeric code and/or string message, not JSON. + if len(b) > 100 { + b = append(b[:97], "..."...) + } + return fmt.Errorf("failed to start codespace: %s", b) } return nil @@ -309,7 +314,7 @@ func (a *API) GetCodespaceRegionLocation(ctx context.Context) (string, error) { } if resp.StatusCode != http.StatusOK { - return "", a.errorResponse(b) + return "", jsonErrorResponse(b) } var response getCodespaceRegionLocationResponse @@ -351,7 +356,7 @@ func (a *API) GetCodespacesSkus(ctx context.Context, user *User, repository *Rep } if resp.StatusCode != http.StatusOK { - return nil, a.errorResponse(b) + return nil, jsonErrorResponse(b) } response := struct { @@ -395,7 +400,7 @@ func (a *API) CreateCodespace(ctx context.Context, user *User, repository *Repos } if resp.StatusCode > http.StatusAccepted { - return nil, a.errorResponse(b) + return nil, jsonErrorResponse(b) } var response Codespace @@ -424,7 +429,7 @@ func (a *API) DeleteCodespace(ctx context.Context, user *User, token, codespaceN if err != nil { return fmt.Errorf("error reading response body: %v", err) } - return a.errorResponse(b) + return jsonErrorResponse(b) } return nil @@ -461,7 +466,7 @@ func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *Cod } if resp.StatusCode != http.StatusOK { - return nil, a.errorResponse(b) + return nil, jsonErrorResponse(b) } var response getCodespaceRepositoryContentsResponse From 272af2fadf4694bf2ed59845aa469c8bd2a8c0ea Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Fri, 27 Aug 2021 18:12:06 -0400 Subject: [PATCH 25/26] add docs --- internal/codespaces/states.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/internal/codespaces/states.go b/internal/codespaces/states.go index 36cf5e5e3..ed41fab9b 100644 --- a/internal/codespaces/states.go +++ b/internal/codespaces/states.go @@ -11,6 +11,7 @@ import ( "github.com/github/ghcs/api" ) +// PostCreateStateStatus is a string value representing the different statuses a state can have. type PostCreateStateStatus string func (p PostCreateStateStatus) String() string { @@ -23,16 +24,15 @@ const ( PostCreateStateFailed PostCreateStateStatus = "failed" ) -type PostCreateStatesResult struct { - PostCreateStates []PostCreateState - Err error -} - +// PostCreateState is a combination of a state and status value that is captured +// during codespace creation. type PostCreateState struct { Name string `json:"name"` Status PostCreateStateStatus `json:"status"` } +// PollPostCreateStates polls the state file in a codespace after creation and calls the poller +// with a slice of states to be processed. 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 { @@ -92,6 +92,7 @@ func getPostCreateOutput(ctx context.Context, tunnelPort int, codespace *api.Cod return output.Steps, nil } +// TODO(josebalius): this won't be needed soon func sshDestination(codespace *api.Codespace) string { user := "codespace" if codespace.RepositoryNWO == "github/github" { From 0cf2640c863898fb3a78f19be2d87f29f2ba677f Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Fri, 27 Aug 2021 18:14:10 -0400 Subject: [PATCH 26/26] better docs and stop ticker --- internal/codespaces/states.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/codespaces/states.go b/internal/codespaces/states.go index ed41fab9b..5c3dcef45 100644 --- a/internal/codespaces/states.go +++ b/internal/codespaces/states.go @@ -31,8 +31,9 @@ type PostCreateState struct { Status PostCreateStateStatus `json:"status"` } -// PollPostCreateStates polls the state file in a codespace after creation and calls the poller -// with a slice of states to be processed. +// PollPostCreateStates watches for state changes in a codespace, +// and calls the supplied poller for each batch of state changes. +// It runs until the context is cancelled or SSH tunnel is closed. 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 { @@ -50,6 +51,8 @@ func PollPostCreateStates(ctx context.Context, log logger, apiClient *api.API, u } t := time.NewTicker(1 * time.Second) + defer t.Stop() + for { select { case <-ctx.Done():