diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index 8bdae6056..9afc965c4 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -564,6 +564,7 @@ type CreateCodespaceParams struct { Branch string Machine string Location string + DevContainerPath string VSCSTarget string VSCSTargetURL string PermissionsOptOut bool @@ -612,6 +613,7 @@ type startCreateRequest struct { Ref string `json:"ref"` Location string `json:"location"` Machine string `json:"machine"` + DevContainerPath string `json:"devcontainer_path,omitempty"` VSCSTarget string `json:"vscs_target,omitempty"` VSCSTargetURL string `json:"vscs_target_url,omitempty"` PermissionsOptOut bool `json:"multi_repo_permissions_opt_out"` @@ -643,6 +645,7 @@ func (a *API) startCreate(ctx context.Context, params *CreateCodespaceParams) (* Ref: params.Branch, Location: params.Location, Machine: params.Machine, + DevContainerPath: params.DevContainerPath, VSCSTarget: params.VSCSTarget, VSCSTargetURL: params.VSCSTargetURL, PermissionsOptOut: params.PermissionsOptOut, @@ -727,6 +730,73 @@ func (a *API) DeleteCodespace(ctx context.Context, codespaceName string) error { return nil } +type DevContainerEntry struct { + Path string `json:"path"` + Name string `json:"name,omitempty"` +} + +// ListDevContainers returns a list of valid devcontainer.json files for the repo. Pass a negative limit to request all pages from +// the API until all devcontainer.json files have been fetched. +func (a *API) ListDevContainers(ctx context.Context, repoID int, branch string, limit int) (devcontainers []DevContainerEntry, err error) { + perPage := 100 + if limit > 0 && limit < 100 { + perPage = limit + } + + v := url.Values{} + v.Set("per_page", strconv.Itoa(perPage)) + if branch != "" { + v.Set("ref", branch) + } + listURL := fmt.Sprintf("%s/repositories/%d/codespaces/devcontainers?%s", a.githubAPI, repoID, v.Encode()) + + for { + req, err := http.NewRequest(http.MethodGet, listURL, nil) + if err != nil { + return nil, fmt.Errorf("error creating request: %w", err) + } + a.setHeaders(req) + + resp, err := a.do(ctx, req, fmt.Sprintf("/repositories/%d/codespaces/devcontainers", repoID)) + if err != nil { + return nil, fmt.Errorf("error making request: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, api.HandleHTTPError(resp) + } + + var response struct { + Devcontainers []DevContainerEntry `json:"devcontainers"` + } + + dec := json.NewDecoder(resp.Body) + if err := dec.Decode(&response); err != nil { + return nil, fmt.Errorf("error unmarshaling response: %w", err) + } + + nextURL := findNextPage(resp.Header.Get("Link")) + devcontainers = append(devcontainers, response.Devcontainers...) + + if nextURL == "" || (limit > 0 && len(devcontainers) >= limit) { + break + } + + if newPerPage := limit - len(devcontainers); limit > 0 && newPerPage < 100 { + u, _ := url.Parse(nextURL) + q := u.Query() + q.Set("per_page", strconv.Itoa(newPerPage)) + u.RawQuery = q.Encode() + listURL = u.String() + } else { + listURL = nextURL + } + } + + return devcontainers, nil +} + type EditCodespaceParams struct { DisplayName string `json:"display_name,omitempty"` IdleTimeoutMinutes int `json:"idle_timeout_minutes,omitempty"` diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index dde1a0f30..f057ca99b 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -73,6 +73,7 @@ type apiClient interface { AuthorizedKeys(ctx context.Context, user string) ([]byte, error) GetCodespacesMachines(ctx context.Context, repoID int, branch, location string) ([]*api.Machine, error) GetCodespaceRepositoryContents(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) + ListDevContainers(ctx context.Context, repoID int, branch string, limit int) (devcontainers []api.DevContainerEntry, err error) GetCodespaceRepoSuggestions(ctx context.Context, partialSearch string, params api.RepoSearchParameters) ([]string, error) } diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index 601d370b9..91d293658 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -15,6 +15,14 @@ import ( "github.com/spf13/cobra" ) +const ( + DEVCONTAINER_PROMPT_DEFAULT = "Default Codespaces configuration" +) + +var ( + DEFAULT_DEVCONTAINER_DEFINITIONS = []string{".devcontainer.json", ".devcontainer/devcontainer.json"} +) + type createOptions struct { repo string branch string @@ -22,6 +30,7 @@ type createOptions struct { machine string showStatus bool permissionsOptOut bool + devContainerPath string idleTimeout time.Duration } @@ -44,6 +53,7 @@ func newCreateCmd(app *App) *cobra.Command { createCmd.Flags().BoolVarP(&opts.permissionsOptOut, "default-permissions", "", false, "do not prompt to accept additional permissions requested by the codespace") createCmd.Flags().BoolVarP(&opts.showStatus, "status", "s", false, "show status of post-create command and dotfiles") createCmd.Flags().DurationVar(&opts.idleTimeout, "idle-timeout", 0, "allowed inactivity before codespace is stopped, e.g. \"10m\", \"1h\"") + createCmd.Flags().StringVar(&opts.devContainerPath, "devcontainer-path", "", "path to the devcontainer.json file to use when creating codespace") return createCmd } @@ -111,6 +121,53 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { branch = repository.DefaultBranch } + devContainerPath := opts.devContainerPath + + // now that we have repo+branch, we can list available devcontainer.json files (if any) + if opts.devContainerPath == "" { + a.StartProgressIndicatorWithLabel("Fetching devcontainer.json files") + devcontainers, err := a.apiClient.ListDevContainers(ctx, repository.ID, branch, 100) + a.StopProgressIndicator() + if err != nil { + return fmt.Errorf("error getting devcontainer.json paths: %w", err) + } + + if len(devcontainers) > 0 { + + // if there is only one devcontainer.json file and it is one of the default paths we can auto-select it + if len(devcontainers) == 1 && utils.StringInSlice(devcontainers[0].Path, DEFAULT_DEVCONTAINER_DEFINITIONS) { + devContainerPath = devcontainers[0].Path + } else { + promptOptions := []string{} + + if !utils.StringInSlice(devcontainers[0].Path, DEFAULT_DEVCONTAINER_DEFINITIONS) { + promptOptions = []string{DEVCONTAINER_PROMPT_DEFAULT} + } + + for _, devcontainer := range devcontainers { + promptOptions = append(promptOptions, devcontainer.Path) + } + + devContainerPathQuestion := &survey.Question{ + Name: "devContainerPath", + Prompt: &survey.Select{ + Message: "Devcontainer definition file:", + Options: promptOptions, + }, + } + + if err := ask([]*survey.Question{devContainerPathQuestion}, &devContainerPath); err != nil { + return fmt.Errorf("failed to prompt: %w", err) + } + } + } + + if devContainerPath == DEVCONTAINER_PROMPT_DEFAULT { + // special arg allows users to opt out of devcontainer.json selection + devContainerPath = "" + } + } + machine, err := getMachineName(ctx, a.apiClient, repository.ID, opts.machine, branch, userInputs.Location) if err != nil { return fmt.Errorf("error getting machine type: %w", err) @@ -127,6 +184,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { VSCSTarget: vscsTarget, VSCSTargetURL: vscsTargetUrl, IdleTimeoutMinutes: int(opts.idleTimeout.Minutes()), + DevContainerPath: devContainerPath, PermissionsOptOut: opts.permissionsOptOut, } diff --git a/pkg/cmd/codespace/create_test.go b/pkg/cmd/codespace/create_test.go index 62e7be91e..3f78a94b2 100644 --- a/pkg/cmd/codespace/create_test.go +++ b/pkg/cmd/codespace/create_test.go @@ -9,6 +9,7 @@ import ( "github.com/cli/cli/v2/internal/codespaces/api" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/stretchr/testify/assert" ) func TestApp_Create(t *testing.T) { @@ -35,6 +36,9 @@ func TestApp_Create(t *testing.T) { DefaultBranch: "main", }, nil }, + ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { + return []api.DevContainerEntry{{Path: ".devcontainer/devcontainer.json"}}, nil + }, GetCodespacesMachinesFunc: func(ctx context.Context, repoID int, branch, location string) ([]*api.Machine, error) { return []*api.Machine{ { @@ -97,6 +101,60 @@ func TestApp_Create(t *testing.T) { if params.IdleTimeoutMinutes != 30 { return nil, fmt.Errorf("idle timeout minutes was %v", params.IdleTimeoutMinutes) } + if params.DevContainerPath != ".devcontainer/foobar/devcontainer.json" { + return nil, fmt.Errorf("got dev container path %q, want %q", params.DevContainerPath, ".devcontainer/foobar/devcontainer.json") + } + return &api.Codespace{ + Name: "monalisa-dotfiles-abcd1234", + }, nil + }, + }, + }, + opts: createOptions{ + repo: "monalisa/dotfiles", + branch: "", + machine: "GIGA", + showStatus: false, + idleTimeout: 30 * time.Minute, + devContainerPath: ".devcontainer/foobar/devcontainer.json", + }, + wantStdout: "monalisa-dotfiles-abcd1234\n", + }, + { + name: "create codespace with default branch with default devcontainer if no path provided and no devcontainer files exist in the repo", + fields: fields{ + apiClient: &apiClientMock{ + GetCodespaceRegionLocationFunc: func(ctx context.Context) (string, error) { + return "EUROPE", nil + }, + GetRepositoryFunc: func(ctx context.Context, nwo string) (*api.Repository, error) { + return &api.Repository{ + ID: 1234, + FullName: nwo, + DefaultBranch: "main", + }, nil + }, + ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { + return []api.DevContainerEntry{}, nil + }, + GetCodespacesMachinesFunc: func(ctx context.Context, repoID int, branch, location string) ([]*api.Machine, error) { + return []*api.Machine{ + { + Name: "GIGA", + DisplayName: "Gigabits of a machine", + }, + }, nil + }, + CreateCodespaceFunc: func(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) { + if params.Branch != "main" { + return nil, fmt.Errorf("got branch %q, want %q", params.Branch, "main") + } + if params.IdleTimeoutMinutes != 30 { + return nil, fmt.Errorf("idle timeout minutes was %v", params.IdleTimeoutMinutes) + } + if params.DevContainerPath != "" { + return nil, fmt.Errorf("got dev container path %q, want %q", params.DevContainerPath, ".devcontainer/foobar/devcontainer.json") + } return &api.Codespace{ Name: "monalisa-dotfiles-abcd1234", IdleTimeoutNotice: "Idle timeout for this codespace is set to 10 minutes in compliance with your organization's policy", @@ -118,6 +176,34 @@ func TestApp_Create(t *testing.T) { wantStderr: "Notice: Idle timeout for this codespace is set to 10 minutes in compliance with your organization's policy\n", isTTY: true, }, + { + name: "returns error when getting devcontainer paths fails", + fields: fields{ + apiClient: &apiClientMock{ + GetCodespaceRegionLocationFunc: func(ctx context.Context) (string, error) { + return "EUROPE", nil + }, + GetRepositoryFunc: func(ctx context.Context, nwo string) (*api.Repository, error) { + return &api.Repository{ + ID: 1234, + FullName: nwo, + DefaultBranch: "main", + }, nil + }, + ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { + return nil, fmt.Errorf("some error") + }, + }, + }, + opts: createOptions{ + repo: "monalisa/dotfiles", + branch: "", + machine: "GIGA", + showStatus: false, + idleTimeout: 30 * time.Minute, + }, + wantErr: fmt.Errorf("error getting devcontainer.json paths: some error"), + }, { name: "create codespace with default branch does not show idle timeout notice if not conntected to terminal", fields: fields{ @@ -132,6 +218,9 @@ func TestApp_Create(t *testing.T) { DefaultBranch: "main", }, nil }, + ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { + return []api.DevContainerEntry{}, nil + }, GetCodespacesMachinesFunc: func(ctx context.Context, repoID int, branch, location string) ([]*api.Machine, error) { return []*api.Machine{ { @@ -179,6 +268,9 @@ func TestApp_Create(t *testing.T) { DefaultBranch: "main", }, nil }, + ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { + return []api.DevContainerEntry{{Path: ".devcontainer/devcontainer.json"}}, nil + }, GetCodespacesMachinesFunc: func(ctx context.Context, repoID int, branch, location string) ([]*api.Machine, error) { return []*api.Machine{ { @@ -228,8 +320,9 @@ Alternatively, you can run "create" with the "--default-permissions" option to c io: io, apiClient: tt.fields.apiClient, } - if err := a.Create(context.Background(), tt.opts); err != tt.wantErr { - t.Errorf("App.Create() error = %v, wantErr %v", err, tt.wantErr) + + if err := a.Create(context.Background(), tt.opts); err != nil && tt.wantErr != nil { + assert.EqualError(t, err, tt.wantErr.Error()) } if got := stdout.String(); got != tt.wantStdout { t.Errorf("stdout = %v, want %v", got, tt.wantStdout) diff --git a/pkg/cmd/codespace/mock_api.go b/pkg/cmd/codespace/mock_api.go index 220750afe..897b39f1d 100644 --- a/pkg/cmd/codespace/mock_api.go +++ b/pkg/cmd/codespace/mock_api.go @@ -52,6 +52,9 @@ import ( // ListCodespacesFunc: func(ctx context.Context, limit int) ([]*api.Codespace, error) { // panic("mock out the ListCodespaces method") // }, +// ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { +// panic("mock out the ListDevContainers method") +// }, // StartCodespaceFunc: func(ctx context.Context, name string) error { // panic("mock out the StartCodespace method") // }, @@ -101,6 +104,9 @@ type apiClientMock struct { // ListCodespacesFunc mocks the ListCodespaces method. ListCodespacesFunc func(ctx context.Context, limit int) ([]*api.Codespace, error) + // ListDevContainersFunc mocks the ListDevContainers method. + ListDevContainersFunc func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) + // StartCodespaceFunc mocks the StartCodespace method. StartCodespaceFunc func(ctx context.Context, name string) error @@ -201,6 +207,17 @@ type apiClientMock struct { // Limit is the limit argument value. Limit int } + // ListDevContainers holds details about calls to the ListDevContainers method. + ListDevContainers []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // RepoID is the repoID argument value. + RepoID int + // Branch is the branch argument value. + Branch string + // Limit is the limit argument value. + Limit int + } // StartCodespace holds details about calls to the StartCodespace method. StartCodespace []struct { // Ctx is the ctx argument value. @@ -228,6 +245,7 @@ type apiClientMock struct { lockGetRepository sync.RWMutex lockGetUser sync.RWMutex lockListCodespaces sync.RWMutex + lockListDevContainers sync.RWMutex lockStartCodespace sync.RWMutex lockStopCodespace sync.RWMutex } @@ -668,6 +686,49 @@ func (mock *apiClientMock) ListCodespacesCalls() []struct { return calls } +// ListDevContainers calls ListDevContainersFunc. +func (mock *apiClientMock) ListDevContainers(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { + if mock.ListDevContainersFunc == nil { + panic("apiClientMock.ListDevContainersFunc: method is nil but apiClient.ListDevContainers was just called") + } + callInfo := struct { + Ctx context.Context + RepoID int + Branch string + Limit int + }{ + Ctx: ctx, + RepoID: repoID, + Branch: branch, + Limit: limit, + } + mock.lockListDevContainers.Lock() + mock.calls.ListDevContainers = append(mock.calls.ListDevContainers, callInfo) + mock.lockListDevContainers.Unlock() + return mock.ListDevContainersFunc(ctx, repoID, branch, limit) +} + +// ListDevContainersCalls gets all the calls that were made to ListDevContainers. +// Check the length with: +// len(mockedapiClient.ListDevContainersCalls()) +func (mock *apiClientMock) ListDevContainersCalls() []struct { + Ctx context.Context + RepoID int + Branch string + Limit int +} { + var calls []struct { + Ctx context.Context + RepoID int + Branch string + Limit int + } + mock.lockListDevContainers.RLock() + calls = mock.calls.ListDevContainers + mock.lockListDevContainers.RUnlock() + return calls +} + // StartCodespace calls StartCodespaceFunc. func (mock *apiClientMock) StartCodespace(ctx context.Context, name string) error { if mock.StartCodespaceFunc == nil { diff --git a/utils/utils.go b/utils/utils.go index e73b2af26..503fdf0af 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -85,6 +85,15 @@ func ValidURL(urlStr string) bool { return len(urlStr) < 8192 } +func StringInSlice(a string, slice []string) bool { + for _, b := range slice { + if b == a { + return true + } + } + return false +} + func IsDebugEnabled() (bool, string) { debugValue, isDebugSet := os.LookupEnv("GH_DEBUG") legacyDebugValue := os.Getenv("DEBUG")