diff --git a/api/client.go b/api/client.go index 5d8265ab5..21cb3febc 100644 --- a/api/client.go +++ b/api/client.go @@ -194,8 +194,7 @@ func graphQLClient(h *http.Client, hostname string) *graphql.Client { // REST performs a REST request and parses the response. func (c Client) REST(hostname string, method string, p string, body io.Reader, data interface{}) error { - url := ghinstance.RESTPrefix(hostname) + p - req, err := http.NewRequest(method, url, body) + req, err := http.NewRequest(method, restURL(hostname, p), body) if err != nil { return err } @@ -230,6 +229,13 @@ func (c Client) REST(hostname string, method string, p string, body io.Reader, d return nil } +func restURL(hostname string, pathOrURL string) string { + if strings.HasPrefix(pathOrURL, "https://") || strings.HasPrefix(pathOrURL, "http://") { + return pathOrURL + } + return ghinstance.RESTPrefix(hostname) + pathOrURL +} + func handleResponse(resp *http.Response, data interface{}) error { success := resp.StatusCode >= 200 && resp.StatusCode < 300 diff --git a/api/client_test.go b/api/client_test.go index 29da3363a..df0fdf8e2 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -80,6 +80,26 @@ func TestRESTGetDelete(t *testing.T) { assert.NoError(t, err) } +func TestRESTWithFullURL(t *testing.T) { + http := &httpmock.Registry{} + client := NewClient(ReplaceTripper(http)) + + http.Register( + httpmock.REST("GET", "api/v3/user/repos"), + httpmock.StatusStringResponse(200, "{}")) + http.Register( + httpmock.REST("GET", "user/repos"), + httpmock.StatusStringResponse(200, "{}")) + + err := client.REST("example.com", "GET", "user/repos", nil, nil) + assert.NoError(t, err) + err = client.REST("example.com", "GET", "https://another.net/user/repos", nil, nil) + assert.NoError(t, err) + + assert.Equal(t, "example.com", http.Requests[0].URL.Hostname()) + assert.Equal(t, "another.net", http.Requests[1].URL.Hostname()) +} + func TestRESTError(t *testing.T) { fakehttp := &httpmock.Registry{} client := NewClient(ReplaceTripper(fakehttp)) diff --git a/pkg/cmd/run/shared/shared.go b/pkg/cmd/run/shared/shared.go index 9f3dcf2e2..d0e2c131d 100644 --- a/pkg/cmd/run/shared/shared.go +++ b/pkg/cmd/run/shared/shared.go @@ -243,13 +243,7 @@ type JobsPayload struct { func GetJobs(client *api.Client, repo ghrepo.Interface, run Run) ([]Job, error) { var result JobsPayload - parsed, err := url.Parse(run.JobsURL) - if err != nil { - return nil, err - } - - err = client.REST(repo.RepoHost(), "GET", parsed.Path[1:], nil, &result) - if err != nil { + if err := client.REST(repo.RepoHost(), "GET", run.JobsURL, nil, &result); err != nil { return nil, err } return result.Jobs, nil diff --git a/pkg/cmd/run/shared/test.go b/pkg/cmd/run/shared/test.go index f70370a54..64f1c801a 100644 --- a/pkg/cmd/run/shared/test.go +++ b/pkg/cmd/run/shared/test.go @@ -26,12 +26,12 @@ func TestRun(name string, id int64, s Status, c Conclusion) Run { Conclusion: c, Event: "push", HeadBranch: "trunk", - JobsURL: fmt.Sprintf("/runs/%d/jobs", id), + JobsURL: fmt.Sprintf("https://api.github.com/runs/%d/jobs", id), HeadCommit: Commit{ Message: "cool commit", }, HeadSha: "1234567890", - URL: fmt.Sprintf("runs/%d", id), + URL: fmt.Sprintf("https://github.com/runs/%d", id), HeadRepository: Repo{ Owner: struct{ Login string }{Login: "OWNER"}, Name: "REPO", @@ -68,7 +68,7 @@ var SuccessfulJob Job = Job{ Name: "cool job", StartedAt: created(), CompletedAt: updated(), - URL: "jobs/10", + URL: "https://github.com/jobs/10", RunID: 3, Steps: []Step{ { @@ -93,7 +93,7 @@ var FailedJob Job = Job{ Name: "sad job", StartedAt: created(), CompletedAt: updated(), - URL: "jobs/20", + URL: "https://github.com/jobs/20", RunID: 1234, Steps: []Step{ { diff --git a/pkg/cmd/run/view/view_test.go b/pkg/cmd/run/view/view_test.go index 8ddde1cd0..807481ca3 100644 --- a/pkg/cmd/run/view/view_test.go +++ b/pkg/cmd/run/view/view_test.go @@ -207,7 +207,7 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/check-runs/10/annotations"), httpmock.JSONResponse([]shared.Annotation{})) }, - wantOut: "\n✓ trunk successful #2898 · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n\nFor more information about a job, try: gh run view --job=\nView this run on GitHub: runs/3\n", + wantOut: "\n✓ trunk successful #2898 · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n\nFor more information about a job, try: gh run view --job=\nView this run on GitHub: https://github.com/runs/3\n", }, { name: "exit status, failed run", @@ -236,7 +236,7 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/check-runs/20/annotations"), httpmock.JSONResponse(shared.FailedJobAnnotations)) }, - wantOut: "\nX trunk failed · 1234\nTriggered via push about 59 minutes ago\n\nJOBS\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nX the job is sad\nsad job: blaze.py#420\n\n\nTo see what failed, try: gh run view 1234 --log-failed\nView this run on GitHub: runs/1234\n", + wantOut: "\nX trunk failed · 1234\nTriggered via push about 59 minutes ago\n\nJOBS\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nX the job is sad\nsad job: blaze.py#420\n\n\nTo see what failed, try: gh run view 1234 --log-failed\nView this run on GitHub: https://github.com/runs/1234\n", wantErr: true, }, { @@ -278,7 +278,7 @@ func TestViewRun(t *testing.T) { artifact-3 For more information about a job, try: gh run view --job= - View this run on GitHub: runs/3 + View this run on GitHub: https://github.com/runs/3 `), }, { @@ -308,7 +308,7 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/check-runs/10/annotations"), httpmock.JSONResponse([]shared.Annotation{})) }, - wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n\nFor more information about a job, try: gh run view --job=\nView this run on GitHub: runs/3\n", + wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n\nFor more information about a job, try: gh run view --job=\nView this run on GitHub: https://github.com/runs/3\n", }, { name: "verbose", @@ -343,7 +343,7 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/check-runs/20/annotations"), httpmock.JSONResponse(shared.FailedJobAnnotations)) }, - wantOut: "\nX trunk failed · 1234\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nX the job is sad\nsad job: blaze.py#420\n\n\nTo see what failed, try: gh run view 1234 --log-failed\nView this run on GitHub: runs/1234\n", + wantOut: "\nX trunk failed · 1234\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nX the job is sad\nsad job: blaze.py#420\n\n\nTo see what failed, try: gh run view 1234 --log-failed\nView this run on GitHub: https://github.com/runs/1234\n", }, { name: "prompts for choice, one job", @@ -380,7 +380,7 @@ func TestViewRun(t *testing.T) { opts: &ViewOptions{ Prompt: true, }, - wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n\nFor more information about a job, try: gh run view --job=\nView this run on GitHub: runs/3\n", + wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n\nFor more information about a job, try: gh run view --job=\nView this run on GitHub: https://github.com/runs/3\n", }, { name: "interactive with log", @@ -664,7 +664,7 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/check-runs/10/annotations"), httpmock.JSONResponse([]shared.Annotation{})) }, - wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\n\nTo see the full job log, try: gh run view --log --job=10\nView this run on GitHub: runs/3\n", + wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\n\nTo see the full job log, try: gh run view --log --job=10\nView this run on GitHub: https://github.com/runs/3\n", }, { name: "interactive, multiple jobs, choose all jobs", @@ -703,7 +703,7 @@ func TestViewRun(t *testing.T) { as.StubOne(2) as.StubOne(0) }, - wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nX the job is sad\nsad job: blaze.py#420\n\n\nFor more information about a job, try: gh run view --job=\nView this run on GitHub: runs/3\n", + wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nX the job is sad\nsad job: blaze.py#420\n\n\nFor more information about a job, try: gh run view --job=\nView this run on GitHub: https://github.com/runs/3\n", }, { name: "interactive, multiple jobs, choose specific jobs", @@ -736,7 +736,7 @@ func TestViewRun(t *testing.T) { as.StubOne(2) as.StubOne(1) }, - wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\n\nTo see the full job log, try: gh run view --log --job=10\nView this run on GitHub: runs/3\n", + wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\n\nTo see the full job log, try: gh run view --log --job=10\nView this run on GitHub: https://github.com/runs/3\n", }, { name: "web run", @@ -750,8 +750,8 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3"), httpmock.JSONResponse(shared.SuccessfulRun)) }, - browsedURL: "runs/3", - wantOut: "Opening runs/3 in your browser.\n", + browsedURL: "https://github.com/runs/3", + wantOut: "Opening github.com/runs/3 in your browser.\n", }, { name: "web job", @@ -768,8 +768,8 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3"), httpmock.JSONResponse(shared.SuccessfulRun)) }, - browsedURL: "jobs/10?check_suite_focus=true", - wantOut: "Opening jobs/10 in your browser.\n", + browsedURL: "https://github.com/jobs/10?check_suite_focus=true", + wantOut: "Opening github.com/jobs/10 in your browser.\n", }, { name: "hide job header, failure", @@ -788,7 +788,7 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/123/artifacts"), httpmock.StringResponse(`{}`)) }, - wantOut: "\nX trunk failed no job · 123\nTriggered via push about 59 minutes ago\n\nX This run likely failed because of a workflow file issue.\n\nFor more information, see: runs/123\n", + wantOut: "\nX trunk failed no job · 123\nTriggered via push about 59 minutes ago\n\nX This run likely failed because of a workflow file issue.\n\nFor more information, see: https://github.com/runs/123\n", }, { name: "hide job header, startup_failure", @@ -807,7 +807,7 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/123/artifacts"), httpmock.StringResponse(`{}`)) }, - wantOut: "\nX trunk failed no job · 123\nTriggered via push about 59 minutes ago\n\nX This run likely failed because of a workflow file issue.\n\nFor more information, see: runs/123\n", + wantOut: "\nX trunk failed no job · 123\nTriggered via push about 59 minutes ago\n\nX This run likely failed because of a workflow file issue.\n\nFor more information, see: https://github.com/runs/123\n", }, } diff --git a/pkg/cmd/secret/list/list.go b/pkg/cmd/secret/list/list.go index d331482b8..e8694c62c 100644 --- a/pkg/cmd/secret/list/list.go +++ b/pkg/cmd/secret/list/list.go @@ -3,7 +3,6 @@ package list import ( "fmt" "net/http" - "net/url" "strings" "time" @@ -160,14 +159,8 @@ func getOrgSecrets(client *api.Client, host, orgName string) ([]*Secret, error) if secret.SelectedReposURL == "" { continue } - u, err := url.Parse(secret.SelectedReposURL) - if err != nil { - return nil, fmt.Errorf("failed determining selected repositories for %s: %w", secret.Name, err) - } - var result responseData - err = client.REST(u.Host, "GET", u.Path[1:], nil, &result) - if err != nil { + if err := client.REST(host, "GET", secret.SelectedReposURL, nil, &result); err != nil { return nil, fmt.Errorf("failed determining selected repositories for %s: %w", secret.Name, err) } secret.NumSelectedRepos = result.TotalCount