From ad96ad3580a9503389745f460d45ffe1a1857802 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 31 Jul 2024 14:55:39 +0000 Subject: [PATCH 1/4] build(deps): bump actions/attest-build-provenance from 1.3.3 to 1.4.0 Bumps [actions/attest-build-provenance](https://github.com/actions/attest-build-provenance) from 1.3.3 to 1.4.0. - [Release notes](https://github.com/actions/attest-build-provenance/releases) - [Changelog](https://github.com/actions/attest-build-provenance/blob/main/RELEASE.md) - [Commits](https://github.com/actions/attest-build-provenance/compare/5e9cb68e95676991667494a6a4e59b8a2f13e1d0...210c1913531870065f03ce1f9440dd87bc0938cd) --- updated-dependencies: - dependency-name: actions/attest-build-provenance dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- .github/workflows/deployment.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/deployment.yml b/.github/workflows/deployment.yml index 2e87e2429..564eb647c 100644 --- a/.github/workflows/deployment.yml +++ b/.github/workflows/deployment.yml @@ -299,7 +299,7 @@ jobs: rpmsign --addsign dist/*.rpm - name: Attest release artifacts if: inputs.environment == 'production' - uses: actions/attest-build-provenance@5e9cb68e95676991667494a6a4e59b8a2f13e1d0 # v1.3.3 + uses: actions/attest-build-provenance@210c1913531870065f03ce1f9440dd87bc0938cd # v1.4.0 with: subject-path: "dist/gh_*" - name: Run createrepo From 22a5a4c899cec911152cbb27acb539ee7505655d Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Fri, 2 Aug 2024 10:36:32 -0400 Subject: [PATCH 2/4] Update `gh variable get` to use repo host Closes #9274 This change addresses a bug where repository and repository environment variables are not being retrieved from the appropriate host, causing GHES users to fail without specifying `GH_HOST` to force `gh variable set` to do the right thing. In addition, the tests for this command have been updated to check both github.com and GHES variations to ensure this works. --- pkg/cmd/variable/get/get.go | 24 +++++++++++--------- pkg/cmd/variable/get/get_test.go | 39 ++++++++++++++++++++++++++++++-- pkg/httpmock/stub.go | 9 ++++++++ 3 files changed, 59 insertions(+), 13 deletions(-) diff --git a/pkg/cmd/variable/get/get.go b/pkg/cmd/variable/get/get.go index 32b4c5e62..e4def5a03 100644 --- a/pkg/cmd/variable/get/get.go +++ b/pkg/cmd/variable/get/get.go @@ -92,22 +92,24 @@ func getRun(opts *GetOptions) error { } } - var path string - switch variableEntity { - case shared.Organization: - path = fmt.Sprintf("orgs/%s/actions/variables/%s", orgName, opts.VariableName) - case shared.Environment: - path = fmt.Sprintf("repos/%s/environments/%s/variables/%s", ghrepo.FullName(baseRepo), envName, opts.VariableName) - case shared.Repository: - path = fmt.Sprintf("repos/%s/actions/variables/%s", ghrepo.FullName(baseRepo), opts.VariableName) - } - cfg, err := opts.Config() if err != nil { return err } - host, _ := cfg.Authentication().DefaultHost() + var path string + var host string + switch variableEntity { + case shared.Organization: + path = fmt.Sprintf("orgs/%s/actions/variables/%s", orgName, opts.VariableName) + host, _ = cfg.Authentication().DefaultHost() + case shared.Environment: + path = fmt.Sprintf("repos/%s/environments/%s/variables/%s", ghrepo.FullName(baseRepo), envName, opts.VariableName) + host = baseRepo.RepoHost() + case shared.Repository: + path = fmt.Sprintf("repos/%s/actions/variables/%s", ghrepo.FullName(baseRepo), opts.VariableName) + host = baseRepo.RepoHost() + } var variable shared.Variable if err = client.REST(host, "GET", path, nil, &variable); err != nil { diff --git a/pkg/cmd/variable/get/get_test.go b/pkg/cmd/variable/get/get_test.go index 4a03beb6a..e85910152 100644 --- a/pkg/cmd/variable/get/get_test.go +++ b/pkg/cmd/variable/get/get_test.go @@ -110,6 +110,7 @@ func Test_getRun(t *testing.T) { tests := []struct { name string opts *GetOptions + host string httpStubs func(*httpmock.Registry) jsonFields []string wantOut string @@ -120,8 +121,23 @@ func Test_getRun(t *testing.T) { opts: &GetOptions{ VariableName: "VARIABLE_ONE", }, + host: "github.com", httpStubs: func(reg *httpmock.Registry) { - reg.Register(httpmock.REST("GET", "repos/owner/repo/actions/variables/VARIABLE_ONE"), + reg.Register(httpmock.WithHost(httpmock.REST("GET", "repos/owner/repo/actions/variables/VARIABLE_ONE"), "api.github.com"), + httpmock.JSONResponse(shared.Variable{ + Value: "repo_var", + })) + }, + wantOut: "repo_var\n", + }, + { + name: "getting GHES repo variable", + opts: &GetOptions{ + VariableName: "VARIABLE_ONE", + }, + host: "ghe.io", + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.WithHost(httpmock.REST("GET", "api/v3/repos/owner/repo/actions/variables/VARIABLE_ONE"), "ghe.io"), httpmock.JSONResponse(shared.Variable{ Value: "repo_var", })) @@ -148,8 +164,24 @@ func Test_getRun(t *testing.T) { EnvName: "Development", VariableName: "VARIABLE_ONE", }, + host: "github.com", httpStubs: func(reg *httpmock.Registry) { - reg.Register(httpmock.REST("GET", "repos/owner/repo/environments/Development/variables/VARIABLE_ONE"), + reg.Register(httpmock.WithHost(httpmock.REST("GET", "repos/owner/repo/environments/Development/variables/VARIABLE_ONE"), "api.github.com"), + httpmock.JSONResponse(shared.Variable{ + Value: "env_var", + })) + }, + wantOut: "env_var\n", + }, + { + name: "getting GHES env variable", + opts: &GetOptions{ + EnvName: "Development", + VariableName: "VARIABLE_ONE", + }, + host: "ghe.io", + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.WithHost(httpmock.REST("GET", "api/v3/repos/owner/repo/environments/Development/variables/VARIABLE_ONE"), "ghe.io"), httpmock.JSONResponse(shared.Variable{ Value: "env_var", })) @@ -226,6 +258,9 @@ func Test_getRun(t *testing.T) { tt.opts.IO = ios tt.opts.BaseRepo = func() (ghrepo.Interface, error) { + if tt.host != "" { + return ghrepo.FromFullNameWithHost("owner/repo", tt.host) + } return ghrepo.FromFullName("owner/repo") } tt.opts.HttpClient = func() (*http.Client, error) { diff --git a/pkg/httpmock/stub.go b/pkg/httpmock/stub.go index 147d36fc8..e7534d7f8 100644 --- a/pkg/httpmock/stub.go +++ b/pkg/httpmock/stub.go @@ -123,6 +123,15 @@ func StringResponse(body string) Responder { } } +func WithHost(matcher Matcher, host string) Matcher { + return func(req *http.Request) bool { + if !strings.EqualFold(req.Host, host) { + return false + } + return matcher(req) + } +} + func WithHeader(responder Responder, header string, value string) Responder { return func(req *http.Request) (*http.Response, error) { resp, _ := responder(req) From d7b8ecf33dee8b0582e2514d9821e6225c58ab7c Mon Sep 17 00:00:00 2001 From: Yukai Chou Date: Fri, 2 Aug 2024 03:55:20 +0800 Subject: [PATCH 3/4] Unify use of tab indent in non-test source files Found with rg '(^ | \t|\t )' -g '*.go' -g '!*_test.go' Mixed indent exceptions: - wrapped long list items with extra 2-space indent - code snippets using space indent - commented code lines having "\t*// \t+" prefix --- api/queries_issue.go | 2 +- api/query_builder.go | 8 +- pkg/cmd/attestation/attestation.go | 2 +- .../attestation/trustedroot/trustedroot.go | 24 +++--- pkg/cmd/codespace/ssh.go | 2 +- pkg/cmd/pr/list/list.go | 2 +- pkg/cmd/pr/merge/merge.go | 2 +- pkg/cmd/pr/status/http.go | 34 ++++---- pkg/cmd/release/create/create.go | 2 +- pkg/cmd/repo/create/create.go | 2 +- pkg/cmd/repo/credits/credits.go | 18 ++--- pkg/cmd/root/help_topic.go | 79 +++++++++---------- pkg/cmd/run/download/download.go | 16 ++-- pkg/cmd/run/list/list.go | 2 +- pkg/cmd/search/commits/commits.go | 4 +- pkg/cmd/search/issues/issues.go | 5 +- pkg/cmd/search/prs/prs.go | 4 +- pkg/cmd/search/repos/repos.go | 4 +- pkg/cmd/variable/variable.go | 2 +- pkg/cmd/workflow/run/run.go | 2 +- pkg/cmd/workflow/view/view.go | 8 +- pkg/surveyext/editor.go | 6 +- 22 files changed, 114 insertions(+), 116 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index 62531e84e..094b6b198 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -286,7 +286,7 @@ func IssueStatus(client *Client, repo ghrepo.Interface, options IssueStatusOptio } } } - }` + }` variables := map[string]interface{}{ "owner": repo.RepoOwner(), diff --git a/api/query_builder.go b/api/query_builder.go index 30dbeb869..755179396 100644 --- a/api/query_builder.go +++ b/api/query_builder.go @@ -152,13 +152,13 @@ func StatusCheckRollupGraphQLWithCountByState() string { contexts { checkRunCount, checkRunCountsByState { - state, - count + state, + count }, statusContextCount, statusContextCountsByState { - state, - count + state, + count } } } diff --git a/pkg/cmd/attestation/attestation.go b/pkg/cmd/attestation/attestation.go index ea1e0c08c..a6229e636 100644 --- a/pkg/cmd/attestation/attestation.go +++ b/pkg/cmd/attestation/attestation.go @@ -18,7 +18,7 @@ func NewCmdAttestation(f *cmdutil.Factory) *cobra.Command { Aliases: []string{"at"}, Long: heredoc.Doc(` Download and verify artifact attestations. - `), + `), } root.AddCommand(download.NewDownloadCmd(f, nil)) diff --git a/pkg/cmd/attestation/trustedroot/trustedroot.go b/pkg/cmd/attestation/trustedroot/trustedroot.go index 09ab9a6c9..e28ceab62 100644 --- a/pkg/cmd/attestation/trustedroot/trustedroot.go +++ b/pkg/cmd/attestation/trustedroot/trustedroot.go @@ -32,22 +32,22 @@ func NewTrustedRootCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Com Long: heredoc.Docf(` ### NOTE: This feature is currently in beta, and subject to change. - Output contents for a trusted_root.jsonl file, likely for offline verification. + Output contents for a trusted_root.jsonl file, likely for offline verification. - When using %[1]sgh attestation verify%[1]s, if your machine is on the internet, - this will happen automatically. But to do offline verification, you need to - supply a trusted root file with %[1]s--custom-trusted-root%[1]s; this command - will help you fetch a %[1]strusted_root.jsonl%[1]s file for that purpose. + When using %[1]sgh attestation verify%[1]s, if your machine is on the internet, + this will happen automatically. But to do offline verification, you need to + supply a trusted root file with %[1]s--custom-trusted-root%[1]s; this command + will help you fetch a %[1]strusted_root.jsonl%[1]s file for that purpose. - You can call this command without any flags to get a trusted root file covering - the Sigstore Public Good Instance as well as GitHub's Sigstore instance. + You can call this command without any flags to get a trusted root file covering + the Sigstore Public Good Instance as well as GitHub's Sigstore instance. - Otherwise you can use %[1]s--tuf-url%[1]s to specify the URL of a custom TUF - repository mirror, and %[1]s--tuf-root%[1]s should be the path to the - %[1]sroot.json%[1]s file that you securely obtained out-of-band. + Otherwise you can use %[1]s--tuf-url%[1]s to specify the URL of a custom TUF + repository mirror, and %[1]s--tuf-root%[1]s should be the path to the + %[1]sroot.json%[1]s file that you securely obtained out-of-band. - If you just want to verify the integrity of your local TUF repository, and don't - want the contents of a trusted_root.jsonl file, use %[1]s--verify-only%[1]s. + If you just want to verify the integrity of your local TUF repository, and don't + want the contents of a trusted_root.jsonl file, use %[1]s--verify-only%[1]s. `, "`"), Example: heredoc.Doc(` # Get a trusted_root.jsonl for both Sigstore Public Good and GitHub's instance diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 571f74267..c6c3943e2 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -60,7 +60,7 @@ func newSSHCmd(app *App) *cobra.Command { The %[1]sssh%[1]s command will automatically create a public/private ssh key pair in the %[1]s~/.ssh%[1]s directory if you do not have an existing valid key pair. When selecting the key pair to use, the preferred order is: - + 1. Key specified by %[1]s-i%[1]s in %[1]s%[1]s 2. Automatic key, if it already exists 3. First valid key pair in ssh config (according to %[1]sssh -G%[1]s) diff --git a/pkg/cmd/pr/list/list.go b/pkg/cmd/pr/list/list.go index 7721ecc1b..93a9b8b4b 100644 --- a/pkg/cmd/pr/list/list.go +++ b/pkg/cmd/pr/list/list.go @@ -72,7 +72,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman Find a PR that introduced a given commit $ gh pr list --search "" --state merged - `), + `), Aliases: []string{"ls"}, Args: cmdutil.NoArgsQuoteReminder, RunE: func(cmd *cobra.Command, args []string) error { diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index 1e7deabcb..e3d1a14ee 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -88,7 +88,7 @@ func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Comm If required checks have not yet passed, auto-merge will be enabled. If required checks have passed, the pull request will be added to the merge queue. To bypass a merge queue and merge directly, pass the %[1]s--admin%[1]s flag. - `, "`"), + `, "`"), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { opts.Finder = shared.NewFinder(f) diff --git a/pkg/cmd/pr/status/http.go b/pkg/cmd/pr/status/http.go index bd7eb6a78..1eb12ed74 100644 --- a/pkg/cmd/pr/status/http.go +++ b/pkg/cmd/pr/status/http.go @@ -101,23 +101,23 @@ func pullRequestStatus(httpClient *http.Client, repo ghrepo.Interface, options r } query := fragments + queryPrefix + ` - viewerCreated: search(query: $viewerQuery, type: ISSUE, first: $per_page) { - totalCount: issueCount - edges { - node { - ...prWithReviews - } - } - } - reviewRequested: search(query: $reviewerQuery, type: ISSUE, first: $per_page) { - totalCount: issueCount - edges { - node { - ...pr - } - } - } - } + viewerCreated: search(query: $viewerQuery, type: ISSUE, first: $per_page) { + totalCount: issueCount + edges { + node { + ...prWithReviews + } + } + } + reviewRequested: search(query: $reviewerQuery, type: ISSUE, first: $per_page) { + totalCount: issueCount + edges { + node { + ...pr + } + } + } + } ` currentUsername := options.Username diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index 1cfc5ba06..95453250e 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -115,7 +115,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co Use release notes from a file $ gh release create v1.2.3 -F changelog.md - + Don't mark the release as latest $ gh release create v1.2.3 --latest=false diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index 05e8909e1..9074ea117 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -88,7 +88,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co To create a remote repository non-interactively, supply the repository name and one of %[1]s--public%[1]s, %[1]s--private%[1]s, or %[1]s--internal%[1]s. Pass %[1]s--clone%[1]s to clone the new repository locally. - If the %[1]sOWNER/%[1]s portion of the %[1]sOWNER/REPO%[1]s name argument is omitted, it + If the %[1]sOWNER/%[1]s portion of the %[1]sOWNER/REPO%[1]s name argument is omitted, it defaults to the name of the authenticating user. To create a remote repository from an existing local repository, specify the source directory with %[1]s--source%[1]s. diff --git a/pkg/cmd/repo/credits/credits.go b/pkg/cmd/repo/credits/credits.go index 4b7b9fb28..7db08a231 100644 --- a/pkg/cmd/repo/credits/credits.go +++ b/pkg/cmd/repo/credits/credits.go @@ -79,18 +79,18 @@ func NewCmdRepoCredits(f *cmdutil.Factory, runF func(*CreditsOptions) error) *co Use: "credits []", Short: "View credits for a repository", Example: heredoc.Doc(` - # view credits for the current repository - $ gh repo credits + # view credits for the current repository + $ gh repo credits - # view credits for a specific repository - $ gh repo credits cool/repo + # view credits for a specific repository + $ gh repo credits cool/repo - # print a non-animated thank you - $ gh repo credits -s + # print a non-animated thank you + $ gh repo credits -s - # pipe to just print the contributors, one per line - $ gh repo credits | cat - `), + # pipe to just print the contributors, one per line + $ gh repo credits | cat + `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { diff --git a/pkg/cmd/root/help_topic.go b/pkg/cmd/root/help_topic.go index 4786a993b..3da5bf476 100644 --- a/pkg/cmd/root/help_topic.go +++ b/pkg/cmd/root/help_topic.go @@ -158,25 +158,25 @@ var HelpTopics = []helpTopic{ $ gh pr list --json number,title,author [ { - "author": { - "login": "monalisa" - }, - "number": 123, - "title": "A helpful contribution" + "author": { + "login": "monalisa" + }, + "number": 123, + "title": "A helpful contribution" }, { - "author": { - "login": "codercat" - }, - "number": 124, - "title": "Improve the docs" + "author": { + "login": "codercat" + }, + "number": 124, + "title": "Improve the docs" }, { - "author": { - "login": "cli-maintainer" - }, - "number": 125, - "title": "An exciting new feature" + "author": { + "login": "cli-maintainer" + }, + "number": 125, + "title": "An exciting new feature" } ] @@ -193,32 +193,31 @@ var HelpTopics = []helpTopic{ | map(.labels = (.labels | map(.name))) # show only the label names | .[:3] # select the first 3 results' [ - { - "labels": [ - "enhancement", - "needs triage" - ], - "number": 123, - "title": "A helpful contribution" - }, - { - "labels": [ - "help wanted", - "docs", - "good first issue" - ], - "number": 125, - "title": "Improve the docs" - }, - { - "labels": [ - "enhancement", - ], - "number": 7221, - "title": "An exciting new feature" - } + { + "labels": [ + "enhancement", + "needs triage" + ], + "number": 123, + "title": "A helpful contribution" + }, + { + "labels": [ + "help wanted", + "docs", + "good first issue" + ], + "number": 125, + "title": "Improve the docs" + }, + { + "labels": [ + "enhancement", + ], + "number": 7221, + "title": "An exciting new feature" + } ] - # using the --template flag with the hyperlink helper gh issue list --json title,url --template '{{range .}}{{hyperlink .url .title}}{{"\n"}}{{end}}' diff --git a/pkg/cmd/run/download/download.go b/pkg/cmd/run/download/download.go index 993e16e52..18a10df80 100644 --- a/pkg/cmd/run/download/download.go +++ b/pkg/cmd/run/download/download.go @@ -51,17 +51,17 @@ func NewCmdDownload(f *cmdutil.Factory, runF func(*DownloadOptions) error) *cobr `), Args: cobra.MaximumNArgs(1), Example: heredoc.Doc(` - # Download all artifacts generated by a workflow run - $ gh run download + # Download all artifacts generated by a workflow run + $ gh run download - # Download a specific artifact within a run - $ gh run download -n + # Download a specific artifact within a run + $ gh run download -n - # Download specific artifacts across all runs in a repository - $ gh run download -n -n + # Download specific artifacts across all runs in a repository + $ gh run download -n -n - # Select artifacts to download interactively - $ gh run download + # Select artifacts to download interactively + $ gh run download `), RunE: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { diff --git a/pkg/cmd/run/list/list.go b/pkg/cmd/run/list/list.go index 21e6c7faf..793bfbb85 100644 --- a/pkg/cmd/run/list/list.go +++ b/pkg/cmd/run/list/list.go @@ -58,7 +58,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman Use: "list", Short: "List recent workflow runs", Long: heredoc.Docf(` - List recent workflow runs. + List recent workflow runs. Note that providing the %[1]sworkflow_name%[1]s to the %[1]s-w%[1]s flag will not fetch disabled workflows. Also pass the %[1]s-a%[1]s flag to fetch disabled workflow runs using the %[1]sworkflow_name%[1]s and the %[1]s-w%[1]s flag. diff --git a/pkg/cmd/search/commits/commits.go b/pkg/cmd/search/commits/commits.go index 698e933bb..6a9f58b86 100644 --- a/pkg/cmd/search/commits/commits.go +++ b/pkg/cmd/search/commits/commits.go @@ -45,7 +45,7 @@ func NewCmdCommits(f *cmdutil.Factory, runF func(*CommitsOptions) error) *cobra. GitHub search syntax is documented at: - `), + `), Example: heredoc.Doc(` # search commits matching set of keywords "readme" and "typo" $ gh search commits readme typo @@ -64,7 +64,7 @@ func NewCmdCommits(f *cmdutil.Factory, runF func(*CommitsOptions) error) *cobra. # search commits authored before February 1st, 2022 $ gh search commits --author-date="<2022-02-01" - `), + `), RunE: func(c *cobra.Command, args []string) error { if len(args) == 0 && c.Flags().NFlag() == 0 { return cmdutil.FlagErrorf("specify search keywords or flags") diff --git a/pkg/cmd/search/issues/issues.go b/pkg/cmd/search/issues/issues.go index ec7909747..a8be32f71 100644 --- a/pkg/cmd/search/issues/issues.go +++ b/pkg/cmd/search/issues/issues.go @@ -34,7 +34,7 @@ func NewCmdIssues(f *cmdutil.Factory, runF func(*shared.IssuesOptions) error) *c GitHub search syntax is documented at: - `), + `), Example: heredoc.Doc(` # search issues matching set of keywords "readme" and "typo" $ gh search issues readme typo @@ -56,8 +56,7 @@ func NewCmdIssues(f *cmdutil.Factory, runF func(*shared.IssuesOptions) error) *c # search issues only from un-archived repositories (default is all repositories) $ gh search issues --owner github --archived=false - - `), + `), RunE: func(c *cobra.Command, args []string) error { if len(args) == 0 && c.Flags().NFlag() == 0 { return cmdutil.FlagErrorf("specify search keywords or flags") diff --git a/pkg/cmd/search/prs/prs.go b/pkg/cmd/search/prs/prs.go index 5da7b18f7..32565a580 100644 --- a/pkg/cmd/search/prs/prs.go +++ b/pkg/cmd/search/prs/prs.go @@ -36,7 +36,7 @@ func NewCmdPrs(f *cmdutil.Factory, runF func(*shared.IssuesOptions) error) *cobr GitHub search syntax is documented at: - `), + `), Example: heredoc.Doc(` # search pull requests matching set of keywords "fix" and "bug" $ gh search prs fix bug @@ -58,7 +58,7 @@ func NewCmdPrs(f *cmdutil.Factory, runF func(*shared.IssuesOptions) error) *cobr # search pull requests only from un-archived repositories (default is all repositories) $ gh search prs --owner github --archived=false - `), + `), RunE: func(c *cobra.Command, args []string) error { if len(args) == 0 && c.Flags().NFlag() == 0 { return cmdutil.FlagErrorf("specify search keywords or flags") diff --git a/pkg/cmd/search/repos/repos.go b/pkg/cmd/search/repos/repos.go index 26bee450e..88b8db13f 100644 --- a/pkg/cmd/search/repos/repos.go +++ b/pkg/cmd/search/repos/repos.go @@ -46,7 +46,7 @@ func NewCmdRepos(f *cmdutil.Factory, runF func(*ReposOptions) error) *cobra.Comm GitHub search syntax is documented at: - `), + `), Example: heredoc.Doc(` # search repositories matching set of keywords "cli" and "shell" $ gh search repos cli shell @@ -68,7 +68,7 @@ func NewCmdRepos(f *cmdutil.Factory, runF func(*ReposOptions) error) *cobra.Comm # search repositories excluding archived repositories $ gh search repos --archived=false - `), + `), RunE: func(c *cobra.Command, args []string) error { if len(args) == 0 && c.Flags().NFlag() == 0 { return cmdutil.FlagErrorf("specify search keywords or flags") diff --git a/pkg/cmd/variable/variable.go b/pkg/cmd/variable/variable.go index 15ff38744..920268c41 100644 --- a/pkg/cmd/variable/variable.go +++ b/pkg/cmd/variable/variable.go @@ -17,7 +17,7 @@ func NewCmdVariable(f *cmdutil.Factory) *cobra.Command { Long: heredoc.Docf(` Variables can be set at the repository, environment or organization level for use in GitHub Actions or Dependabot. Run %[1]sgh help variable set%[1]s to learn how to get started. - `, "`"), + `, "`"), } cmdutil.EnableRepoOverride(cmd, f) diff --git a/pkg/cmd/workflow/run/run.go b/pkg/cmd/workflow/run/run.go index 19c650984..b7ab53098 100644 --- a/pkg/cmd/workflow/run/run.go +++ b/pkg/cmd/workflow/run/run.go @@ -64,7 +64,7 @@ func NewCmdRun(f *cmdutil.Factory, runF func(*RunOptions) error) *cobra.Command - Interactively - Via %[1]s-f/--raw-field%[1]s or %[1]s-F/--field%[1]s flags - As JSON, via standard input - `, "`"), + `, "`"), Example: heredoc.Doc(` # Have gh prompt you for what workflow you'd like to run and interactively collect inputs $ gh workflow run diff --git a/pkg/cmd/workflow/view/view.go b/pkg/cmd/workflow/view/view.go index 28e561766..188d79e22 100644 --- a/pkg/cmd/workflow/view/view.go +++ b/pkg/cmd/workflow/view/view.go @@ -56,11 +56,11 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman Short: "View the summary of a workflow", Args: cobra.MaximumNArgs(1), Example: heredoc.Doc(` - # Interactively select a workflow to view - $ gh workflow view + # Interactively select a workflow to view + $ gh workflow view - # View a specific workflow - $ gh workflow view 0451 + # View a specific workflow + $ gh workflow view 0451 `), RunE: func(cmd *cobra.Command, args []string) error { // support `-R, --repo` override diff --git a/pkg/surveyext/editor.go b/pkg/surveyext/editor.go index 420ecc4e5..82533f913 100644 --- a/pkg/surveyext/editor.go +++ b/pkg/surveyext/editor.go @@ -46,10 +46,10 @@ var EditorQuestionTemplate = ` {{- color .Config.Icons.Question.Format }}{{ .Config.Icons.Question.Text }} {{color "reset"}} {{- color "default+hb"}}{{ .Message }} {{color "reset"}} {{- if .ShowAnswer}} - {{- color "cyan"}}{{.Answer}}{{color "reset"}}{{"\n"}} + {{- color "cyan"}}{{.Answer}}{{color "reset"}}{{"\n"}} {{- else }} - {{- if and .Help (not .ShowHelp)}}{{color "cyan"}}[{{ .Config.HelpInput }} for help]{{color "reset"}} {{end}} - {{- if and .Default (not .HideDefault)}}{{color "white"}}({{.Default}}) {{color "reset"}}{{end}} + {{- if and .Help (not .ShowHelp)}}{{color "cyan"}}[{{ .Config.HelpInput }} for help]{{color "reset"}} {{end}} + {{- if and .Default (not .HideDefault)}}{{color "white"}}({{.Default}}) {{color "reset"}}{{end}} {{- color "cyan"}}[(e) to launch {{ .EditorCommand }}{{- if .BlankAllowed }}, enter to skip{{ end }}] {{color "reset"}} {{- end}}` From c088951944082eb4c474097edb1861ed07d6c856 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Fri, 2 Aug 2024 15:11:22 -0400 Subject: [PATCH 4/4] Fix host handling in variable and secret delete This change updates `gh variable delete` and `gh secret delete` to use the host associated with the repository rather than the default host. Along with these changes is minor refactoring of the tests to ensure appropriate Dotcom vs GHES targeting works as expected. Minor edit to `gh variable get` testing to simplify some conditional logic in test setup. --- pkg/cmd/secret/delete/delete.go | 29 +++--- pkg/cmd/secret/delete/delete_test.go | 127 ++++++++++++++++++------- pkg/cmd/variable/delete/delete.go | 24 ++--- pkg/cmd/variable/delete/delete_test.go | 60 +++++++++--- pkg/cmd/variable/get/get_test.go | 17 ++-- 5 files changed, 176 insertions(+), 81 deletions(-) diff --git a/pkg/cmd/secret/delete/delete.go b/pkg/cmd/secret/delete/delete.go index 9a299ce4f..564e8633f 100644 --- a/pkg/cmd/secret/delete/delete.go +++ b/pkg/cmd/secret/delete/delete.go @@ -105,24 +105,27 @@ func removeRun(opts *DeleteOptions) error { } } - var path string - switch secretEntity { - case shared.Organization: - path = fmt.Sprintf("orgs/%s/%s/secrets/%s", orgName, secretApp, opts.SecretName) - case shared.Environment: - path = fmt.Sprintf("repos/%s/environments/%s/secrets/%s", ghrepo.FullName(baseRepo), envName, opts.SecretName) - case shared.User: - path = fmt.Sprintf("user/codespaces/secrets/%s", opts.SecretName) - case shared.Repository: - path = fmt.Sprintf("repos/%s/%s/secrets/%s", ghrepo.FullName(baseRepo), secretApp, opts.SecretName) - } - cfg, err := opts.Config() if err != nil { return err } - host, _ := cfg.Authentication().DefaultHost() + var path string + var host string + switch secretEntity { + case shared.Organization: + path = fmt.Sprintf("orgs/%s/%s/secrets/%s", orgName, secretApp, opts.SecretName) + host, _ = cfg.Authentication().DefaultHost() + case shared.Environment: + path = fmt.Sprintf("repos/%s/environments/%s/secrets/%s", ghrepo.FullName(baseRepo), envName, opts.SecretName) + host = baseRepo.RepoHost() + case shared.User: + path = fmt.Sprintf("user/codespaces/secrets/%s", opts.SecretName) + host, _ = cfg.Authentication().DefaultHost() + case shared.Repository: + path = fmt.Sprintf("repos/%s/%s/secrets/%s", ghrepo.FullName(baseRepo), secretApp, opts.SecretName) + host = baseRepo.RepoHost() + } err = client.REST(host, "DELETE", path, nil, nil) if err != nil { diff --git a/pkg/cmd/secret/delete/delete_test.go b/pkg/cmd/secret/delete/delete_test.go index 9d2c078c7..bd5053f4d 100644 --- a/pkg/cmd/secret/delete/delete_test.go +++ b/pkg/cmd/secret/delete/delete_test.go @@ -13,6 +13,7 @@ import ( "github.com/cli/cli/v2/pkg/iostreams" "github.com/google/shlex" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNewCmdDelete(t *testing.T) { @@ -121,9 +122,10 @@ func TestNewCmdDelete(t *testing.T) { func Test_removeRun_repo(t *testing.T) { tests := []struct { - name string - opts *DeleteOptions - wantPath string + name string + opts *DeleteOptions + host string + httpStubs func(*httpmock.Registry) }{ { name: "Actions", @@ -131,7 +133,21 @@ func Test_removeRun_repo(t *testing.T) { Application: "actions", SecretName: "cool_secret", }, - wantPath: "repos/owner/repo/actions/secrets/cool_secret", + host: "github.com", + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "repos/owner/repo/actions/secrets/cool_secret"), "api.github.com"), httpmock.StatusStringResponse(204, "No Content")) + }, + }, + { + name: "Actions GHES", + opts: &DeleteOptions{ + Application: "actions", + SecretName: "cool_secret", + }, + host: "example.com", + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "api/v3/repos/owner/repo/actions/secrets/cool_secret"), "example.com"), httpmock.StatusStringResponse(204, "No Content")) + }, }, { name: "Dependabot", @@ -139,23 +155,38 @@ func Test_removeRun_repo(t *testing.T) { Application: "dependabot", SecretName: "cool_dependabot_secret", }, - wantPath: "repos/owner/repo/dependabot/secrets/cool_dependabot_secret", + host: "github.com", + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "repos/owner/repo/dependabot/secrets/cool_dependabot_secret"), "api.github.com"), httpmock.StatusStringResponse(204, "No Content")) + }, + }, + { + name: "Dependabot GHES", + opts: &DeleteOptions{ + Application: "dependabot", + SecretName: "cool_dependabot_secret", + }, + host: "example.com", + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "api/v3/repos/owner/repo/dependabot/secrets/cool_dependabot_secret"), "example.com"), httpmock.StatusStringResponse(204, "No Content")) + }, }, { name: "defaults to Actions", opts: &DeleteOptions{ SecretName: "cool_secret", }, - wantPath: "repos/owner/repo/actions/secrets/cool_secret", + host: "github.com", + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "repos/owner/repo/actions/secrets/cool_secret"), "api.github.com"), httpmock.StatusStringResponse(204, "No Content")) + }, }, } for _, tt := range tests { reg := &httpmock.Registry{} - - reg.Register( - httpmock.REST("DELETE", tt.wantPath), - httpmock.StatusStringResponse(204, "No Content")) + tt.httpStubs(reg) + defer reg.Verify(t) ios, _, _, _ := iostreams.Test() @@ -167,44 +198,70 @@ func Test_removeRun_repo(t *testing.T) { return config.NewBlankConfig(), nil } tt.opts.BaseRepo = func() (ghrepo.Interface, error) { - return ghrepo.FromFullName("owner/repo") + return ghrepo.FromFullNameWithHost("owner/repo", tt.host) } err := removeRun(tt.opts) assert.NoError(t, err) - - reg.Verify(t) } } func Test_removeRun_env(t *testing.T) { - reg := &httpmock.Registry{} - - reg.Register( - httpmock.REST("DELETE", "repos/owner/repo/environments/development/secrets/cool_secret"), - httpmock.StatusStringResponse(204, "No Content")) - - ios, _, _, _ := iostreams.Test() - - opts := &DeleteOptions{ - IO: ios, - HttpClient: func() (*http.Client, error) { - return &http.Client{Transport: reg}, nil + tests := []struct { + name string + opts *DeleteOptions + host string + httpStubs func(*httpmock.Registry) + }{ + { + name: "delete environment secret", + opts: &DeleteOptions{ + SecretName: "cool_secret", + EnvName: "development", + }, + host: "github.com", + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "repos/owner/repo/environments/development/secrets/cool_secret"), "api.github.com"), + httpmock.StatusStringResponse(204, "No Content")) + }, }, - Config: func() (gh.Config, error) { - return config.NewBlankConfig(), nil + { + name: "delete environment secret GHES", + opts: &DeleteOptions{ + SecretName: "cool_secret", + EnvName: "development", + }, + host: "example.com", + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "api/v3/repos/owner/repo/environments/development/secrets/cool_secret"), "example.com"), + httpmock.StatusStringResponse(204, "No Content")) + }, }, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.FromFullName("owner/repo") - }, - SecretName: "cool_secret", - EnvName: "development", } - err := removeRun(opts) - assert.NoError(t, err) + for _, tt := range tests { + reg := &httpmock.Registry{} + tt.httpStubs(reg) + defer reg.Verify(t) - reg.Verify(t) + ios, _, _, _ := iostreams.Test() + tt.opts.IO = ios + tt.opts.BaseRepo = func() (ghrepo.Interface, error) { + if tt.host != "" { + return ghrepo.FromFullNameWithHost("owner/repo", tt.host) + } + 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 + } + + err := removeRun(tt.opts) + require.NoError(t, err) + } } func Test_removeRun_org(t *testing.T) { diff --git a/pkg/cmd/variable/delete/delete.go b/pkg/cmd/variable/delete/delete.go index 3617f3188..d51320167 100644 --- a/pkg/cmd/variable/delete/delete.go +++ b/pkg/cmd/variable/delete/delete.go @@ -91,22 +91,24 @@ func removeRun(opts *DeleteOptions) error { } } - var path string - switch variableEntity { - case shared.Organization: - path = fmt.Sprintf("orgs/%s/actions/variables/%s", orgName, opts.VariableName) - case shared.Environment: - path = fmt.Sprintf("repos/%s/environments/%s/variables/%s", ghrepo.FullName(baseRepo), envName, opts.VariableName) - case shared.Repository: - path = fmt.Sprintf("repos/%s/actions/variables/%s", ghrepo.FullName(baseRepo), opts.VariableName) - } - cfg, err := opts.Config() if err != nil { return err } - host, _ := cfg.Authentication().DefaultHost() + var path string + var host string + switch variableEntity { + case shared.Organization: + path = fmt.Sprintf("orgs/%s/actions/variables/%s", orgName, opts.VariableName) + host, _ = cfg.Authentication().DefaultHost() + case shared.Environment: + path = fmt.Sprintf("repos/%s/environments/%s/variables/%s", ghrepo.FullName(baseRepo), envName, opts.VariableName) + host = baseRepo.RepoHost() + case shared.Repository: + path = fmt.Sprintf("repos/%s/actions/variables/%s", ghrepo.FullName(baseRepo), opts.VariableName) + host = baseRepo.RepoHost() + } err = client.REST(host, "DELETE", path, nil, nil) if err != nil { diff --git a/pkg/cmd/variable/delete/delete_test.go b/pkg/cmd/variable/delete/delete_test.go index e659f96a6..d00bef8fb 100644 --- a/pkg/cmd/variable/delete/delete_test.go +++ b/pkg/cmd/variable/delete/delete_test.go @@ -13,6 +13,7 @@ import ( "github.com/cli/cli/v2/pkg/iostreams" "github.com/google/shlex" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNewCmdDelete(t *testing.T) { @@ -87,16 +88,32 @@ func TestNewCmdDelete(t *testing.T) { func TestRemoveRun(t *testing.T) { tests := []struct { - name string - opts *DeleteOptions - wantPath string + name string + opts *DeleteOptions + host string + httpStubs func(*httpmock.Registry) }{ { name: "repo", opts: &DeleteOptions{ VariableName: "cool_variable", }, - wantPath: "repos/owner/repo/actions/variables/cool_variable", + host: "github.com", + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "repos/owner/repo/actions/variables/cool_variable"), "api.github.com"), + httpmock.StatusStringResponse(204, "No Content")) + }, + }, + { + name: "repo GHES", + opts: &DeleteOptions{ + VariableName: "cool_variable", + }, + host: "example.com", + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "api/v3/repos/owner/repo/actions/variables/cool_variable"), "example.com"), + httpmock.StatusStringResponse(204, "No Content")) + }, }, { name: "env", @@ -104,7 +121,23 @@ func TestRemoveRun(t *testing.T) { VariableName: "cool_variable", EnvName: "development", }, - wantPath: "repos/owner/repo/environments/development/variables/cool_variable", + host: "github.com", + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "repos/owner/repo/environments/development/variables/cool_variable"), "api.github.com"), + httpmock.StatusStringResponse(204, "No Content")) + }, + }, + { + name: "env GHES", + opts: &DeleteOptions{ + VariableName: "cool_variable", + EnvName: "development", + }, + host: "example.com", + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "api/v3/repos/owner/repo/environments/development/variables/cool_variable"), "example.com"), + httpmock.StatusStringResponse(204, "No Content")) + }, }, { name: "org", @@ -112,35 +145,34 @@ func TestRemoveRun(t *testing.T) { VariableName: "cool_variable", OrgName: "UmbrellaCorporation", }, - wantPath: "orgs/UmbrellaCorporation/actions/variables/cool_variable", + host: "github.com", + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "orgs/UmbrellaCorporation/actions/variables/cool_variable"), "api.github.com"), + httpmock.StatusStringResponse(204, "No Content")) + }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { reg := &httpmock.Registry{} + tt.httpStubs(reg) defer reg.Verify(t) - 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") + return ghrepo.FromFullNameWithHost("owner/repo", tt.host) } err := removeRun(tt.opts) - assert.NoError(t, err) + require.NoError(t, err) }) } } diff --git a/pkg/cmd/variable/get/get_test.go b/pkg/cmd/variable/get/get_test.go index e85910152..82b602c9e 100644 --- a/pkg/cmd/variable/get/get_test.go +++ b/pkg/cmd/variable/get/get_test.go @@ -135,9 +135,9 @@ func Test_getRun(t *testing.T) { opts: &GetOptions{ VariableName: "VARIABLE_ONE", }, - host: "ghe.io", + host: "example.com", httpStubs: func(reg *httpmock.Registry) { - reg.Register(httpmock.WithHost(httpmock.REST("GET", "api/v3/repos/owner/repo/actions/variables/VARIABLE_ONE"), "ghe.io"), + reg.Register(httpmock.WithHost(httpmock.REST("GET", "api/v3/repos/owner/repo/actions/variables/VARIABLE_ONE"), "example.com"), httpmock.JSONResponse(shared.Variable{ Value: "repo_var", })) @@ -150,6 +150,7 @@ func Test_getRun(t *testing.T) { OrgName: "TestOrg", VariableName: "VARIABLE_ONE", }, + host: "github.com", httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", "orgs/TestOrg/actions/variables/VARIABLE_ONE"), httpmock.JSONResponse(shared.Variable{ @@ -179,9 +180,9 @@ func Test_getRun(t *testing.T) { EnvName: "Development", VariableName: "VARIABLE_ONE", }, - host: "ghe.io", + host: "example.com", httpStubs: func(reg *httpmock.Registry) { - reg.Register(httpmock.WithHost(httpmock.REST("GET", "api/v3/repos/owner/repo/environments/Development/variables/VARIABLE_ONE"), "ghe.io"), + reg.Register(httpmock.WithHost(httpmock.REST("GET", "api/v3/repos/owner/repo/environments/Development/variables/VARIABLE_ONE"), "example.com"), httpmock.JSONResponse(shared.Variable{ Value: "env_var", })) @@ -193,6 +194,7 @@ func Test_getRun(t *testing.T) { opts: &GetOptions{ VariableName: "VARIABLE_ONE", }, + host: "github.com", jsonFields: []string{"name", "value", "visibility", "updatedAt", "createdAt", "numSelectedRepos", "selectedReposURL"}, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", "repos/owner/repo/actions/variables/VARIABLE_ONE"), @@ -225,6 +227,7 @@ func Test_getRun(t *testing.T) { opts: &GetOptions{ VariableName: "VARIABLE_ONE", }, + host: "github.com", httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", "repos/owner/repo/actions/variables/VARIABLE_ONE"), httpmock.StatusStringResponse(404, "not found"), @@ -237,6 +240,7 @@ func Test_getRun(t *testing.T) { opts: &GetOptions{ VariableName: "VARIABLE_ONE", }, + host: "github.com", httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", "repos/owner/repo/actions/variables/VARIABLE_ONE"), httpmock.StatusStringResponse(400, "not found"), @@ -258,10 +262,7 @@ func Test_getRun(t *testing.T) { tt.opts.IO = ios tt.opts.BaseRepo = func() (ghrepo.Interface, error) { - if tt.host != "" { - return ghrepo.FromFullNameWithHost("owner/repo", tt.host) - } - return ghrepo.FromFullName("owner/repo") + return ghrepo.FromFullNameWithHost("owner/repo", tt.host) } tt.opts.HttpClient = func() (*http.Client, error) { return &http.Client{Transport: reg}, nil