From a3efb53c443d07dfab92a2dae7d66073d6e47124 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Mon, 4 Oct 2021 08:32:02 -0400 Subject: [PATCH] Update API.StartCodespace to use new API endpoint - Switch to using name instead of GUID - Remove GUID from the code since it is not used anywhere else - Add docs to the api client methods - Re-gen mocked client --- internal/codespaces/api/api.go | 44 ++++++++++++++++++++++--------- internal/codespaces/codespaces.go | 4 +-- pkg/cmd/codespace/common.go | 2 +- pkg/cmd/codespace/mock_api.go | 34 ++++++++++-------------- 4 files changed, 49 insertions(+), 35 deletions(-) diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index ea2e8fcaa..6305d6f06 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -43,6 +43,7 @@ import ( const githubAPI = "https://api.github.com" +// API is the interface to the codespace service. type API struct { token string client httpClient @@ -53,6 +54,7 @@ type httpClient interface { Do(req *http.Request) (*http.Response, error) } +// New creates a new API client with the given token and HTTP client. func New(token string, httpClient httpClient) *API { return &API{ token: token, @@ -61,10 +63,12 @@ func New(token string, httpClient httpClient) *API { } } +// User represents a GitHub user. type User struct { Login string `json:"login"` } +// GetUser returns the user associated with the given token. func (a *API) GetUser(ctx context.Context) (*User, error) { req, err := http.NewRequest(http.MethodGet, a.githubAPI+"/user", nil) if err != nil { @@ -95,6 +99,7 @@ func (a *API) GetUser(ctx context.Context) (*User, error) { return &response, nil } +// jsonErrorResponse returns the error message from a JSON response. func jsonErrorResponse(b []byte) error { var response struct { Message string `json:"message"` @@ -106,10 +111,12 @@ func jsonErrorResponse(b []byte) error { return errors.New(response.Message) } +// Repository represents a GitHub repository. type Repository struct { ID int `json:"id"` } +// GetRepository returns the repository associated with the given owner and name. func (a *API) GetRepository(ctx context.Context, nwo string) (*Repository, error) { req, err := http.NewRequest(http.MethodGet, a.githubAPI+"/repos/"+strings.ToLower(nwo), nil) if err != nil { @@ -140,9 +147,9 @@ func (a *API) GetRepository(ctx context.Context, nwo string) (*Repository, error return &response, nil } +// Codespace represents a codespace. type Codespace struct { Name string `json:"name"` - GUID string `json:"guid"` CreatedAt string `json:"created_at"` LastUsedAt string `json:"last_used_at"` Branch string `json:"branch"` @@ -168,6 +175,7 @@ type CodespaceEnvironmentGitStatus struct { } const ( + // CodespaceEnvironmentStateAvailable is the state for a running codespace environment. CodespaceEnvironmentStateAvailable = "Available" ) @@ -179,6 +187,7 @@ type CodespaceEnvironmentConnection struct { HostPublicKeys []string `json:"hostPublicKeys"` } +// ListCodespaces returns a list of codespaces for the user. func (a *API) ListCodespaces(ctx context.Context) ([]*Codespace, error) { req, err := http.NewRequest( http.MethodGet, a.githubAPI+"/user/codespaces", nil, @@ -212,6 +221,7 @@ func (a *API) ListCodespaces(ctx context.Context) ([]*Codespace, error) { return response.Codespaces, nil } +// getCodespaceTokenRequest is the request body for the get codespace token endpoint. type getCodespaceTokenRequest struct { MintRepositoryToken bool `json:"mint_repository_token"` } @@ -224,6 +234,7 @@ type getCodespaceTokenResponse struct { // creation of a codespace is not yet complete and that the caller should try again. var ErrNotProvisioned = errors.New("codespace not provisioned") +// GetCodespaceToken returns a codespace token for the user. func (a *API) GetCodespaceToken(ctx context.Context, ownerLogin, codespaceName string) (string, error) { reqBody, err := json.Marshal(getCodespaceTokenRequest{true}) if err != nil { @@ -267,6 +278,7 @@ func (a *API) GetCodespaceToken(ctx context.Context, ownerLogin, codespaceName s return response.RepositoryToken, nil } +// GetCodespace returns a codespace for the user. func (a *API) GetCodespace(ctx context.Context, token, owner, codespace string) (*Codespace, error) { req, err := http.NewRequest( http.MethodGet, @@ -302,19 +314,20 @@ func (a *API) GetCodespace(ctx context.Context, token, owner, codespace string) return &response, nil } -func (a *API) StartCodespace(ctx context.Context, token string, codespace *Codespace) error { +// StartCodespace starts a codespace for the user. +// If the codespace is already running, the returned error from the API is ignored. +func (a *API) StartCodespace(ctx context.Context, codespaceName string) error { req, err := http.NewRequest( http.MethodPost, - a.githubAPI+"/vscs_internal/proxy/environments/"+codespace.GUID+"/start", + a.githubAPI+"/user/codespaces/"+codespaceName+"/start", nil, ) if err != nil { return fmt.Errorf("error creating request: %w", err) } - // TODO: use a.setHeaders() - req.Header.Set("Authorization", "Bearer "+token) - resp, err := a.do(ctx, req, "/vscs_internal/proxy/environments/*/start") + a.setHeaders(req) + resp, err := a.do(ctx, req, "/user/codespaces/*/start") if err != nil { return fmt.Errorf("error making request: %w", err) } @@ -326,19 +339,20 @@ func (a *API) StartCodespace(ctx context.Context, token string, codespace *Codes } if resp.StatusCode != http.StatusOK { + if resp.StatusCode == http.StatusConflict { + // 409 means the codespace is already running which we can safely ignore + return nil + } + // Error response may be a numeric code or a JSON {"message": "..."}. if bytes.HasPrefix(b, []byte("{")) { return jsonErrorResponse(b) // probably JSON } + if len(b) > 100 { b = append(b[:97], "..."...) } - if strings.TrimSpace(string(b)) == "7" { - // Non-HTTP 200 with error code 7 (EnvironmentNotShutdown) is benign. - // Ignore it. - } else { - return fmt.Errorf("failed to start codespace: %s", b) - } + return fmt.Errorf("failed to start codespace: %s", b) } return nil @@ -348,6 +362,7 @@ type getCodespaceRegionLocationResponse struct { Current string `json:"current"` } +// GetCodespaceRegionLocation returns the closest codespace location for the user. func (a *API) GetCodespaceRegionLocation(ctx context.Context) (string, error) { req, err := http.NewRequest(http.MethodGet, "https://online.visualstudio.com/api/v1/locations", nil) if err != nil { @@ -382,6 +397,7 @@ type SKU struct { DisplayName string `json:"display_name"` } +// GetCodespacesSKUs returns the available SKUs for the user for a given repo, branch and location. func (a *API) GetCodespacesSKUs(ctx context.Context, user *User, repository *Repository, branch, location string) ([]*SKU, error) { req, err := http.NewRequest(http.MethodGet, a.githubAPI+"/vscs_internal/user/"+user.Login+"/skus", nil) if err != nil { @@ -520,6 +536,7 @@ func (a *API) startCreate(ctx context.Context, repoID int, machine, branch, loca return &response, nil } +// DeleteCodespace deletes the given codespace. func (a *API) DeleteCodespace(ctx context.Context, codespaceName string) error { req, err := http.NewRequest(http.MethodDelete, a.githubAPI+"/user/codespaces/"+codespaceName, nil) if err != nil { @@ -616,6 +633,8 @@ func (a *API) AuthorizedKeys(ctx context.Context, user string) ([]byte, error) { return b, nil } +// do executes the given request and returns the response. It creates an +// opentracing span to track the length of the request. func (a *API) do(ctx context.Context, req *http.Request, spanName string) (*http.Response, error) { // TODO(adonovan): use NewRequestWithContext(ctx) and drop ctx parameter. span, ctx := opentracing.StartSpanFromContext(ctx, spanName) @@ -624,6 +643,7 @@ func (a *API) do(ctx context.Context, req *http.Request, spanName string) (*http return a.client.Do(req) } +// setHeaders sets the required headers for the API. func (a *API) setHeaders(req *http.Request) { if a.token != "" { req.Header.Set("Authorization", "Bearer "+a.token) diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index 654504205..968217280 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -26,7 +26,7 @@ func connectionReady(codespace *api.Codespace) bool { type apiClient interface { GetCodespace(ctx context.Context, token, user, name string) (*api.Codespace, error) GetCodespaceToken(ctx context.Context, user, codespace string) (string, error) - StartCodespace(ctx context.Context, token string, codespace *api.Codespace) error + StartCodespace(ctx context.Context, name string) error } // ConnectToLiveshare waits for a Codespace to become running, @@ -36,7 +36,7 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient apiClient, us if codespace.Environment.State != api.CodespaceEnvironmentStateAvailable { startedCodespace = true log.Print("Starting your codespace...") - if err := apiClient.StartCodespace(ctx, token, codespace); err != nil { + if err := apiClient.StartCodespace(ctx, codespace.Name); err != nil { return nil, fmt.Errorf("error starting codespace: %w", err) } } diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index bc2b04bbc..c43e618f9 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -37,7 +37,7 @@ type apiClient interface { GetCodespace(ctx context.Context, token, user, name string) (*api.Codespace, error) ListCodespaces(ctx context.Context) ([]*api.Codespace, error) DeleteCodespace(ctx context.Context, name string) error - StartCodespace(ctx context.Context, token string, codespace *api.Codespace) error + StartCodespace(ctx context.Context, name string) error CreateCodespace(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) GetRepository(ctx context.Context, nwo string) (*api.Repository, error) AuthorizedKeys(ctx context.Context, user string) ([]byte, error) diff --git a/pkg/cmd/codespace/mock_api.go b/pkg/cmd/codespace/mock_api.go index b4edf6cdd..bda277e34 100644 --- a/pkg/cmd/codespace/mock_api.go +++ b/pkg/cmd/codespace/mock_api.go @@ -49,7 +49,7 @@ import ( // ListCodespacesFunc: func(ctx context.Context) ([]*api.Codespace, error) { // panic("mock out the ListCodespaces method") // }, -// StartCodespaceFunc: func(ctx context.Context, token string, codespace *api.Codespace) error { +// StartCodespaceFunc: func(ctx context.Context, name string) error { // panic("mock out the StartCodespace method") // }, // } @@ -93,7 +93,7 @@ type apiClientMock struct { ListCodespacesFunc func(ctx context.Context) ([]*api.Codespace, error) // StartCodespaceFunc mocks the StartCodespace method. - StartCodespaceFunc func(ctx context.Context, token string, codespace *api.Codespace) error + StartCodespaceFunc func(ctx context.Context, name string) error // calls tracks calls to the methods. calls struct { @@ -186,10 +186,8 @@ type apiClientMock struct { StartCodespace []struct { // Ctx is the ctx argument value. Ctx context.Context - // Token is the token argument value. - Token string - // Codespace is the codespace argument value. - Codespace *api.Codespace + // Name is the name argument value. + Name string } } lockAuthorizedKeys sync.RWMutex @@ -608,37 +606,33 @@ func (mock *apiClientMock) ListCodespacesCalls() []struct { } // StartCodespace calls StartCodespaceFunc. -func (mock *apiClientMock) StartCodespace(ctx context.Context, token string, codespace *api.Codespace) error { +func (mock *apiClientMock) StartCodespace(ctx context.Context, name string) error { if mock.StartCodespaceFunc == nil { panic("apiClientMock.StartCodespaceFunc: method is nil but apiClient.StartCodespace was just called") } callInfo := struct { - Ctx context.Context - Token string - Codespace *api.Codespace + Ctx context.Context + Name string }{ - Ctx: ctx, - Token: token, - Codespace: codespace, + Ctx: ctx, + Name: name, } mock.lockStartCodespace.Lock() mock.calls.StartCodespace = append(mock.calls.StartCodespace, callInfo) mock.lockStartCodespace.Unlock() - return mock.StartCodespaceFunc(ctx, token, codespace) + return mock.StartCodespaceFunc(ctx, name) } // StartCodespaceCalls gets all the calls that were made to StartCodespace. // Check the length with: // len(mockedapiClient.StartCodespaceCalls()) func (mock *apiClientMock) StartCodespaceCalls() []struct { - Ctx context.Context - Token string - Codespace *api.Codespace + Ctx context.Context + Name string } { var calls []struct { - Ctx context.Context - Token string - Codespace *api.Codespace + Ctx context.Context + Name string } mock.lockStartCodespace.RLock() calls = mock.calls.StartCodespace