From cbaaf77822bd764dbce526acce4a0b51d772fcb9 Mon Sep 17 00:00:00 2001 From: Mateus Marquezini Date: Mon, 9 Dec 2024 08:35:01 -0300 Subject: [PATCH 01/45] fixing gh gist view prompts with no TTY --- pkg/cmd/gist/view/view.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/gist/view/view.go b/pkg/cmd/gist/view/view.go index 2a19f5958..a6f4d528a 100644 --- a/pkg/cmd/gist/view/view.go +++ b/pkg/cmd/gist/view/view.go @@ -55,7 +55,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman } if !opts.IO.IsStdoutTTY() { - opts.Raw = true + return cmdutil.FlagErrorf("run or gist ID required when not running interactively\n\nUsage: gh gist view [ | ] [flags]\n\nFlags:\n -f, --filename string Display a single file from the gist\n --files List file names from the gist\n -r, --raw Print raw instead of rendered gist contents\n -w, --web Open gist in the browser\n") } if runF != nil { From b740486b13046ad1560aa1c461947ef27c145eb2 Mon Sep 17 00:00:00 2001 From: Mateus Marquezini Date: Mon, 9 Dec 2024 10:59:09 -0300 Subject: [PATCH 02/45] #10042 fixed test --- pkg/cmd/gist/view/view_test.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/gist/view/view_test.go b/pkg/cmd/gist/view/view_test.go index 52e616f7c..2e80ee082 100644 --- a/pkg/cmd/gist/view/view_test.go +++ b/pkg/cmd/gist/view/view_test.go @@ -20,10 +20,11 @@ import ( func TestNewCmdView(t *testing.T) { tests := []struct { - name string - cli string - wants ViewOptions - tty bool + name string + cli string + wants ViewOptions + tty bool + wantsErr bool }{ { name: "tty no arguments", @@ -37,12 +38,14 @@ func TestNewCmdView(t *testing.T) { }, { name: "nontty no arguments", + tty: false, cli: "123", wants: ViewOptions{ Raw: true, Selector: "123", ListFiles: false, }, + wantsErr: true, }, { name: "filename passed", @@ -94,6 +97,12 @@ func TestNewCmdView(t *testing.T) { gotOpts = opts return nil }) + + _, err = cmd.ExecuteC() + if tt.wantsErr { + assert.Error(t, err) + return + } cmd.SetArgs(argv) cmd.SetIn(&bytes.Buffer{}) cmd.SetOut(&bytes.Buffer{}) From f4f8840c3f53587f398240571c5affebc149d8c1 Mon Sep 17 00:00:00 2001 From: Mateus Marquezini Date: Thu, 9 Jan 2025 12:07:15 -0300 Subject: [PATCH 03/45] #10042: Add error messages for 'gh gist view/edit' prompts when no TTY is detected --- pkg/cmd/gist/edit/edit.go | 3 +++ pkg/cmd/gist/view/view.go | 6 +++++- pkg/cmd/gist/view/view_test.go | 17 +++++------------ 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index d777e9581..ced995f63 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -108,6 +108,9 @@ func editRun(opts *EditOptions) error { if gistID == "" { cs := opts.IO.ColorScheme() if gistID == "" { + if !opts.IO.CanPrompt() { + return cmdutil.FlagErrorf("gist ID or URL required when not running interactively") + } gistID, err = shared.PromptGists(opts.Prompter, client, host, cs) if err != nil { return err diff --git a/pkg/cmd/gist/view/view.go b/pkg/cmd/gist/view/view.go index a6f4d528a..25feccdc1 100644 --- a/pkg/cmd/gist/view/view.go +++ b/pkg/cmd/gist/view/view.go @@ -55,7 +55,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman } if !opts.IO.IsStdoutTTY() { - return cmdutil.FlagErrorf("run or gist ID required when not running interactively\n\nUsage: gh gist view [ | ] [flags]\n\nFlags:\n -f, --filename string Display a single file from the gist\n --files List file names from the gist\n -r, --raw Print raw instead of rendered gist contents\n -w, --web Open gist in the browser\n") + opts.Raw = true } if runF != nil { @@ -89,6 +89,10 @@ func viewRun(opts *ViewOptions) error { cs := opts.IO.ColorScheme() if gistID == "" { + if !opts.IO.CanPrompt() { + return cmdutil.FlagErrorf("gist ID or URL required when not running interactively") + } + gistID, err = shared.PromptGists(opts.Prompter, client, hostname, cs) if err != nil { return err diff --git a/pkg/cmd/gist/view/view_test.go b/pkg/cmd/gist/view/view_test.go index 2e80ee082..e52a3b9cb 100644 --- a/pkg/cmd/gist/view/view_test.go +++ b/pkg/cmd/gist/view/view_test.go @@ -20,11 +20,10 @@ import ( func TestNewCmdView(t *testing.T) { tests := []struct { - name string - cli string - wants ViewOptions - tty bool - wantsErr bool + name string + cli string + wants ViewOptions + tty bool }{ { name: "tty no arguments", @@ -45,7 +44,6 @@ func TestNewCmdView(t *testing.T) { Selector: "123", ListFiles: false, }, - wantsErr: true, }, { name: "filename passed", @@ -98,11 +96,6 @@ func TestNewCmdView(t *testing.T) { return nil }) - _, err = cmd.ExecuteC() - if tt.wantsErr { - assert.Error(t, err) - return - } cmd.SetArgs(argv) cmd.SetIn(&bytes.Buffer{}) cmd.SetOut(&bytes.Buffer{}) @@ -166,7 +159,7 @@ func Test_viewRun(t *testing.T) { }, }, }, - wantOut: "test interactive mode\n", + wantErr: true, }, { name: "filename selected", From 4c2ba5681d4777dc364bad989eda6a68189ca6a5 Mon Sep 17 00:00:00 2001 From: Mateus Marquezini Date: Thu, 9 Jan 2025 13:14:27 -0300 Subject: [PATCH 04/45] #10042 removed unnecessary field --- pkg/cmd/gist/view/view_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/cmd/gist/view/view_test.go b/pkg/cmd/gist/view/view_test.go index e52a3b9cb..37dda42d1 100644 --- a/pkg/cmd/gist/view/view_test.go +++ b/pkg/cmd/gist/view/view_test.go @@ -37,7 +37,6 @@ func TestNewCmdView(t *testing.T) { }, { name: "nontty no arguments", - tty: false, cli: "123", wants: ViewOptions{ Raw: true, From c5d6ae6cf698553e47b1fcd92385d7975296f35f Mon Sep 17 00:00:00 2001 From: Mateus Marquezini Date: Sun, 12 Jan 2025 14:53:20 -0300 Subject: [PATCH 05/45] Update pkg/cmd/gist/edit/edit.go Update pkg/cmd/gist/edit/edit.go after code review Co-authored-by: Kynan Ware <47394200+BagToad@users.noreply.github.com> --- pkg/cmd/gist/edit/edit.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index ced995f63..5f2815682 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -111,6 +111,7 @@ func editRun(opts *EditOptions) error { if !opts.IO.CanPrompt() { return cmdutil.FlagErrorf("gist ID or URL required when not running interactively") } + gistID, err = shared.PromptGists(opts.Prompter, client, host, cs) if err != nil { return err From 24e9fed7e0720a47870a495bd54e45cca2b4a5a5 Mon Sep 17 00:00:00 2001 From: Mateus Marquezini Date: Sun, 12 Jan 2025 14:57:24 -0300 Subject: [PATCH 06/45] removed unnecessary space --- pkg/cmd/gist/edit/edit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 5f2815682..b201cdc14 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -111,7 +111,7 @@ func editRun(opts *EditOptions) error { if !opts.IO.CanPrompt() { return cmdutil.FlagErrorf("gist ID or URL required when not running interactively") } - + gistID, err = shared.PromptGists(opts.Prompter, client, host, cs) if err != nil { return err From b41b77165bc8d0d6cdf7a031c62d2e09c3160aa7 Mon Sep 17 00:00:00 2001 From: Rajhans Jadhao Date: Mon, 13 Jan 2025 19:21:58 +0530 Subject: [PATCH 07/45] show error message for rerun workflow older than a month ago --- pkg/cmd/run/rerun/rerun.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/cmd/run/rerun/rerun.go b/pkg/cmd/run/rerun/rerun.go index 1c7d6b53f..20e94b68f 100644 --- a/pkg/cmd/run/rerun/rerun.go +++ b/pkg/cmd/run/rerun/rerun.go @@ -202,6 +202,9 @@ func rerunRun(client *api.Client, repo ghrepo.Interface, run *shared.Run, onlyFa if err != nil { var httpError api.HTTPError if errors.As(err, &httpError) && httpError.StatusCode == 403 { + if httpError.Message == "Unable to retry this workflow run because it was created over a month ago" { + return fmt.Errorf("run %d cannot be rerun; %s", run.ID, httpError.Message) + } return fmt.Errorf("run %d cannot be rerun; its workflow file may be broken", run.ID) } return fmt.Errorf("failed to rerun: %w", err) From 977e2326a25d48c7fb4250b2927760ca28f18a79 Mon Sep 17 00:00:00 2001 From: Mateus Marquezini Date: Mon, 13 Jan 2025 11:06:13 -0300 Subject: [PATCH 08/45] #10042: Applied code review suggestions to view_test.go --- pkg/cmd/gist/edit/edit_test.go | 7 ++++ pkg/cmd/gist/view/view_test.go | 58 +++++++++++++++++++++++----------- 2 files changed, 47 insertions(+), 18 deletions(-) diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index 11a32bc92..0bf7cb51a 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -519,6 +519,11 @@ func Test_editRun(t *testing.T) { }, }, }, + { + name: "no arguments notty", + nontty: true, + wantErr: "gist ID or URL required when not running interactively", + }, } for _, tt := range tests { @@ -552,6 +557,8 @@ func Test_editRun(t *testing.T) { stdin.WriteString(tt.stdin) ios.SetStdoutTTY(!tt.nontty) ios.SetStdinTTY(!tt.nontty) + ios.SetStderrTTY(!tt.nontty) + tt.opts.IO = ios tt.opts.Selector = "1234" diff --git a/pkg/cmd/gist/view/view_test.go b/pkg/cmd/gist/view/view_test.go index 37dda42d1..820401f1e 100644 --- a/pkg/cmd/gist/view/view_test.go +++ b/pkg/cmd/gist/view/view_test.go @@ -16,6 +16,7 @@ import ( "github.com/cli/cli/v2/pkg/iostreams" "github.com/google/shlex" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNewCmdView(t *testing.T) { @@ -116,19 +117,22 @@ func Test_viewRun(t *testing.T) { opts *ViewOptions wantOut string gist *shared.Gist - wantErr bool + isTTY bool + wantErr string mockGistList bool }{ { - name: "no such gist", + name: "no such gist", + isTTY: false, opts: &ViewOptions{ Selector: "1234", ListFiles: false, }, - wantErr: true, + wantErr: "not found", }, { - name: "one file", + name: "one file", + isTTY: true, opts: &ViewOptions{ Selector: "1234", ListFiles: false, @@ -144,7 +148,8 @@ func Test_viewRun(t *testing.T) { wantOut: "bwhiizzzbwhuiiizzzz\n", }, { - name: "one file, no ID supplied", + name: "one file, no ID supplied", + isTTY: true, opts: &ViewOptions{ Selector: "", ListFiles: false, @@ -158,10 +163,16 @@ func Test_viewRun(t *testing.T) { }, }, }, - wantErr: true, + wantOut: "test interactive mode\n", }, { - name: "filename selected", + name: "no arguments notty", + isTTY: false, + wantErr: "gist ID or URL required when not running interactively", + }, + { + name: "filename selected", + isTTY: true, opts: &ViewOptions{ Selector: "1234", Filename: "cicada.txt", @@ -182,7 +193,8 @@ func Test_viewRun(t *testing.T) { wantOut: "bwhiizzzbwhuiiizzzz\n", }, { - name: "filename selected, raw", + name: "filename selected, raw", + isTTY: true, opts: &ViewOptions{ Selector: "1234", Filename: "cicada.txt", @@ -204,7 +216,8 @@ func Test_viewRun(t *testing.T) { wantOut: "bwhiizzzbwhuiiizzzz\n", }, { - name: "multiple files, no description", + name: "multiple files, no description", + isTTY: true, opts: &ViewOptions{ Selector: "1234", ListFiles: false, @@ -224,7 +237,8 @@ func Test_viewRun(t *testing.T) { wantOut: "cicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.md\n\n\n # foo \n\n", }, { - name: "multiple files, trailing newlines", + name: "multiple files, trailing newlines", + isTTY: true, opts: &ViewOptions{ Selector: "1234", ListFiles: false, @@ -244,7 +258,8 @@ func Test_viewRun(t *testing.T) { wantOut: "cicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.txt\n\nbar\n", }, { - name: "multiple files, description", + name: "multiple files, description", + isTTY: true, opts: &ViewOptions{ Selector: "1234", ListFiles: false, @@ -265,7 +280,8 @@ func Test_viewRun(t *testing.T) { wantOut: "some files\n\ncicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.md\n\n\n \n • foo \n\n", }, { - name: "multiple files, raw", + name: "multiple files, raw", + isTTY: true, opts: &ViewOptions{ Selector: "1234", Raw: true, @@ -287,7 +303,8 @@ func Test_viewRun(t *testing.T) { wantOut: "some files\n\ncicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.md\n\n- foo\n", }, { - name: "one file, list files", + name: "one file, list files", + isTTY: true, opts: &ViewOptions{ Selector: "1234", Raw: false, @@ -305,7 +322,8 @@ func Test_viewRun(t *testing.T) { wantOut: "cicada.txt\n", }, { - name: "multiple file, list files", + name: "multiple file, list files", + isTTY: true, opts: &ViewOptions{ Selector: "1234", Raw: false, @@ -377,16 +395,20 @@ func Test_viewRun(t *testing.T) { } ios, _, stdout, _ := iostreams.Test() - ios.SetStdoutTTY(true) + ios.SetStdoutTTY(tt.isTTY) + ios.SetStdinTTY(tt.isTTY) + ios.SetStderrTTY(tt.isTTY) + tt.opts.IO = ios t.Run(tt.name, func(t *testing.T) { err := viewRun(tt.opts) - if tt.wantErr { - assert.Error(t, err) + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) return + } else { + require.NoError(t, err) } - assert.NoError(t, err) assert.Equal(t, tt.wantOut, stdout.String()) reg.Verify(t) From c0f6eb0598bad1463f8dca4b574c69342d653aa0 Mon Sep 17 00:00:00 2001 From: Mateus Marquezini Date: Mon, 13 Jan 2025 11:39:48 -0300 Subject: [PATCH 09/45] #10042: Attempt to add a new test scenario for handling error messages when TTY is unavailable --- pkg/cmd/gist/edit/edit_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index 0bf7cb51a..be610b6b4 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -520,8 +520,11 @@ func Test_editRun(t *testing.T) { }, }, { - name: "no arguments notty", - nontty: true, + name: "no arguments notty", + nontty: true, + opts: &EditOptions{ + Selector: "", + }, wantErr: "gist ID or URL required when not running interactively", }, } From 7300f0d56888dd74605f8921aba8e796612ffcd6 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 13 Jan 2025 15:23:32 +0000 Subject: [PATCH 10/45] Bump github.com/gabriel-vasile/mimetype from 1.4.7 to 1.4.8 Bumps [github.com/gabriel-vasile/mimetype](https://github.com/gabriel-vasile/mimetype) from 1.4.7 to 1.4.8. - [Release notes](https://github.com/gabriel-vasile/mimetype/releases) - [Commits](https://github.com/gabriel-vasile/mimetype/compare/v1.4.7...v1.4.8) --- updated-dependencies: - dependency-name: github.com/gabriel-vasile/mimetype dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 5661ac091..00d18bbd2 100644 --- a/go.mod +++ b/go.mod @@ -19,7 +19,7 @@ require ( github.com/creack/pty v1.1.24 github.com/digitorus/timestamp v0.0.0-20231217203849-220c5c2851b7 github.com/distribution/reference v0.5.0 - github.com/gabriel-vasile/mimetype v1.4.7 + github.com/gabriel-vasile/mimetype v1.4.8 github.com/gdamore/tcell/v2 v2.5.4 github.com/golang/snappy v0.0.4 github.com/google/go-cmp v0.6.0 diff --git a/go.sum b/go.sum index d17d1251d..16bcb6ed7 100644 --- a/go.sum +++ b/go.sum @@ -149,8 +149,8 @@ github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHk github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA= github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM= -github.com/gabriel-vasile/mimetype v1.4.7 h1:SKFKl7kD0RiPdbht0s7hFtjl489WcQ1VyPW8ZzUMYCA= -github.com/gabriel-vasile/mimetype v1.4.7/go.mod h1:GDlAgAyIRT27BhFl53XNAFtfjzOkLaF35JdEG0P7LtU= +github.com/gabriel-vasile/mimetype v1.4.8 h1:FfZ3gj38NjllZIeJAmMhr+qKL8Wu+nOoI3GqacKw1NM= +github.com/gabriel-vasile/mimetype v1.4.8/go.mod h1:ByKUIKGjh1ODkGM1asKUbQZOLGrPjydw3hYPU2YU9t8= github.com/gdamore/encoding v1.0.0 h1:+7OoQ1Bc6eTm5niUzBa0Ctsh6JbMW6Ra+YNuAtDBdko= github.com/gdamore/encoding v1.0.0/go.mod h1:alR0ol34c49FCSBLjhosxzcPHQbf2trDkoo5dl+VrEg= github.com/gdamore/tcell/v2 v2.5.4 h1:TGU4tSjD3sCL788vFNeJnTdzpNKIw1H5dgLnJRQVv/k= From 960f533234dcf327e1e9456c417f0b81ad09647c Mon Sep 17 00:00:00 2001 From: AMS21 Date: Mon, 13 Jan 2025 20:01:28 +0100 Subject: [PATCH 11/45] add install instructions for Manjaro Linux --- docs/install_linux.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/install_linux.md b/docs/install_linux.md index 33339d6df..43ba876a1 100644 --- a/docs/install_linux.md +++ b/docs/install_linux.md @@ -275,6 +275,13 @@ Void Linux users can install from the [official distribution repo](https://voidl sudo xbps-install github-cli ``` +### Manjaro Linux +Manjaro Linux users can install from the [official extra repository](https://manjaristas.org/branch_compare?q=github-cli): + +```bash +pamac install github-cli +``` + [releases page]: https://github.com/cli/cli/releases/latest [arch linux repo]: https://www.archlinux.org/packages/extra/x86_64/github-cli [arch linux aur]: https://aur.archlinux.org/packages/github-cli-git From bc8c46b0b1c1fd411e286990b950aad254cfb78f Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Mon, 13 Jan 2025 20:38:03 -0500 Subject: [PATCH 12/45] Make extension update check non-blocking Fixes #10235 This commit updates the Cobra command logic around extension upgrade checks to be non-blocking. Previously, we were waiting for 1 second after the extension completed to allow the update checking logic complete, however users want the GitHub CLI to run as far as possible. --- pkg/cmd/root/extension.go | 31 ++++++++++++--------- pkg/cmd/root/extension_test.go | 51 ++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 13 deletions(-) diff --git a/pkg/cmd/root/extension.go b/pkg/cmd/root/extension.go index 7f2325e18..7e2d7aca7 100644 --- a/pkg/cmd/root/extension.go +++ b/pkg/cmd/root/extension.go @@ -52,20 +52,25 @@ func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ex }, // PostRun handles communicating extension release information if found PostRun: func(c *cobra.Command, args []string) { - releaseInfo := <-updateMessageChan - if releaseInfo != nil { - stderr := io.ErrOut - fmt.Fprintf(stderr, "\n\n%s %s → %s\n", - cs.Yellowf("A new release of %s is available:", ext.Name()), - cs.Cyan(strings.TrimPrefix(ext.CurrentVersion(), "v")), - cs.Cyan(strings.TrimPrefix(releaseInfo.Version, "v"))) - if ext.IsPinned() { - fmt.Fprintf(stderr, "To upgrade, run: gh extension upgrade %s --force\n", ext.Name()) - } else { - fmt.Fprintf(stderr, "To upgrade, run: gh extension upgrade %s\n", ext.Name()) + select { + case releaseInfo := <-updateMessageChan: + if releaseInfo != nil { + stderr := io.ErrOut + fmt.Fprintf(stderr, "\n\n%s %s → %s\n", + cs.Yellowf("A new release of %s is available:", ext.Name()), + cs.Cyan(strings.TrimPrefix(ext.CurrentVersion(), "v")), + cs.Cyan(strings.TrimPrefix(releaseInfo.Version, "v"))) + if ext.IsPinned() { + fmt.Fprintf(stderr, "To upgrade, run: gh extension upgrade %s --force\n", ext.Name()) + } else { + fmt.Fprintf(stderr, "To upgrade, run: gh extension upgrade %s\n", ext.Name()) + } + fmt.Fprintf(stderr, "%s\n\n", + cs.Yellow(releaseInfo.URL)) } - fmt.Fprintf(stderr, "%s\n\n", - cs.Yellow(releaseInfo.URL)) + default: + // Do not make the user wait for extension update check if incomplete by this time. + // This is being handled in non-blocking default as there is no context to cancel like in gh update checks. } }, GroupID: "extension", diff --git a/pkg/cmd/root/extension_test.go b/pkg/cmd/root/extension_test.go index 32b250de5..7dd66df31 100644 --- a/pkg/cmd/root/extension_test.go +++ b/pkg/cmd/root/extension_test.go @@ -3,6 +3,7 @@ package root_test import ( "io" "testing" + "time" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/update" @@ -121,6 +122,8 @@ func TestNewCmdExtension_Updates(t *testing.T) { em := &extensions.ExtensionManagerMock{ DispatchFunc: func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) { // Assume extension executed / dispatched without problems as test is focused on upgrade checking. + // Sleep for 1 second to allow update checking logic to complete. + time.Sleep(1 * time.Second) return true, nil }, } @@ -169,3 +172,51 @@ func TestNewCmdExtension_Updates(t *testing.T) { } } } + +func TestNewCmdExtension_UpdateCheckIsNonblocking(t *testing.T) { + ios, _, _, stderr := iostreams.Test() + + em := &extensions.ExtensionManagerMock{ + DispatchFunc: func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) { + // Assume extension executed / dispatched without problems as test is focused on upgrade checking. + return true, nil + }, + } + + ext := &extensions.ExtensionMock{ + CurrentVersionFunc: func() string { + return "1.0.0" + }, + IsPinnedFunc: func() bool { + return false + }, + LatestVersionFunc: func() string { + return "2.0.0" + }, + NameFunc: func() string { + return "major-update" + }, + UpdateAvailableFunc: func() bool { + return true + }, + URLFunc: func() string { + return "https//github.com/dne/major-update" + }, + } + + checkFunc := func(em extensions.ExtensionManager, ext extensions.Extension) (*update.ReleaseInfo, error) { + // This function runs in the background as a goroutine while the extension is dispatched. + // Testing whether the extension cobra command is non-blocking for extension update check, + // this function will cause goroutine to sleep for a sufficiently long time before panicking. + // The idea is that the test will conclude because the dispatch function completes immediately + // and exits before the check update function completes. + time.Sleep(30 * time.Second) + panic("It seems like the extension cobra command might be blocking on update checking when it should not block on long update checks") + } + + cmd := root.NewCmdExtension(ios, em, ext, checkFunc) + + _, err := cmd.ExecuteC() + require.NoError(t, err) + require.Emptyf(t, stderr.String(), "long running, non-blocking extension update check should not have displayed anything on stderr") +} From 88988374b51ae71952c708a4d01a814b40c564ee Mon Sep 17 00:00:00 2001 From: Wing Date: Wed, 4 Dec 2024 15:24:41 +0100 Subject: [PATCH 13/45] Add remote check to secret commands Co-authored-by: William Martin --- pkg/cmd/secret/delete/delete.go | 12 ++++++++++++ pkg/cmd/secret/list/list.go | 12 ++++++++++++ pkg/cmd/secret/set/set.go | 13 +++++++++++++ pkg/cmdutil/errors.go | 16 ++++++++++++++++ 4 files changed, 53 insertions(+) diff --git a/pkg/cmd/secret/delete/delete.go b/pkg/cmd/secret/delete/delete.go index 564e8633f..75d061e7f 100644 --- a/pkg/cmd/secret/delete/delete.go +++ b/pkg/cmd/secret/delete/delete.go @@ -6,6 +6,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" + ghContext "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/secret/shared" @@ -19,12 +20,15 @@ type DeleteOptions struct { IO *iostreams.IOStreams Config func() (gh.Config, error) BaseRepo func() (ghrepo.Interface, error) + Remotes func() (ghContext.Remotes, error) SecretName string OrgName string EnvName string UserSecrets bool Application string + + HasRepoOverride bool } func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Command { @@ -32,6 +36,7 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co IO: f.IOStreams, Config: f.Config, HttpClient: f.HttpClient, + Remotes: f.Remotes, } cmd := &cobra.Command{ @@ -53,6 +58,8 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co return err } + opts.HasRepoOverride = cmd.Flags().Changed("repo") + opts.SecretName = args[0] if runF != nil { @@ -103,6 +110,11 @@ func removeRun(opts *DeleteOptions) error { if err != nil { return err } + + err = cmdutil.ValidateHasOnlyOneRemote(opts.HasRepoOverride, opts.Remotes) + if err != nil { + return err + } } cfg, err := opts.Config() diff --git a/pkg/cmd/secret/list/list.go b/pkg/cmd/secret/list/list.go index 9e1fd305d..b87c31998 100644 --- a/pkg/cmd/secret/list/list.go +++ b/pkg/cmd/secret/list/list.go @@ -9,6 +9,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" + ghContext "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/tableprinter" @@ -23,6 +24,7 @@ type ListOptions struct { IO *iostreams.IOStreams Config func() (gh.Config, error) BaseRepo func() (ghrepo.Interface, error) + Remotes func() (ghContext.Remotes, error) Now func() time.Time Exporter cmdutil.Exporter @@ -30,6 +32,8 @@ type ListOptions struct { EnvName string UserSecrets bool Application string + + HasRepoOverride bool } var secretFields = []string{ @@ -47,6 +51,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman IO: f.IOStreams, Config: f.Config, HttpClient: f.HttpClient, + Remotes: f.Remotes, Now: time.Now, } @@ -70,6 +75,8 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman return err } + opts.HasRepoOverride = cmd.Flags().Changed("repo") + if runF != nil { return runF(opts) } @@ -101,6 +108,11 @@ func listRun(opts *ListOptions) error { if err != nil { return err } + + err = cmdutil.ValidateHasOnlyOneRemote(opts.HasRepoOverride, opts.Remotes) + if err != nil { + return err + } } secretEntity, err := shared.GetSecretEntity(orgName, envName, opts.UserSecrets) diff --git a/pkg/cmd/secret/set/set.go b/pkg/cmd/secret/set/set.go index 755bdda1f..dade133af 100644 --- a/pkg/cmd/secret/set/set.go +++ b/pkg/cmd/secret/set/set.go @@ -11,6 +11,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" + ghContext "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/secret/shared" @@ -27,6 +28,7 @@ type SetOptions struct { IO *iostreams.IOStreams Config func() (gh.Config, error) BaseRepo func() (ghrepo.Interface, error) + Remotes func() (ghContext.Remotes, error) Prompter iprompter RandomOverride func() io.Reader @@ -41,6 +43,8 @@ type SetOptions struct { RepositoryNames []string EnvFile string Application string + + HasRepoOverride bool } type iprompter interface { @@ -52,6 +56,7 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command IO: f.IOStreams, Config: f.Config, HttpClient: f.HttpClient, + Remotes: f.Remotes, Prompter: f.Prompter, } @@ -144,6 +149,8 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command } } + opts.HasRepoOverride = cmd.Flags().Changed("repo") + if runF != nil { return runF(opts) } @@ -187,6 +194,12 @@ func setRun(opts *SetOptions) error { if err != nil { return err } + + err = cmdutil.ValidateHasOnlyOneRemote(opts.HasRepoOverride, opts.Remotes) + if err != nil { + return err + } + host = baseRepo.RepoHost() } else { cfg, err := opts.Config() diff --git a/pkg/cmdutil/errors.go b/pkg/cmdutil/errors.go index edb97abcd..389a40d25 100644 --- a/pkg/cmdutil/errors.go +++ b/pkg/cmdutil/errors.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/AlecAivazis/survey/v2/terminal" + ghContext "github.com/cli/cli/v2/context" ) // FlagErrorf returns a new FlagError that wraps an error produced by @@ -57,6 +58,21 @@ func MutuallyExclusive(message string, conditions ...bool) error { return nil } +func ValidateHasOnlyOneRemote(hasRepoOverride bool, remotes func() (ghContext.Remotes, error)) error { + if !hasRepoOverride { + remotes, err := remotes() + if err != nil { + return err + } + + if remotes.Len() > 1 { + return fmt.Errorf("multiple remotes detected %v. please specify which repo to use by providing the -R or --repo argument", remotes) + } + } + + return nil +} + type NoResultsError struct { message string } From 57c9ee0ad2257d4b68a90764cb2d973f4e04dcbc Mon Sep 17 00:00:00 2001 From: Wing Date: Wed, 4 Dec 2024 15:25:47 +0100 Subject: [PATCH 14/45] Add tests for secret commands Co-authored-by: William Martin --- pkg/cmd/secret/delete/delete_test.go | 118 +++++++++++++++++++ pkg/cmd/secret/list/list_test.go | 169 +++++++++++++++++++++++++++ pkg/cmd/secret/set/set_test.go | 139 ++++++++++++++++++++++ pkg/cmdutil/errors.go | 2 +- 4 files changed, 427 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/secret/delete/delete_test.go b/pkg/cmd/secret/delete/delete_test.go index bd5053f4d..45091a3b8 100644 --- a/pkg/cmd/secret/delete/delete_test.go +++ b/pkg/cmd/secret/delete/delete_test.go @@ -5,6 +5,8 @@ import ( "net/http" "testing" + ghContext "github.com/cli/cli/v2/context" + "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" @@ -351,3 +353,119 @@ func Test_removeRun_user(t *testing.T) { reg.Verify(t) } + +func Test_removeRun_remote_validation(t *testing.T) { + tests := []struct { + name string + opts *DeleteOptions + wantPath string + wantErr bool + errMsg string + }{ + { + name: "single repo detected", + opts: &DeleteOptions{ + Application: "actions", + SecretName: "cool_secret", + Remotes: func() (ghContext.Remotes, error) { + remote := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "repo"), + } + + return ghContext.Remotes{ + remote, + }, nil + }}, + wantPath: "repos/owner/repo/actions/secrets/cool_secret", + }, + { + name: "multi repo detected", + opts: &DeleteOptions{ + Application: "actions", + SecretName: "cool_secret", + Remotes: func() (ghContext.Remotes, error) { + remote := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "repo"), + } + remote2 := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "upstream", + }, + Repo: ghrepo.New("owner", "repo"), + } + + return ghContext.Remotes{ + remote, + remote2, + }, nil + }}, + wantErr: true, + errMsg: "multiple remotes detected [origin upstream]. please specify which repo to use by providing the -R or --repo argument", + }, + { + name: "multi repo detected - single repo given", + opts: &DeleteOptions{ + Application: "actions", + SecretName: "cool_secret", + HasRepoOverride: true, + Remotes: func() (ghContext.Remotes, error) { + remote := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "repo"), + } + remote2 := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "upstream", + }, + Repo: ghrepo.New("owner", "repo"), + } + + return ghContext.Remotes{ + remote, + remote2, + }, nil + }}, + wantPath: "repos/owner/repo/actions/secrets/cool_secret", + }, + } + + for _, tt := range tests { + reg := &httpmock.Registry{} + + if tt.wantPath != "" { + reg.Register( + httpmock.REST("DELETE", tt.wantPath), + httpmock.StatusStringResponse(204, "No Content")) + } + + ios, _, _, _ := iostreams.Test() + + tt.opts.IO = ios + tt.opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + } + tt.opts.Config = func() (gh.Config, error) { + return config.NewBlankConfig(), nil + } + tt.opts.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.FromFullName("owner/repo") + } + + err := removeRun(tt.opts) + if tt.wantErr { + assert.EqualError(t, err, tt.errMsg) + } else { + assert.NoError(t, err) + } + + reg.Verify(t) + } +} diff --git a/pkg/cmd/secret/list/list_test.go b/pkg/cmd/secret/list/list_test.go index 2e7b19065..a66b913ea 100644 --- a/pkg/cmd/secret/list/list_test.go +++ b/pkg/cmd/secret/list/list_test.go @@ -9,6 +9,8 @@ import ( "testing" "time" + ghContext "github.com/cli/cli/v2/context" + "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" @@ -442,6 +444,173 @@ func Test_listRun(t *testing.T) { } } +func Test_listRunRemoteValidation(t *testing.T) { + tests := []struct { + name string + tty bool + json bool + opts *ListOptions + wantOut []string + wantErr bool + errMsg string + }{ + { + name: "single repo detected", + tty: false, + opts: &ListOptions{ + Remotes: func() (ghContext.Remotes, error) { + remote := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "repo"), + } + + return ghContext.Remotes{ + remote, + }, nil + }, + }, + wantOut: []string{ + "SECRET_ONE\t1988-10-11T00:00:00Z", + "SECRET_TWO\t2020-12-04T00:00:00Z", + "SECRET_THREE\t1975-11-30T00:00:00Z", + }, + }, + { + name: "multi repo detected", + tty: false, + opts: &ListOptions{ + Remotes: func() (ghContext.Remotes, error) { + remote := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "repo"), + } + remote2 := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "upstream", + }, + Repo: ghrepo.New("owner", "repo"), + } + + return ghContext.Remotes{ + remote, + remote2, + }, nil + }, + }, + wantOut: []string{}, + wantErr: true, + errMsg: "multiple remotes detected [origin upstream]. please specify which repo to use by providing the -R or --repo argument", + }, + { + name: "multi repo detected - single repo given", + tty: false, + opts: &ListOptions{ + HasRepoOverride: true, + Remotes: func() (ghContext.Remotes, error) { + remote := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "repo"), + } + remote2 := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "upstream", + }, + Repo: ghrepo.New("owner", "repo"), + } + + return ghContext.Remotes{ + remote, + remote2, + }, nil + }, + }, + wantOut: []string{ + "SECRET_ONE\t1988-10-11T00:00:00Z", + "SECRET_TWO\t2020-12-04T00:00:00Z", + "SECRET_THREE\t1975-11-30T00:00:00Z", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + reg.Verify(t) + + path := "repos/owner/repo/actions/secrets" + + t0, _ := time.Parse("2006-01-02", "1988-10-11") + t1, _ := time.Parse("2006-01-02", "2020-12-04") + t2, _ := time.Parse("2006-01-02", "1975-11-30") + payload := struct { + Secrets []Secret + }{ + Secrets: []Secret{ + { + Name: "SECRET_ONE", + UpdatedAt: t0, + }, + { + Name: "SECRET_TWO", + UpdatedAt: t1, + }, + { + Name: "SECRET_THREE", + UpdatedAt: t2, + }, + }, + } + + reg.Register(httpmock.REST("GET", path), httpmock.JSONResponse(payload)) + + ios, _, stdout, _ := iostreams.Test() + + ios.SetStdoutTTY(tt.tty) + + tt.opts.IO = ios + tt.opts.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.FromFullName("owner/repo") + } + tt.opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + } + tt.opts.Config = func() (gh.Config, error) { + return config.NewBlankConfig(), nil + } + tt.opts.Now = func() time.Time { + t, _ := time.Parse(time.RFC822, "15 Mar 23 00:00 UTC") + return t + } + + if tt.json { + exporter := cmdutil.NewJSONExporter() + exporter.SetFields(secretFields) + tt.opts.Exporter = exporter + } + + err := listRun(tt.opts) + if tt.wantErr { + assert.EqualError(t, err, tt.errMsg) + + return + } + + assert.NoError(t, err) + + if len(tt.wantOut) > 1 { + expected := fmt.Sprintf("%s\n", strings.Join(tt.wantOut, "\n")) + assert.Equal(t, expected, stdout.String()) + } + }) + } +} + // Test_listRun_populatesNumSelectedReposIfRequired asserts that NumSelectedRepos // field is populated **only** when it's going to be presented in the output. Since // populating this field costs further API requests (one per secret), it's important diff --git a/pkg/cmd/secret/set/set_test.go b/pkg/cmd/secret/set/set_test.go index 5c8d6f693..68c7624dd 100644 --- a/pkg/cmd/secret/set/set_test.go +++ b/pkg/cmd/secret/set/set_test.go @@ -10,6 +10,8 @@ import ( "testing" "github.com/MakeNowJust/heredoc" + ghContext "github.com/cli/cli/v2/context" + "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" @@ -700,6 +702,143 @@ func Test_getSecretsFromOptions(t *testing.T) { } } +func Test_setRun_remote_validation(t *testing.T) { + tests := []struct { + name string + opts *SetOptions + wantApp string + wantErr bool + errMsg string + }{ + { + name: "single repo detected", + opts: &SetOptions{ + Application: "actions", + Remotes: func() (ghContext.Remotes, error) { + remote := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "repo"), + } + + return ghContext.Remotes{ + remote, + }, nil + }, + }, + wantApp: "actions", + }, + { + name: "multi repo detected", + opts: &SetOptions{ + Application: "actions", + Remotes: func() (ghContext.Remotes, error) { + remote := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "repo"), + } + remote2 := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "upstream", + }, + Repo: ghrepo.New("owner", "repo"), + } + + return ghContext.Remotes{ + remote, + remote2, + }, nil + }, + }, + wantErr: true, + errMsg: "multiple remotes detected [origin upstream]. please specify which repo to use by providing the -R or --repo argument", + }, + { + name: "multi repo detected - single repo given", + opts: &SetOptions{ + Application: "actions", + HasRepoOverride: true, + Remotes: func() (ghContext.Remotes, error) { + remote := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "repo"), + } + remote2 := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "upstream", + }, + Repo: ghrepo.New("owner", "repo"), + } + + return ghContext.Remotes{ + remote, + remote2, + }, nil + }, + }, + wantApp: "actions", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + + if tt.wantApp != "" { + reg.Register(httpmock.REST("GET", fmt.Sprintf("repos/owner/repo/%s/secrets/public-key", tt.wantApp)), + httpmock.JSONResponse(PubKey{ID: "123", Key: "CDjXqf7AJBXWhMczcy+Fs7JlACEptgceysutztHaFQI="})) + + reg.Register(httpmock.REST("PUT", fmt.Sprintf("repos/owner/repo/%s/secrets/cool_secret", tt.wantApp)), + httpmock.StatusStringResponse(201, `{}`)) + } + + ios, _, _, _ := iostreams.Test() + + opts := &SetOptions{ + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, + Config: func() (gh.Config, error) { return config.NewBlankConfig(), nil }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.FromFullName("owner/repo") + }, + IO: ios, + SecretName: "cool_secret", + Body: "a secret", + RandomOverride: fakeRandom, + Application: tt.opts.Application, + HasRepoOverride: tt.opts.HasRepoOverride, + Remotes: tt.opts.Remotes, + } + + err := setRun(opts) + if tt.wantErr { + assert.EqualError(t, err, tt.errMsg) + } else { + assert.NoError(t, err) + } + + reg.Verify(t) + + if tt.wantApp != "" && !tt.wantErr { + data, err := io.ReadAll(reg.Requests[1].Body) + assert.NoError(t, err) + + var payload SecretPayload + err = json.Unmarshal(data, &payload) + assert.NoError(t, err) + assert.Equal(t, payload.KeyID, "123") + assert.Equal(t, payload.EncryptedValue, "UKYUCbHd0DJemxa3AOcZ6XcsBwALG9d4bpB8ZT0gSV39vl3BHiGSgj8zJapDxgB2BwqNqRhpjC4=") + } + }) + } +} + func fakeRandom() io.Reader { return bytes.NewReader(bytes.Repeat([]byte{5}, 32)) } diff --git a/pkg/cmdutil/errors.go b/pkg/cmdutil/errors.go index 389a40d25..e1a37d0c8 100644 --- a/pkg/cmdutil/errors.go +++ b/pkg/cmdutil/errors.go @@ -59,7 +59,7 @@ func MutuallyExclusive(message string, conditions ...bool) error { } func ValidateHasOnlyOneRemote(hasRepoOverride bool, remotes func() (ghContext.Remotes, error)) error { - if !hasRepoOverride { + if !hasRepoOverride && remotes != nil { remotes, err := remotes() if err != nil { return err From 3de2fd94b3caea368c5bd9d0273c0eb47ffc7091 Mon Sep 17 00:00:00 2001 From: Wing Date: Wed, 4 Dec 2024 15:27:01 +0100 Subject: [PATCH 15/45] Prompt for secret commands Co-authored-by: William Martin --- pkg/cmd/secret/set/set.go | 29 +++++++++++++++++++++++------ pkg/cmdutil/flags.go | 8 ++++++++ pkg/cmdutil/repo_override.go | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/secret/set/set.go b/pkg/cmd/secret/set/set.go index dade133af..9f3797008 100644 --- a/pkg/cmd/secret/set/set.go +++ b/pkg/cmd/secret/set/set.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/base64" "fmt" + "github.com/cli/cli/v2/internal/prompter" "io" "net/http" "os" @@ -29,7 +30,7 @@ type SetOptions struct { Config func() (gh.Config, error) BaseRepo func() (ghrepo.Interface, error) Remotes func() (ghContext.Remotes, error) - Prompter iprompter + Prompter prompter.Prompter RandomOverride func() io.Reader @@ -45,10 +46,7 @@ type SetOptions struct { Application string HasRepoOverride bool -} - -type iprompter interface { - Password(string) (string, error) + Interactive bool } func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command { @@ -82,6 +80,9 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command # Read secret value from an environment variable $ gh secret set MYSECRET --body "$ENV_VALUE" + # Set secret for a specific remote repository + $ gh secret set MYSECRET --repo origin/repo --body "$ENV_VALUE" + # Read secret value from a file $ gh secret set MYSECRET < myfile.txt @@ -111,6 +112,8 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command // support `-R, --repo` override opts.BaseRepo = f.BaseRepo + flagCount := cmdutil.CountSetFlags(cmd.Flags()) + if err := cmdutil.MutuallyExclusive("specify only one of `--org`, `--env`, or `--user`", opts.OrgName != "", opts.EnvName != "", opts.UserSecrets); err != nil { return err } @@ -129,6 +132,10 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command } } else { opts.SecretName = args[0] + + if flagCount == 0 { + opts.Interactive = true + } } if cmd.Flags().Changed("visibility") { @@ -197,7 +204,17 @@ func setRun(opts *SetOptions) error { err = cmdutil.ValidateHasOnlyOneRemote(opts.HasRepoOverride, opts.Remotes) if err != nil { - return err + if opts.Interactive { + selectedRepo, errSelectedRepo := cmdutil.PromptForRepo(baseRepo, opts.Remotes, opts.Prompter) + + if errSelectedRepo != nil { + return errSelectedRepo + } + + baseRepo = selectedRepo + } else { + return err + } } host = baseRepo.RepoHost() diff --git a/pkg/cmdutil/flags.go b/pkg/cmdutil/flags.go index c0064099c..fadbc5fbe 100644 --- a/pkg/cmdutil/flags.go +++ b/pkg/cmdutil/flags.go @@ -180,3 +180,11 @@ func isIncluded(value string, opts []string) bool { } return false } + +func CountSetFlags(flags *pflag.FlagSet) int { + count := 0 + flags.Visit(func(f *pflag.Flag) { + count++ + }) + return count +} diff --git a/pkg/cmdutil/repo_override.go b/pkg/cmdutil/repo_override.go index 791dd919a..41a4a4799 100644 --- a/pkg/cmdutil/repo_override.go +++ b/pkg/cmdutil/repo_override.go @@ -1,11 +1,13 @@ package cmdutil import ( + ghContext "github.com/cli/cli/v2/context" "os" "sort" "strings" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/spf13/cobra" ) @@ -68,3 +70,34 @@ func OverrideBaseRepoFunc(f *Factory, override string) func() (ghrepo.Interface, } return f.BaseRepo } + +func PromptForRepo(baseRepo ghrepo.Interface, remotes func() (ghContext.Remotes, error), survey prompter.Prompter) (ghrepo.Interface, error) { + var defaultRepo string + var remoteArray []string + + if remotes, _ := remotes(); remotes != nil { + if defaultRemote, _ := remotes.ResolvedRemote(); defaultRemote != nil { + // this is a remote explicitly chosen via `repo set-default` + defaultRepo = ghrepo.FullName(defaultRemote) + } else if len(remotes) > 0 { + // as a fallback, just pick the first remote + defaultRepo = ghrepo.FullName(remotes[0]) + } + + for _, remote := range remotes { + remoteArray = append(remoteArray, ghrepo.FullName(remote)) + } + } + + baseRepoInput, errInput := survey.Select("Select a base repo", defaultRepo, remoteArray) + if errInput != nil { + return baseRepo, errInput + } + + selectedRepo, errSelectedRepo := ghrepo.FromFullName(remoteArray[baseRepoInput]) + if errSelectedRepo != nil { + return baseRepo, errSelectedRepo + } + + return selectedRepo, nil +} From cad59036f5aabb62e70723fb8ac7da61b34ed719 Mon Sep 17 00:00:00 2001 From: Wing Date: Wed, 21 Aug 2024 18:16:06 +0200 Subject: [PATCH 16/45] Update docs for set-default NOTE: gh does not use the default repository for managing repository and environment secrets. --- pkg/cmd/repo/setdefault/setdefault.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/repo/setdefault/setdefault.go b/pkg/cmd/repo/setdefault/setdefault.go index eb8fcfb5a..69be625d6 100644 --- a/pkg/cmd/repo/setdefault/setdefault.go +++ b/pkg/cmd/repo/setdefault/setdefault.go @@ -30,7 +30,8 @@ func explainer() string { - viewing and creating issues - viewing and creating releases - working with GitHub Actions - - adding repository and environment secrets`) + + ### NOTE: gh does not use the default repository for managing repository and environment secrets or variables.`) } type iprompter interface { From 0be5720c1cd1ffacebead919be7091e53d1e17b6 Mon Sep 17 00:00:00 2001 From: Wing Date: Wed, 21 Aug 2024 18:31:27 +0200 Subject: [PATCH 17/45] Update setdefault test --- pkg/cmd/repo/setdefault/setdefault.go | 2 +- pkg/cmd/repo/setdefault/setdefault_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/repo/setdefault/setdefault.go b/pkg/cmd/repo/setdefault/setdefault.go index 69be625d6..3e1554a69 100644 --- a/pkg/cmd/repo/setdefault/setdefault.go +++ b/pkg/cmd/repo/setdefault/setdefault.go @@ -31,7 +31,7 @@ func explainer() string { - viewing and creating releases - working with GitHub Actions - ### NOTE: gh does not use the default repository for managing repository and environment secrets or variables.`) + ### NOTE: gh does not use the default repository for managing repository and environment secrets.`) } type iprompter interface { diff --git a/pkg/cmd/repo/setdefault/setdefault_test.go b/pkg/cmd/repo/setdefault/setdefault_test.go index 55a0193d5..dfd72d83e 100644 --- a/pkg/cmd/repo/setdefault/setdefault_test.go +++ b/pkg/cmd/repo/setdefault/setdefault_test.go @@ -382,7 +382,7 @@ func TestDefaultRun(t *testing.T) { } } }, - wantStdout: "This command sets the default remote repository to use when querying the\nGitHub API for the locally cloned repository.\n\ngh uses the default repository for things like:\n\n - viewing and creating pull requests\n - viewing and creating issues\n - viewing and creating releases\n - working with GitHub Actions\n - adding repository and environment secrets\n\n✓ Set OWNER2/REPO2 as the default repository for the current directory\n", + wantStdout: "This command sets the default remote repository to use when querying the\nGitHub API for the locally cloned repository.\n\ngh uses the default repository for things like:\n\n - viewing and creating pull requests\n - viewing and creating issues\n - viewing and creating releases\n - working with GitHub Actions\n\n### NOTE: gh does not use the default repository for managing repository and environment secrets.\n\n✓ Set OWNER2/REPO2 as the default repository for the current directory\n", }, { name: "interactive mode only one known host", @@ -456,7 +456,7 @@ func TestDefaultRun(t *testing.T) { } } }, - wantStdout: "This command sets the default remote repository to use when querying the\nGitHub API for the locally cloned repository.\n\ngh uses the default repository for things like:\n\n - viewing and creating pull requests\n - viewing and creating issues\n - viewing and creating releases\n - working with GitHub Actions\n - adding repository and environment secrets\n\n✓ Set OWNER2/REPO2 as the default repository for the current directory\n", + wantStdout: "This command sets the default remote repository to use when querying the\nGitHub API for the locally cloned repository.\n\ngh uses the default repository for things like:\n\n - viewing and creating pull requests\n - viewing and creating issues\n - viewing and creating releases\n - working with GitHub Actions\n\n### NOTE: gh does not use the default repository for managing repository and environment secrets.\n\n✓ Set OWNER2/REPO2 as the default repository for the current directory\n", }, } From df8bb51c9cf92ac3ae72fb914799e004f54d6b10 Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 9 Dec 2024 15:59:01 +0100 Subject: [PATCH 18/45] Always prompt on secret set when multiple remotes --- pkg/cmd/secret/set/set.go | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/secret/set/set.go b/pkg/cmd/secret/set/set.go index 9f3797008..74bd7b33c 100644 --- a/pkg/cmd/secret/set/set.go +++ b/pkg/cmd/secret/set/set.go @@ -4,12 +4,13 @@ import ( "bytes" "encoding/base64" "fmt" - "github.com/cli/cli/v2/internal/prompter" "io" "net/http" "os" "strings" + "github.com/cli/cli/v2/internal/prompter" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" ghContext "github.com/cli/cli/v2/context" @@ -46,7 +47,7 @@ type SetOptions struct { Application string HasRepoOverride bool - Interactive bool + CanPrompt bool } func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command { @@ -112,8 +113,6 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command // support `-R, --repo` override opts.BaseRepo = f.BaseRepo - flagCount := cmdutil.CountSetFlags(cmd.Flags()) - if err := cmdutil.MutuallyExclusive("specify only one of `--org`, `--env`, or `--user`", opts.OrgName != "", opts.EnvName != "", opts.UserSecrets); err != nil { return err } @@ -132,10 +131,6 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command } } else { opts.SecretName = args[0] - - if flagCount == 0 { - opts.Interactive = true - } } if cmd.Flags().Changed("visibility") { @@ -157,6 +152,7 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command } opts.HasRepoOverride = cmd.Flags().Changed("repo") + opts.CanPrompt = opts.IO.CanPrompt() if runF != nil { return runF(opts) @@ -202,19 +198,17 @@ func setRun(opts *SetOptions) error { return err } - err = cmdutil.ValidateHasOnlyOneRemote(opts.HasRepoOverride, opts.Remotes) - if err != nil { - if opts.Interactive { - selectedRepo, errSelectedRepo := cmdutil.PromptForRepo(baseRepo, opts.Remotes, opts.Prompter) - - if errSelectedRepo != nil { - return errSelectedRepo - } - - baseRepo = selectedRepo - } else { + if err = cmdutil.ValidateHasOnlyOneRemote(opts.HasRepoOverride, opts.Remotes); err != nil { + if !opts.CanPrompt { return err } + + selectedRepo, errSelectingRepo := cmdutil.PromptForRepo(baseRepo, opts.Remotes, opts.Prompter) + if errSelectingRepo != nil { + return errSelectingRepo + } + + baseRepo = selectedRepo } host = baseRepo.RepoHost() From 73244c010eddcb89534b090203059d1ee3dd4dd5 Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 9 Dec 2024 16:04:41 +0100 Subject: [PATCH 19/45] Move secret repo validation into secrets subpackage --- pkg/cmd/secret/delete/delete.go | 2 +- pkg/cmd/secret/list/list.go | 2 +- pkg/cmd/secret/set/set.go | 4 +-- pkg/cmd/secret/shared/base_repo.go | 55 ++++++++++++++++++++++++++++++ pkg/cmdutil/errors.go | 16 --------- pkg/cmdutil/repo_override.go | 33 ------------------ 6 files changed, 59 insertions(+), 53 deletions(-) create mode 100644 pkg/cmd/secret/shared/base_repo.go diff --git a/pkg/cmd/secret/delete/delete.go b/pkg/cmd/secret/delete/delete.go index 75d061e7f..70aab4be5 100644 --- a/pkg/cmd/secret/delete/delete.go +++ b/pkg/cmd/secret/delete/delete.go @@ -111,7 +111,7 @@ func removeRun(opts *DeleteOptions) error { return err } - err = cmdutil.ValidateHasOnlyOneRemote(opts.HasRepoOverride, opts.Remotes) + err = shared.ValidateHasOnlyOneRemote(opts.HasRepoOverride, opts.Remotes) if err != nil { return err } diff --git a/pkg/cmd/secret/list/list.go b/pkg/cmd/secret/list/list.go index b87c31998..5c13234d0 100644 --- a/pkg/cmd/secret/list/list.go +++ b/pkg/cmd/secret/list/list.go @@ -109,7 +109,7 @@ func listRun(opts *ListOptions) error { return err } - err = cmdutil.ValidateHasOnlyOneRemote(opts.HasRepoOverride, opts.Remotes) + err = shared.ValidateHasOnlyOneRemote(opts.HasRepoOverride, opts.Remotes) if err != nil { return err } diff --git a/pkg/cmd/secret/set/set.go b/pkg/cmd/secret/set/set.go index 74bd7b33c..b4630e067 100644 --- a/pkg/cmd/secret/set/set.go +++ b/pkg/cmd/secret/set/set.go @@ -198,12 +198,12 @@ func setRun(opts *SetOptions) error { return err } - if err = cmdutil.ValidateHasOnlyOneRemote(opts.HasRepoOverride, opts.Remotes); err != nil { + if err = shared.ValidateHasOnlyOneRemote(opts.HasRepoOverride, opts.Remotes); err != nil { if !opts.CanPrompt { return err } - selectedRepo, errSelectingRepo := cmdutil.PromptForRepo(baseRepo, opts.Remotes, opts.Prompter) + selectedRepo, errSelectingRepo := shared.PromptForRepo(baseRepo, opts.Remotes, opts.Prompter) if errSelectingRepo != nil { return errSelectingRepo } diff --git a/pkg/cmd/secret/shared/base_repo.go b/pkg/cmd/secret/shared/base_repo.go new file mode 100644 index 000000000..db2dfc8f1 --- /dev/null +++ b/pkg/cmd/secret/shared/base_repo.go @@ -0,0 +1,55 @@ +package shared + +import ( + "fmt" + + ghContext "github.com/cli/cli/v2/context" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" +) + +func ValidateHasOnlyOneRemote(hasRepoOverride bool, remotes func() (ghContext.Remotes, error)) error { + if !hasRepoOverride && remotes != nil { + remotes, err := remotes() + if err != nil { + return err + } + + if remotes.Len() > 1 { + return fmt.Errorf("multiple remotes detected %v. please specify which repo to use by providing the -R or --repo argument", remotes) + } + } + + return nil +} + +func PromptForRepo(baseRepo ghrepo.Interface, remotes func() (ghContext.Remotes, error), survey prompter.Prompter) (ghrepo.Interface, error) { + var defaultRepo string + var remoteArray []string + + if remotes, _ := remotes(); remotes != nil { + if defaultRemote, _ := remotes.ResolvedRemote(); defaultRemote != nil { + // this is a remote explicitly chosen via `repo set-default` + defaultRepo = ghrepo.FullName(defaultRemote) + } else if len(remotes) > 0 { + // as a fallback, just pick the first remote + defaultRepo = ghrepo.FullName(remotes[0]) + } + + for _, remote := range remotes { + remoteArray = append(remoteArray, ghrepo.FullName(remote)) + } + } + + baseRepoInput, errInput := survey.Select("Select a base repo", defaultRepo, remoteArray) + if errInput != nil { + return baseRepo, errInput + } + + selectedRepo, errSelectedRepo := ghrepo.FromFullName(remoteArray[baseRepoInput]) + if errSelectedRepo != nil { + return baseRepo, errSelectedRepo + } + + return selectedRepo, nil +} diff --git a/pkg/cmdutil/errors.go b/pkg/cmdutil/errors.go index e1a37d0c8..edb97abcd 100644 --- a/pkg/cmdutil/errors.go +++ b/pkg/cmdutil/errors.go @@ -5,7 +5,6 @@ import ( "fmt" "github.com/AlecAivazis/survey/v2/terminal" - ghContext "github.com/cli/cli/v2/context" ) // FlagErrorf returns a new FlagError that wraps an error produced by @@ -58,21 +57,6 @@ func MutuallyExclusive(message string, conditions ...bool) error { return nil } -func ValidateHasOnlyOneRemote(hasRepoOverride bool, remotes func() (ghContext.Remotes, error)) error { - if !hasRepoOverride && remotes != nil { - remotes, err := remotes() - if err != nil { - return err - } - - if remotes.Len() > 1 { - return fmt.Errorf("multiple remotes detected %v. please specify which repo to use by providing the -R or --repo argument", remotes) - } - } - - return nil -} - type NoResultsError struct { message string } diff --git a/pkg/cmdutil/repo_override.go b/pkg/cmdutil/repo_override.go index 41a4a4799..791dd919a 100644 --- a/pkg/cmdutil/repo_override.go +++ b/pkg/cmdutil/repo_override.go @@ -1,13 +1,11 @@ package cmdutil import ( - ghContext "github.com/cli/cli/v2/context" "os" "sort" "strings" "github.com/cli/cli/v2/internal/ghrepo" - "github.com/cli/cli/v2/internal/prompter" "github.com/spf13/cobra" ) @@ -70,34 +68,3 @@ func OverrideBaseRepoFunc(f *Factory, override string) func() (ghrepo.Interface, } return f.BaseRepo } - -func PromptForRepo(baseRepo ghrepo.Interface, remotes func() (ghContext.Remotes, error), survey prompter.Prompter) (ghrepo.Interface, error) { - var defaultRepo string - var remoteArray []string - - if remotes, _ := remotes(); remotes != nil { - if defaultRemote, _ := remotes.ResolvedRemote(); defaultRemote != nil { - // this is a remote explicitly chosen via `repo set-default` - defaultRepo = ghrepo.FullName(defaultRemote) - } else if len(remotes) > 0 { - // as a fallback, just pick the first remote - defaultRepo = ghrepo.FullName(remotes[0]) - } - - for _, remote := range remotes { - remoteArray = append(remoteArray, ghrepo.FullName(remote)) - } - } - - baseRepoInput, errInput := survey.Select("Select a base repo", defaultRepo, remoteArray) - if errInput != nil { - return baseRepo, errInput - } - - selectedRepo, errSelectedRepo := ghrepo.FromFullName(remoteArray[baseRepoInput]) - if errSelectedRepo != nil { - return baseRepo, errSelectedRepo - } - - return selectedRepo, nil -} From 870da79886e418e7c095247653440635edc0fd99 Mon Sep 17 00:00:00 2001 From: William Martin Date: Wed, 18 Dec 2024 13:36:53 +0100 Subject: [PATCH 20/45] Use smarter base repo funcs for secret commands --- pkg/cmd/secret/delete/delete.go | 25 ++- pkg/cmd/secret/delete/delete_test.go | 221 +++++++++---------- pkg/cmd/secret/list/list.go | 33 +-- pkg/cmd/secret/list/list_test.go | 271 +++++++++--------------- pkg/cmd/secret/set/set.go | 35 ++- pkg/cmd/secret/set/set_test.go | 240 +++++++++------------ pkg/cmd/secret/shared/base_repo.go | 75 +++++-- pkg/cmd/secret/shared/base_repo_test.go | 239 +++++++++++++++++++++ pkg/cmdutil/flags.go | 8 - 9 files changed, 647 insertions(+), 500 deletions(-) create mode 100644 pkg/cmd/secret/shared/base_repo_test.go diff --git a/pkg/cmd/secret/delete/delete.go b/pkg/cmd/secret/delete/delete.go index 70aab4be5..64b02c9d9 100644 --- a/pkg/cmd/secret/delete/delete.go +++ b/pkg/cmd/secret/delete/delete.go @@ -6,7 +6,6 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" - ghContext "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/secret/shared" @@ -20,15 +19,12 @@ type DeleteOptions struct { IO *iostreams.IOStreams Config func() (gh.Config, error) BaseRepo func() (ghrepo.Interface, error) - Remotes func() (ghContext.Remotes, error) SecretName string OrgName string EnvName string UserSecrets bool Application string - - HasRepoOverride bool } func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Command { @@ -36,7 +32,6 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co IO: f.IOStreams, Config: f.Config, HttpClient: f.HttpClient, - Remotes: f.Remotes, } cmd := &cobra.Command{ @@ -51,15 +46,24 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co `), Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - // support `-R, --repo` override + // If the user specified a repo directly, then we're using the OverrideBaseRepoFunc set by EnableRepoOverride + // So there's no reason to use the specialised BaseRepoFunc that requires remote disambiguation. opts.BaseRepo = f.BaseRepo + if !cmd.Flags().Changed("repo") { + // If they haven't specified a repo directly, then we will wrap the BaseRepoFunc in one that error if + // there might be multiple valid remotes. + opts.BaseRepo = shared.RequireNoAmbiguityBaseRepoFunc(opts.BaseRepo, f.Remotes) + // But if we are able to prompt, then we will wrap that up in a BaseRepoFunc that can prompt the user to + // resolve the ambiguity. + if opts.IO.CanPrompt() { + opts.BaseRepo = shared.PromptWhenMultipleRemotesBaseRepoFunc(opts.BaseRepo, f.Prompter) + } + } if err := cmdutil.MutuallyExclusive("specify only one of `--org`, `--env`, or `--user`", opts.OrgName != "", opts.EnvName != "", opts.UserSecrets); err != nil { return err } - opts.HasRepoOverride = cmd.Flags().Changed("repo") - opts.SecretName = args[0] if runF != nil { @@ -110,11 +114,6 @@ func removeRun(opts *DeleteOptions) error { if err != nil { return err } - - err = shared.ValidateHasOnlyOneRemote(opts.HasRepoOverride, opts.Remotes) - if err != nil { - return err - } } cfg, err := opts.Config() diff --git a/pkg/cmd/secret/delete/delete_test.go b/pkg/cmd/secret/delete/delete_test.go index 45091a3b8..91d688a89 100644 --- a/pkg/cmd/secret/delete/delete_test.go +++ b/pkg/cmd/secret/delete/delete_test.go @@ -2,6 +2,7 @@ package delete import ( "bytes" + "io" "net/http" "testing" @@ -10,6 +11,8 @@ import ( "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" + "github.com/cli/cli/v2/pkg/cmd/secret/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" @@ -122,6 +125,108 @@ func TestNewCmdDelete(t *testing.T) { } } +func TestNewCmdDeleteBaseRepoFuncs(t *testing.T) { + remotes := ghContext.Remotes{ + &ghContext.Remote{ + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "fork"), + }, + &ghContext.Remote{ + Remote: &git.Remote{ + Name: "upstream", + }, + Repo: ghrepo.New("owner", "repo"), + }, + } + + tests := []struct { + name string + args string + prompterStubs func(*prompter.MockPrompter) + wantRepo ghrepo.Interface + wantErr error + }{ + { + name: "when there is a repo flag provided, the factory base repo func is used", + args: "SECRET_NAME --repo owner/repo", + wantRepo: ghrepo.New("owner", "repo"), + }, + { + name: "when there is no repo flag provided, and no prompting, the base func requiring no ambiguity is used", + args: "SECRET_NAME", + wantErr: shared.MultipleRemotesError{ + Remotes: remotes, + }, + }, + { + name: "when there is no repo flag provided, and can prompt, the base func resolving ambiguity is used", + args: "SECRET_NAME", + prompterStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect( + "Select a base repo", + []string{"owner/fork", "owner/repo"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "owner/fork") + }, + ) + }, + wantRepo: ghrepo.New("owner", "fork"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + var pm *prompter.MockPrompter + if tt.prompterStubs != nil { + ios.SetStdinTTY(true) + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) + pm = prompter.NewMockPrompter(t) + tt.prompterStubs(pm) + } + + f := &cmdutil.Factory{ + IOStreams: ios, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.FromFullName("owner/repo") + }, + Prompter: pm, + Remotes: func() (ghContext.Remotes, error) { + return remotes, nil + }, + } + + argv, err := shlex.Split(tt.args) + assert.NoError(t, err) + + var gotOpts *DeleteOptions + cmd := NewCmdDelete(f, func(opts *DeleteOptions) error { + gotOpts = opts + return nil + }) + // Require to support --repo flag + cmdutil.EnableRepoOverride(cmd, f) + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + _, err = cmd.ExecuteC() + require.NoError(t, err) + + baseRepo, err := gotOpts.BaseRepo() + if tt.wantErr != nil { + require.Equal(t, tt.wantErr, err) + return + } + require.True(t, ghrepo.IsSame(tt.wantRepo, baseRepo)) + }) + } +} + func Test_removeRun_repo(t *testing.T) { tests := []struct { name string @@ -353,119 +458,3 @@ func Test_removeRun_user(t *testing.T) { reg.Verify(t) } - -func Test_removeRun_remote_validation(t *testing.T) { - tests := []struct { - name string - opts *DeleteOptions - wantPath string - wantErr bool - errMsg string - }{ - { - name: "single repo detected", - opts: &DeleteOptions{ - Application: "actions", - SecretName: "cool_secret", - Remotes: func() (ghContext.Remotes, error) { - remote := &ghContext.Remote{ - Remote: &git.Remote{ - Name: "origin", - }, - Repo: ghrepo.New("owner", "repo"), - } - - return ghContext.Remotes{ - remote, - }, nil - }}, - wantPath: "repos/owner/repo/actions/secrets/cool_secret", - }, - { - name: "multi repo detected", - opts: &DeleteOptions{ - Application: "actions", - SecretName: "cool_secret", - Remotes: func() (ghContext.Remotes, error) { - remote := &ghContext.Remote{ - Remote: &git.Remote{ - Name: "origin", - }, - Repo: ghrepo.New("owner", "repo"), - } - remote2 := &ghContext.Remote{ - Remote: &git.Remote{ - Name: "upstream", - }, - Repo: ghrepo.New("owner", "repo"), - } - - return ghContext.Remotes{ - remote, - remote2, - }, nil - }}, - wantErr: true, - errMsg: "multiple remotes detected [origin upstream]. please specify which repo to use by providing the -R or --repo argument", - }, - { - name: "multi repo detected - single repo given", - opts: &DeleteOptions{ - Application: "actions", - SecretName: "cool_secret", - HasRepoOverride: true, - Remotes: func() (ghContext.Remotes, error) { - remote := &ghContext.Remote{ - Remote: &git.Remote{ - Name: "origin", - }, - Repo: ghrepo.New("owner", "repo"), - } - remote2 := &ghContext.Remote{ - Remote: &git.Remote{ - Name: "upstream", - }, - Repo: ghrepo.New("owner", "repo"), - } - - return ghContext.Remotes{ - remote, - remote2, - }, nil - }}, - wantPath: "repos/owner/repo/actions/secrets/cool_secret", - }, - } - - for _, tt := range tests { - reg := &httpmock.Registry{} - - if tt.wantPath != "" { - reg.Register( - httpmock.REST("DELETE", tt.wantPath), - httpmock.StatusStringResponse(204, "No Content")) - } - - ios, _, _, _ := iostreams.Test() - - tt.opts.IO = ios - tt.opts.HttpClient = func() (*http.Client, error) { - return &http.Client{Transport: reg}, nil - } - tt.opts.Config = func() (gh.Config, error) { - return config.NewBlankConfig(), nil - } - tt.opts.BaseRepo = func() (ghrepo.Interface, error) { - return ghrepo.FromFullName("owner/repo") - } - - err := removeRun(tt.opts) - if tt.wantErr { - assert.EqualError(t, err, tt.errMsg) - } else { - assert.NoError(t, err) - } - - reg.Verify(t) - } -} diff --git a/pkg/cmd/secret/list/list.go b/pkg/cmd/secret/list/list.go index 5c13234d0..49ad47aff 100644 --- a/pkg/cmd/secret/list/list.go +++ b/pkg/cmd/secret/list/list.go @@ -9,9 +9,9 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" - ghContext "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/internal/tableprinter" "github.com/cli/cli/v2/pkg/cmd/secret/shared" "github.com/cli/cli/v2/pkg/cmdutil" @@ -24,16 +24,15 @@ type ListOptions struct { IO *iostreams.IOStreams Config func() (gh.Config, error) BaseRepo func() (ghrepo.Interface, error) - Remotes func() (ghContext.Remotes, error) - Now func() time.Time - Exporter cmdutil.Exporter + Prompter prompter.Prompter + + Now func() time.Time + Exporter cmdutil.Exporter OrgName string EnvName string UserSecrets bool Application string - - HasRepoOverride bool } var secretFields = []string{ @@ -51,8 +50,8 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman IO: f.IOStreams, Config: f.Config, HttpClient: f.HttpClient, - Remotes: f.Remotes, Now: time.Now, + Prompter: f.Prompter, } cmd := &cobra.Command{ @@ -68,15 +67,24 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman Aliases: []string{"ls"}, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - // support `-R, --repo` override + // If the user specified a repo directly, then we're using the OverrideBaseRepoFunc set by EnableRepoOverride + // So there's no reason to use the specialised BaseRepoFunc that requires remote disambiguation. opts.BaseRepo = f.BaseRepo + if !cmd.Flags().Changed("repo") { + // If they haven't specified a repo directly, then we will wrap the BaseRepoFunc in one that error if + // there might be multiple valid remotes. + opts.BaseRepo = shared.RequireNoAmbiguityBaseRepoFunc(opts.BaseRepo, f.Remotes) + // But if we are able to prompt, then we will wrap that up in a BaseRepoFunc that can prompt the user to + // resolve the ambiguity. + if opts.IO.CanPrompt() { + opts.BaseRepo = shared.PromptWhenMultipleRemotesBaseRepoFunc(opts.BaseRepo, f.Prompter) + } + } if err := cmdutil.MutuallyExclusive("specify only one of `--org`, `--env`, or `--user`", opts.OrgName != "", opts.EnvName != "", opts.UserSecrets); err != nil { return err } - opts.HasRepoOverride = cmd.Flags().Changed("repo") - if runF != nil { return runF(opts) } @@ -108,11 +116,6 @@ func listRun(opts *ListOptions) error { if err != nil { return err } - - err = shared.ValidateHasOnlyOneRemote(opts.HasRepoOverride, opts.Remotes) - if err != nil { - return err - } } secretEntity, err := shared.GetSecretEntity(orgName, envName, opts.UserSecrets) diff --git a/pkg/cmd/secret/list/list_test.go b/pkg/cmd/secret/list/list_test.go index a66b913ea..63a899b77 100644 --- a/pkg/cmd/secret/list/list_test.go +++ b/pkg/cmd/secret/list/list_test.go @@ -3,6 +3,7 @@ package list import ( "bytes" "fmt" + "io" "net/http" "net/url" "strings" @@ -14,6 +15,7 @@ import ( "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmd/secret/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" @@ -103,6 +105,108 @@ func Test_NewCmdList(t *testing.T) { } } +func TestNewCmdListBaseRepoFuncs(t *testing.T) { + remotes := ghContext.Remotes{ + &ghContext.Remote{ + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "fork"), + }, + &ghContext.Remote{ + Remote: &git.Remote{ + Name: "upstream", + }, + Repo: ghrepo.New("owner", "repo"), + }, + } + + tests := []struct { + name string + args string + prompterStubs func(*prompter.MockPrompter) + wantRepo ghrepo.Interface + wantErr error + }{ + { + name: "when there is a repo flag provided, the factory base repo func is used", + args: "--repo owner/repo", + wantRepo: ghrepo.New("owner", "repo"), + }, + { + name: "when there is no repo flag provided, and no prompting, the base func requiring no ambiguity is used", + args: "", + wantErr: shared.MultipleRemotesError{ + Remotes: remotes, + }, + }, + { + name: "when there is no repo flag provided, and can prompt, the base func resolving ambiguity is used", + args: "", + prompterStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect( + "Select a base repo", + []string{"owner/fork", "owner/repo"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "owner/fork") + }, + ) + }, + wantRepo: ghrepo.New("owner", "fork"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + var pm *prompter.MockPrompter + if tt.prompterStubs != nil { + ios.SetStdinTTY(true) + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) + pm = prompter.NewMockPrompter(t) + tt.prompterStubs(pm) + } + + f := &cmdutil.Factory{ + IOStreams: ios, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.FromFullName("owner/repo") + }, + Prompter: pm, + Remotes: func() (ghContext.Remotes, error) { + return remotes, nil + }, + } + + argv, err := shlex.Split(tt.args) + assert.NoError(t, err) + + var gotOpts *ListOptions + cmd := NewCmdList(f, func(opts *ListOptions) error { + gotOpts = opts + return nil + }) + // Require to support --repo flag + cmdutil.EnableRepoOverride(cmd, f) + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + _, err = cmd.ExecuteC() + require.NoError(t, err) + + baseRepo, err := gotOpts.BaseRepo() + if tt.wantErr != nil { + require.Equal(t, tt.wantErr, err) + return + } + require.True(t, ghrepo.IsSame(tt.wantRepo, baseRepo)) + }) + } +} + func Test_listRun(t *testing.T) { tests := []struct { name string @@ -444,173 +548,6 @@ func Test_listRun(t *testing.T) { } } -func Test_listRunRemoteValidation(t *testing.T) { - tests := []struct { - name string - tty bool - json bool - opts *ListOptions - wantOut []string - wantErr bool - errMsg string - }{ - { - name: "single repo detected", - tty: false, - opts: &ListOptions{ - Remotes: func() (ghContext.Remotes, error) { - remote := &ghContext.Remote{ - Remote: &git.Remote{ - Name: "origin", - }, - Repo: ghrepo.New("owner", "repo"), - } - - return ghContext.Remotes{ - remote, - }, nil - }, - }, - wantOut: []string{ - "SECRET_ONE\t1988-10-11T00:00:00Z", - "SECRET_TWO\t2020-12-04T00:00:00Z", - "SECRET_THREE\t1975-11-30T00:00:00Z", - }, - }, - { - name: "multi repo detected", - tty: false, - opts: &ListOptions{ - Remotes: func() (ghContext.Remotes, error) { - remote := &ghContext.Remote{ - Remote: &git.Remote{ - Name: "origin", - }, - Repo: ghrepo.New("owner", "repo"), - } - remote2 := &ghContext.Remote{ - Remote: &git.Remote{ - Name: "upstream", - }, - Repo: ghrepo.New("owner", "repo"), - } - - return ghContext.Remotes{ - remote, - remote2, - }, nil - }, - }, - wantOut: []string{}, - wantErr: true, - errMsg: "multiple remotes detected [origin upstream]. please specify which repo to use by providing the -R or --repo argument", - }, - { - name: "multi repo detected - single repo given", - tty: false, - opts: &ListOptions{ - HasRepoOverride: true, - Remotes: func() (ghContext.Remotes, error) { - remote := &ghContext.Remote{ - Remote: &git.Remote{ - Name: "origin", - }, - Repo: ghrepo.New("owner", "repo"), - } - remote2 := &ghContext.Remote{ - Remote: &git.Remote{ - Name: "upstream", - }, - Repo: ghrepo.New("owner", "repo"), - } - - return ghContext.Remotes{ - remote, - remote2, - }, nil - }, - }, - wantOut: []string{ - "SECRET_ONE\t1988-10-11T00:00:00Z", - "SECRET_TWO\t2020-12-04T00:00:00Z", - "SECRET_THREE\t1975-11-30T00:00:00Z", - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - reg := &httpmock.Registry{} - reg.Verify(t) - - path := "repos/owner/repo/actions/secrets" - - t0, _ := time.Parse("2006-01-02", "1988-10-11") - t1, _ := time.Parse("2006-01-02", "2020-12-04") - t2, _ := time.Parse("2006-01-02", "1975-11-30") - payload := struct { - Secrets []Secret - }{ - Secrets: []Secret{ - { - Name: "SECRET_ONE", - UpdatedAt: t0, - }, - { - Name: "SECRET_TWO", - UpdatedAt: t1, - }, - { - Name: "SECRET_THREE", - UpdatedAt: t2, - }, - }, - } - - reg.Register(httpmock.REST("GET", path), httpmock.JSONResponse(payload)) - - ios, _, stdout, _ := iostreams.Test() - - ios.SetStdoutTTY(tt.tty) - - tt.opts.IO = ios - tt.opts.BaseRepo = func() (ghrepo.Interface, error) { - return ghrepo.FromFullName("owner/repo") - } - tt.opts.HttpClient = func() (*http.Client, error) { - return &http.Client{Transport: reg}, nil - } - tt.opts.Config = func() (gh.Config, error) { - return config.NewBlankConfig(), nil - } - tt.opts.Now = func() time.Time { - t, _ := time.Parse(time.RFC822, "15 Mar 23 00:00 UTC") - return t - } - - if tt.json { - exporter := cmdutil.NewJSONExporter() - exporter.SetFields(secretFields) - tt.opts.Exporter = exporter - } - - err := listRun(tt.opts) - if tt.wantErr { - assert.EqualError(t, err, tt.errMsg) - - return - } - - assert.NoError(t, err) - - if len(tt.wantOut) > 1 { - expected := fmt.Sprintf("%s\n", strings.Join(tt.wantOut, "\n")) - assert.Equal(t, expected, stdout.String()) - } - }) - } -} - // Test_listRun_populatesNumSelectedReposIfRequired asserts that NumSelectedRepos // field is populated **only** when it's going to be presented in the output. Since // populating this field costs further API requests (one per secret), it's important diff --git a/pkg/cmd/secret/set/set.go b/pkg/cmd/secret/set/set.go index b4630e067..ba9b023a6 100644 --- a/pkg/cmd/secret/set/set.go +++ b/pkg/cmd/secret/set/set.go @@ -13,7 +13,6 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" - ghContext "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/secret/shared" @@ -30,7 +29,6 @@ type SetOptions struct { IO *iostreams.IOStreams Config func() (gh.Config, error) BaseRepo func() (ghrepo.Interface, error) - Remotes func() (ghContext.Remotes, error) Prompter prompter.Prompter RandomOverride func() io.Reader @@ -45,9 +43,6 @@ type SetOptions struct { RepositoryNames []string EnvFile string Application string - - HasRepoOverride bool - CanPrompt bool } func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command { @@ -55,7 +50,6 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command IO: f.IOStreams, Config: f.Config, HttpClient: f.HttpClient, - Remotes: f.Remotes, Prompter: f.Prompter, } @@ -110,8 +104,19 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - // support `-R, --repo` override + // If the user specified a repo directly, then we're using the OverrideBaseRepoFunc set by EnableRepoOverride + // So there's no reason to use the specialised BaseRepoFunc that requires remote disambiguation. opts.BaseRepo = f.BaseRepo + if !cmd.Flags().Changed("repo") { + // If they haven't specified a repo directly, then we will wrap the BaseRepoFunc in one that error if + // there might be multiple valid remotes. + opts.BaseRepo = shared.RequireNoAmbiguityBaseRepoFunc(opts.BaseRepo, f.Remotes) + // But if we are able to prompt, then we will wrap that up in a BaseRepoFunc that can prompt the user to + // resolve the ambiguity. + if opts.IO.CanPrompt() { + opts.BaseRepo = shared.PromptWhenMultipleRemotesBaseRepoFunc(opts.BaseRepo, f.Prompter) + } + } if err := cmdutil.MutuallyExclusive("specify only one of `--org`, `--env`, or `--user`", opts.OrgName != "", opts.EnvName != "", opts.UserSecrets); err != nil { return err @@ -151,9 +156,6 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command } } - opts.HasRepoOverride = cmd.Flags().Changed("repo") - opts.CanPrompt = opts.IO.CanPrompt() - if runF != nil { return runF(opts) } @@ -198,19 +200,6 @@ func setRun(opts *SetOptions) error { return err } - if err = shared.ValidateHasOnlyOneRemote(opts.HasRepoOverride, opts.Remotes); err != nil { - if !opts.CanPrompt { - return err - } - - selectedRepo, errSelectingRepo := shared.PromptForRepo(baseRepo, opts.Remotes, opts.Prompter) - if errSelectingRepo != nil { - return errSelectingRepo - } - - baseRepo = selectedRepo - } - host = baseRepo.RepoHost() } else { cfg, err := opts.Config() diff --git a/pkg/cmd/secret/set/set_test.go b/pkg/cmd/secret/set/set_test.go index 68c7624dd..63834521f 100644 --- a/pkg/cmd/secret/set/set_test.go +++ b/pkg/cmd/secret/set/set_test.go @@ -22,6 +22,7 @@ import ( "github.com/cli/cli/v2/pkg/iostreams" "github.com/google/shlex" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNewCmdSet(t *testing.T) { @@ -223,6 +224,108 @@ func TestNewCmdSet(t *testing.T) { } } +func TestNewCmdSetBaseRepoFuncs(t *testing.T) { + remotes := ghContext.Remotes{ + &ghContext.Remote{ + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "fork"), + }, + &ghContext.Remote{ + Remote: &git.Remote{ + Name: "upstream", + }, + Repo: ghrepo.New("owner", "repo"), + }, + } + + tests := []struct { + name string + args string + prompterStubs func(*prompter.MockPrompter) + wantRepo ghrepo.Interface + wantErr error + }{ + { + name: "when there is a repo flag provided, the factory base repo func is used", + args: "SECRET_NAME --repo owner/repo", + wantRepo: ghrepo.New("owner", "repo"), + }, + { + name: "when there is no repo flag provided, and no prompting, the base func requiring no ambiguity is used", + args: "SECRET_NAME", + wantErr: shared.MultipleRemotesError{ + Remotes: remotes, + }, + }, + { + name: "when there is no repo flag provided, and can prompt, the base func resolving ambiguity is used", + args: "SECRET_NAME", + prompterStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect( + "Select a base repo", + []string{"owner/fork", "owner/repo"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "owner/fork") + }, + ) + }, + wantRepo: ghrepo.New("owner", "fork"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + var pm *prompter.MockPrompter + if tt.prompterStubs != nil { + ios.SetStdinTTY(true) + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) + pm = prompter.NewMockPrompter(t) + tt.prompterStubs(pm) + } + + f := &cmdutil.Factory{ + IOStreams: ios, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.FromFullName("owner/repo") + }, + Prompter: pm, + Remotes: func() (ghContext.Remotes, error) { + return remotes, nil + }, + } + + argv, err := shlex.Split(tt.args) + assert.NoError(t, err) + + var gotOpts *SetOptions + cmd := NewCmdSet(f, func(opts *SetOptions) error { + gotOpts = opts + return nil + }) + // Require to support --repo flag + cmdutil.EnableRepoOverride(cmd, f) + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + _, err = cmd.ExecuteC() + require.NoError(t, err) + + baseRepo, err := gotOpts.BaseRepo() + if tt.wantErr != nil { + require.Equal(t, tt.wantErr, err) + return + } + require.True(t, ghrepo.IsSame(tt.wantRepo, baseRepo)) + }) + } +} + func Test_setRun_repo(t *testing.T) { tests := []struct { name string @@ -702,143 +805,6 @@ func Test_getSecretsFromOptions(t *testing.T) { } } -func Test_setRun_remote_validation(t *testing.T) { - tests := []struct { - name string - opts *SetOptions - wantApp string - wantErr bool - errMsg string - }{ - { - name: "single repo detected", - opts: &SetOptions{ - Application: "actions", - Remotes: func() (ghContext.Remotes, error) { - remote := &ghContext.Remote{ - Remote: &git.Remote{ - Name: "origin", - }, - Repo: ghrepo.New("owner", "repo"), - } - - return ghContext.Remotes{ - remote, - }, nil - }, - }, - wantApp: "actions", - }, - { - name: "multi repo detected", - opts: &SetOptions{ - Application: "actions", - Remotes: func() (ghContext.Remotes, error) { - remote := &ghContext.Remote{ - Remote: &git.Remote{ - Name: "origin", - }, - Repo: ghrepo.New("owner", "repo"), - } - remote2 := &ghContext.Remote{ - Remote: &git.Remote{ - Name: "upstream", - }, - Repo: ghrepo.New("owner", "repo"), - } - - return ghContext.Remotes{ - remote, - remote2, - }, nil - }, - }, - wantErr: true, - errMsg: "multiple remotes detected [origin upstream]. please specify which repo to use by providing the -R or --repo argument", - }, - { - name: "multi repo detected - single repo given", - opts: &SetOptions{ - Application: "actions", - HasRepoOverride: true, - Remotes: func() (ghContext.Remotes, error) { - remote := &ghContext.Remote{ - Remote: &git.Remote{ - Name: "origin", - }, - Repo: ghrepo.New("owner", "repo"), - } - remote2 := &ghContext.Remote{ - Remote: &git.Remote{ - Name: "upstream", - }, - Repo: ghrepo.New("owner", "repo"), - } - - return ghContext.Remotes{ - remote, - remote2, - }, nil - }, - }, - wantApp: "actions", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - reg := &httpmock.Registry{} - - if tt.wantApp != "" { - reg.Register(httpmock.REST("GET", fmt.Sprintf("repos/owner/repo/%s/secrets/public-key", tt.wantApp)), - httpmock.JSONResponse(PubKey{ID: "123", Key: "CDjXqf7AJBXWhMczcy+Fs7JlACEptgceysutztHaFQI="})) - - reg.Register(httpmock.REST("PUT", fmt.Sprintf("repos/owner/repo/%s/secrets/cool_secret", tt.wantApp)), - httpmock.StatusStringResponse(201, `{}`)) - } - - ios, _, _, _ := iostreams.Test() - - opts := &SetOptions{ - HttpClient: func() (*http.Client, error) { - return &http.Client{Transport: reg}, nil - }, - Config: func() (gh.Config, error) { return config.NewBlankConfig(), nil }, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.FromFullName("owner/repo") - }, - IO: ios, - SecretName: "cool_secret", - Body: "a secret", - RandomOverride: fakeRandom, - Application: tt.opts.Application, - HasRepoOverride: tt.opts.HasRepoOverride, - Remotes: tt.opts.Remotes, - } - - err := setRun(opts) - if tt.wantErr { - assert.EqualError(t, err, tt.errMsg) - } else { - assert.NoError(t, err) - } - - reg.Verify(t) - - if tt.wantApp != "" && !tt.wantErr { - data, err := io.ReadAll(reg.Requests[1].Body) - assert.NoError(t, err) - - var payload SecretPayload - err = json.Unmarshal(data, &payload) - assert.NoError(t, err) - assert.Equal(t, payload.KeyID, "123") - assert.Equal(t, payload.EncryptedValue, "UKYUCbHd0DJemxa3AOcZ6XcsBwALG9d4bpB8ZT0gSV39vl3BHiGSgj8zJapDxgB2BwqNqRhpjC4=") - } - }) - } -} - func fakeRandom() io.Reader { return bytes.NewReader(bytes.Repeat([]byte{5}, 32)) } diff --git a/pkg/cmd/secret/shared/base_repo.go b/pkg/cmd/secret/shared/base_repo.go index db2dfc8f1..3420f6da1 100644 --- a/pkg/cmd/secret/shared/base_repo.go +++ b/pkg/cmd/secret/shared/base_repo.go @@ -1,47 +1,80 @@ package shared import ( - "fmt" + "errors" ghContext "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/prompter" ) -func ValidateHasOnlyOneRemote(hasRepoOverride bool, remotes func() (ghContext.Remotes, error)) error { - if !hasRepoOverride && remotes != nil { +type MultipleRemotesError struct { + Remotes ghContext.Remotes +} + +func (e MultipleRemotesError) Error() string { + return "multiple remotes detected. please specify which repo to use by providing the -R or --repo argument" +} + +type baseRepoFn func() (ghrepo.Interface, error) +type remotesFn func() (ghContext.Remotes, error) + +func PromptWhenMultipleRemotesBaseRepoFunc(baseRepoFn baseRepoFn, prompter prompter.Prompter) baseRepoFn { + return func() (ghrepo.Interface, error) { + baseRepo, err := baseRepoFn() + if err != nil { + var multipleRemotesError MultipleRemotesError + if !errors.As(err, &multipleRemotesError) { + return nil, err + } + + // prompt for the base repo + baseRepo, err = promptForRepo(baseRepo, multipleRemotesError.Remotes, prompter) + if err != nil { + return nil, err + } + } + + return baseRepo, nil + } +} + +// RequireNoAmbiguityBaseRepoFunc returns a function to resolve the base repo, ensuring that +// there was only one remote. +func RequireNoAmbiguityBaseRepoFunc(baseRepo baseRepoFn, remotes remotesFn) baseRepoFn { + return func() (ghrepo.Interface, error) { + // TODO: Is this really correct? Some remotes may not be in the same network. We probably need to resolve the + // network rather than looking at the remotes? remotes, err := remotes() if err != nil { - return err + return nil, err } if remotes.Len() > 1 { - return fmt.Errorf("multiple remotes detected %v. please specify which repo to use by providing the -R or --repo argument", remotes) + return nil, MultipleRemotesError{Remotes: remotes} } - } - return nil + return baseRepo() + } } -func PromptForRepo(baseRepo ghrepo.Interface, remotes func() (ghContext.Remotes, error), survey prompter.Prompter) (ghrepo.Interface, error) { +func promptForRepo(baseRepo ghrepo.Interface, remotes ghContext.Remotes, prompter prompter.Prompter) (ghrepo.Interface, error) { var defaultRepo string var remoteArray []string - if remotes, _ := remotes(); remotes != nil { - if defaultRemote, _ := remotes.ResolvedRemote(); defaultRemote != nil { - // this is a remote explicitly chosen via `repo set-default` - defaultRepo = ghrepo.FullName(defaultRemote) - } else if len(remotes) > 0 { - // as a fallback, just pick the first remote - defaultRepo = ghrepo.FullName(remotes[0]) - } - - for _, remote := range remotes { - remoteArray = append(remoteArray, ghrepo.FullName(remote)) - } + if defaultRemote, _ := remotes.ResolvedRemote(); defaultRemote != nil { + // this is a remote explicitly chosen via `repo set-default` + defaultRepo = ghrepo.FullName(defaultRemote) + } else if len(remotes) > 0 { + // as a fallback, just pick the first remote + defaultRepo = ghrepo.FullName(remotes[0]) } - baseRepoInput, errInput := survey.Select("Select a base repo", defaultRepo, remoteArray) + for _, remote := range remotes { + remoteArray = append(remoteArray, ghrepo.FullName(remote)) + } + + baseRepoInput, errInput := prompter.Select("Select a base repo", defaultRepo, remoteArray) if errInput != nil { return baseRepo, errInput } diff --git a/pkg/cmd/secret/shared/base_repo_test.go b/pkg/cmd/secret/shared/base_repo_test.go new file mode 100644 index 000000000..64b2a3d55 --- /dev/null +++ b/pkg/cmd/secret/shared/base_repo_test.go @@ -0,0 +1,239 @@ +package shared_test + +import ( + "errors" + "testing" + + ghContext "github.com/cli/cli/v2/context" + "github.com/cli/cli/v2/pkg/cmd/secret/shared" + "github.com/stretchr/testify/require" + + "github.com/cli/cli/v2/git" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" +) + +func TestRequireNoAmbiguityBaseRepoFunc(t *testing.T) { + t.Parallel() + + t.Run("succeeds when there is only one remote", func(t *testing.T) { + t.Parallel() + + // Given there is only one remote + baseRepoFn := shared.RequireNoAmbiguityBaseRepoFunc(baseRepoStubFn, oneRemoteStubFn) + + // When fetching the base repo + baseRepo, err := baseRepoFn() + + // It succeeds and returns the inner base repo + require.NoError(t, err) + require.True(t, ghrepo.IsSame(ghrepo.New("owner", "repo"), baseRepo)) + }) + + t.Run("returns specific error when there are multiple remotes", func(t *testing.T) { + t.Parallel() + + // Given there are multiple remotes + baseRepoFn := shared.RequireNoAmbiguityBaseRepoFunc(baseRepoStubFn, twoRemotesStubFn) + + // When fetching the base repo + _, err := baseRepoFn() + + // It succeeds and returns the inner base repo + var multipleRemotesError shared.MultipleRemotesError + require.ErrorAs(t, err, &multipleRemotesError) + require.Equal(t, ghContext.Remotes{ + { + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "fork"), + }, + { + Remote: &git.Remote{ + Name: "upstream", + }, + Repo: ghrepo.New("owner", "repo"), + }, + }, multipleRemotesError.Remotes) + }) + + t.Run("when the remote fetching function fails, it returns the error", func(t *testing.T) { + t.Parallel() + + // Given the remote fetching function fails + baseRepoFn := shared.RequireNoAmbiguityBaseRepoFunc(baseRepoStubFn, errRemoteStubFn) + + // When fetching the base repo + _, err := baseRepoFn() + + // It returns the error + require.Equal(t, errors.New("test remote error"), err) + }) + + t.Run("when the wrapped base repo function fails, it returns the error", func(t *testing.T) { + t.Parallel() + + // Given the wrapped base repo function fails + baseRepoFn := shared.RequireNoAmbiguityBaseRepoFunc(errBaseRepoStubFn, oneRemoteStubFn) + + // When fetching the base repo + _, err := baseRepoFn() + + // It returns the error + require.Equal(t, errors.New("test base repo error"), err) + }) +} + +func TestPromptWhenMultipleRemotesBaseRepoFunc(t *testing.T) { + t.Parallel() + + t.Run("when there is no error from wrapped base repo func, then it succeeds without prompting", func(t *testing.T) { + t.Parallel() + + // Given the base repo function succeeds + baseRepoFn := shared.PromptWhenMultipleRemotesBaseRepoFunc(baseRepoStubFn, nil) + + // When fetching the base repo + baseRepo, err := baseRepoFn() + + // It succeeds and returns the inner base repo + require.NoError(t, err) + require.True(t, ghrepo.IsSame(ghrepo.New("owner", "repo"), baseRepo)) + }) + + t.Run("when the wrapped base repo func returns a specific error, then the prompter is used for disambiguation, with the resolved remote as the default", func(t *testing.T) { + t.Parallel() + + pm := prompter.NewMockPrompter(t) + pm.RegisterSelect( + "Select a base repo", + []string{"owner/fork", "owner/repo"}, + func(_, def string, opts []string) (int, error) { + t.Helper() + require.Equal(t, "owner/repo", def) + return prompter.IndexFor(opts, "owner/repo") + }, + ) + + // Given the wrapped base repo func returns a specific error + baseRepoFn := shared.PromptWhenMultipleRemotesBaseRepoFunc(errMultipleRemotesStubFn, pm) + + // When fetching the base repo + baseRepo, err := baseRepoFn() + + // It uses the prompter for disambiguation + require.NoError(t, err) + require.True(t, ghrepo.IsSame(ghrepo.New("owner", "repo"), baseRepo)) + }) + + t.Run("when the prompter returns an error, then it is returned", func(t *testing.T) { + t.Parallel() + + // Given the prompter returns an error + pm := prompter.NewMockPrompter(t) + pm.RegisterSelect( + "Select a base repo", + []string{"owner/fork", "owner/repo"}, + func(_, _ string, opts []string) (int, error) { + return 0, errors.New("test prompt error") + }, + ) + + // Given the wrapped base repo func returns a specific error + baseRepoFn := shared.PromptWhenMultipleRemotesBaseRepoFunc(errMultipleRemotesStubFn, pm) + + // When fetching the base repo + _, err := baseRepoFn() + + // It returns the error + require.Equal(t, errors.New("test prompt error"), err) + }) + + t.Run("when the wrapped base repo func returns a non-specific error, then it is returned", func(t *testing.T) { + t.Parallel() + + // Given the wrapped base repo func returns a non-specific error + baseRepoFn := shared.PromptWhenMultipleRemotesBaseRepoFunc(errBaseRepoStubFn, nil) + + // When fetching the base repo + _, err := baseRepoFn() + + // It returns the error + require.Equal(t, errors.New("test base repo error"), err) + }) +} + +func TestMultipleRemotesErrorMessage(t *testing.T) { + err := shared.MultipleRemotesError{} + require.EqualError(t, err, "multiple remotes detected. please specify which repo to use by providing the -R or --repo argument") +} + +func errMultipleRemotesStubFn() (ghrepo.Interface, error) { + remote1 := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "fork"), + } + + remote2 := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "upstream", + Resolved: "base", + }, + Repo: ghrepo.New("owner", "repo"), + } + + return nil, shared.MultipleRemotesError{ + Remotes: ghContext.Remotes{ + remote1, + remote2, + }, + } +} + +func baseRepoStubFn() (ghrepo.Interface, error) { + return ghrepo.New("owner", "repo"), nil +} + +func oneRemoteStubFn() (ghContext.Remotes, error) { + remote := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "repo"), + } + + return ghContext.Remotes{ + remote, + }, nil +} + +func twoRemotesStubFn() (ghContext.Remotes, error) { + remote1 := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "fork"), + } + + remote2 := &ghContext.Remote{ + Remote: &git.Remote{ + Name: "upstream", + }, + Repo: ghrepo.New("owner", "repo"), + } + return ghContext.Remotes{ + remote1, + remote2, + }, nil +} + +func errRemoteStubFn() (ghContext.Remotes, error) { + return nil, errors.New("test remote error") +} + +func errBaseRepoStubFn() (ghrepo.Interface, error) { + return nil, errors.New("test base repo error") +} diff --git a/pkg/cmdutil/flags.go b/pkg/cmdutil/flags.go index fadbc5fbe..c0064099c 100644 --- a/pkg/cmdutil/flags.go +++ b/pkg/cmdutil/flags.go @@ -180,11 +180,3 @@ func isIncluded(value string, opts []string) bool { } return false } - -func CountSetFlags(flags *pflag.FlagSet) int { - count := 0 - flags.Visit(func(f *pflag.Flag) { - count++ - }) - return count -} From 4da4c82090d82c40ce13b43e0941d59e16181e04 Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 20 Dec 2024 14:33:47 +0100 Subject: [PATCH 21/45] Rename secret BaseRepo func --- pkg/cmd/factory/remote_resolver.go | 1 + pkg/cmd/secret/delete/delete.go | 2 +- pkg/cmd/secret/delete/delete_test.go | 2 +- pkg/cmd/secret/list/list.go | 2 +- pkg/cmd/secret/list/list_test.go | 2 +- pkg/cmd/secret/set/set.go | 2 +- pkg/cmd/secret/set/set_test.go | 2 +- pkg/cmd/secret/shared/base_repo.go | 17 ++++++++--------- pkg/cmd/secret/shared/base_repo_test.go | 16 ++++++++-------- 9 files changed, 23 insertions(+), 23 deletions(-) diff --git a/pkg/cmd/factory/remote_resolver.go b/pkg/cmd/factory/remote_resolver.go index 7e27834e2..b008ade7e 100644 --- a/pkg/cmd/factory/remote_resolver.go +++ b/pkg/cmd/factory/remote_resolver.go @@ -69,6 +69,7 @@ func (rr *remoteResolver) Resolver() func() (context.Remotes, error) { sort.Sort(resolvedRemotes) // Filter remotes by hosts + // Note that this is not caching correctly: https://github.com/cli/cli/issues/10103 cachedRemotes := resolvedRemotes.FilterByHosts(hosts) // Filter again by default host if one is set diff --git a/pkg/cmd/secret/delete/delete.go b/pkg/cmd/secret/delete/delete.go index 64b02c9d9..87b96bee1 100644 --- a/pkg/cmd/secret/delete/delete.go +++ b/pkg/cmd/secret/delete/delete.go @@ -56,7 +56,7 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co // But if we are able to prompt, then we will wrap that up in a BaseRepoFunc that can prompt the user to // resolve the ambiguity. if opts.IO.CanPrompt() { - opts.BaseRepo = shared.PromptWhenMultipleRemotesBaseRepoFunc(opts.BaseRepo, f.Prompter) + opts.BaseRepo = shared.PromptWhenAmbiguousBaseRepoFunc(opts.BaseRepo, f.Prompter) } } diff --git a/pkg/cmd/secret/delete/delete_test.go b/pkg/cmd/secret/delete/delete_test.go index 91d688a89..f1f19a591 100644 --- a/pkg/cmd/secret/delete/delete_test.go +++ b/pkg/cmd/secret/delete/delete_test.go @@ -156,7 +156,7 @@ func TestNewCmdDeleteBaseRepoFuncs(t *testing.T) { { name: "when there is no repo flag provided, and no prompting, the base func requiring no ambiguity is used", args: "SECRET_NAME", - wantErr: shared.MultipleRemotesError{ + wantErr: shared.AmbiguousBaseRepoError{ Remotes: remotes, }, }, diff --git a/pkg/cmd/secret/list/list.go b/pkg/cmd/secret/list/list.go index 49ad47aff..4da9bc569 100644 --- a/pkg/cmd/secret/list/list.go +++ b/pkg/cmd/secret/list/list.go @@ -77,7 +77,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman // But if we are able to prompt, then we will wrap that up in a BaseRepoFunc that can prompt the user to // resolve the ambiguity. if opts.IO.CanPrompt() { - opts.BaseRepo = shared.PromptWhenMultipleRemotesBaseRepoFunc(opts.BaseRepo, f.Prompter) + opts.BaseRepo = shared.PromptWhenAmbiguousBaseRepoFunc(opts.BaseRepo, f.Prompter) } } diff --git a/pkg/cmd/secret/list/list_test.go b/pkg/cmd/secret/list/list_test.go index 63a899b77..729984deb 100644 --- a/pkg/cmd/secret/list/list_test.go +++ b/pkg/cmd/secret/list/list_test.go @@ -136,7 +136,7 @@ func TestNewCmdListBaseRepoFuncs(t *testing.T) { { name: "when there is no repo flag provided, and no prompting, the base func requiring no ambiguity is used", args: "", - wantErr: shared.MultipleRemotesError{ + wantErr: shared.AmbiguousBaseRepoError{ Remotes: remotes, }, }, diff --git a/pkg/cmd/secret/set/set.go b/pkg/cmd/secret/set/set.go index ba9b023a6..87deee71c 100644 --- a/pkg/cmd/secret/set/set.go +++ b/pkg/cmd/secret/set/set.go @@ -114,7 +114,7 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command // But if we are able to prompt, then we will wrap that up in a BaseRepoFunc that can prompt the user to // resolve the ambiguity. if opts.IO.CanPrompt() { - opts.BaseRepo = shared.PromptWhenMultipleRemotesBaseRepoFunc(opts.BaseRepo, f.Prompter) + opts.BaseRepo = shared.PromptWhenAmbiguousBaseRepoFunc(opts.BaseRepo, f.Prompter) } } diff --git a/pkg/cmd/secret/set/set_test.go b/pkg/cmd/secret/set/set_test.go index 63834521f..dd3124e82 100644 --- a/pkg/cmd/secret/set/set_test.go +++ b/pkg/cmd/secret/set/set_test.go @@ -255,7 +255,7 @@ func TestNewCmdSetBaseRepoFuncs(t *testing.T) { { name: "when there is no repo flag provided, and no prompting, the base func requiring no ambiguity is used", args: "SECRET_NAME", - wantErr: shared.MultipleRemotesError{ + wantErr: shared.AmbiguousBaseRepoError{ Remotes: remotes, }, }, diff --git a/pkg/cmd/secret/shared/base_repo.go b/pkg/cmd/secret/shared/base_repo.go index 3420f6da1..39a59149e 100644 --- a/pkg/cmd/secret/shared/base_repo.go +++ b/pkg/cmd/secret/shared/base_repo.go @@ -8,28 +8,27 @@ import ( "github.com/cli/cli/v2/internal/prompter" ) -type MultipleRemotesError struct { +type AmbiguousBaseRepoError struct { Remotes ghContext.Remotes } -func (e MultipleRemotesError) Error() string { +func (e AmbiguousBaseRepoError) Error() string { return "multiple remotes detected. please specify which repo to use by providing the -R or --repo argument" } type baseRepoFn func() (ghrepo.Interface, error) type remotesFn func() (ghContext.Remotes, error) -func PromptWhenMultipleRemotesBaseRepoFunc(baseRepoFn baseRepoFn, prompter prompter.Prompter) baseRepoFn { +func PromptWhenAmbiguousBaseRepoFunc(baseRepoFn baseRepoFn, prompter prompter.Prompter) baseRepoFn { return func() (ghrepo.Interface, error) { baseRepo, err := baseRepoFn() if err != nil { - var multipleRemotesError MultipleRemotesError - if !errors.As(err, &multipleRemotesError) { + var ambiguousBaseRepoErr AmbiguousBaseRepoError + if !errors.As(err, &ambiguousBaseRepoErr) { return nil, err } - // prompt for the base repo - baseRepo, err = promptForRepo(baseRepo, multipleRemotesError.Remotes, prompter) + baseRepo, err = promptForRepo(baseRepo, ambiguousBaseRepoErr.Remotes, prompter) if err != nil { return nil, err } @@ -40,7 +39,7 @@ func PromptWhenMultipleRemotesBaseRepoFunc(baseRepoFn baseRepoFn, prompter promp } // RequireNoAmbiguityBaseRepoFunc returns a function to resolve the base repo, ensuring that -// there was only one remote. +// there was only one option, regardless of whether the base repo had been set. func RequireNoAmbiguityBaseRepoFunc(baseRepo baseRepoFn, remotes remotesFn) baseRepoFn { return func() (ghrepo.Interface, error) { // TODO: Is this really correct? Some remotes may not be in the same network. We probably need to resolve the @@ -51,7 +50,7 @@ func RequireNoAmbiguityBaseRepoFunc(baseRepo baseRepoFn, remotes remotesFn) base } if remotes.Len() > 1 { - return nil, MultipleRemotesError{Remotes: remotes} + return nil, AmbiguousBaseRepoError{Remotes: remotes} } return baseRepo() diff --git a/pkg/cmd/secret/shared/base_repo_test.go b/pkg/cmd/secret/shared/base_repo_test.go index 64b2a3d55..469a9722c 100644 --- a/pkg/cmd/secret/shared/base_repo_test.go +++ b/pkg/cmd/secret/shared/base_repo_test.go @@ -40,7 +40,7 @@ func TestRequireNoAmbiguityBaseRepoFunc(t *testing.T) { _, err := baseRepoFn() // It succeeds and returns the inner base repo - var multipleRemotesError shared.MultipleRemotesError + var multipleRemotesError shared.AmbiguousBaseRepoError require.ErrorAs(t, err, &multipleRemotesError) require.Equal(t, ghContext.Remotes{ { @@ -92,7 +92,7 @@ func TestPromptWhenMultipleRemotesBaseRepoFunc(t *testing.T) { t.Parallel() // Given the base repo function succeeds - baseRepoFn := shared.PromptWhenMultipleRemotesBaseRepoFunc(baseRepoStubFn, nil) + baseRepoFn := shared.PromptWhenAmbiguousBaseRepoFunc(baseRepoStubFn, nil) // When fetching the base repo baseRepo, err := baseRepoFn() @@ -117,7 +117,7 @@ func TestPromptWhenMultipleRemotesBaseRepoFunc(t *testing.T) { ) // Given the wrapped base repo func returns a specific error - baseRepoFn := shared.PromptWhenMultipleRemotesBaseRepoFunc(errMultipleRemotesStubFn, pm) + baseRepoFn := shared.PromptWhenAmbiguousBaseRepoFunc(errMultipleRemotesStubFn, pm) // When fetching the base repo baseRepo, err := baseRepoFn() @@ -141,7 +141,7 @@ func TestPromptWhenMultipleRemotesBaseRepoFunc(t *testing.T) { ) // Given the wrapped base repo func returns a specific error - baseRepoFn := shared.PromptWhenMultipleRemotesBaseRepoFunc(errMultipleRemotesStubFn, pm) + baseRepoFn := shared.PromptWhenAmbiguousBaseRepoFunc(errMultipleRemotesStubFn, pm) // When fetching the base repo _, err := baseRepoFn() @@ -154,7 +154,7 @@ func TestPromptWhenMultipleRemotesBaseRepoFunc(t *testing.T) { t.Parallel() // Given the wrapped base repo func returns a non-specific error - baseRepoFn := shared.PromptWhenMultipleRemotesBaseRepoFunc(errBaseRepoStubFn, nil) + baseRepoFn := shared.PromptWhenAmbiguousBaseRepoFunc(errBaseRepoStubFn, nil) // When fetching the base repo _, err := baseRepoFn() @@ -165,8 +165,8 @@ func TestPromptWhenMultipleRemotesBaseRepoFunc(t *testing.T) { } func TestMultipleRemotesErrorMessage(t *testing.T) { - err := shared.MultipleRemotesError{} - require.EqualError(t, err, "multiple remotes detected. please specify which repo to use by providing the -R or --repo argument") + err := shared.AmbiguousBaseRepoError{} + require.EqualError(t, err, "multiple remotes detected. please specify which repo to use by providing the -R, --repo argument") } func errMultipleRemotesStubFn() (ghrepo.Interface, error) { @@ -185,7 +185,7 @@ func errMultipleRemotesStubFn() (ghrepo.Interface, error) { Repo: ghrepo.New("owner", "repo"), } - return nil, shared.MultipleRemotesError{ + return nil, shared.AmbiguousBaseRepoError{ Remotes: ghContext.Remotes{ remote1, remote2, From d831e3e1dbb67e81cf1cbc057f1fe6b86028b5af Mon Sep 17 00:00:00 2001 From: William Martin Date: Thu, 9 Jan 2025 17:33:26 +0100 Subject: [PATCH 22/45] Remove validated TODO and add review warning --- pkg/cmd/secret/shared/base_repo.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/secret/shared/base_repo.go b/pkg/cmd/secret/shared/base_repo.go index 39a59149e..652d92064 100644 --- a/pkg/cmd/secret/shared/base_repo.go +++ b/pkg/cmd/secret/shared/base_repo.go @@ -13,7 +13,7 @@ type AmbiguousBaseRepoError struct { } func (e AmbiguousBaseRepoError) Error() string { - return "multiple remotes detected. please specify which repo to use by providing the -R or --repo argument" + return "multiple remotes detected. please specify which repo to use by providing the -R, --repo argument" } type baseRepoFn func() (ghrepo.Interface, error) @@ -42,8 +42,6 @@ func PromptWhenAmbiguousBaseRepoFunc(baseRepoFn baseRepoFn, prompter prompter.Pr // there was only one option, regardless of whether the base repo had been set. func RequireNoAmbiguityBaseRepoFunc(baseRepo baseRepoFn, remotes remotesFn) baseRepoFn { return func() (ghrepo.Interface, error) { - // TODO: Is this really correct? Some remotes may not be in the same network. We probably need to resolve the - // network rather than looking at the remotes? remotes, err := remotes() if err != nil { return nil, err @@ -57,10 +55,13 @@ func RequireNoAmbiguityBaseRepoFunc(baseRepo baseRepoFn, remotes remotesFn) base } } +// REVIEW WARNING: I have not had a close look at this function yet, I do not vouch for it. func promptForRepo(baseRepo ghrepo.Interface, remotes ghContext.Remotes, prompter prompter.Prompter) (ghrepo.Interface, error) { var defaultRepo string var remoteArray []string + // TODO: consider whether we should just go with the default order of remotes because then + // users that are familiar can just hit enter and achieve the behaviour they had before. if defaultRemote, _ := remotes.ResolvedRemote(); defaultRemote != nil { // this is a remote explicitly chosen via `repo set-default` defaultRepo = ghrepo.FullName(defaultRemote) From 5630252f7861cf7b4b6bea82c10842f7e7931302 Mon Sep 17 00:00:00 2001 From: William Martin Date: Wed, 15 Jan 2025 13:22:10 +0100 Subject: [PATCH 23/45] Add acceptance test for secrets remote disambiguation --- ...secret-require-remote-disambiguation.txtar | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 acceptance/testdata/secret/secret-require-remote-disambiguation.txtar diff --git a/acceptance/testdata/secret/secret-require-remote-disambiguation.txtar b/acceptance/testdata/secret/secret-require-remote-disambiguation.txtar new file mode 100644 index 000000000..f4d1bbb4a --- /dev/null +++ b/acceptance/testdata/secret/secret-require-remote-disambiguation.txtar @@ -0,0 +1,36 @@ +# Set up env vars +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create ${ORG}/${REPO} --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes ${ORG}/${REPO} + +# Create a fork +exec gh repo fork ${ORG}/${REPO} --org ${ORG} --fork-name ${REPO}-fork + +# Defer fork cleanup +defer gh repo delete --yes ${ORG}/${REPO}-fork + +# Sleep to allow the fork to be created before cloning +sleep 2 + +# Clone and move into the fork repo +exec gh repo clone ${ORG}/${REPO}-fork +cd ${REPO}-fork + +# Secret list requires disambiguation +! exec gh secret list +stderr 'multiple remotes detected. please specify which repo to use by providing the -R, --repo argument' + +# Secret set requires disambiguation +! exec gh secret set 'TEST_SECRET_NAME' --body 'TEST_SECRET_VALUE' +stderr 'multiple remotes detected. please specify which repo to use by providing the -R, --repo argument' + +# Secret delete requires disambiguation +! exec gh secret delete 'TEST_SECRET_NAME' +stderr 'multiple remotes detected. please specify which repo to use by providing the -R, --repo argument' From ce47fabc27b1fbb1ffb275c33ce4097233dd0ddb Mon Sep 17 00:00:00 2001 From: William Martin Date: Wed, 15 Jan 2025 13:55:36 +0100 Subject: [PATCH 24/45] Move secret base repo prompting earlier --- pkg/cmd/secret/delete/delete.go | 16 ++++++++-------- pkg/cmd/secret/set/set.go | 23 ++++++++++++----------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/secret/delete/delete.go b/pkg/cmd/secret/delete/delete.go index 87b96bee1..8f17fdf59 100644 --- a/pkg/cmd/secret/delete/delete.go +++ b/pkg/cmd/secret/delete/delete.go @@ -99,6 +99,14 @@ func removeRun(opts *DeleteOptions) error { return err } + var baseRepo ghrepo.Interface + if secretEntity == shared.Repository || secretEntity == shared.Environment { + baseRepo, err = opts.BaseRepo() + if err != nil { + return err + } + } + secretApp, err := shared.GetSecretApp(opts.Application, secretEntity) if err != nil { return err @@ -108,14 +116,6 @@ func removeRun(opts *DeleteOptions) error { return fmt.Errorf("%s secrets are not supported for %s", secretEntity, secretApp) } - var baseRepo ghrepo.Interface - if secretEntity == shared.Repository || secretEntity == shared.Environment { - baseRepo, err = opts.BaseRepo() - if err != nil { - return err - } - } - cfg, err := opts.Config() if err != nil { return err diff --git a/pkg/cmd/secret/set/set.go b/pkg/cmd/secret/set/set.go index 87deee71c..a35c40aa3 100644 --- a/pkg/cmd/secret/set/set.go +++ b/pkg/cmd/secret/set/set.go @@ -178,23 +178,13 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command } func setRun(opts *SetOptions) error { - secrets, err := getSecretsFromOptions(opts) - if err != nil { - return err - } - - c, err := opts.HttpClient() - if err != nil { - return fmt.Errorf("could not create http client: %w", err) - } - client := api.NewClientFromHTTP(c) - orgName := opts.OrgName envName := opts.EnvName var host string var baseRepo ghrepo.Interface if orgName == "" && !opts.UserSecrets { + var err error baseRepo, err = opts.BaseRepo() if err != nil { return err @@ -209,6 +199,17 @@ func setRun(opts *SetOptions) error { host, _ = cfg.Authentication().DefaultHost() } + secrets, err := getSecretsFromOptions(opts) + if err != nil { + return err + } + + c, err := opts.HttpClient() + if err != nil { + return fmt.Errorf("could not create http client: %w", err) + } + client := api.NewClientFromHTTP(c) + secretEntity, err := shared.GetSecretEntity(orgName, envName, opts.UserSecrets) if err != nil { return err From a47327aee6396df90349e3b61a20da3d11c2ed6c Mon Sep 17 00:00:00 2001 From: William Martin Date: Wed, 15 Jan 2025 14:11:24 +0100 Subject: [PATCH 25/45] Secret base repo prompting should not use resolved remote This is because the secret commands don't use the SmartBaseRepo behaviour, and therefore don't care about the resolved remote. --- pkg/cmd/secret/shared/base_repo.go | 46 +++++++------------------ pkg/cmd/secret/shared/base_repo_test.go | 8 ++--- 2 files changed, 16 insertions(+), 38 deletions(-) diff --git a/pkg/cmd/secret/shared/base_repo.go b/pkg/cmd/secret/shared/base_repo.go index 652d92064..c46752794 100644 --- a/pkg/cmd/secret/shared/base_repo.go +++ b/pkg/cmd/secret/shared/base_repo.go @@ -28,10 +28,22 @@ func PromptWhenAmbiguousBaseRepoFunc(baseRepoFn baseRepoFn, prompter prompter.Pr return nil, err } - baseRepo, err = promptForRepo(baseRepo, ambiguousBaseRepoErr.Remotes, prompter) + baseRepoOptions := make([]string, len(ambiguousBaseRepoErr.Remotes)) + for i, remote := range ambiguousBaseRepoErr.Remotes { + baseRepoOptions[i] = ghrepo.FullName(remote) + } + + selectedBaseRepo, err := prompter.Select("Select a base repo", baseRepoOptions[0], baseRepoOptions) if err != nil { return nil, err } + + selectedRepo, err := ghrepo.FromFullName(baseRepoOptions[selectedBaseRepo]) + if err != nil { + return nil, err + } + + return selectedRepo, nil } return baseRepo, nil @@ -54,35 +66,3 @@ func RequireNoAmbiguityBaseRepoFunc(baseRepo baseRepoFn, remotes remotesFn) base return baseRepo() } } - -// REVIEW WARNING: I have not had a close look at this function yet, I do not vouch for it. -func promptForRepo(baseRepo ghrepo.Interface, remotes ghContext.Remotes, prompter prompter.Prompter) (ghrepo.Interface, error) { - var defaultRepo string - var remoteArray []string - - // TODO: consider whether we should just go with the default order of remotes because then - // users that are familiar can just hit enter and achieve the behaviour they had before. - if defaultRemote, _ := remotes.ResolvedRemote(); defaultRemote != nil { - // this is a remote explicitly chosen via `repo set-default` - defaultRepo = ghrepo.FullName(defaultRemote) - } else if len(remotes) > 0 { - // as a fallback, just pick the first remote - defaultRepo = ghrepo.FullName(remotes[0]) - } - - for _, remote := range remotes { - remoteArray = append(remoteArray, ghrepo.FullName(remote)) - } - - baseRepoInput, errInput := prompter.Select("Select a base repo", defaultRepo, remoteArray) - if errInput != nil { - return baseRepo, errInput - } - - selectedRepo, errSelectedRepo := ghrepo.FromFullName(remoteArray[baseRepoInput]) - if errSelectedRepo != nil { - return baseRepo, errSelectedRepo - } - - return selectedRepo, nil -} diff --git a/pkg/cmd/secret/shared/base_repo_test.go b/pkg/cmd/secret/shared/base_repo_test.go index 469a9722c..d5cdc5e23 100644 --- a/pkg/cmd/secret/shared/base_repo_test.go +++ b/pkg/cmd/secret/shared/base_repo_test.go @@ -102,7 +102,7 @@ func TestPromptWhenMultipleRemotesBaseRepoFunc(t *testing.T) { require.True(t, ghrepo.IsSame(ghrepo.New("owner", "repo"), baseRepo)) }) - t.Run("when the wrapped base repo func returns a specific error, then the prompter is used for disambiguation, with the resolved remote as the default", func(t *testing.T) { + t.Run("when the wrapped base repo func returns a specific error, then the prompter is used for disambiguation, with the remote ordering remaining unchanged", func(t *testing.T) { t.Parallel() pm := prompter.NewMockPrompter(t) @@ -110,8 +110,7 @@ func TestPromptWhenMultipleRemotesBaseRepoFunc(t *testing.T) { "Select a base repo", []string{"owner/fork", "owner/repo"}, func(_, def string, opts []string) (int, error) { - t.Helper() - require.Equal(t, "owner/repo", def) + require.Equal(t, "owner/fork", def) return prompter.IndexFor(opts, "owner/repo") }, ) @@ -179,8 +178,7 @@ func errMultipleRemotesStubFn() (ghrepo.Interface, error) { remote2 := &ghContext.Remote{ Remote: &git.Remote{ - Name: "upstream", - Resolved: "base", + Name: "upstream", }, Repo: ghrepo.New("owner", "repo"), } From b382b2472823a929117c751cbbfdf20e4c232c61 Mon Sep 17 00:00:00 2001 From: William Martin Date: Wed, 15 Jan 2025 14:22:43 +0100 Subject: [PATCH 26/45] Print informative message before prompting for secret repo --- pkg/cmd/secret/delete/delete.go | 2 +- pkg/cmd/secret/list/list.go | 2 +- pkg/cmd/secret/set/set.go | 2 +- pkg/cmd/secret/shared/base_repo.go | 5 ++++- pkg/cmd/secret/shared/base_repo_test.go | 22 +++++++++++++++++----- 5 files changed, 24 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/secret/delete/delete.go b/pkg/cmd/secret/delete/delete.go index 8f17fdf59..78550f928 100644 --- a/pkg/cmd/secret/delete/delete.go +++ b/pkg/cmd/secret/delete/delete.go @@ -56,7 +56,7 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co // But if we are able to prompt, then we will wrap that up in a BaseRepoFunc that can prompt the user to // resolve the ambiguity. if opts.IO.CanPrompt() { - opts.BaseRepo = shared.PromptWhenAmbiguousBaseRepoFunc(opts.BaseRepo, f.Prompter) + opts.BaseRepo = shared.PromptWhenAmbiguousBaseRepoFunc(opts.BaseRepo, f.IOStreams, f.Prompter) } } diff --git a/pkg/cmd/secret/list/list.go b/pkg/cmd/secret/list/list.go index 4da9bc569..41cf88e64 100644 --- a/pkg/cmd/secret/list/list.go +++ b/pkg/cmd/secret/list/list.go @@ -77,7 +77,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman // But if we are able to prompt, then we will wrap that up in a BaseRepoFunc that can prompt the user to // resolve the ambiguity. if opts.IO.CanPrompt() { - opts.BaseRepo = shared.PromptWhenAmbiguousBaseRepoFunc(opts.BaseRepo, f.Prompter) + opts.BaseRepo = shared.PromptWhenAmbiguousBaseRepoFunc(opts.BaseRepo, f.IOStreams, f.Prompter) } } diff --git a/pkg/cmd/secret/set/set.go b/pkg/cmd/secret/set/set.go index a35c40aa3..ba90e1f67 100644 --- a/pkg/cmd/secret/set/set.go +++ b/pkg/cmd/secret/set/set.go @@ -114,7 +114,7 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command // But if we are able to prompt, then we will wrap that up in a BaseRepoFunc that can prompt the user to // resolve the ambiguity. if opts.IO.CanPrompt() { - opts.BaseRepo = shared.PromptWhenAmbiguousBaseRepoFunc(opts.BaseRepo, f.Prompter) + opts.BaseRepo = shared.PromptWhenAmbiguousBaseRepoFunc(opts.BaseRepo, f.IOStreams, f.Prompter) } } diff --git a/pkg/cmd/secret/shared/base_repo.go b/pkg/cmd/secret/shared/base_repo.go index c46752794..d902105aa 100644 --- a/pkg/cmd/secret/shared/base_repo.go +++ b/pkg/cmd/secret/shared/base_repo.go @@ -2,10 +2,12 @@ package shared import ( "errors" + "fmt" ghContext "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/prompter" + "github.com/cli/cli/v2/pkg/iostreams" ) type AmbiguousBaseRepoError struct { @@ -19,7 +21,7 @@ func (e AmbiguousBaseRepoError) Error() string { type baseRepoFn func() (ghrepo.Interface, error) type remotesFn func() (ghContext.Remotes, error) -func PromptWhenAmbiguousBaseRepoFunc(baseRepoFn baseRepoFn, prompter prompter.Prompter) baseRepoFn { +func PromptWhenAmbiguousBaseRepoFunc(baseRepoFn baseRepoFn, ios *iostreams.IOStreams, prompter prompter.Prompter) baseRepoFn { return func() (ghrepo.Interface, error) { baseRepo, err := baseRepoFn() if err != nil { @@ -33,6 +35,7 @@ func PromptWhenAmbiguousBaseRepoFunc(baseRepoFn baseRepoFn, prompter prompter.Pr baseRepoOptions[i] = ghrepo.FullName(remote) } + fmt.Fprintf(ios.Out, "%s Multiple remotes detected. Due to the sensitive nature of secrets, requiring disambiguation.\n", ios.ColorScheme().WarningIcon()) selectedBaseRepo, err := prompter.Select("Select a base repo", baseRepoOptions[0], baseRepoOptions) if err != nil { return nil, err diff --git a/pkg/cmd/secret/shared/base_repo_test.go b/pkg/cmd/secret/shared/base_repo_test.go index d5cdc5e23..6b7aa02ff 100644 --- a/pkg/cmd/secret/shared/base_repo_test.go +++ b/pkg/cmd/secret/shared/base_repo_test.go @@ -6,6 +6,7 @@ import ( ghContext "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/pkg/cmd/secret/shared" + "github.com/cli/cli/v2/pkg/iostreams" "github.com/stretchr/testify/require" "github.com/cli/cli/v2/git" @@ -91,8 +92,10 @@ func TestPromptWhenMultipleRemotesBaseRepoFunc(t *testing.T) { t.Run("when there is no error from wrapped base repo func, then it succeeds without prompting", func(t *testing.T) { t.Parallel() + ios, _, _, _ := iostreams.Test() + // Given the base repo function succeeds - baseRepoFn := shared.PromptWhenAmbiguousBaseRepoFunc(baseRepoStubFn, nil) + baseRepoFn := shared.PromptWhenAmbiguousBaseRepoFunc(baseRepoStubFn, ios, nil) // When fetching the base repo baseRepo, err := baseRepoFn() @@ -105,6 +108,8 @@ func TestPromptWhenMultipleRemotesBaseRepoFunc(t *testing.T) { t.Run("when the wrapped base repo func returns a specific error, then the prompter is used for disambiguation, with the remote ordering remaining unchanged", func(t *testing.T) { t.Parallel() + ios, _, stdout, _ := iostreams.Test() + pm := prompter.NewMockPrompter(t) pm.RegisterSelect( "Select a base repo", @@ -116,12 +121,15 @@ func TestPromptWhenMultipleRemotesBaseRepoFunc(t *testing.T) { ) // Given the wrapped base repo func returns a specific error - baseRepoFn := shared.PromptWhenAmbiguousBaseRepoFunc(errMultipleRemotesStubFn, pm) + baseRepoFn := shared.PromptWhenAmbiguousBaseRepoFunc(errMultipleRemotesStubFn, ios, pm) // When fetching the base repo baseRepo, err := baseRepoFn() - // It uses the prompter for disambiguation + // It prints an informative message + require.Equal(t, "! Multiple remotes detected. Due to the sensitive nature of secrets, requiring disambiguation.\n", stdout.String()) + + // And it uses the prompter for disambiguation require.NoError(t, err) require.True(t, ghrepo.IsSame(ghrepo.New("owner", "repo"), baseRepo)) }) @@ -129,6 +137,8 @@ func TestPromptWhenMultipleRemotesBaseRepoFunc(t *testing.T) { t.Run("when the prompter returns an error, then it is returned", func(t *testing.T) { t.Parallel() + ios, _, _, _ := iostreams.Test() + // Given the prompter returns an error pm := prompter.NewMockPrompter(t) pm.RegisterSelect( @@ -140,7 +150,7 @@ func TestPromptWhenMultipleRemotesBaseRepoFunc(t *testing.T) { ) // Given the wrapped base repo func returns a specific error - baseRepoFn := shared.PromptWhenAmbiguousBaseRepoFunc(errMultipleRemotesStubFn, pm) + baseRepoFn := shared.PromptWhenAmbiguousBaseRepoFunc(errMultipleRemotesStubFn, ios, pm) // When fetching the base repo _, err := baseRepoFn() @@ -152,8 +162,10 @@ func TestPromptWhenMultipleRemotesBaseRepoFunc(t *testing.T) { t.Run("when the wrapped base repo func returns a non-specific error, then it is returned", func(t *testing.T) { t.Parallel() + ios, _, _, _ := iostreams.Test() + // Given the wrapped base repo func returns a non-specific error - baseRepoFn := shared.PromptWhenAmbiguousBaseRepoFunc(errBaseRepoStubFn, nil) + baseRepoFn := shared.PromptWhenAmbiguousBaseRepoFunc(errBaseRepoStubFn, ios, nil) // When fetching the base repo _, err := baseRepoFn() From 88a64d2a1135db1a3cfa508d43ebdb9c9b1cf4f8 Mon Sep 17 00:00:00 2001 From: William Martin Date: Wed, 15 Jan 2025 14:31:32 +0100 Subject: [PATCH 27/45] Change wording on secret repo prompt --- pkg/cmd/secret/delete/delete_test.go | 2 +- pkg/cmd/secret/list/list_test.go | 2 +- pkg/cmd/secret/set/set_test.go | 2 +- pkg/cmd/secret/shared/base_repo.go | 2 +- pkg/cmd/secret/shared/base_repo_test.go | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/secret/delete/delete_test.go b/pkg/cmd/secret/delete/delete_test.go index f1f19a591..377879cf2 100644 --- a/pkg/cmd/secret/delete/delete_test.go +++ b/pkg/cmd/secret/delete/delete_test.go @@ -165,7 +165,7 @@ func TestNewCmdDeleteBaseRepoFuncs(t *testing.T) { args: "SECRET_NAME", prompterStubs: func(pm *prompter.MockPrompter) { pm.RegisterSelect( - "Select a base repo", + "Select a repo", []string{"owner/fork", "owner/repo"}, func(_, _ string, opts []string) (int, error) { return prompter.IndexFor(opts, "owner/fork") diff --git a/pkg/cmd/secret/list/list_test.go b/pkg/cmd/secret/list/list_test.go index 729984deb..601de2572 100644 --- a/pkg/cmd/secret/list/list_test.go +++ b/pkg/cmd/secret/list/list_test.go @@ -145,7 +145,7 @@ func TestNewCmdListBaseRepoFuncs(t *testing.T) { args: "", prompterStubs: func(pm *prompter.MockPrompter) { pm.RegisterSelect( - "Select a base repo", + "Select a repo", []string{"owner/fork", "owner/repo"}, func(_, _ string, opts []string) (int, error) { return prompter.IndexFor(opts, "owner/fork") diff --git a/pkg/cmd/secret/set/set_test.go b/pkg/cmd/secret/set/set_test.go index dd3124e82..99c8c6cbe 100644 --- a/pkg/cmd/secret/set/set_test.go +++ b/pkg/cmd/secret/set/set_test.go @@ -264,7 +264,7 @@ func TestNewCmdSetBaseRepoFuncs(t *testing.T) { args: "SECRET_NAME", prompterStubs: func(pm *prompter.MockPrompter) { pm.RegisterSelect( - "Select a base repo", + "Select a repo", []string{"owner/fork", "owner/repo"}, func(_, _ string, opts []string) (int, error) { return prompter.IndexFor(opts, "owner/fork") diff --git a/pkg/cmd/secret/shared/base_repo.go b/pkg/cmd/secret/shared/base_repo.go index d902105aa..308117ac6 100644 --- a/pkg/cmd/secret/shared/base_repo.go +++ b/pkg/cmd/secret/shared/base_repo.go @@ -36,7 +36,7 @@ func PromptWhenAmbiguousBaseRepoFunc(baseRepoFn baseRepoFn, ios *iostreams.IOStr } fmt.Fprintf(ios.Out, "%s Multiple remotes detected. Due to the sensitive nature of secrets, requiring disambiguation.\n", ios.ColorScheme().WarningIcon()) - selectedBaseRepo, err := prompter.Select("Select a base repo", baseRepoOptions[0], baseRepoOptions) + selectedBaseRepo, err := prompter.Select("Select a repo", baseRepoOptions[0], baseRepoOptions) if err != nil { return nil, err } diff --git a/pkg/cmd/secret/shared/base_repo_test.go b/pkg/cmd/secret/shared/base_repo_test.go index 6b7aa02ff..d78fad7e5 100644 --- a/pkg/cmd/secret/shared/base_repo_test.go +++ b/pkg/cmd/secret/shared/base_repo_test.go @@ -112,7 +112,7 @@ func TestPromptWhenMultipleRemotesBaseRepoFunc(t *testing.T) { pm := prompter.NewMockPrompter(t) pm.RegisterSelect( - "Select a base repo", + "Select a repo", []string{"owner/fork", "owner/repo"}, func(_, def string, opts []string) (int, error) { require.Equal(t, "owner/fork", def) @@ -142,7 +142,7 @@ func TestPromptWhenMultipleRemotesBaseRepoFunc(t *testing.T) { // Given the prompter returns an error pm := prompter.NewMockPrompter(t) pm.RegisterSelect( - "Select a base repo", + "Select a repo", []string{"owner/fork", "owner/repo"}, func(_, _ string, opts []string) (int, error) { return 0, errors.New("test prompt error") From e7ffb1e435e4faa081af3c6638f7f648e94f94f0 Mon Sep 17 00:00:00 2001 From: William Martin Date: Wed, 15 Jan 2025 14:38:05 +0100 Subject: [PATCH 28/45] Fix typo in secret base repo selection comment --- pkg/cmd/secret/delete/delete.go | 2 +- pkg/cmd/secret/list/list.go | 2 +- pkg/cmd/secret/set/set.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/secret/delete/delete.go b/pkg/cmd/secret/delete/delete.go index 78550f928..84a5e7a83 100644 --- a/pkg/cmd/secret/delete/delete.go +++ b/pkg/cmd/secret/delete/delete.go @@ -50,7 +50,7 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co // So there's no reason to use the specialised BaseRepoFunc that requires remote disambiguation. opts.BaseRepo = f.BaseRepo if !cmd.Flags().Changed("repo") { - // If they haven't specified a repo directly, then we will wrap the BaseRepoFunc in one that error if + // If they haven't specified a repo directly, then we will wrap the BaseRepoFunc in one that errors if // there might be multiple valid remotes. opts.BaseRepo = shared.RequireNoAmbiguityBaseRepoFunc(opts.BaseRepo, f.Remotes) // But if we are able to prompt, then we will wrap that up in a BaseRepoFunc that can prompt the user to diff --git a/pkg/cmd/secret/list/list.go b/pkg/cmd/secret/list/list.go index 41cf88e64..af3526386 100644 --- a/pkg/cmd/secret/list/list.go +++ b/pkg/cmd/secret/list/list.go @@ -71,7 +71,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman // So there's no reason to use the specialised BaseRepoFunc that requires remote disambiguation. opts.BaseRepo = f.BaseRepo if !cmd.Flags().Changed("repo") { - // If they haven't specified a repo directly, then we will wrap the BaseRepoFunc in one that error if + // If they haven't specified a repo directly, then we will wrap the BaseRepoFunc in one that errors if // there might be multiple valid remotes. opts.BaseRepo = shared.RequireNoAmbiguityBaseRepoFunc(opts.BaseRepo, f.Remotes) // But if we are able to prompt, then we will wrap that up in a BaseRepoFunc that can prompt the user to diff --git a/pkg/cmd/secret/set/set.go b/pkg/cmd/secret/set/set.go index ba90e1f67..3c12b4192 100644 --- a/pkg/cmd/secret/set/set.go +++ b/pkg/cmd/secret/set/set.go @@ -108,7 +108,7 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command // So there's no reason to use the specialised BaseRepoFunc that requires remote disambiguation. opts.BaseRepo = f.BaseRepo if !cmd.Flags().Changed("repo") { - // If they haven't specified a repo directly, then we will wrap the BaseRepoFunc in one that error if + // If they haven't specified a repo directly, then we will wrap the BaseRepoFunc in one that errors if // there might be multiple valid remotes. opts.BaseRepo = shared.RequireNoAmbiguityBaseRepoFunc(opts.BaseRepo, f.Remotes) // But if we are able to prompt, then we will wrap that up in a BaseRepoFunc that can prompt the user to From 2757d22b4b236ad0d24104a5027d6ec87cc1e657 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 15 Jan 2025 14:49:31 +0000 Subject: [PATCH 29/45] Bump google.golang.org/protobuf from 1.36.2 to 1.36.3 Bumps google.golang.org/protobuf from 1.36.2 to 1.36.3. --- updated-dependencies: - dependency-name: google.golang.org/protobuf dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 0921a544f..910ab484d 100644 --- a/go.mod +++ b/go.mod @@ -51,7 +51,7 @@ require ( golang.org/x/term v0.27.0 golang.org/x/text v0.21.0 google.golang.org/grpc v1.64.1 - google.golang.org/protobuf v1.36.2 + google.golang.org/protobuf v1.36.3 gopkg.in/h2non/gock.v1 v1.1.2 gopkg.in/yaml.v3 v3.0.1 ) diff --git a/go.sum b/go.sum index 73158e4e0..4314215db 100644 --- a/go.sum +++ b/go.sum @@ -548,8 +548,8 @@ google.golang.org/genproto/googleapis/rpc v0.0.0-20240520151616-dc85e6b867a5 h1: google.golang.org/genproto/googleapis/rpc v0.0.0-20240520151616-dc85e6b867a5/go.mod h1:EfXuqaE1J41VCDicxHzUDm+8rk+7ZdXzHV0IhO/I6s0= google.golang.org/grpc v1.64.1 h1:LKtvyfbX3UGVPFcGqJ9ItpVWW6oN/2XqTxfAnwRRXiA= google.golang.org/grpc v1.64.1/go.mod h1:hiQF4LFZelK2WKaP6W0L92zGHtiQdZxk8CrSdvyjeP0= -google.golang.org/protobuf v1.36.2 h1:R8FeyR1/eLmkutZOM5CWghmo5itiG9z0ktFlTVLuTmU= -google.golang.org/protobuf v1.36.2/go.mod h1:9fA7Ob0pmnwhb644+1+CVWFRbNajQ6iRojtC/QF5bRE= +google.golang.org/protobuf v1.36.3 h1:82DV7MYdb8anAVi3qge1wSnMDrnKK7ebr+I0hHRN1BU= +google.golang.org/protobuf v1.36.3/go.mod h1:9fA7Ob0pmnwhb644+1+CVWFRbNajQ6iRojtC/QF5bRE= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= From 243acaf5792d62234f769c2a9125f81616cd6328 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Thu, 16 Jan 2025 00:08:45 -0500 Subject: [PATCH 30/45] Refactor test based on PR feedback --- pkg/cmd/root/extension_test.go | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/root/extension_test.go b/pkg/cmd/root/extension_test.go index 7dd66df31..8246c0e2a 100644 --- a/pkg/cmd/root/extension_test.go +++ b/pkg/cmd/root/extension_test.go @@ -1,6 +1,7 @@ package root_test import ( + "fmt" "io" "testing" "time" @@ -122,8 +123,6 @@ func TestNewCmdExtension_Updates(t *testing.T) { em := &extensions.ExtensionManagerMock{ DispatchFunc: func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) { // Assume extension executed / dispatched without problems as test is focused on upgrade checking. - // Sleep for 1 second to allow update checking logic to complete. - time.Sleep(1 * time.Second) return true, nil }, } @@ -174,7 +173,7 @@ func TestNewCmdExtension_Updates(t *testing.T) { } func TestNewCmdExtension_UpdateCheckIsNonblocking(t *testing.T) { - ios, _, _, stderr := iostreams.Test() + ios, _, _, _ := iostreams.Test() em := &extensions.ExtensionManagerMock{ DispatchFunc: func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) { @@ -204,19 +203,30 @@ func TestNewCmdExtension_UpdateCheckIsNonblocking(t *testing.T) { }, } + // When the extension command is executed, the checkFunc will run in the background longer than the extension dispatch. + // If the update check is non-blocking, then the extension command will complete immediately while checkFunc is still running. checkFunc := func(em extensions.ExtensionManager, ext extensions.Extension) (*update.ReleaseInfo, error) { - // This function runs in the background as a goroutine while the extension is dispatched. - // Testing whether the extension cobra command is non-blocking for extension update check, - // this function will cause goroutine to sleep for a sufficiently long time before panicking. - // The idea is that the test will conclude because the dispatch function completes immediately - // and exits before the check update function completes. time.Sleep(30 * time.Second) - panic("It seems like the extension cobra command might be blocking on update checking when it should not block on long update checks") + return nil, fmt.Errorf("update check should not have completed") } cmd := root.NewCmdExtension(ios, em, ext, checkFunc) - _, err := cmd.ExecuteC() - require.NoError(t, err) - require.Emptyf(t, stderr.String(), "long running, non-blocking extension update check should not have displayed anything on stderr") + // The test whether update check is non-blocking is based on how long it takes for the extension command execution. + // If there is no wait time as checkFunc is sleeping sufficiently long, we can trust update check is non-blocking. + // Otherwise, if any amount of wait is encountered, it is a decent indicator that update checking is blocking. + // This is not an ideal test and indicates the update design should be revisited to be easier to understand and manage. + completed := make(chan struct{}) + go func() { + _, err := cmd.ExecuteC() + require.NoError(t, err) + close(completed) + }() + + select { + case <-completed: + // Expected behavior assuming extension dispatch exits immediately while checkFunc is still running. + case <-time.After(1 * time.Second): + t.Fatal("extension update check should have exited") + } } From cc4bf0fc9fc58da7f4eb522aad3fa41f0f0068f5 Mon Sep 17 00:00:00 2001 From: William Martin Date: Thu, 16 Jan 2025 14:10:42 +0100 Subject: [PATCH 31/45] Add small wait to extension update tests --- pkg/cmd/root/extension_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/cmd/root/extension_test.go b/pkg/cmd/root/extension_test.go index 8246c0e2a..5e9e9b9bc 100644 --- a/pkg/cmd/root/extension_test.go +++ b/pkg/cmd/root/extension_test.go @@ -123,6 +123,10 @@ func TestNewCmdExtension_Updates(t *testing.T) { em := &extensions.ExtensionManagerMock{ DispatchFunc: func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) { // Assume extension executed / dispatched without problems as test is focused on upgrade checking. + // Sleep for 100 milliseconds to allow update checking logic to complete. This would be better + // served by making the behaviour controllable by channels, but it's a larger change than desired + // just to improve the test. + time.Sleep(100 * time.Millisecond) return true, nil }, } From 35d81e3ff596071005ce07d39c0e8e5a4d09fb50 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 16 Jan 2025 10:03:48 -0700 Subject: [PATCH 32/45] Fix: Gist edit tests for interactivity This changes the gist edit tests to use the positive `istty` instead of the previous inverse `nontty`, which is consistent with the way other commands are written. --- pkg/cmd/gist/edit/edit_test.go | 223 ++++++++++++++++++++++----------- 1 file changed, 148 insertions(+), 75 deletions(-) diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index be610b6b4..a40695c39 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -3,11 +3,13 @@ package edit import ( "bytes" "encoding/json" + "fmt" "io" "net/http" "os" "path/filepath" "testing" + "time" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/gh" @@ -141,23 +143,31 @@ func Test_editRun(t *testing.T) { require.NoError(t, err) tests := []struct { - name string - opts *EditOptions - gist *shared.Gist - httpStubs func(*httpmock.Registry) - prompterStubs func(*prompter.MockPrompter) - nontty bool - stdin string - wantErr string - wantParams map[string]interface{} + name string + opts *EditOptions + mockGist *shared.Gist + mockGistList bool + httpStubs func(*httpmock.Registry) + prompterStubs func(*prompter.MockPrompter) + istty bool + stdin string + wantErr string + wantLastRequestParameters map[string]interface{} }{ { name: "no such gist", wantErr: "gist not found: 1234", + opts: &EditOptions{ + Selector: "1234", + }, }, { - name: "one file", - gist: &shared.Gist{ + name: "one file", + istty: false, + opts: &EditOptions{ + Selector: "1234", + }, + mockGist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ "cicada.txt": { @@ -172,7 +182,7 @@ func Test_editRun(t *testing.T) { reg.Register(httpmock.REST("POST", "gists/1234"), httpmock.StatusStringResponse(201, "{}")) }, - wantParams: map[string]interface{}{ + wantLastRequestParameters: map[string]interface{}{ "description": "", "files": map[string]interface{}{ "cicada.txt": map[string]interface{}{ @@ -183,7 +193,9 @@ func Test_editRun(t *testing.T) { }, }, { - name: "multiple files, submit", + name: "multiple files, submit, with TTY", + istty: true, + mockGistList: true, prompterStubs: func(pm *prompter.MockPrompter) { pm.RegisterSelect("Edit which file?", []string{"cicada.txt", "unix.md"}, @@ -196,7 +208,7 @@ func Test_editRun(t *testing.T) { return prompter.IndexFor(opts, "Submit") }) }, - gist: &shared.Gist{ + mockGist: &shared.Gist{ ID: "1234", Description: "catbug", Files: map[string]*shared.GistFile{ @@ -215,7 +227,7 @@ func Test_editRun(t *testing.T) { reg.Register(httpmock.REST("POST", "gists/1234"), httpmock.StatusStringResponse(201, "{}")) }, - wantParams: map[string]interface{}{ + wantLastRequestParameters: map[string]interface{}{ "description": "catbug", "files": map[string]interface{}{ "cicada.txt": map[string]interface{}{ @@ -230,7 +242,11 @@ func Test_editRun(t *testing.T) { }, }, { - name: "multiple files, cancel", + name: "multiple files, cancel", + istty: true, + opts: &EditOptions{ + Selector: "1234", + }, prompterStubs: func(pm *prompter.MockPrompter) { pm.RegisterSelect("Edit which file?", []string{"cicada.txt", "unix.md"}, @@ -244,7 +260,7 @@ func Test_editRun(t *testing.T) { }) }, wantErr: "CancelError", - gist: &shared.Gist{ + mockGist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ "cicada.txt": { @@ -263,7 +279,10 @@ func Test_editRun(t *testing.T) { }, { name: "not change", - gist: &shared.Gist{ + opts: &EditOptions{ + Selector: "1234", + }, + mockGist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ "cicada.txt": { @@ -277,7 +296,10 @@ func Test_editRun(t *testing.T) { }, { name: "another user's gist", - gist: &shared.Gist{ + opts: &EditOptions{ + Selector: "1234", + }, + mockGist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ "cicada.txt": { @@ -292,7 +314,11 @@ func Test_editRun(t *testing.T) { }, { name: "add file to existing gist", - gist: &shared.Gist{ + opts: &EditOptions{ + AddFilename: fileToAdd, + Selector: "1234", + }, + mockGist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ "sample.txt": { @@ -307,16 +333,14 @@ func Test_editRun(t *testing.T) { reg.Register(httpmock.REST("POST", "gists/1234"), httpmock.StatusStringResponse(201, "{}")) }, - opts: &EditOptions{ - AddFilename: fileToAdd, - }, }, { name: "change description", opts: &EditOptions{ Description: "my new description", + Selector: "1234", }, - gist: &shared.Gist{ + mockGist: &shared.Gist{ ID: "1234", Description: "my old description", Files: map[string]*shared.GistFile{ @@ -331,7 +355,7 @@ func Test_editRun(t *testing.T) { reg.Register(httpmock.REST("POST", "gists/1234"), httpmock.StatusStringResponse(201, "{}")) }, - wantParams: map[string]interface{}{ + wantLastRequestParameters: map[string]interface{}{ "description": "my new description", "files": map[string]interface{}{ "sample.txt": map[string]interface{}{ @@ -343,7 +367,12 @@ func Test_editRun(t *testing.T) { }, { name: "add file to existing gist from source parameter", - gist: &shared.Gist{ + opts: &EditOptions{ + AddFilename: "from_source.txt", + SourceFile: fileToAdd, + Selector: "1234", + }, + mockGist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ "sample.txt": { @@ -358,11 +387,7 @@ func Test_editRun(t *testing.T) { reg.Register(httpmock.REST("POST", "gists/1234"), httpmock.StatusStringResponse(201, "{}")) }, - opts: &EditOptions{ - AddFilename: "from_source.txt", - SourceFile: fileToAdd, - }, - wantParams: map[string]interface{}{ + wantLastRequestParameters: map[string]interface{}{ "description": "", "files": map[string]interface{}{ "from_source.txt": map[string]interface{}{ @@ -374,7 +399,12 @@ func Test_editRun(t *testing.T) { }, { name: "add file to existing gist from stdin", - gist: &shared.Gist{ + opts: &EditOptions{ + AddFilename: "from_source.txt", + SourceFile: "-", + Selector: "1234", + }, + mockGist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ "sample.txt": { @@ -389,12 +419,8 @@ func Test_editRun(t *testing.T) { reg.Register(httpmock.REST("POST", "gists/1234"), httpmock.StatusStringResponse(201, "{}")) }, - opts: &EditOptions{ - AddFilename: "from_source.txt", - SourceFile: "-", - }, stdin: "data from stdin", - wantParams: map[string]interface{}{ + wantLastRequestParameters: map[string]interface{}{ "description": "", "files": map[string]interface{}{ "from_source.txt": map[string]interface{}{ @@ -406,7 +432,11 @@ func Test_editRun(t *testing.T) { }, { name: "remove file, file does not exist", - gist: &shared.Gist{ + opts: &EditOptions{ + RemoveFilename: "sample2.txt", + Selector: "1234", + }, + mockGist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ "sample.txt": { @@ -417,14 +447,15 @@ func Test_editRun(t *testing.T) { }, Owner: &shared.GistOwner{Login: "octocat"}, }, - opts: &EditOptions{ - RemoveFilename: "sample2.txt", - }, wantErr: "gist has no file \"sample2.txt\"", }, { name: "remove file from existing gist", - gist: &shared.Gist{ + opts: &EditOptions{ + RemoveFilename: "sample2.txt", + Selector: "1234", + }, + mockGist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ "sample.txt": { @@ -444,10 +475,7 @@ func Test_editRun(t *testing.T) { reg.Register(httpmock.REST("POST", "gists/1234"), httpmock.StatusStringResponse(201, "{}")) }, - opts: &EditOptions{ - RemoveFilename: "sample2.txt", - }, - wantParams: map[string]interface{}{ + wantLastRequestParameters: map[string]interface{}{ "description": "", "files": map[string]interface{}{ "sample.txt": map[string]interface{}{ @@ -460,7 +488,11 @@ func Test_editRun(t *testing.T) { }, { name: "edit gist using file from source parameter", - gist: &shared.Gist{ + opts: &EditOptions{ + SourceFile: fileToAdd, + Selector: "1234", + }, + mockGist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ "sample.txt": { @@ -475,10 +507,7 @@ func Test_editRun(t *testing.T) { reg.Register(httpmock.REST("POST", "gists/1234"), httpmock.StatusStringResponse(201, "{}")) }, - opts: &EditOptions{ - SourceFile: fileToAdd, - }, - wantParams: map[string]interface{}{ + wantLastRequestParameters: map[string]interface{}{ "description": "", "files": map[string]interface{}{ "sample.txt": map[string]interface{}{ @@ -490,7 +519,11 @@ func Test_editRun(t *testing.T) { }, { name: "edit gist using stdin", - gist: &shared.Gist{ + opts: &EditOptions{ + SourceFile: "-", + Selector: "1234", + }, + mockGist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ "sample.txt": { @@ -505,11 +538,8 @@ func Test_editRun(t *testing.T) { reg.Register(httpmock.REST("POST", "gists/1234"), httpmock.StatusStringResponse(201, "{}")) }, - opts: &EditOptions{ - SourceFile: "-", - }, stdin: "data from stdin", - wantParams: map[string]interface{}{ + wantLastRequestParameters: map[string]interface{}{ "description": "", "files": map[string]interface{}{ "sample.txt": map[string]interface{}{ @@ -520,8 +550,8 @@ func Test_editRun(t *testing.T) { }, }, { - name: "no arguments notty", - nontty: true, + name: "no arguments notty", + istty: false, opts: &EditOptions{ Selector: "", }, @@ -531,24 +561,62 @@ func Test_editRun(t *testing.T) { for _, tt := range tests { reg := &httpmock.Registry{} - if tt.gist == nil { + pm := prompter.NewMockPrompter(t) + + if tt.opts == nil { + tt.opts = &EditOptions{} + } + + if tt.opts.Selector != "" { + // Only register the HTTP stubs for a direct gist lookup if a selector is provided. + if tt.mockGist == nil { + // If no gist is provided, we expect a 404. + reg.Register(httpmock.REST("GET", fmt.Sprintf("gists/%s", tt.opts.Selector)), + httpmock.StatusStringResponse(404, "Not Found")) + } else { + // If a gist is provided, we expect the gist to be fetched. + reg.Register(httpmock.REST("GET", fmt.Sprintf("gists/%s", tt.opts.Selector)), + httpmock.JSONResponse(tt.mockGist)) + reg.Register(httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) + } + } + + if tt.mockGistList { + sixHours, _ := time.ParseDuration("6h") + sixHoursAgo := time.Now().Add(-sixHours) + reg.Register(httpmock.GraphQL(`query GistList\b`), + httpmock.StringResponse( + fmt.Sprintf(`{ "data": { "viewer": { "gists": { "nodes": [ + { + "description": "whatever", + "files": [{ "name": "cicada.txt" }, { "name": "unix.md" }], + "isPublic": true, + "name": "1234", + "updatedAt": "%s" + } + ], + "pageInfo": { + "hasNextPage": false, + "endCursor": "somevaluedoesnotmatter" + } } } } }`, sixHoursAgo.Format(time.RFC3339)))) reg.Register(httpmock.REST("GET", "gists/1234"), - httpmock.StatusStringResponse(404, "Not Found")) - } else { - reg.Register(httpmock.REST("GET", "gists/1234"), - httpmock.JSONResponse(tt.gist)) + httpmock.JSONResponse(tt.mockGist)) reg.Register(httpmock.GraphQL(`query UserCurrent\b`), httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) + + gistList := "cicada.txt whatever about 6 hours ago" + pm.RegisterSelect("Select a gist", + []string{gistList}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, gistList) + }) } if tt.httpStubs != nil { tt.httpStubs(reg) } - if tt.opts == nil { - tt.opts = &EditOptions{} - } - tt.opts.Edit = func(_, _, _ string, _ *iostreams.IOStreams) (string, error) { return "new file content", nil } @@ -558,19 +626,17 @@ func Test_editRun(t *testing.T) { } ios, stdin, stdout, stderr := iostreams.Test() stdin.WriteString(tt.stdin) - ios.SetStdoutTTY(!tt.nontty) - ios.SetStdinTTY(!tt.nontty) - ios.SetStderrTTY(!tt.nontty) + ios.SetStdoutTTY(tt.istty) + ios.SetStdinTTY(tt.istty) + ios.SetStderrTTY(tt.istty) tt.opts.IO = ios - tt.opts.Selector = "1234" tt.opts.Config = func() (gh.Config, error) { return config.NewBlankConfig(), nil } t.Run(tt.name, func(t *testing.T) { - pm := prompter.NewMockPrompter(t) if tt.prompterStubs != nil { tt.prompterStubs(pm) } @@ -584,14 +650,21 @@ func Test_editRun(t *testing.T) { } assert.NoError(t, err) - if tt.wantParams != nil { - bodyBytes, _ := io.ReadAll(reg.Requests[2].Body) + if tt.wantLastRequestParameters != nil { + // Currently only checking that the last request has + // the expected request parameters. + // + // This might need to be changed, if a test were to be added + // that needed to check that a request other than the last + // has the desired parameters. + lastRequest := reg.Requests[len(reg.Requests)-1] + bodyBytes, _ := io.ReadAll(lastRequest.Body) reqBody := make(map[string]interface{}) err = json.Unmarshal(bodyBytes, &reqBody) if err != nil { t.Fatalf("error decoding JSON: %v", err) } - assert.Equal(t, tt.wantParams, reqBody) + assert.Equal(t, tt.wantLastRequestParameters, reqBody) } assert.Equal(t, "", stdout.String()) From dec46670bb356a214e0ae4ecc8fd689a55df5214 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 16 Jan 2025 11:14:33 -0700 Subject: [PATCH 33/45] Fix: gist edit/view tests var name consistency --- pkg/cmd/gist/edit/edit_test.go | 16 ++++++++-------- pkg/cmd/gist/view/view_test.go | 28 ++++++++++++++-------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index a40695c39..5a3f0a268 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -149,7 +149,7 @@ func Test_editRun(t *testing.T) { mockGistList bool httpStubs func(*httpmock.Registry) prompterStubs func(*prompter.MockPrompter) - istty bool + isTTY bool stdin string wantErr string wantLastRequestParameters map[string]interface{} @@ -163,7 +163,7 @@ func Test_editRun(t *testing.T) { }, { name: "one file", - istty: false, + isTTY: false, opts: &EditOptions{ Selector: "1234", }, @@ -194,7 +194,7 @@ func Test_editRun(t *testing.T) { }, { name: "multiple files, submit, with TTY", - istty: true, + isTTY: true, mockGistList: true, prompterStubs: func(pm *prompter.MockPrompter) { pm.RegisterSelect("Edit which file?", @@ -243,7 +243,7 @@ func Test_editRun(t *testing.T) { }, { name: "multiple files, cancel", - istty: true, + isTTY: true, opts: &EditOptions{ Selector: "1234", }, @@ -551,7 +551,7 @@ func Test_editRun(t *testing.T) { }, { name: "no arguments notty", - istty: false, + isTTY: false, opts: &EditOptions{ Selector: "", }, @@ -626,9 +626,9 @@ func Test_editRun(t *testing.T) { } ios, stdin, stdout, stderr := iostreams.Test() stdin.WriteString(tt.stdin) - ios.SetStdoutTTY(tt.istty) - ios.SetStdinTTY(tt.istty) - ios.SetStderrTTY(tt.istty) + ios.SetStdoutTTY(tt.isTTY) + ios.SetStdinTTY(tt.isTTY) + ios.SetStderrTTY(tt.isTTY) tt.opts.IO = ios diff --git a/pkg/cmd/gist/view/view_test.go b/pkg/cmd/gist/view/view_test.go index 820401f1e..706b85f10 100644 --- a/pkg/cmd/gist/view/view_test.go +++ b/pkg/cmd/gist/view/view_test.go @@ -116,10 +116,10 @@ func Test_viewRun(t *testing.T) { name string opts *ViewOptions wantOut string - gist *shared.Gist + mockGist *shared.Gist + mockGistList bool isTTY bool wantErr string - mockGistList bool }{ { name: "no such gist", @@ -137,7 +137,7 @@ func Test_viewRun(t *testing.T) { Selector: "1234", ListFiles: false, }, - gist: &shared.Gist{ + mockGist: &shared.Gist{ Files: map[string]*shared.GistFile{ "cicada.txt": { Content: "bwhiizzzbwhuiiizzzz", @@ -155,7 +155,7 @@ func Test_viewRun(t *testing.T) { ListFiles: false, }, mockGistList: true, - gist: &shared.Gist{ + mockGist: &shared.Gist{ Files: map[string]*shared.GistFile{ "cicada.txt": { Content: "test interactive mode", @@ -178,7 +178,7 @@ func Test_viewRun(t *testing.T) { Filename: "cicada.txt", ListFiles: false, }, - gist: &shared.Gist{ + mockGist: &shared.Gist{ Files: map[string]*shared.GistFile{ "cicada.txt": { Content: "bwhiizzzbwhuiiizzzz", @@ -201,7 +201,7 @@ func Test_viewRun(t *testing.T) { Raw: true, ListFiles: false, }, - gist: &shared.Gist{ + mockGist: &shared.Gist{ Files: map[string]*shared.GistFile{ "cicada.txt": { Content: "bwhiizzzbwhuiiizzzz", @@ -222,7 +222,7 @@ func Test_viewRun(t *testing.T) { Selector: "1234", ListFiles: false, }, - gist: &shared.Gist{ + mockGist: &shared.Gist{ Files: map[string]*shared.GistFile{ "cicada.txt": { Content: "bwhiizzzbwhuiiizzzz", @@ -243,7 +243,7 @@ func Test_viewRun(t *testing.T) { Selector: "1234", ListFiles: false, }, - gist: &shared.Gist{ + mockGist: &shared.Gist{ Files: map[string]*shared.GistFile{ "cicada.txt": { Content: "bwhiizzzbwhuiiizzzz\n", @@ -264,7 +264,7 @@ func Test_viewRun(t *testing.T) { Selector: "1234", ListFiles: false, }, - gist: &shared.Gist{ + mockGist: &shared.Gist{ Description: "some files", Files: map[string]*shared.GistFile{ "cicada.txt": { @@ -287,7 +287,7 @@ func Test_viewRun(t *testing.T) { Raw: true, ListFiles: false, }, - gist: &shared.Gist{ + mockGist: &shared.Gist{ Description: "some files", Files: map[string]*shared.GistFile{ "cicada.txt": { @@ -310,7 +310,7 @@ func Test_viewRun(t *testing.T) { Raw: false, ListFiles: true, }, - gist: &shared.Gist{ + mockGist: &shared.Gist{ Description: "some files", Files: map[string]*shared.GistFile{ "cicada.txt": { @@ -329,7 +329,7 @@ func Test_viewRun(t *testing.T) { Raw: false, ListFiles: true, }, - gist: &shared.Gist{ + mockGist: &shared.Gist{ Description: "some files", Files: map[string]*shared.GistFile{ "cicada.txt": { @@ -348,12 +348,12 @@ func Test_viewRun(t *testing.T) { for _, tt := range tests { reg := &httpmock.Registry{} - if tt.gist == nil { + if tt.mockGist == nil { reg.Register(httpmock.REST("GET", "gists/1234"), httpmock.StatusStringResponse(404, "Not Found")) } else { reg.Register(httpmock.REST("GET", "gists/1234"), - httpmock.JSONResponse(tt.gist)) + httpmock.JSONResponse(tt.mockGist)) } if tt.opts == nil { From 5b6fd53a88498caab47c0e24e06c427f53574814 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 16 Jan 2025 11:36:48 -0700 Subject: [PATCH 34/45] Fix: gist edit test name --- pkg/cmd/gist/edit/edit_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index 5a3f0a268..728f6e894 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -242,7 +242,7 @@ func Test_editRun(t *testing.T) { }, }, { - name: "multiple files, cancel", + name: "multiple files, cancel, with TTY", isTTY: true, opts: &EditOptions{ Selector: "1234", From facd0caa2971ad3620f451d1968e04aab2cce17c Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 16 Jan 2025 11:44:46 -0700 Subject: [PATCH 35/45] Fix: accidental whitespace in gist edit --- pkg/cmd/gist/edit/edit.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index b85118937..e0d6e3b84 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -113,7 +113,6 @@ func editRun(opts *EditOptions) error { } gist, err := shared.PromptGists(opts.Prompter, client, host, cs) - if err != nil { return err } From dcf5cc872412bb8a40554c22b0601394db3d3ff0 Mon Sep 17 00:00:00 2001 From: Rob Morgan Date: Fri, 17 Jan 2025 18:08:59 +0800 Subject: [PATCH 36/45] fix: tiny typo fix --- internal/gh/gh.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/gh/gh.go b/internal/gh/gh.go index e4431fdab..8e640c41a 100644 --- a/internal/gh/gh.go +++ b/internal/gh/gh.go @@ -90,7 +90,7 @@ type Migration interface { } // AuthConfig is used for interacting with some persistent configuration for gh, -// with knowledge on how to access encrypted storage when neccesarry. +// with knowledge on how to access encrypted storage when necessary. // Behavior is scoped to authentication specific tasks. type AuthConfig interface { // HasActiveToken returns true when a token for the hostname is present. From 7942be56aad37d163e970df17d0b5a4cdf2c09c1 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Fri, 17 Jan 2025 12:07:32 -0700 Subject: [PATCH 37/45] Add affected version to bug report form --- .github/ISSUE_TEMPLATE/bug_report.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 00f2a0ba5..ae0d29096 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -9,7 +9,11 @@ assignees: '' ### Describe the bug -A clear and concise description of what the bug is. Include version by typing `gh --version`. +A clear and concise description of what the bug is. + +### Affected version + +Please run `gh version` and paste the output below. ### Steps to reproduce the behavior From 31bff5b5a8020b684566c73e2618fc65b3b8daa4 Mon Sep 17 00:00:00 2001 From: Caleb Brose <5447118+cmbrose@users.noreply.github.com> Date: Wed, 8 Jan 2025 02:30:08 +0000 Subject: [PATCH 38/45] Better handling for codespace state polling --- internal/codespaces/api/api.go | 2 + internal/codespaces/codespaces.go | 70 ++++++---- internal/codespaces/codespaces_test.go | 175 +++++++++++++++++++++++++ 3 files changed, 220 insertions(+), 27 deletions(-) create mode 100644 internal/codespaces/codespaces_test.go diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index 9e79eafe1..0d1eaf5b3 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -240,6 +240,8 @@ const ( CodespaceStateAvailable = "Available" // CodespaceStateShutdown is the state for a shutdown codespace environment. CodespaceStateShutdown = "Shutdown" + // CodespaceStateShuttingDown is the state for a shutting down codespace environment. + CodespaceStateShuttingDown = "ShuttingDown" // CodespaceStateStarting is the state for a starting codespace environment. CodespaceStateStarting = "Starting" // CodespaceStateRebuilding is the state for a rebuilding codespace environment. diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index da2eacd44..38a2e903e 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -13,6 +13,10 @@ import ( "github.com/cli/cli/v2/internal/codespaces/connection" ) +// codespaceStatePollingBackoff is the delay between state polls while waiting for codespaces to become +// available. It's only exposed so that it can be shortened for testing, otherwise it should not be changed +var codespaceStatePollingBackoff = backoff.NewConstantBackOff(10 * time.Second) + func connectionReady(codespace *api.Codespace) bool { // If the codespace is not available, it is not ready if codespace.State != api.CodespaceStateAvailable { @@ -67,41 +71,53 @@ func GetCodespaceConnection(ctx context.Context, progress progressIndicator, api // waitUntilCodespaceConnectionReady waits for a Codespace to be running and is able to be connected to. func waitUntilCodespaceConnectionReady(ctx context.Context, progress progressIndicator, apiClient apiClient, codespace *api.Codespace) (*api.Codespace, error) { - if codespace.State != api.CodespaceStateAvailable { - progress.StartProgressIndicatorWithLabel("Starting codespace") - defer progress.StopProgressIndicator() - if err := apiClient.StartCodespace(ctx, codespace.Name); err != nil { - return nil, fmt.Errorf("error starting codespace: %w", err) - } + if connectionReady(codespace) { + return codespace, nil } - if !connectionReady(codespace) { - expBackoff := backoff.NewExponentialBackOff() - expBackoff.Multiplier = 1.1 - expBackoff.MaxInterval = 10 * time.Second - expBackoff.MaxElapsedTime = 5 * time.Minute + progress.StartProgressIndicatorWithLabel("Waiting for codespace to become ready") + defer progress.StopProgressIndicator() - err := backoff.Retry(func() error { - var err error + lastState := "" + firstRetry := true + + err := backoff.Retry(func() error { + var err error + if firstRetry { + firstRetry = false + } else { codespace, err = apiClient.GetCodespace(ctx, codespace.Name, true) if err != nil { return backoff.Permanent(fmt.Errorf("error getting codespace: %w", err)) } - - if connectionReady(codespace) { - return nil - } - - return &TimeoutError{message: "codespace not ready yet"} - }, backoff.WithContext(expBackoff, ctx)) - if err != nil { - var timeoutErr *TimeoutError - if errors.As(err, &timeoutErr) { - return nil, errors.New("timed out while waiting for the codespace to start") - } - - return nil, err } + + if connectionReady(codespace) { + return nil + } + + // Only react to changes in the state (so that we don't try to start the codespace twice) + if codespace.State != lastState { + if codespace.State == api.CodespaceStateShutdown { + err = apiClient.StartCodespace(ctx, codespace.Name) + if err != nil { + return backoff.Permanent(fmt.Errorf("error starting codespace: %w", err)) + } + } + } + + lastState = codespace.State + + return &TimeoutError{message: "codespace not ready yet"} + }, backoff.WithContext(codespaceStatePollingBackoff, ctx)) + + if err != nil { + var timeoutErr *TimeoutError + if errors.As(err, &timeoutErr) { + return nil, errors.New("timed out while waiting for the codespace to start") + } + + return nil, err } return codespace, nil diff --git a/internal/codespaces/codespaces_test.go b/internal/codespaces/codespaces_test.go new file mode 100644 index 000000000..c9d37aca2 --- /dev/null +++ b/internal/codespaces/codespaces_test.go @@ -0,0 +1,175 @@ +package codespaces + +import ( + "context" + "net/http" + "testing" + "time" + + "github.com/cenkalti/backoff/v4" + "github.com/cli/cli/v2/internal/codespaces/api" +) + +func init() { + // Set the backoff to 0 for testing so that they run quickly + codespaceStatePollingBackoff = backoff.NewConstantBackOff(time.Second * 0) +} + +// This is just enough to trick `connectionReady` +var readyCodespace = &api.Codespace{ + State: api.CodespaceStateAvailable, + Connection: api.CodespaceConnection{ + TunnelProperties: api.TunnelProperties{ + ConnectAccessToken: "test", + ManagePortsAccessToken: "test", + ServiceUri: "test", + TunnelId: "test", + ClusterId: "test", + Domain: "test", + }, + }, +} + +func TestWaitUntilCodespaceConnectionReady_WhenAlreadyReady(t *testing.T) { + t.Parallel() + + apiClient := &mockApiClient{} + waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, readyCodespace) +} + +func TestWaitUntilCodespaceConnectionReady_PollsApi(t *testing.T) { + t.Parallel() + + apiClient := &mockApiClient{ + onGetCodespace: func() (*api.Codespace, error) { + return readyCodespace, nil + }, + } + waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, &api.Codespace{State: api.CodespaceStateStarting}) +} + +func TestWaitUntilCodespaceConnectionReady_StartsCodespace(t *testing.T) { + t.Parallel() + + codespace := &api.Codespace{State: api.CodespaceStateShutdown} + + apiClient := &mockApiClient{ + onGetCodespace: func() (*api.Codespace, error) { + return codespace, nil + }, + onStartCodespace: func() error { + *codespace = *readyCodespace + return nil + }, + } + waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, codespace) +} + +func TestWaitUntilCodespaceConnectionReady_PollsCodespaceUntilReady(t *testing.T) { + t.Parallel() + + codespace := &api.Codespace{State: api.CodespaceStateShutdown} + hasPolled := false + + apiClient := &mockApiClient{ + onGetCodespace: func() (*api.Codespace, error) { + if hasPolled { + *codespace = *readyCodespace + } + + hasPolled = true + + return codespace, nil + }, + onStartCodespace: func() error { + codespace.State = api.CodespaceStateStarting + return nil + }, + } + waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, codespace) +} + +func TestWaitUntilCodespaceConnectionReady_WaitsForShutdownBeforeStarting(t *testing.T) { + t.Parallel() + + codespace := &api.Codespace{State: api.CodespaceStateShuttingDown} + + apiClient := &mockApiClient{ + onGetCodespace: func() (*api.Codespace, error) { + // Make sure that we poll at least once before going to shutdown + if codespace.State == api.CodespaceStateShuttingDown { + codespace.State = api.CodespaceStateShutdown + } + return codespace, nil + }, + onStartCodespace: func() error { + if codespace.State != api.CodespaceStateShutdown { + t.Fatalf("Codespace started from non-shutdown state: %s", codespace.State) + } + *codespace = *readyCodespace + return nil + }, + } + waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, codespace) +} + +func TestWaitUntilCodespaceConnectionReady_DoesntStartTwice(t *testing.T) { + t.Parallel() + + codespace := &api.Codespace{State: api.CodespaceStateShutdown} + didStart := false + didPollAfterStart := false + + apiClient := &mockApiClient{ + onGetCodespace: func() (*api.Codespace, error) { + // Make sure that we are in shutdown state for one poll after starting to make sure we don't try to start again + if didPollAfterStart { + *codespace = *readyCodespace + } + + if didStart { + didPollAfterStart = true + } + + return codespace, nil + }, + onStartCodespace: func() error { + if didStart { + t.Fatal("Should not start multiple times") + } + didStart = true + return nil + }, + } + waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, codespace) +} + +type mockApiClient struct { + onStartCodespace func() error + onGetCodespace func() (*api.Codespace, error) +} + +func (m *mockApiClient) StartCodespace(ctx context.Context, name string) error { + if m.onStartCodespace == nil { + panic("onStartCodespace not set and StartCodespace was called") + } + + return m.onStartCodespace() +} + +func (m *mockApiClient) GetCodespace(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) { + if m.onGetCodespace == nil { + panic("onGetCodespace not set and GetCodespace was called") + } + + return m.onGetCodespace() +} + +func (m *mockApiClient) HTTPClient() (*http.Client, error) { + panic("Not implemented") +} + +type mockProgressIndicator struct{} + +func (m *mockProgressIndicator) StartProgressIndicatorWithLabel(s string) {} +func (m *mockProgressIndicator) StopProgressIndicator() {} From 9b8cd7babd342a0a295d92078ef960cbc3cb0c2a Mon Sep 17 00:00:00 2001 From: Caleb Brose <5447118+cmbrose@users.noreply.github.com> Date: Fri, 10 Jan 2025 20:11:17 +0000 Subject: [PATCH 39/45] Linter --- internal/codespaces/codespaces_test.go | 51 ++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/internal/codespaces/codespaces_test.go b/internal/codespaces/codespaces_test.go index c9d37aca2..d931b96ef 100644 --- a/internal/codespaces/codespaces_test.go +++ b/internal/codespaces/codespaces_test.go @@ -34,7 +34,13 @@ func TestWaitUntilCodespaceConnectionReady_WhenAlreadyReady(t *testing.T) { t.Parallel() apiClient := &mockApiClient{} - waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, readyCodespace) + result, err := waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, readyCodespace) + if err != nil { + t.Fatalf("Expected nil error, but was %v", err) + } + if result.State != api.CodespaceStateAvailable { + t.Fatalf("Expected final state to be %s, but was %s", api.CodespaceStateAvailable, result.State) + } } func TestWaitUntilCodespaceConnectionReady_PollsApi(t *testing.T) { @@ -45,7 +51,14 @@ func TestWaitUntilCodespaceConnectionReady_PollsApi(t *testing.T) { return readyCodespace, nil }, } - waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, &api.Codespace{State: api.CodespaceStateStarting}) + result, err := waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, &api.Codespace{State: api.CodespaceStateStarting}) + + if err != nil { + t.Fatalf("Expected nil error, but was %v", err) + } + if result.State != api.CodespaceStateAvailable { + t.Fatalf("Expected final state to be %s, but was %s", api.CodespaceStateAvailable, result.State) + } } func TestWaitUntilCodespaceConnectionReady_StartsCodespace(t *testing.T) { @@ -62,7 +75,13 @@ func TestWaitUntilCodespaceConnectionReady_StartsCodespace(t *testing.T) { return nil }, } - waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, codespace) + result, err := waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, codespace) + if err != nil { + t.Fatalf("Expected nil error, but was %v", err) + } + if result.State != api.CodespaceStateAvailable { + t.Fatalf("Expected final state to be %s, but was %s", api.CodespaceStateAvailable, result.State) + } } func TestWaitUntilCodespaceConnectionReady_PollsCodespaceUntilReady(t *testing.T) { @@ -86,7 +105,13 @@ func TestWaitUntilCodespaceConnectionReady_PollsCodespaceUntilReady(t *testing.T return nil }, } - waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, codespace) + result, err := waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, codespace) + if err != nil { + t.Fatalf("Expected nil error, but was %v", err) + } + if result.State != api.CodespaceStateAvailable { + t.Fatalf("Expected final state to be %s, but was %s", api.CodespaceStateAvailable, result.State) + } } func TestWaitUntilCodespaceConnectionReady_WaitsForShutdownBeforeStarting(t *testing.T) { @@ -110,10 +135,16 @@ func TestWaitUntilCodespaceConnectionReady_WaitsForShutdownBeforeStarting(t *tes return nil }, } - waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, codespace) + result, err := waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, codespace) + if err != nil { + t.Fatalf("Expected nil error, but was %v", err) + } + if result.State != api.CodespaceStateAvailable { + t.Fatalf("Expected final state to be %s, but was %s", api.CodespaceStateAvailable, result.State) + } } -func TestWaitUntilCodespaceConnectionReady_DoesntStartTwice(t *testing.T) { +func TestUntilCodespaceConnectionReady_DoesntStartTwice(t *testing.T) { t.Parallel() codespace := &api.Codespace{State: api.CodespaceStateShutdown} @@ -141,7 +172,13 @@ func TestWaitUntilCodespaceConnectionReady_DoesntStartTwice(t *testing.T) { return nil }, } - waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, codespace) + result, err := waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, codespace) + if err != nil { + t.Fatalf("Expected nil error, but was %v", err) + } + if result.State != api.CodespaceStateAvailable { + t.Fatalf("Expected final state to be %s, but was %s", api.CodespaceStateAvailable, result.State) + } } type mockApiClient struct { From 0c8591064dec9ed95d7fba10a9decb5fff1ee605 Mon Sep 17 00:00:00 2001 From: Caleb Brose <5447118+cmbrose@users.noreply.github.com> Date: Mon, 13 Jan 2025 22:09:11 +0000 Subject: [PATCH 40/45] Change back to exponential backoff --- internal/codespaces/codespaces.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index 38a2e903e..92185120a 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -15,7 +15,12 @@ import ( // codespaceStatePollingBackoff is the delay between state polls while waiting for codespaces to become // available. It's only exposed so that it can be shortened for testing, otherwise it should not be changed -var codespaceStatePollingBackoff = backoff.NewConstantBackOff(10 * time.Second) +var codespaceStatePollingBackoff backoff.BackOff = backoff.NewExponentialBackOff( + backoff.WithInitialInterval(1*time.Second), + backoff.WithMultiplier(1.02), + backoff.WithMaxInterval(10*time.Second), + backoff.WithMaxElapsedTime(5*time.Minute), +) func connectionReady(codespace *api.Codespace) bool { // If the codespace is not available, it is not ready From 4396e40a31de072f6390c6a50cb67ef2e09ba551 Mon Sep 17 00:00:00 2001 From: Mikel Olasagasti Uranga Date: Mon, 20 Jan 2025 16:27:25 +0100 Subject: [PATCH 41/45] Fix: Ensure constant format strings in fmt and printf calls Go 1.24 introduces stricter checks for format string validation. This commit fixes instances where non-constant format strings were used in calls to functions like `fmt.Errorf`, `fmt.Printf`, and similar. Signed-off-by: Mikel Olasagasti Uranga --- pkg/cmd/auth/token/token.go | 2 +- pkg/cmd/codespace/create_test.go | 6 +++--- pkg/cmd/pr/merge/merge.go | 7 ++++--- pkg/cmd/repo/view/view.go | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/auth/token/token.go b/pkg/cmd/auth/token/token.go index 9f4dcc1ac..7ac65d37e 100644 --- a/pkg/cmd/auth/token/token.go +++ b/pkg/cmd/auth/token/token.go @@ -88,7 +88,7 @@ func tokenRun(opts *TokenOptions) error { if opts.Username != "" { errMsg += fmt.Sprintf(" account %s", opts.Username) } - return fmt.Errorf(errMsg) + return fmt.Errorf("%s", errMsg) } if val != "" { diff --git a/pkg/cmd/codespace/create_test.go b/pkg/cmd/codespace/create_test.go index 6466d9be7..85d311a47 100644 --- a/pkg/cmd/codespace/create_test.go +++ b/pkg/cmd/codespace/create_test.go @@ -648,14 +648,14 @@ Alternatively, you can run "create" with the "--default-permissions" option to c assert.EqualError(t, err, tt.wantErr.Error()) } if err != nil && tt.wantErr == nil { - t.Logf(err.Error()) + t.Logf("%s", err.Error()) } if got := stdout.String(); got != tt.wantStdout { - t.Logf(t.Name()) + t.Logf("%s", t.Name()) t.Errorf(" stdout = %v, want %v", got, tt.wantStdout) } if got := stderr.String(); got != tt.wantStderr { - t.Logf(t.Name()) + t.Logf("%s", t.Name()) t.Errorf(" stderr = %v, want %v", got, tt.wantStderr) } diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index 6696fac85..422b5e93e 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -384,13 +384,14 @@ func (m *mergeContext) deleteLocalBranch() error { if m.merged { if m.opts.IO.CanPrompt() && !m.opts.IsDeleteBranchIndicated { - confirmed, err := m.opts.Prompter.Confirm(fmt.Sprintf("Pull request %s#%d was already merged. Delete the branch locally?", ghrepo.FullName(m.baseRepo), m.pr.Number), false) + message := fmt.Sprintf("Pull request %s#%d was already merged. Delete the branch locally?", ghrepo.FullName(m.baseRepo), m.pr.Number) + confirmed, err := m.opts.Prompter.Confirm(message, false) if err != nil { return fmt.Errorf("could not prompt: %w", err) } m.deleteBranch = confirmed } else { - _ = m.warnf(fmt.Sprintf("%s Pull request %s#%d was already merged\n", m.cs.WarningIcon(), ghrepo.FullName(m.baseRepo), m.pr.Number)) + _ = m.warnf("%s Pull request %s#%d was already merged\n", m.cs.WarningIcon(), ghrepo.FullName(m.baseRepo), m.pr.Number) } } @@ -431,7 +432,7 @@ func (m *mergeContext) deleteLocalBranch() error { } if err := m.opts.GitClient.Pull(ctx, baseRemote.Name, targetBranch); err != nil { - _ = m.warnf(fmt.Sprintf("%s warning: not possible to fast-forward to: %q\n", m.cs.WarningIcon(), targetBranch)) + _ = m.warnf("%s warning: not possible to fast-forward to: %q\n", m.cs.WarningIcon(), targetBranch) } switchedToBranch = targetBranch diff --git a/pkg/cmd/repo/view/view.go b/pkg/cmd/repo/view/view.go index d0370203a..f8cbcb18e 100644 --- a/pkg/cmd/repo/view/view.go +++ b/pkg/cmd/repo/view/view.go @@ -156,7 +156,7 @@ func viewRun(opts *ViewOptions) error { fmt.Fprintf(stdout, "description:\t%s\n", repo.Description) if readme != nil { fmt.Fprintln(stdout, "--") - fmt.Fprintf(stdout, readme.Content) + fmt.Fprintf(stdout, "%s", readme.Content) fmt.Fprintln(stdout) } From 0ead3398a71663edf8e24dae836c957aaf2cc280 Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 21 Jan 2025 17:29:39 +0100 Subject: [PATCH 42/45] Bump golang ci lint to work with go 1.24 --- .github/workflows/lint.yml | 4 ++-- pkg/cmd/ruleset/shared/shared.go | 2 +- pkg/cmdutil/json_flags.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 0a6c0d9db..f1ae1e522 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -32,7 +32,7 @@ jobs: go mod verify go mod download - LINT_VERSION=1.59.1 + LINT_VERSION=1.63.4 curl -fsSL https://github.com/golangci/golangci-lint/releases/download/v${LINT_VERSION}/golangci-lint-${LINT_VERSION}-linux-amd64.tar.gz | \ tar xz --strip-components 1 --wildcards \*/golangci-lint mkdir -p bin && mv golangci-lint bin/ @@ -53,6 +53,6 @@ jobs: assert-nothing-changed go fmt ./... assert-nothing-changed go mod tidy - bin/golangci-lint run --out-format=github-actions --timeout=3m || STATUS=$? + bin/golangci-lint run --out-format=colored-line-number --timeout=3m || STATUS=$? exit $STATUS diff --git a/pkg/cmd/ruleset/shared/shared.go b/pkg/cmd/ruleset/shared/shared.go index 3ec1c83ee..536e8a5a9 100644 --- a/pkg/cmd/ruleset/shared/shared.go +++ b/pkg/cmd/ruleset/shared/shared.go @@ -78,7 +78,7 @@ func ParseRulesForDisplay(rules []RulesetRule) string { for _, rule := range rules { display.WriteString(fmt.Sprintf("- %s", rule.Type)) - if rule.Parameters != nil && len(rule.Parameters) > 0 { + if len(rule.Parameters) > 0 { display.WriteString(": ") // sort these keys too for consistency diff --git a/pkg/cmdutil/json_flags.go b/pkg/cmdutil/json_flags.go index 440e0e0b8..596c2f216 100644 --- a/pkg/cmdutil/json_flags.go +++ b/pkg/cmdutil/json_flags.go @@ -254,7 +254,7 @@ func (e *jsonExporter) exportData(v reflect.Value) interface{} { } return m.Interface() case reflect.Struct: - if v.CanAddr() && reflect.PtrTo(v.Type()).Implements(exportableType) { + if v.CanAddr() && reflect.PointerTo(v.Type()).Implements(exportableType) { ve := v.Addr().Interface().(exportable) return ve.ExportData(e.fields) } else if v.Type().Implements(exportableType) { From 0a3706a4042f5ef319b8a8e619cfafa741b8dc86 Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 21 Jan 2025 18:04:47 +0100 Subject: [PATCH 43/45] Remove unncessary printf usage --- pkg/cmd/auth/token/token.go | 3 ++- pkg/cmd/codespace/create_test.go | 6 +++--- pkg/cmd/repo/view/view.go | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/auth/token/token.go b/pkg/cmd/auth/token/token.go index 7ac65d37e..d9faac7d8 100644 --- a/pkg/cmd/auth/token/token.go +++ b/pkg/cmd/auth/token/token.go @@ -1,6 +1,7 @@ package token import ( + "errors" "fmt" "github.com/MakeNowJust/heredoc" @@ -88,7 +89,7 @@ func tokenRun(opts *TokenOptions) error { if opts.Username != "" { errMsg += fmt.Sprintf(" account %s", opts.Username) } - return fmt.Errorf("%s", errMsg) + return errors.New(errMsg) } if val != "" { diff --git a/pkg/cmd/codespace/create_test.go b/pkg/cmd/codespace/create_test.go index 85d311a47..c5f11bd2f 100644 --- a/pkg/cmd/codespace/create_test.go +++ b/pkg/cmd/codespace/create_test.go @@ -648,14 +648,14 @@ Alternatively, you can run "create" with the "--default-permissions" option to c assert.EqualError(t, err, tt.wantErr.Error()) } if err != nil && tt.wantErr == nil { - t.Logf("%s", err.Error()) + t.Log(err.Error()) } if got := stdout.String(); got != tt.wantStdout { - t.Logf("%s", t.Name()) + t.Log(t.Name()) t.Errorf(" stdout = %v, want %v", got, tt.wantStdout) } if got := stderr.String(); got != tt.wantStderr { - t.Logf("%s", t.Name()) + t.Log(t.Name()) t.Errorf(" stderr = %v, want %v", got, tt.wantStderr) } diff --git a/pkg/cmd/repo/view/view.go b/pkg/cmd/repo/view/view.go index f8cbcb18e..06f85d048 100644 --- a/pkg/cmd/repo/view/view.go +++ b/pkg/cmd/repo/view/view.go @@ -156,7 +156,7 @@ func viewRun(opts *ViewOptions) error { fmt.Fprintf(stdout, "description:\t%s\n", repo.Description) if readme != nil { fmt.Fprintln(stdout, "--") - fmt.Fprintf(stdout, "%s", readme.Content) + fmt.Fprint(stdout, readme.Content) fmt.Fprintln(stdout) } From 9eb8002764575cef00d11900ac982a76d28dfd7a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 16 Jan 2025 20:58:01 +0000 Subject: [PATCH 44/45] Bump github.com/google/go-containerregistry from 0.20.2 to 0.20.3 Bumps [github.com/google/go-containerregistry](https://github.com/google/go-containerregistry) from 0.20.2 to 0.20.3. - [Release notes](https://github.com/google/go-containerregistry/releases) - [Changelog](https://github.com/google/go-containerregistry/blob/main/.goreleaser.yml) - [Commits](https://github.com/google/go-containerregistry/compare/v0.20.2...v0.20.3) --- updated-dependencies: - dependency-name: github.com/google/go-containerregistry dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- go.mod | 42 +++++++++++------------ go.sum | 106 +++++++++++++++++++++++++++------------------------------ 2 files changed, 71 insertions(+), 77 deletions(-) diff --git a/go.mod b/go.mod index cf08aa2a7..e3a729287 100644 --- a/go.mod +++ b/go.mod @@ -1,8 +1,7 @@ module github.com/cli/cli/v2 go 1.22.5 - -toolchain go1.22.6 +toolchain go1.23.5 require ( github.com/AlecAivazis/survey/v2 v2.3.7 @@ -18,12 +17,12 @@ require ( github.com/cpuguy83/go-md2man/v2 v2.0.6 github.com/creack/pty v1.1.24 github.com/digitorus/timestamp v0.0.0-20231217203849-220c5c2851b7 - github.com/distribution/reference v0.5.0 + github.com/distribution/reference v0.6.0 github.com/gabriel-vasile/mimetype v1.4.8 github.com/gdamore/tcell/v2 v2.5.4 github.com/golang/snappy v0.0.4 github.com/google/go-cmp v0.6.0 - github.com/google/go-containerregistry v0.20.2 + github.com/google/go-containerregistry v0.20.3 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/gorilla/websocket v1.5.3 github.com/hashicorp/go-multierror v1.1.1 @@ -44,11 +43,11 @@ require ( github.com/sigstore/sigstore-go v0.6.2 github.com/spf13/cobra v1.8.1 github.com/spf13/pflag v1.0.5 - github.com/stretchr/testify v1.9.0 + github.com/stretchr/testify v1.10.0 github.com/zalando/go-keyring v0.2.5 - golang.org/x/crypto v0.31.0 + golang.org/x/crypto v0.32.0 golang.org/x/sync v0.10.0 - golang.org/x/term v0.27.0 + golang.org/x/term v0.28.0 golang.org/x/text v0.21.0 google.golang.org/grpc v1.64.1 google.golang.org/protobuf v1.36.3 @@ -66,21 +65,21 @@ require ( github.com/charmbracelet/x/exp/term v0.0.0-20240425164147-ba2a9512b05f // indirect github.com/cli/browser v1.3.0 // indirect github.com/cli/shurcooL-graphql v0.0.4 // indirect - github.com/containerd/stargz-snapshotter/estargz v0.14.3 // indirect + github.com/containerd/stargz-snapshotter/estargz v0.16.3 // indirect github.com/cyberphone/json-canonicalization v0.0.0-20220623050100-57a0ce2678a7 // indirect github.com/danieljoos/wincred v1.2.1 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/digitorus/pkcs7 v0.0.0-20230818184609-3a137a874352 // indirect github.com/dlclark/regexp2 v1.4.0 // indirect - github.com/docker/cli v27.1.1+incompatible // indirect - github.com/docker/distribution v2.8.2+incompatible // indirect - github.com/docker/docker-credential-helpers v0.7.0 // indirect + github.com/docker/cli v27.5.0+incompatible // indirect + github.com/docker/distribution v2.8.3+incompatible // indirect + github.com/docker/docker-credential-helpers v0.8.2 // indirect github.com/fatih/color v1.16.0 // indirect github.com/fsnotify/fsnotify v1.7.0 // indirect github.com/gdamore/encoding v1.0.0 // indirect github.com/go-chi/chi v4.1.2+incompatible // indirect github.com/go-jose/go-jose/v4 v4.0.2 // indirect - github.com/go-logr/logr v1.4.1 // indirect + github.com/go-logr/logr v1.4.2 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-openapi/analysis v0.23.0 // indirect github.com/go-openapi/errors v0.22.0 // indirect @@ -107,7 +106,7 @@ require ( github.com/itchyny/timefmt-go v0.1.5 // indirect github.com/jedisct1/go-minisign v0.0.0-20211028175153-1c139d1cc84b // indirect github.com/josharian/intern v1.0.0 // indirect - github.com/klauspost/compress v1.17.4 // indirect + github.com/klauspost/compress v1.17.11 // indirect github.com/letsencrypt/boulder v0.0.0-20240620165639-de9c06129bec // indirect github.com/lucasb-eyer/go-colorful v1.2.0 // indirect github.com/magiconair/properties v1.8.7 // indirect @@ -121,7 +120,7 @@ require ( github.com/oklog/ulid v1.3.1 // indirect github.com/olekukonko/tablewriter v0.0.5 // indirect github.com/opencontainers/go-digest v1.0.0 // indirect - github.com/opencontainers/image-spec v1.1.0-rc5 // indirect + github.com/opencontainers/image-spec v1.1.0 // indirect github.com/pelletier/go-toml/v2 v2.1.0 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect @@ -149,20 +148,21 @@ require ( github.com/thlib/go-timezone-local v0.0.0-20210907160436-ef149e42d28e // indirect github.com/titanous/rocacheck v0.0.0-20171023193734-afe73141d399 // indirect github.com/transparency-dev/merkle v0.0.2 // indirect - github.com/vbatts/tar-split v0.11.3 // indirect + github.com/vbatts/tar-split v0.11.6 // indirect github.com/yuin/goldmark v1.5.4 // indirect github.com/yuin/goldmark-emoji v1.0.2 // indirect go.mongodb.org/mongo-driver v1.14.0 // indirect - go.opentelemetry.io/otel v1.27.0 // indirect - go.opentelemetry.io/otel/metric v1.27.0 // indirect - go.opentelemetry.io/otel/trace v1.27.0 // indirect + go.opentelemetry.io/auto/sdk v1.1.0 // indirect + go.opentelemetry.io/otel v1.33.0 // indirect + go.opentelemetry.io/otel/metric v1.33.0 // indirect + go.opentelemetry.io/otel/trace v1.33.0 // indirect go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.27.0 // indirect golang.org/x/exp v0.0.0-20240112132812-db7319d0e0e3 // indirect - golang.org/x/mod v0.21.0 // indirect - golang.org/x/net v0.33.0 // indirect + golang.org/x/mod v0.22.0 // indirect + golang.org/x/net v0.34.0 // indirect golang.org/x/sys v0.29.0 // indirect - golang.org/x/tools v0.26.0 // indirect + golang.org/x/tools v0.29.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20240520151616-dc85e6b867a5 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20240520151616-dc85e6b867a5 // indirect gopkg.in/ini.v1 v1.67.0 // indirect diff --git a/go.sum b/go.sum index 5bb8cb8f7..5745095b8 100644 --- a/go.sum +++ b/go.sum @@ -1,8 +1,7 @@ cloud.google.com/go v0.112.1 h1:uJSeirPke5UNZHIb4SxfZklVSiWWVqW4oXlETwZziwM= cloud.google.com/go/compute v1.25.1 h1:ZRpHJedLtTpKgr3RV1Fx23NuaAEN1Zfx9hw1u4aJdjU= -cloud.google.com/go/compute v1.25.1/go.mod h1:oopOIR53ly6viBYxaDhBfJwzUAxf1zE//uf3IB011ls= -cloud.google.com/go/compute/metadata v0.2.3 h1:mg4jlk7mCAj6xXp9UJ4fjI9VUI5rubuGBW5aJ7UnBMY= -cloud.google.com/go/compute/metadata v0.2.3/go.mod h1:VAV5nSsACxMJvgaAuX6Pk2AawlZn8kiOGuCv6gTkwuA= +cloud.google.com/go/compute/metadata v0.6.0 h1:A6hENjEsCDtC1k8byVsgwvVcioamEHvZ4j01OwKxG9I= +cloud.google.com/go/compute/metadata v0.6.0/go.mod h1:FjyFAW1MW0C203CEOMDTu3Dk1FlqW3Rga40jzHL4hfg= cloud.google.com/go/iam v1.1.6 h1:bEa06k05IO4f4uJonbB5iAgKTPpABy1ayxaIZV/GHVc= cloud.google.com/go/iam v1.1.6/go.mod h1:O0zxdPeGBoFdWW3HWmBxJsk0pfvNM/p/qa82rWOGTwI= cloud.google.com/go/kms v1.15.8 h1:szIeDCowID8th2i8XE4uRev5PMxQFqW+JjwYxL9h6xs= @@ -25,7 +24,6 @@ github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.0.0 h1:D3occ github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.0.0/go.mod h1:bTSOgj05NGRuHHhQwAdPnYr9TOdNmKlZTgGLL6nyAdI= github.com/AzureAD/microsoft-authentication-library-for-go v1.2.2 h1:XHOnouVk1mxXfQidrMEnLlPk9UMeRtyBTnEFtxkV0kU= github.com/AzureAD/microsoft-authentication-library-for-go v1.2.2/go.mod h1:wP83P5OoQ5p6ip3ScPr0BAq0BvuPAvacpEuSzyouqAI= -github.com/BurntSushi/toml v1.2.1/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= github.com/MakeNowJust/heredoc v1.0.0 h1:cXCdzVdstXyiTqTvfqk9SDHpKNjxuom+DOlyEeQ4pzQ= github.com/MakeNowJust/heredoc v1.0.0/go.mod h1:mG5amYoWBHf8vpLOuehzbGGw0EHxpZZ6lCpQ4fNJ8LE= github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2 h1:+vx7roKuyA63nhn5WAunQHLTznkw5W8b1Xc0dNjp83s= @@ -108,9 +106,8 @@ github.com/cli/shurcooL-graphql v0.0.4 h1:6MogPnQJLjKkaXPyGqPRXOI2qCsQdqNfUY1QSJ github.com/cli/shurcooL-graphql v0.0.4/go.mod h1:3waN4u02FiZivIV+p1y4d0Jo1jc6BViMA73C+sZo2fk= github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb h1:EDmT6Q9Zs+SbUoc7Ik9EfrFqcylYqgPZ9ANSbTAntnE= github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb/go.mod h1:ZjrT6AXHbDs86ZSdt/osfBi5qfexBrKUdONk989Wnk4= -github.com/containerd/stargz-snapshotter/estargz v0.14.3 h1:OqlDCK3ZVUO6C3B/5FSkDwbkEETK84kQgEeFwDC+62k= -github.com/containerd/stargz-snapshotter/estargz v0.14.3/go.mod h1:KY//uOCIkSuNAHhJogcZtrNHdKrA99/FCCRjE3HD36o= -github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= +github.com/containerd/stargz-snapshotter/estargz v0.16.3 h1:7evrXtoh1mSbGj/pfRccTampEyKpjpOnS3CyiV1Ebr8= +github.com/containerd/stargz-snapshotter/estargz v0.16.3/go.mod h1:uyr4BfYfOj3G9WBVE8cOlQmXAbPN9VEQpBBeJIuOipU= github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/cpuguy83/go-md2man/v2 v2.0.6 h1:XJtiaUW6dEEqVuZiMTn1ldk455QWwEIsMIJlo5vtkx0= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= @@ -130,16 +127,16 @@ github.com/digitorus/pkcs7 v0.0.0-20230818184609-3a137a874352 h1:ge14PCmCvPjpMQM github.com/digitorus/pkcs7 v0.0.0-20230818184609-3a137a874352/go.mod h1:SKVExuS+vpu2l9IoOc0RwqE7NYnb0JlcFHFnEJkVDzc= github.com/digitorus/timestamp v0.0.0-20231217203849-220c5c2851b7 h1:lxmTCgmHE1GUYL7P0MlNa00M67axePTq+9nBSGddR8I= github.com/digitorus/timestamp v0.0.0-20231217203849-220c5c2851b7/go.mod h1:GvWntX9qiTlOud0WkQ6ewFm0LPy5JUR1Xo0Ngbd1w6Y= -github.com/distribution/reference v0.5.0 h1:/FUIFXtfc/x2gpa5/VGfiGLuOIdYa1t65IKK2OFGvA0= -github.com/distribution/reference v0.5.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E= +github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk= +github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E= github.com/dlclark/regexp2 v1.4.0 h1:F1rxgk7p4uKjwIQxBs9oAXe5CqrXlCduYEJvrF4u93E= github.com/dlclark/regexp2 v1.4.0/go.mod h1:2pZnwuY/m+8K6iRw6wQdMtk+rH5tNGR1i55kozfMjCc= -github.com/docker/cli v27.1.1+incompatible h1:goaZxOqs4QKxznZjjBWKONQci/MywhtRv2oNn0GkeZE= -github.com/docker/cli v27.1.1+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= -github.com/docker/distribution v2.8.2+incompatible h1:T3de5rq0dB1j30rp0sA2rER+m322EBzniBPB6ZIzuh8= -github.com/docker/distribution v2.8.2+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w= -github.com/docker/docker-credential-helpers v0.7.0 h1:xtCHsjxogADNZcdv1pKUHXryefjlVRqWqIhk/uXJp0A= -github.com/docker/docker-credential-helpers v0.7.0/go.mod h1:rETQfLdHNT3foU5kuNkFR1R1V12OJRRO5lzt2D1b5X0= +github.com/docker/cli v27.5.0+incompatible h1:aMphQkcGtpHixwwhAXJT1rrK/detk2JIvDaFkLctbGM= +github.com/docker/cli v27.5.0+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= +github.com/docker/distribution v2.8.3+incompatible h1:AtKxIZ36LoNK51+Z6RpzLpddBirtxJnzDrHLEKxTAYk= +github.com/docker/distribution v2.8.3+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w= +github.com/docker/docker-credential-helpers v0.8.2 h1:bX3YxiGzFP5sOXWc3bTPEXdEaZSeVMrFgOr3T+zrFAo= +github.com/docker/docker-credential-helpers v0.8.2/go.mod h1:P3ci7E3lwkZg6XiHdRKft1KckHiO9a2rNtyFbZ/ry9M= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM= github.com/fatih/color v1.16.0/go.mod h1:fL2Sau1YI5c0pdGEVCbKQbLXB6edEj1ZgiY4NijnWvE= @@ -162,8 +159,8 @@ github.com/go-jose/go-jose/v3 v3.0.3/go.mod h1:5b+7YgP7ZICgJDBdfjZaIt+H/9L9T/YQr github.com/go-jose/go-jose/v4 v4.0.2 h1:R3l3kkBds16bO7ZFAEEcofK0MkrAJt3jlJznWZG0nvk= github.com/go-jose/go-jose/v4 v4.0.2/go.mod h1:WVf9LFMHh/QVrmqrOfqun0C45tMe3RoiKJMPvgWwLfY= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= -github.com/go-logr/logr v1.4.1 h1:pKouT5E8xu9zeFC39JXRDukb6JFQPXM5p5I91188VAQ= -github.com/go-logr/logr v1.4.1/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= +github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY= +github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/go-openapi/analysis v0.23.0 h1:aGday7OWupfMs+LbmLZG4k0MYXIANxcuBTYUC03zFCU= @@ -204,8 +201,8 @@ github.com/google/certificate-transparency-go v1.2.1 h1:4iW/NwzqOqYEEoCBEFP+jPbB github.com/google/certificate-transparency-go v1.2.1/go.mod h1:bvn/ytAccv+I6+DGkqpvSsEdiVGramgaSC6RD3tEmeE= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= -github.com/google/go-containerregistry v0.20.2 h1:B1wPJ1SN/S7pB+ZAimcciVD+r+yV/l/DSArMxlbwseo= -github.com/google/go-containerregistry v0.20.2/go.mod h1:z38EKdKh4h7IP2gSfUUqEvalZBqs6AoLeWfUy34nQC8= +github.com/google/go-containerregistry v0.20.3 h1:oNx7IdTI936V8CQRveCjaxOiegWwvM7kqkbXTpyiovI= +github.com/google/go-containerregistry v0.20.3/go.mod h1:w00pIgBRDVUDFM6bq+Qx8lwNWK+cxgCuX1vd3PIBDNI= github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0= github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/s2a-go v0.1.7 h1:60BLSyTrOV4/haCDW4zb1guZItoSq8foHCXrAnjBo/o= @@ -288,8 +285,8 @@ github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8Hm github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs= github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8= -github.com/klauspost/compress v1.17.4 h1:Ej5ixsIri7BrIjBkRZLTo6ghwrEtHFk7ijlczPW4fZ4= -github.com/klauspost/compress v1.17.4/go.mod h1:/dCuZOvVtNoHsyb+cuJD3itjs3NbnF6KH9zAO4BDxPM= +github.com/klauspost/compress v1.17.11 h1:In6xLpyWOi1+C7tXUUWv2ot1QvBjxevKAaI6IXrJmUc= +github.com/klauspost/compress v1.17.11/go.mod h1:pMDklpSncoRMuLFrf1W9Ss9KT+0rH90U12bZKk7uwG0= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= @@ -340,8 +337,8 @@ github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N github.com/olekukonko/tablewriter v0.0.5/go.mod h1:hPp6KlRPjbx+hW8ykQs1w3UBbZlj6HuIJcUGPhkA7kY= github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U= github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= -github.com/opencontainers/image-spec v1.1.0-rc5 h1:Ygwkfw9bpDvs+c9E34SdgGOj41dX/cbdlwvlWt0pnFI= -github.com/opencontainers/image-spec v1.1.0-rc5/go.mod h1:X4pATf0uXsnn3g5aiGIsVnJBR4mxhKzfwmvK/B2NTm8= +github.com/opencontainers/image-spec v1.1.0 h1:8SG7/vwALn54lVB/0yZ/MMwhFrPYtpEHQb2IpWsCzug= +github.com/opencontainers/image-spec v1.1.0/go.mod h1:W4s4sFTMaBeK1BQLXbG4AdM2szdn85PY75RI83NrTrM= github.com/opentracing/opentracing-go v1.2.0 h1:uEJPy/1a5RIPAJ0Ov+OIO8OxWu77jEv+1B0VhjKrZUs= github.com/opentracing/opentracing-go v1.2.0/go.mod h1:GxEUsuufX4nBwe+T+Wl9TAgYrxe9dPLANfrWvHYVTgc= github.com/pelletier/go-toml/v2 v2.1.0 h1:FnwAJ4oYMvbT/34k9zzHuZNrhlz48GB3/s6at6/MHO4= @@ -369,8 +366,8 @@ github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= github.com/rodaine/table v1.0.1 h1:U/VwCnUxlVYxw8+NJiLIuCxA/xa6jL38MY3FYysVWWQ= github.com/rodaine/table v1.0.1/go.mod h1:UVEtfBsflpeEcD56nF4F5AocNFta0ZuolpSVdPtlmP4= -github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M= -github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA= +github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII= +github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o= github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/ryanuber/go-glob v1.0.0 h1:iQh3xXAumdQ+4Ufa5b25cRpC5TYKlno6hsv6Cb3pkBk= @@ -411,7 +408,6 @@ github.com/sigstore/sigstore/pkg/signature/kms/hashivault v1.8.3 h1:h9G8j+Ds21zq github.com/sigstore/sigstore/pkg/signature/kms/hashivault v1.8.3/go.mod h1:zgCeHOuqF6k7A7TTEvftcA9V3FRzB7mrPtHOhXAQBnc= github.com/sigstore/timestamp-authority v1.2.2 h1:X4qyutnCQqJ0apMewFyx+3t7Tws00JQ/JonBiu3QvLE= github.com/sigstore/timestamp-authority v1.2.2/go.mod h1:nEah4Eq4wpliDjlY342rXclGSO7Kb9hoRrl9tqLW13A= -github.com/sirupsen/logrus v1.9.0/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ= github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= github.com/sourcegraph/conc v0.3.0 h1:OQTbbt6P72L20UqAkXXuLOj79LfEanQ+YQFNpLA9ySo= @@ -436,10 +432,9 @@ github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= -github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= -github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= -github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= +github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8= github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU= github.com/theupdateframework/go-tuf v0.7.0 h1:CqbQFrWo1ae3/I0UCblSbczevCCbS31Qvs5LdxRWqRI= @@ -452,9 +447,8 @@ github.com/titanous/rocacheck v0.0.0-20171023193734-afe73141d399 h1:e/5i7d4oYZ+C github.com/titanous/rocacheck v0.0.0-20171023193734-afe73141d399/go.mod h1:LdwHTNJT99C5fTAzDz0ud328OgXz+gierycbcIx2fRs= github.com/transparency-dev/merkle v0.0.2 h1:Q9nBoQcZcgPamMkGn7ghV8XiTZ/kRxn1yCG81+twTK4= github.com/transparency-dev/merkle v0.0.2/go.mod h1:pqSy+OXefQ1EDUVmAJ8MUhHB9TXGuzVAT58PqBoHz1A= -github.com/urfave/cli v1.22.12/go.mod h1:sSBEIC79qR6OvcmsD4U3KABeOTxDqQtdDnaFuUN30b8= -github.com/vbatts/tar-split v0.11.3 h1:hLFqsOLQ1SsppQNTMpkpPXClLDfC2A3Zgy9OUU+RVck= -github.com/vbatts/tar-split v0.11.3/go.mod h1:9QlHN18E+fEH7RdG+QAJJcuya3rqT7eXSTY7wGrAokY= +github.com/vbatts/tar-split v0.11.6 h1:4SjTW5+PU11n6fZenf2IPoV8/tz3AaYHMWjf23envGs= +github.com/vbatts/tar-split v0.11.6/go.mod h1:dqKNtesIOr2j2Qv3W/cHjnvk9I8+G7oAkFDFN6TCBEI= github.com/yuin/goldmark v1.3.7/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= github.com/yuin/goldmark v1.5.4 h1:2uY/xC0roWy8IBEGLgB1ywIoEJFGmRrX21YQcvGZzjU= @@ -467,18 +461,20 @@ go.mongodb.org/mongo-driver v1.14.0 h1:P98w8egYRjYe3XDjxhYJagTokP/H6HzlsnojRgZRd go.mongodb.org/mongo-driver v1.14.0/go.mod h1:Vzb0Mk/pa7e6cWw85R4F/endUC3u0U9jGcNU603k65c= go.opencensus.io v0.24.0 h1:y73uSU6J157QMP2kn2r30vwW1A2W2WFwSCGnAVxeaD0= go.opencensus.io v0.24.0/go.mod h1:vNK8G9p7aAivkbmorf4v+7Hgx+Zs0yY+0fOtgBfjQKo= +go.opentelemetry.io/auto/sdk v1.1.0 h1:cH53jehLUN6UFLY71z+NDOiNJqDdPRaXzTel0sJySYA= +go.opentelemetry.io/auto/sdk v1.1.0/go.mod h1:3wSPjt5PWp2RhlCcmmOial7AvC4DQqZb7a7wCow3W8A= go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.52.0 h1:vS1Ao/R55RNV4O7TA2Qopok8yN+X0LIP6RVWLFkprck= go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.52.0/go.mod h1:BMsdeOxN04K0L5FNUBfjFdvwWGNe/rkmSwH4Aelu/X0= -go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.52.0 h1:9l89oX4ba9kHbBol3Xin3leYJ+252h0zszDtBwyKe2A= -go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.52.0/go.mod h1:XLZfZboOJWHNKUv7eH0inh0E9VV6eWDFB/9yJyTLPp0= -go.opentelemetry.io/otel v1.27.0 h1:9BZoF3yMK/O1AafMiQTVu0YDj5Ea4hPhxCs7sGva+cg= -go.opentelemetry.io/otel v1.27.0/go.mod h1:DMpAK8fzYRzs+bi3rS5REupisuqTheUlSZJ1WnZaPAQ= -go.opentelemetry.io/otel/metric v1.27.0 h1:hvj3vdEKyeCi4YaYfNjv2NUje8FqKqUY8IlF0FxV/ik= -go.opentelemetry.io/otel/metric v1.27.0/go.mod h1:mVFgmRlhljgBiuk/MP/oKylr4hs85GZAylncepAX/ak= -go.opentelemetry.io/otel/sdk v1.27.0 h1:mlk+/Y1gLPLn84U4tI8d3GNJmGT/eXe3ZuOXN9kTWmI= -go.opentelemetry.io/otel/sdk v1.27.0/go.mod h1:Ha9vbLwJE6W86YstIywK2xFfPjbWlCuwPtMkKdz/Y4A= -go.opentelemetry.io/otel/trace v1.27.0 h1:IqYb813p7cmbHk0a5y6pD5JPakbVfftRXABGt5/Rscw= -go.opentelemetry.io/otel/trace v1.27.0/go.mod h1:6RiD1hkAprV4/q+yd2ln1HG9GoPx39SuvvstaLBl+l4= +go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.58.0 h1:yd02MEjBdJkG3uabWP9apV+OuWRIXGDuJEUJbOHmCFU= +go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.58.0/go.mod h1:umTcuxiv1n/s/S6/c2AT/g2CQ7u5C59sHDNmfSwgz7Q= +go.opentelemetry.io/otel v1.33.0 h1:/FerN9bax5LoK51X/sI0SVYrjSE0/yUL7DpxW4K3FWw= +go.opentelemetry.io/otel v1.33.0/go.mod h1:SUUkR6csvUQl+yjReHu5uM3EtVV7MBm5FHKRlNx4I8I= +go.opentelemetry.io/otel/metric v1.33.0 h1:r+JOocAyeRVXD8lZpjdQjzMadVZp2M4WmQ+5WtEnklQ= +go.opentelemetry.io/otel/metric v1.33.0/go.mod h1:L9+Fyctbp6HFTddIxClbQkjtubW6O9QS3Ann/M82u6M= +go.opentelemetry.io/otel/sdk v1.33.0 h1:iax7M131HuAm9QkZotNHEfstof92xM+N8sr3uHXc2IM= +go.opentelemetry.io/otel/sdk v1.33.0/go.mod h1:A1Q5oi7/9XaMlIWzPSxLRWOI8nG3FnzHJNbiENQuihM= +go.opentelemetry.io/otel/trace v1.33.0 h1:cCJuF7LRjUFso9LPnEAHJDB2pqzp+hbO8eu1qqW2d/s= +go.opentelemetry.io/otel/trace v1.33.0/go.mod h1:uIcdVUZMpTAmz0tI1z04GoVSezK37CbGV4fr1f2nBck= go.step.sm/crypto v0.44.2 h1:t3p3uQ7raP2jp2ha9P6xkQF85TJZh+87xmjSLaib+jk= go.step.sm/crypto v0.44.2/go.mod h1:x1439EnFhadzhkuaGX7sz03LEMQ+jV4gRamf5LCZJQQ= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= @@ -489,20 +485,20 @@ go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8= go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -golang.org/x/crypto v0.31.0 h1:ihbySMvVjLAeSH1IbfcRTkD/iNscyz8rGzjF/E5hV6U= -golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk= +golang.org/x/crypto v0.32.0 h1:euUpcYgM8WcP71gNpTqQCn6rC2t6ULUPiOzfWaXVVfc= +golang.org/x/crypto v0.32.0/go.mod h1:ZnnJkOaASj8g0AjIduWNlq2NRxL0PlBrbKVyZ6V/Ugc= golang.org/x/exp v0.0.0-20240112132812-db7319d0e0e3 h1:hNQpMuAJe5CtcUqCXaWga3FHu+kQvCqcsoVaQgSV60o= golang.org/x/exp v0.0.0-20240112132812-db7319d0e0e3/go.mod h1:idGWGoKP1toJGkd5/ig9ZLuPcZBC3ewk7SzmH0uou08= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= -golang.org/x/mod v0.21.0 h1:vvrHzRwRfVKSiLrG+d4FMl/Qi4ukBCE6kZlTUkDYRT0= -golang.org/x/mod v0.21.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY= +golang.org/x/mod v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4= +golang.org/x/mod v0.22.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= -golang.org/x/net v0.33.0 h1:74SYHlV8BIgHIFC/LrYkOGIwL19eTYXQ5wc6TBuO36I= -golang.org/x/net v0.33.0/go.mod h1:HXLR5J+9DxmrqMwG9qjGCxZ+zKXxBru04zlTvWlWuN4= -golang.org/x/oauth2 v0.22.0 h1:BzDx2FehcG7jJwgWLELCdmLuxk2i+x9UDpSiss2u0ZA= -golang.org/x/oauth2 v0.22.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI= +golang.org/x/net v0.34.0 h1:Mb7Mrk043xzHgnRM88suvJFwzVrRfHEHJEl5/71CKw0= +golang.org/x/net v0.34.0/go.mod h1:di0qlW3YNM5oh6GqDGQr92MyTozJPmybPK4Ev/Gm31k= +golang.org/x/oauth2 v0.25.0 h1:CY4y7XT9v0cRI9oupztF8AgiIu99L/ksR/Xp/6jrZ70= +golang.org/x/oauth2 v0.25.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ= @@ -515,14 +511,13 @@ golang.org/x/sys v0.0.0-20210831042530-f4d43177bf5e/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220906165534-d0df966e6959/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.29.0 h1:TPYlXGxvx1MGTn2GiZDhnjPA9wZzZeGKHHmKhHYvgaU= golang.org/x/sys v0.29.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= -golang.org/x/term v0.27.0 h1:WP60Sv1nlK1T6SupCHbXzSaN0b9wUmsPoRS9b61A23Q= -golang.org/x/term v0.27.0/go.mod h1:iMsnZpn0cago0GOrHO2+Y7u7JPn5AylBrcoWkElMTSM= +golang.org/x/term v0.28.0 h1:/Ts8HFuMR2E6IP/jlo7QVLZHggjKQbhu/7H0LJFr3Gg= +golang.org/x/term v0.28.0/go.mod h1:Sw/lC2IAUZ92udQNf3WodGtn4k/XoLyZoh8v/8uiwek= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= @@ -535,8 +530,8 @@ golang.org/x/time v0.5.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= -golang.org/x/tools v0.26.0 h1:v/60pFQmzmT9ExmjDv2gGIfi3OqfKoEP6I5+umXlbnQ= -golang.org/x/tools v0.26.0/go.mod h1:TPVVj70c7JJ3WCazhD8OdXcZg/og+b9+tH/KxylGwH0= +golang.org/x/tools v0.29.0 h1:Xx0h3TtM9rzQpQuR4dKLrdglAmCEN5Oi+P74JdhdzXE= +golang.org/x/tools v0.29.0/go.mod h1:KMQVMRsVxU6nHCFXrBPhDB8XncLNLM0lIy/F14RP588= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/api v0.172.0 h1:/1OcMZGPmW1rX2LCu2CmGUD1KXK1+pfzxotxyRUCCdk= google.golang.org/api v0.172.0/go.mod h1:+fJZq6QXWfa9pXhnIzsjx4yI22d4aI9ZpLb58gvXjis= @@ -557,7 +552,6 @@ gopkg.in/h2non/gock.v1 v1.1.2 h1:jBbHXgGBK/AoPVfJh5x4r/WxIrElvbLel8TCZkkZJoY= gopkg.in/h2non/gock.v1 v1.1.2/go.mod h1:n7UGz/ckNChHiK05rDoiC4MYSunEC/lyaUm2WWaDva0= gopkg.in/ini.v1 v1.67.0 h1:Dgnx+6+nfE+IfzjUEISNeydPJh9AXNNsWbGP9KzCsOA= gopkg.in/ini.v1 v1.67.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= -gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From 113fe646f35aa4cb10f0e29446e7b1a4e17ffe99 Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 21 Jan 2025 17:13:31 +0100 Subject: [PATCH 45/45] Bump go module version to 1.23 --- docs/source.md | 2 +- go.mod | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/source.md b/docs/source.md index d2b7c5b31..29bf51e39 100644 --- a/docs/source.md +++ b/docs/source.md @@ -1,6 +1,6 @@ # Installation from source -1. Verify that you have Go 1.22+ installed +1. Verify that you have Go 1.23+ installed ```sh $ go version diff --git a/go.mod b/go.mod index e3a729287..ef56b8a73 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,7 @@ module github.com/cli/cli/v2 -go 1.22.5 +go 1.23.0 + toolchain go1.23.5 require (