Merge pull request #5234 from markphelps/codespaces-accept-perms

Codespaces Create: Allow Accepting Permissions
This commit is contained in:
Nate Smith 2022-02-28 16:54:45 -06:00 committed by GitHub
commit 91c4a5d828
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 196 additions and 25 deletions

View file

@ -565,6 +565,7 @@ type CreateCodespaceParams struct {
Branch string
Machine string
Location string
PermissionsOptOut bool
}
// CreateCodespace creates a codespace with the given parameters and returns a non-nil error if it
@ -610,10 +611,20 @@ type startCreateRequest struct {
Ref string `json:"ref"`
Location string `json:"location"`
Machine string `json:"machine"`
PermissionsOptOut bool `json:"devcontainer_permissions_opt_out"`
}
var errProvisioningInProgress = errors.New("provisioning in progress")
type AcceptPermissionsRequiredError struct {
Message string `json:"message"`
AllowPermissionsURL string `json:"allow_permissions_url"`
}
func (e AcceptPermissionsRequiredError) Error() string {
return e.Message
}
// startCreate starts the creation of a codespace.
// It may return success or an error, or errProvisioningInProgress indicating that the operation
// did not complete before the GitHub API's time limit for RPCs (10s), in which case the caller
@ -629,6 +640,7 @@ func (a *API) startCreate(ctx context.Context, params *CreateCodespaceParams) (*
Ref: params.Branch,
Location: params.Location,
Machine: params.Machine,
PermissionsOptOut: params.PermissionsOptOut,
})
if err != nil {
return nil, fmt.Errorf("error marshaling request: %w", err)
@ -648,6 +660,29 @@ func (a *API) startCreate(ctx context.Context, params *CreateCodespaceParams) (*
if resp.StatusCode == http.StatusAccepted {
return nil, errProvisioningInProgress // RPC finished before result of creation known
} else if resp.StatusCode == http.StatusUnauthorized {
var (
ue AcceptPermissionsRequiredError
bodyCopy = &bytes.Buffer{}
r = io.TeeReader(resp.Body, bodyCopy)
)
b, err := ioutil.ReadAll(r)
if err != nil {
return nil, fmt.Errorf("error reading response body: %w", err)
}
if err := json.Unmarshal(b, &ue); err != nil {
return nil, fmt.Errorf("error unmarshaling response: %w", err)
}
if ue.AllowPermissionsURL != "" {
return nil, ue
}
resp.Body = ioutil.NopCloser(bodyCopy)
return nil, api.HandleHTTPError(resp)
} else if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated {
return nil, api.HandleHTTPError(resp)
}

View file

@ -3,17 +3,11 @@ package codespace
import (
"context"
"fmt"
"io/ioutil"
"net/url"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/spf13/cobra"
)
type browser interface {
Browse(string) error
}
func newCodeCmd(app *App) *cobra.Command {
var (
codespace string
@ -25,8 +19,7 @@ func newCodeCmd(app *App) *cobra.Command {
Short: "Open a codespace in Visual Studio Code",
Args: noArgsConstraint,
RunE: func(cmd *cobra.Command, args []string) error {
b := cmdutil.NewBrowser("", ioutil.Discard, app.io.ErrOut)
return app.VSCode(cmd.Context(), b, codespace, useInsiders)
return app.VSCode(cmd.Context(), codespace, useInsiders)
},
}
@ -37,7 +30,7 @@ func newCodeCmd(app *App) *cobra.Command {
}
// VSCode opens a codespace in the local VS VSCode application.
func (a *App) VSCode(ctx context.Context, browser browser, codespaceName string, useInsiders bool) error {
func (a *App) VSCode(ctx context.Context, codespaceName string, useInsiders bool) error {
if codespaceName == "" {
codespace, err := chooseCodespace(ctx, a.apiClient)
if err != nil {
@ -50,7 +43,7 @@ func (a *App) VSCode(ctx context.Context, browser browser, codespaceName string,
}
url := vscodeProtocolURL(codespaceName, useInsiders)
if err := browser.Browse(url); err != nil {
if err := a.browser.Browse(url); err != nil {
return fmt.Errorf("error opening Visual Studio Code: %w", err)
}

View file

@ -40,8 +40,10 @@ func TestApp_VSCode(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
b := &cmdutil.TestBrowser{}
a := &App{}
if err := a.VSCode(context.Background(), b, tt.args.codespaceName, tt.args.useInsiders); (err != nil) != tt.wantErr {
a := &App{
browser: b,
}
if err := a.VSCode(context.Background(), tt.args.codespaceName, tt.args.useInsiders); (err != nil) != tt.wantErr {
t.Errorf("App.VSCode() error = %v, wantErr %v", err, tt.wantErr)
}
b.Verify(t, tt.wantURL)

View file

@ -21,6 +21,10 @@ import (
"golang.org/x/term"
)
type browser interface {
Browse(string) error
}
type executable interface {
Executable() string
}
@ -30,9 +34,10 @@ type App struct {
apiClient apiClient
errLogger *log.Logger
executable executable
browser browser
}
func NewApp(io *iostreams.IOStreams, exe executable, apiClient apiClient) *App {
func NewApp(io *iostreams.IOStreams, exe executable, apiClient apiClient, browser browser) *App {
errLogger := log.New(io.ErrOut, "", 0)
return &App{
@ -40,6 +45,7 @@ func NewApp(io *iostreams.IOStreams, exe executable, apiClient apiClient) *App {
apiClient: apiClient,
errLogger: errLogger,
executable: exe,
browser: browser,
}
}

View file

@ -9,15 +9,18 @@ import (
"github.com/AlecAivazis/survey/v2"
"github.com/cli/cli/v2/internal/codespaces"
"github.com/cli/cli/v2/internal/codespaces/api"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/utils"
"github.com/spf13/cobra"
)
type createOptions struct {
repo string
branch string
machine string
showStatus bool
idleTimeout time.Duration
repo string
branch string
machine string
showStatus bool
permissionsOptOut bool
idleTimeout time.Duration
}
func newCreateCmd(app *App) *cobra.Command {
@ -35,6 +38,7 @@ func newCreateCmd(app *App) *cobra.Command {
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.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\"")
@ -108,17 +112,30 @@ func (a *App) Create(ctx context.Context, opts createOptions) error {
return errors.New("there are no available machine types for this repository")
}
a.StartProgressIndicatorWithLabel("Creating codespace")
codespace, err := a.apiClient.CreateCodespace(ctx, &api.CreateCodespaceParams{
createParams := &api.CreateCodespaceParams{
RepositoryID: repository.ID,
Branch: branch,
Machine: machine,
Location: locationResult.Location,
IdleTimeoutMinutes: int(opts.idleTimeout.Minutes()),
})
PermissionsOptOut: opts.permissionsOptOut,
}
a.StartProgressIndicatorWithLabel("Creating codespace")
codespace, err := a.apiClient.CreateCodespace(ctx, createParams)
a.StopProgressIndicator()
if err != nil {
return fmt.Errorf("error creating codespace: %w", err)
var aerr api.AcceptPermissionsRequiredError
if !errors.As(err, &aerr) || aerr.AllowPermissionsURL == "" {
return fmt.Errorf("error creating codespace: %w", err)
}
codespace, err = a.handleAdditionalPermissions(ctx, createParams, aerr.AllowPermissionsURL)
if err != nil {
// this error could be a cmdutil.SilentError (in the case that the user opened the browser) so we don't want to wrap it
return err
}
}
if opts.showStatus {
@ -131,6 +148,71 @@ func (a *App) Create(ctx context.Context, opts createOptions) error {
return nil
}
func (a *App) handleAdditionalPermissions(ctx context.Context, createParams *api.CreateCodespaceParams, allowPermissionsURL string) (*api.Codespace, error) {
var (
isInteractive = a.io.CanPrompt()
cs = a.io.ColorScheme()
displayURL = utils.DisplayURL(allowPermissionsURL)
)
fmt.Fprintf(a.io.ErrOut, "You must authorize or deny additional permissions requested by this codespace before continuing.\n")
if !isInteractive {
fmt.Fprintf(a.io.ErrOut, "%s in your browser to review and authorize additional permissions: %s\n", cs.Bold("Open this URL"), displayURL)
fmt.Fprintf(a.io.ErrOut, "Alternatively, you can run %q with the %q option to continue without authorizing additional permissions.\n", a.io.ColorScheme().Bold("create"), cs.Bold("--default-permissions"))
return nil, cmdutil.SilentError
}
choices := []string{
"Continue in browser to review and authorize additional permissions",
"Continue without authorizing additional permissions",
}
permsSurvey := []*survey.Question{
{
Name: "accept",
Prompt: &survey.Select{
Message: "What would you like to do?",
Options: choices,
Default: choices[0],
},
Validate: survey.Required,
},
}
var answers struct {
Accept string
}
if err := ask(permsSurvey, &answers); err != nil {
return nil, fmt.Errorf("error getting answers: %w", err)
}
// if the user chose to continue in the browser, open the URL
if answers.Accept == choices[0] {
if err := a.browser.Browse(allowPermissionsURL); err != nil {
return nil, fmt.Errorf("error opening browser: %w", err)
}
// browser opened successfully but we do not know if they accepted the permissions
// so we must exit and wait for the user to attempt the create again
return nil, cmdutil.SilentError
}
// if the user chose to create the codespace without the permissions,
// we can continue with the create opting out of the additional permissions
createParams.PermissionsOptOut = true
a.StartProgressIndicatorWithLabel("Creating codespace")
codespace, err := a.apiClient.CreateCodespace(ctx, createParams)
a.StopProgressIndicator()
if err != nil {
return nil, fmt.Errorf("error creating codespace: %w", err)
}
return codespace, nil
}
// showStatus polls the codespace for a list of post create states and their status. It will keep polling
// until all states have finished. Once all states have finished, we poll once more to check if any new
// states have been introduced and stop polling otherwise.

View file

@ -7,6 +7,7 @@ import (
"time"
"github.com/cli/cli/v2/internal/codespaces/api"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
)
@ -18,7 +19,7 @@ func TestApp_Create(t *testing.T) {
name string
fields fields
opts createOptions
wantErr bool
wantErr error
wantStdout string
wantStderr string
}{
@ -69,6 +70,57 @@ func TestApp_Create(t *testing.T) {
},
wantStdout: "monalisa-dotfiles-abcd1234\n",
},
{
name: "create codespace that requires accepting additional permissions",
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
},
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)
}
return &api.Codespace{}, api.AcceptPermissionsRequiredError{
AllowPermissionsURL: "https://example.com/permissions",
}
},
GetCodespaceRepoSuggestionsFunc: func(ctx context.Context, partialSearch string, params api.RepoSearchParameters) ([]string, error) {
return nil, nil // We can't ask for suggestions without a terminal.
},
},
},
opts: createOptions{
repo: "monalisa/dotfiles",
branch: "",
machine: "GIGA",
showStatus: false,
idleTimeout: 30 * time.Minute,
},
wantErr: cmdutil.SilentError,
wantStderr: `You must authorize or deny additional permissions requested by this codespace before continuing.
Open this URL in your browser to review and authorize additional permissions: example.com/permissions
Alternatively, you can run "create" with the "--default-permissions" option to continue without authorizing additional permissions.
`,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -77,7 +129,7 @@ func TestApp_Create(t *testing.T) {
io: io,
apiClient: tt.fields.apiClient,
}
if err := a.Create(context.Background(), tt.opts); (err != nil) != tt.wantErr {
if err := a.Create(context.Background(), tt.opts); err != tt.wantErr {
t.Errorf("App.Create() error = %v, wantErr %v", err, tt.wantErr)
}
if got := stdout.String(); got != tt.wantStdout {

View file

@ -190,7 +190,7 @@ func TestDelete(t *testing.T) {
io, _, stdout, stderr := iostreams.Test()
io.SetStdinTTY(true)
io.SetStdoutTTY(true)
app := NewApp(io, nil, apiMock)
app := NewApp(io, nil, apiMock, nil)
err := app.Delete(context.Background(), opts)
if (err != nil) != tt.wantErr {
t.Errorf("delete() error = %v, wantErr %v", err, tt.wantErr)

View file

@ -139,6 +139,7 @@ func newCodespaceCmd(f *cmdutil.Factory) *cobra.Command {
vscsURL,
&lazyLoadedHTTPClient{factory: f},
),
f.Browser,
)
cmd := codespaceCmd.NewRootCmd(app)
cmd.Use = "codespace"