Merge pull request #3461 from cli/jobs-url-enterprise

Fix requesting REST sub-resources on GHE
This commit is contained in:
Mislav Marohnić 2021-04-20 11:31:23 +02:00 committed by GitHub
commit ae99ad4fbe
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 49 additions and 36 deletions

View file

@ -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

View file

@ -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))

View file

@ -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

View file

@ -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{
{

View file

@ -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=<job-id>\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=<job-id>\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=<job-id>
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=<job-id>\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=<job-id>\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=<job-id>\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=<job-id>\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=<job-id>\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=<job-id>\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",
},
}

View file

@ -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