From f1991f816b5383b7d307de632a6f763299e659dd Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Tue, 27 Oct 2026 20:56:19 -0700 Subject: [PATCH] github: Fix native stacks on GHE.com Native stack updates and atomic merges are skipped when `spice.forge.github.apiUrl` points to `https://api..ghe.com`. The gateway appends `/v3` to REST requests for every host except `api.github.com`, but GHE.com serves REST at the API host root. The stack probe interprets the resulting 404 as an unsupported feature, so callers silently fall back to ordinary PR operations. GraphQL requests already use the correct `/graphql` endpoint. Recognize GHE.com API hosts alongside `api.github.com` when selecting the REST base, preserving `/api/v3` for Enterprise Server. Centralize web URL defaults and API endpoint derivation in `Deployment`: github.Deployment{URL: o.URL, APIURL: o.APIURL} The forge forwards its raw URL settings, and the gateway resolves the protocol endpoints, so deployment URL rules have one owner. Keep web URL selection independent of API parsing, which still occurs when opening a repository. Fixes #1498 --- .../unreleased/Fixed-20261027-205433.yaml | 3 + internal/forge/github/change_match_test.go | 2 +- internal/forge/github/forge.go | 40 ++-- internal/forge/github/forge_test.go | 182 +++++++++++++++--- internal/forge/github/integration_test.go | 9 +- internal/forge/github/main_test.go | 28 +-- internal/forge/github/merge_test.go | 5 +- internal/forge/github/submit_test.go | 20 +- internal/gateway/github/deployment.go | 74 +++++++ internal/gateway/github/gateway.go | 62 +++--- internal/gateway/github/gateway_test.go | 99 +++++----- internal/gateway/github/graphql.go | 2 +- internal/gateway/github/rest_test.go | 18 +- 13 files changed, 377 insertions(+), 167 deletions(-) create mode 100644 .changes/unreleased/Fixed-20261027-205433.yaml create mode 100644 internal/gateway/github/deployment.go diff --git a/.changes/unreleased/Fixed-20261027-205433.yaml b/.changes/unreleased/Fixed-20261027-205433.yaml new file mode 100644 index 000000000..8819cd456 --- /dev/null +++ b/.changes/unreleased/Fixed-20261027-205433.yaml @@ -0,0 +1,3 @@ +kind: Fixed +body: 'github: Fix GHE.com API hosts skipped during stack updates and merges.' +time: 2026-10-27T20:54:33.723586-07:00 diff --git a/internal/forge/github/change_match_test.go b/internal/forge/github/change_match_test.go index e9b8ef09b..e9a00c02a 100644 --- a/internal/forge/github/change_match_test.go +++ b/internal/forge/github/change_match_test.go @@ -171,7 +171,7 @@ func TestRepository_MatchChangesToBranches_filtersPushRepository(t *testing.T) { []string{"feature"}, &forge.MatchChangesToBranchesOptions{ PushRepository: &RepositoryID{ - url: DefaultURL, + url: "https://github.com", owner: "other", name: "fork", }, diff --git a/internal/forge/github/forge.go b/internal/forge/github/forge.go index 722a01249..ce9a87ed9 100644 --- a/internal/forge/github/forge.go +++ b/internal/forge/github/forge.go @@ -7,7 +7,6 @@ import ( "context" "fmt" "net/http" - "net/url" "go.abhg.dev/gs/internal/forge" "go.abhg.dev/gs/internal/gateway/github" @@ -15,12 +14,6 @@ import ( "go.abhg.dev/gs/internal/silog" ) -// Default URLs for GitHub and its API. -const ( - DefaultURL = "https://github.com" - DefaultAPIURL = "https://api.github.com" -) - // Options defines command line options for the GitHub Forge. // These are all hidden in the CLI, // and are expected to be set only via environment variables. @@ -44,6 +37,10 @@ type Options struct { Stacks bool `name:"github-stacks" hidden:"" config:"forge.github.stacks" default:"true" help:"Whether to use GitHub native stack operations. One of 'true' and 'false'."` } +func (o Options) deployment() github.Deployment { + return github.Deployment{URL: o.URL, APIURL: o.APIURL} +} + // Definition configures GitHub forge instances. type Definition struct { changeMetadataCodec @@ -65,7 +62,7 @@ func (*Definition) ID() string { return "github" } // BaseURL reports the GitHub web URL used for host matching. func (d *Definition) BaseURL() string { - return cmp.Or(d.Options.URL, DefaultURL) + return d.Options.deployment().WebURL() } // CLIPlugin returns the CLI plugin for the GitHub Forge. @@ -116,7 +113,7 @@ func (f *Forge) logger() *silog.Logger { // URL returns the base URL configured for the GitHub Forge // or the default URL if none is set. func (f *Forge) URL() string { - return cmp.Or(f.Options.URL, DefaultURL) + return f.Options.deployment().WebURL() } // BaseURL reports the GitHub web URL used for host matching and links. @@ -124,25 +121,6 @@ func (f *Forge) BaseURL() string { return f.baseURL } -// APIURL returns the base API URL configured for the GitHub Forge -// or the default URL if none is set. -func (f *Forge) APIURL() string { - if f.Options.APIURL != "" { - return f.Options.APIURL - } - - // If the API URL is not set, and base URL is NOT github.com, - // assume API URL is $baseURL/api. - if f.Options.URL != "" && f.Options.URL != DefaultURL { - apiURL, err := url.JoinPath(f.Options.URL, "/api") - if err == nil { - return apiURL - } - } - - return DefaultAPIURL -} - // ID reports a unique key for this forge. func (*Forge) ID() string { return "github" } @@ -171,7 +149,11 @@ func (f *Forge) OpenRepository(ctx context.Context, tok forge.AuthenticationToke } gatewayTokens := newGatewayTokenSource(tokenSource) - gatewayClient, err := github.NewGateway(f.APIURL(), f.HTTPClient, gatewayTokens) + gatewayClient, err := github.NewGateway(github.Config{ + Deployment: f.Options.deployment(), + Tokens: gatewayTokens, + HTTPClient: f.HTTPClient, + }) if err != nil { return nil, fmt.Errorf("create GitHub gateway: %w", err) } diff --git a/internal/forge/github/forge_test.go b/internal/forge/github/forge_test.go index 060049e3f..323dd383e 100644 --- a/internal/forge/github/forge_test.go +++ b/internal/forge/github/forge_test.go @@ -1,6 +1,9 @@ package github import ( + "io" + "net/http" + "strings" "testing" "github.com/alecthomas/kong" @@ -51,25 +54,21 @@ func TestURLs(t *testing.T) { name string opts Options - wantURL string - wantAPIURL string + wantURL string }{ { - name: "Default", - wantURL: DefaultURL, - wantAPIURL: DefaultAPIURL, + name: "Default", + wantURL: "https://github.com", }, { - name: "ExplicitURL", - opts: Options{URL: DefaultURL}, - wantURL: DefaultURL, - wantAPIURL: DefaultAPIURL, + name: "ExplicitURL", + opts: Options{URL: "https://github.com"}, + wantURL: "https://github.com", }, { - name: "CustomURL", - opts: Options{URL: "https://example.com"}, - wantURL: "https://example.com", - wantAPIURL: "https://example.com/api", + name: "CustomURL", + opts: Options{URL: "https://example.com"}, + wantURL: "https://example.com", }, { name: "CustomBoth", @@ -77,27 +76,160 @@ func TestURLs(t *testing.T) { URL: "https://example.com", APIURL: "https://api.example.com", }, - wantURL: "https://example.com", - wantAPIURL: "https://api.example.com", + wantURL: "https://example.com", }, { - name: "InvalidURL", - opts: Options{URL: ":/"}, - wantURL: ":/", - wantAPIURL: DefaultAPIURL, + name: "InvalidURL", + opts: Options{URL: ":/"}, + wantURL: ":/", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { f := Forge{Options: tt.opts} + d := Definition{Options: tt.opts} assert.Equal(t, tt.wantURL, f.URL()) - assert.Equal(t, tt.wantAPIURL, f.APIURL()) + assert.Equal(t, tt.wantURL, d.BaseURL()) }) } } +func TestForge_OpenRepository_endpoints(t *testing.T) { + tests := []struct { + name string + opts Options + wantGQL string + wantREST string + }{ + { + name: "GitHub", + wantGQL: "https://api.github.com/graphql", + wantREST: "https://api.github.com", + }, + { + name: "ExplicitGitHub", + opts: Options{URL: "https://github.com"}, + wantGQL: "https://api.github.com/graphql", + wantREST: "https://api.github.com", + }, + { + name: "MalformedWebURL", + opts: Options{URL: ":/"}, + wantGQL: "https://api.github.com/graphql", + wantREST: "https://api.github.com", + }, + { + name: "EnterpriseCloud", + opts: Options{APIURL: "https://api.octocorp.ghe.com"}, + wantGQL: "https://api.octocorp.ghe.com/graphql", + wantREST: "https://api.octocorp.ghe.com", + }, + { + name: "EnterpriseCloudMixedCasePort", + opts: Options{APIURL: "https://API.OctoCorp.GHE.COM:443"}, + wantGQL: "https://API.OctoCorp.GHE.COM:443/graphql", + wantREST: "https://API.OctoCorp.GHE.COM:443", + }, + { + name: "Enterprise", + opts: Options{URL: "https://github.example.com"}, + wantGQL: "https://github.example.com/api/graphql", + wantREST: "https://github.example.com/api/v3", + }, + { + name: "WebPrefix", + opts: Options{URL: "https://github.example.com/service"}, + wantGQL: "https://github.example.com/service/api/graphql", + wantREST: "https://github.example.com/service/api/v3", + }, + { + name: "CustomAPI", + opts: Options{ + URL: "https://github.example.com", + APIURL: "https://proxy.example.com/github", + }, + wantGQL: "https://proxy.example.com/github/graphql", + wantREST: "https://proxy.example.com/github/v3", + }, + { + name: "EnterpriseCloudWebHost", + opts: Options{URL: "https://octocorp.ghe.com"}, + wantGQL: "https://octocorp.ghe.com/api/graphql", + wantREST: "https://octocorp.ghe.com/api/v3", + }, + { + name: "EnterpriseSimilarCloudHost", + opts: Options{APIURL: "https://api.octocorp.ghe.com.example.net/api"}, + wantGQL: "https://api.octocorp.ghe.com.example.net/api/graphql", + wantREST: "https://api.octocorp.ghe.com.example.net/api/v3", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stackURL := tt.wantREST + "/repos/octo/hello/stacks" + var requests []string + f := Forge{ + Options: tt.opts, + HTTPClient: &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + requests = append(requests, r.URL.String()) + assert.Equal(t, "Bearer token", r.Header.Get("Authorization")) + status, body := http.StatusOK, "[]" + switch r.URL.String() { + case tt.wantGQL: + assert.Equal(t, http.MethodPost, r.Method) + body = `{"data":{"repository":{"id":"R_123"}}}` + case stackURL: + assert.Equal(t, http.MethodGet, r.Method) + default: + status, body = http.StatusNotFound, `{"message":"Not Found"}` + } + return &http.Response{ + StatusCode: status, + Body: io.NopCloser(strings.NewReader(body)), + }, nil + })}, + } + f.Options.Stacks = true + rid, err := f.ParseRepositoryPath("octo/hello") + require.NoError(t, err) + repo, err := f.OpenRepository(t.Context(), &AuthenticationToken{AccessToken: "token"}, rid) + require.NoError(t, err) + + _, err = repo.(forge.StackRepository).PlanMergeRanges(t.Context(), nil) + assert.NoError(t, err) + assert.Equal(t, []string{tt.wantGQL, stackURL}, requests) + }) + } +} + +func TestDefinition_New_invalidAPIURL(t *testing.T) { + d := Definition{Options: Options{ + URL: "https://github.example.com", + APIURL: ":/", + }} + assert.Equal(t, "https://github.example.com", d.BaseURL()) + remoteURL, err := giturl.Parse("https://github.example.com/octo/hello.git") + require.NoError(t, err) + f, err := d.New(remoteURL, &forge.NewOptions{ + HTTPClient: &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + t.Fatal("unexpected HTTP request") + return nil, nil + })}, + }) + require.NoError(t, err) + + ep, err := f.(*Forge).oauth2Endpoint() + require.NoError(t, err) + assert.Equal(t, "https://github.example.com/login/oauth/access_token", ep.TokenURL) + + rid, err := f.ParseRepositoryPath("octo/hello") + require.NoError(t, err) + _, err = f.OpenRepository(t.Context(), &AuthenticationToken{AccessToken: "token"}, rid) + assert.ErrorContains(t, err, "parse API URL") +} + func TestExtractRepoInfo(t *testing.T) { tests := []struct { name string @@ -229,7 +361,7 @@ func TestForge_ParseRepositoryPath_knownForge(t *testing.T) { func TestChangeURL(t *testing.T) { repoID := RepositoryID{ - url: DefaultURL, + url: "https://github.com", owner: "example", name: "repo", } @@ -242,7 +374,7 @@ func TestRepository_ComparisonURL(t *testing.T) { r := &Repository{ owner: "example", repo: "repo", - forge: &Forge{Options: Options{URL: DefaultURL}}, + forge: &Forge{Options: Options{URL: "https://github.com"}}, } t.Run("SameRepository", func(t *testing.T) { @@ -258,7 +390,7 @@ func TestRepository_ComparisonURL(t *testing.T) { Base: "main", Head: "feat#review", HeadRepository: &RepositoryID{ - url: DefaultURL, + url: "https://github.com", owner: "fork", name: "repo", }, @@ -268,3 +400,7 @@ func TestRepository_ComparisonURL(t *testing.T) { got) }) } + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } diff --git a/internal/forge/github/integration_test.go b/internal/forge/github/integration_test.go index 031f16a05..9fe2c2d9a 100644 --- a/internal/forge/github/integration_test.go +++ b/internal/forge/github/integration_test.go @@ -60,11 +60,10 @@ func newRecorder( func newGateway(t *testing.T, httpClient *http.Client) *githubgateway.Gateway { t.Helper() - client, err := githubgateway.NewGateway( - github.DefaultAPIURL, - &http.Client{Transport: httpClient.Transport}, - testTokenSource("token"), - ) + client, err := githubgateway.NewGateway(githubgateway.Config{ + HTTPClient: &http.Client{Transport: httpClient.Transport}, + Tokens: testTokenSource("token"), + }) require.NoError(t, err) return client } diff --git a/internal/forge/github/main_test.go b/internal/forge/github/main_test.go index 1f7d920ae..eb0fa350c 100644 --- a/internal/forge/github/main_test.go +++ b/internal/forge/github/main_test.go @@ -13,18 +13,6 @@ import ( "go.abhg.dev/gs/internal/gateway/github" ) -type testGatewayTokenSource struct{} - -func (testGatewayTokenSource) Token(context.Context) (string, error) { - return "test-token", nil -} - -func newTestGateway(t *testing.T, apiURL string) *github.Gateway { - client, err := github.NewGateway(apiURL, nil, testGatewayTokenSource{}) - require.NoError(t, err) - return client -} - func TestMain(m *testing.M) { name := filepath.Base(os.Args[0]) if runtime.GOOS == "windows" { @@ -46,3 +34,19 @@ password=test-token os.Exit(m.Run()) } + +func newTestGateway(t *testing.T, apiURL string) *github.Gateway { + t.Helper() + client, err := github.NewGateway(github.Config{ + Deployment: github.Deployment{APIURL: apiURL}, + Tokens: testGatewayTokenSource{}, + }) + require.NoError(t, err) + return client +} + +type testGatewayTokenSource struct{} + +func (testGatewayTokenSource) Token(context.Context) (string, error) { + return "test-token", nil +} diff --git a/internal/forge/github/merge_test.go b/internal/forge/github/merge_test.go index 86a62d879..04edbe498 100644 --- a/internal/forge/github/merge_test.go +++ b/internal/forge/github/merge_test.go @@ -82,7 +82,10 @@ func TestRepository_MergeChange_method(t *testing.T) { })) defer srv.Close() - gateway, err := github.NewGateway(srv.URL, nil, gatewayTestTokenSource("token")) + gateway, err := github.NewGateway(github.Config{ + Deployment: github.Deployment{APIURL: srv.URL}, + Tokens: gatewayTestTokenSource("token"), + }) require.NoError(t, err) repo, err := newRepository( t.Context(), new(Forge), diff --git a/internal/forge/github/submit_test.go b/internal/forge/github/submit_test.go index 90c336262..08482b26f 100644 --- a/internal/forge/github/submit_test.go +++ b/internal/forge/github/submit_test.go @@ -67,11 +67,11 @@ func TestRepository_SubmitChange_fromPushRepository(t *testing.T) { })) })) defer srv.Close() - gatewayClient, err := github.NewGateway( - srv.URL, - srv.Client(), - gatewayTestTokenSource("token"), - ) + gatewayClient, err := github.NewGateway(github.Config{ + Deployment: github.Deployment{APIURL: srv.URL}, + HTTPClient: srv.Client(), + Tokens: gatewayTestTokenSource("token"), + }) require.NoError(t, err) repo, err := newRepository( @@ -176,11 +176,11 @@ func TestRepository_addPullRequestMetadata(t *testing.T) { } })) defer srv.Close() - gatewayClient, err := github.NewGateway( - srv.URL, - srv.Client(), - gatewayTestTokenSource("token"), - ) + gatewayClient, err := github.NewGateway(github.Config{ + Deployment: github.Deployment{APIURL: srv.URL}, + HTTPClient: srv.Client(), + Tokens: gatewayTestTokenSource("token"), + }) require.NoError(t, err) repo, err := newRepository( diff --git a/internal/gateway/github/deployment.go b/internal/gateway/github/deployment.go new file mode 100644 index 000000000..5fcca36aa --- /dev/null +++ b/internal/gateway/github/deployment.go @@ -0,0 +1,74 @@ +package github + +import ( + "cmp" + "fmt" + "net/url" + "strings" +) + +// Deployment configures the addresses of a GitHub installation. +// The zero value selects GitHub.com. +// API addresses are resolved when constructing a [Gateway]. +type Deployment struct { + // URL is the web URL. If empty, https://github.com is used. + URL string + + // APIURL overrides the common base URL for the GraphQL and REST APIs. + // For GitHub Enterprise Server, this normally ends in /api, + // before the REST API's /v3 suffix. + // If empty, GitHub.com uses its public API host, + // and custom web URLs use an /api subpath. + APIURL string +} + +// WebURL returns the configured web URL or the GitHub.com default. +// It does not parse or validate either address. +func (d Deployment) WebURL() string { + return cmp.Or(d.URL, defaultWebURL) +} + +type apiEndpoints struct { + GraphQLEndpoint *url.URL + RESTBaseURL *url.URL +} + +// apiEndpoints resolves the common API base into privately owned protocol URLs. +// It returns an error if an explicit APIURL cannot be parsed. +func (d Deployment) apiEndpoints() (apiEndpoints, error) { + apiURL := &url.URL{Scheme: "https", Host: defaultAPIHost} + switch { + case d.APIURL != "": + var err error + apiURL, err = url.Parse(d.APIURL) + if err != nil { + return apiEndpoints{}, fmt.Errorf("parse API URL: %w", err) + } + case d.URL != "" && d.URL != defaultWebURL: + // Retain the API default if the web URL cannot be parsed. + // Web operations validate their own URL. + if webURL, err := url.Parse(d.URL); err == nil { + apiURL = webURL.JoinPath("/api") + } + } + + // GitHub.com and GHE.com serve GraphQL below the REST API host, + // while GitHub Enterprise Server uses /api/graphql and /api/v3. + apiHost := strings.ToLower(apiURL.Hostname()) + isGitHubCloud := apiHost == defaultAPIHost || + (strings.HasPrefix(apiHost, "api.") && strings.HasSuffix(apiHost, ".ghe.com")) + restBaseURL := apiURL + if !isGitHubCloud { + restBaseURL = apiURL.JoinPath("v3") + } + + return apiEndpoints{ + GraphQLEndpoint: apiURL.JoinPath("graphql"), + RESTBaseURL: restBaseURL, + }, nil +} + +const ( + defaultWebURL = "https://github.com" + defaultAPIHost = "api.github.com" +) diff --git a/internal/gateway/github/gateway.go b/internal/gateway/github/gateway.go index 239cbc73d..3c243df2a 100644 --- a/internal/gateway/github/gateway.go +++ b/internal/gateway/github/gateway.go @@ -1,10 +1,11 @@ // Package github provides the typed GitHub API operations needed by git-spice. // -// The package owns GitHub's GraphQL and REST wire protocols, +// The package owns GitHub's deployment URL conventions, +// GraphQL and REST wire protocols, // authenticated request execution, // and response error models. -// Callers supply credentials through [TokenSource] and adapt the typed results -// to their own domain models. +// Callers supply a [Deployment] through [Config] and credentials through +// [TokenSource], and adapt the typed results to their own domain models. // The package does not own credential discovery, persistence, or login flows. package github @@ -15,7 +16,6 @@ import ( "io" "net/http" "net/url" - "strings" ) // maxErrorBody limits the diagnostic content retained from an HTTP error @@ -34,16 +34,16 @@ type TokenSource interface { // A Gateway is safe for concurrent use when its HTTP client and token source are // safe for concurrent use. // Each HTTP request retrieves a token with the operation context and uses the -// configured GitHub endpoint. +// configured GitHub endpoints. // A typed operation may issue multiple requests when GitHub exposes related // data through separate API nodes. // GitHub responses containing both data and errors return only the errors; // callers never observe a partial result. type Gateway struct { - // graphQLEndpoint is derived from the configured API base URL. - graphQLEndpoint string + // graphQLEndpoint is the complete URL for GraphQL requests. + graphQLEndpoint *url.URL - // restBaseURL is the REST API root derived from the configured API base URL. + // restBaseURL is the root to which REST resource paths are appended. restBaseURL *url.URL // httpClient performs requests after the gateway has supplied authentication. @@ -53,42 +53,40 @@ type Gateway struct { tokens TokenSource } -// NewGateway builds a client for a GitHub API base URL. -// -// apiURLStr is the API base URL reported by the forge configuration. -// For GitHub Enterprise Server, this is the common `/api` root from which -// NewGateway derives `/api/graphql` and `/api/v3`. -// A nil HTTP client uses [http.DefaultClient]. -// The tokens source is required and is consulted for every HTTP request. -func NewGateway(apiURLStr string, httpClient *http.Client, tokens TokenSource) (*Gateway, error) { - apiURL, err := url.Parse(apiURLStr) - if err != nil { - return nil, fmt.Errorf("parse API URL: %w", err) - } +// Config configures a Gateway's deployment and authenticated HTTP requests. +type Config struct { + // Deployment selects the GitHub installation. + // Its zero value selects GitHub.com. + Deployment Deployment - graphQLURL := apiURL.JoinPath("graphql") + // Tokens supplies credentials for every HTTP request. + Tokens TokenSource // required - // TODO: Accept complete GraphQL and REST endpoint URLs instead of deriving - // both endpoints from GitHub's common API base URL. - // GitHub.com serves GraphQL below the REST API host, - // while GitHub Enterprise Server uses /api/graphql and /api/v3. - restBaseURL := apiURL - if !strings.EqualFold(apiURL.Hostname(), "api.github.com") { - restBaseURL = apiURL.JoinPath("v3") + // HTTPClient sends requests to both endpoints. + // If nil, [http.DefaultClient] is used. + HTTPClient *http.Client +} + +// NewGateway resolves the deployment's API addresses and builds a client. +func NewGateway(cfg Config) (*Gateway, error) { + endpoints, err := cfg.Deployment.apiEndpoints() + if err != nil { + return nil, err } + httpClient := cfg.HTTPClient if httpClient == nil { httpClient = http.DefaultClient } - if tokens == nil { + if cfg.Tokens == nil { return nil, errors.New("token source is required") } return &Gateway{ - graphQLEndpoint: graphQLURL.String(), - restBaseURL: restBaseURL, + graphQLEndpoint: endpoints.GraphQLEndpoint, + restBaseURL: endpoints.RESTBaseURL, httpClient: httpClient, - tokens: tokens, + tokens: cfg.Tokens, }, nil } diff --git a/internal/gateway/github/gateway_test.go b/internal/gateway/github/gateway_test.go index 4fd836c26..9881e01ed 100644 --- a/internal/gateway/github/gateway_test.go +++ b/internal/gateway/github/gateway_test.go @@ -14,36 +14,38 @@ import ( "github.com/stretchr/testify/require" ) -func TestNewGateway_endpoint(t *testing.T) { - tests := []struct { - name string - give string - wantGQL string - wantREST string - }{ - { - name: "GitHub", - give: "https://api.github.com", - wantGQL: "https://api.github.com/graphql", - wantREST: "https://api.github.com", - }, - { - name: "Enterprise", - give: "https://github.example.com/api", - wantGQL: "https://github.example.com/api/graphql", - wantREST: "https://github.example.com/api/v3", +func TestNewGateway_endpoints(t *testing.T) { + var requests []string + gateway, err := NewGateway(Config{ + Deployment: Deployment{ + URL: "https://github.example.com", + APIURL: "https://proxy.example.com/service", }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - gateway, err := NewGateway(tt.give, nil, tokenSourceFunc(func(context.Context) (string, error) { - return "token", nil - })) - require.NoError(t, err) - assert.Equal(t, tt.wantGQL, gateway.graphQLEndpoint) - assert.Equal(t, tt.wantREST, gateway.restBaseURL.String()) - }) - } + Tokens: tokenSourceFunc(func(context.Context) (string, error) { + return "token", nil + }), + HTTPClient: &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + requests = append(requests, r.URL.String()) + body := "[]" + if r.Method == http.MethodPost { + body = `{"data":{"repository":{"id":"R_123"}}}` + } + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(body)), + }, nil + })}, + }) + require.NoError(t, err) + + id, err := gateway.RepositoryID(t.Context(), "octo", "hello") + require.NoError(t, err) + assert.Equal(t, ID("R_123"), id) + require.NoError(t, gateway.CheckPullRequestStacks(t.Context(), "octo", "hello")) + assert.Equal(t, []string{ + "https://proxy.example.com/service/graphql", + "https://proxy.example.com/service/v3/repos/octo/hello/stacks", + }, requests) } func TestGateway_RepositoryID_request(t *testing.T) { @@ -69,11 +71,15 @@ func TestGateway_RepositoryID_request(t *testing.T) { }) var tokenCalls int - gateway, err := NewGateway("https://github.example.com/api", &http.Client{Transport: transport}, tokenSourceFunc(func(ctx context.Context) (string, error) { - assert.Same(t, t.Context(), ctx) - tokenCalls++ - return "secret", nil - })) + gateway, err := NewGateway(Config{ + Deployment: Deployment{URL: "https://github.example.com"}, + HTTPClient: &http.Client{Transport: transport}, + Tokens: tokenSourceFunc(func(ctx context.Context) (string, error) { + assert.Same(t, t.Context(), ctx) + tokenCalls++ + return "secret", nil + }), + }) require.NoError(t, err) id, err := gateway.RepositoryID(t.Context(), "octo", "hello") @@ -84,9 +90,11 @@ func TestGateway_RepositoryID_request(t *testing.T) { func TestGateway_RepositoryID_tokenError(t *testing.T) { want := errors.New("token unavailable") - gateway, err := NewGateway("https://api.github.com", nil, tokenSourceFunc(func(context.Context) (string, error) { - return "", want - })) + gateway, err := NewGateway(Config{ + Tokens: tokenSourceFunc(func(context.Context) (string, error) { + return "", want + }), + }) require.NoError(t, err) _, err = gateway.RepositoryID(t.Context(), "octo", "hello") @@ -96,9 +104,11 @@ func TestGateway_RepositoryID_tokenError(t *testing.T) { func TestGateway_RepositoryID_cancellation(t *testing.T) { ctx, cancel := context.WithCancel(t.Context()) cancel() - gateway, err := NewGateway("https://api.github.com", nil, tokenSourceFunc(func(ctx context.Context) (string, error) { - return "", ctx.Err() - })) + gateway, err := NewGateway(Config{ + Tokens: tokenSourceFunc(func(ctx context.Context) (string, error) { + return "", ctx.Err() + }), + }) require.NoError(t, err) _, err = gateway.RepositoryID(ctx, "octo", "hello") @@ -258,9 +268,12 @@ func newResponseGateway(t *testing.T, body string) *Gateway { func newTestGateway(t *testing.T, transport http.RoundTripper) *Gateway { t.Helper() - gateway, err := NewGateway("https://api.github.com", &http.Client{Transport: transport}, tokenSourceFunc(func(context.Context) (string, error) { - return "token", nil - })) + gateway, err := NewGateway(Config{ + HTTPClient: &http.Client{Transport: transport}, + Tokens: tokenSourceFunc(func(context.Context) (string, error) { + return "token", nil + }), + }) require.NoError(t, err) return gateway } diff --git a/internal/gateway/github/graphql.go b/internal/gateway/github/graphql.go index 620850738..20dbd8455 100644 --- a/internal/gateway/github/graphql.go +++ b/internal/gateway/github/graphql.go @@ -63,7 +63,7 @@ func (c *Gateway) executeGQL(ctx context.Context, query string, variables, resul return fmt.Errorf("encode GraphQL request: %w", err) } - req, err := c.newRequest(ctx, http.MethodPost, c.graphQLEndpoint, &body) + req, err := c.newRequest(ctx, http.MethodPost, c.graphQLEndpoint.String(), &body) if err != nil { return err } diff --git a/internal/gateway/github/rest_test.go b/internal/gateway/github/rest_test.go index abd0cadd8..d42662fa4 100644 --- a/internal/gateway/github/rest_test.go +++ b/internal/gateway/github/rest_test.go @@ -15,18 +15,17 @@ import ( func TestGateway_RESTAuthenticationRefresh(t *testing.T) { var tokenCalls int - gateway, err := NewGateway( - "https://api.github.com", - &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + gateway, err := NewGateway(Config{ + HTTPClient: &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { assert.Equal(t, fmt.Sprintf("Bearer token-%d", tokenCalls), r.Header.Get("Authorization")) return restJSONResponse(http.StatusCreated, `{}`), nil })}, - tokenSourceFunc(func(ctx context.Context) (string, error) { + Tokens: tokenSourceFunc(func(ctx context.Context) (string, error) { assert.Same(t, t.Context(), ctx) tokenCalls++ return fmt.Sprintf("token-%d", tokenCalls), nil }), - ) + }) require.NoError(t, err) for range 2 { @@ -42,16 +41,15 @@ func TestGateway_RESTAuthenticationRefresh(t *testing.T) { func TestGateway_RESTTokenError(t *testing.T) { want := errors.New("token unavailable") - gateway, err := NewGateway( - "https://api.github.com", - &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + gateway, err := NewGateway(Config{ + HTTPClient: &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { t.Fatal("HTTP request made after token failure") return nil, nil })}, - tokenSourceFunc(func(context.Context) (string, error) { + Tokens: tokenSourceFunc(func(context.Context) (string, error) { return "", want }), - ) + }) require.NoError(t, err) err = gateway.CreatePullRequestStack(t.Context(), &CreatePullRequestStackInput{