From 33aa410a6a11a52e3d4f5d23caf541d83e7872f4 Mon Sep 17 00:00:00 2001 From: jonnii Date: 2026年6月23日 08:04:06 -0400 Subject: [PATCH] feat(server): serve repos at /repos/{owner}/{repo} path routes Replace the {repoID} and unscoped API routes with GitHub-style /api/v1/repos/{owner}/{repo}/... routes, resolved case-insensitively through the registry's owner/repo index. Branch-diff moves from a ?branch= query to a /branch-diff/{name...} path segment: branch names contain slashes and Go's mux requires a trailing wildcard to be the final segment, so .../branches/{name}/diff is not expressible. Rewrite the OpenAPI spec to the scoped shape (it had drifted to the old unscoped paths) and document the submit, sync, and events routes. Drops the legacy default-fallback resolution. The web client moves to these routes in the next change. --- api/openapi/stackit.yaml | 217 ++++++++++++++++++----- docs/deploy.md | 4 +- internal/api/auth/middleware_test.go | 4 +- internal/api/handlers/branch_diff.go | 4 +- internal/api/handlers/branches_test.go | 57 ++++-- internal/api/handlers/common.go | 22 +-- internal/api/handlers/events_test.go | 18 +- internal/api/handlers/sync_test.go | 20 +-- internal/api/handlers/validation_test.go | 16 +- internal/api/handlers/visibility_test.go | 12 +- internal/api/readonly_test.go | 2 +- internal/api/server.go | 43 ++--- internal/api/sync_route_test.go | 2 +- 13 files changed, 274 insertions(+), 147 deletions(-) diff --git a/api/openapi/stackit.yaml b/api/openapi/stackit.yaml index bd7b9480e..8bed904e8 100644 --- a/api/openapi/stackit.yaml +++ b/api/openapi/stackit.yaml @@ -6,7 +6,54 @@ info: servers: - url: / paths: - /api/v1/view: + /api/v1/repos: + get: + summary: List repositories the server is serving (filtered to the caller). + operationId: listRepos + responses: + "200": + description: Repository index. + content: + application/json: + schema: + $ref: "#/components/schemas/ReposListResponse" + post: + summary: Onboard a GitHub repository (clone and start serving it). + description:> + Clones a GitHub repository the authenticated user can access and starts + serving it, recorded against that user so only they see it. Requires an + authenticated, writable, DB-backed server with a repos root configured; + read-only servers refuse with 405. + operationId: onboardRepo + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/OnboardRepoRequest" + responses: + "201": + description: The newly served repository. + content: + application/json: + schema: + $ref: "#/components/schemas/RepoSummary" + "400": + description: Missing or invalid owner/name. + "401": + description: Authentication required. + "404": + description: Repository not found or not accessible to the caller. + "405": + description: Server is read-only. + "409": + description: Repository already onboarded. + "503": + description: Onboarding not available (no auth, database, or repos root). + /api/v1/repos/{owner}/{repo}/view: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" get: summary: Combined view payload for the dashboard. operationId: getView @@ -17,7 +64,12 @@ paths: application/json: schema: $ref: "#/components/schemas/ViewResponse" - /api/v1/repo: + "404": + description: Repository not found or not visible to the caller. + /api/v1/repos/{owner}/{repo}/repo: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" get: summary: Repository metadata. operationId: getRepo @@ -28,7 +80,12 @@ paths: application/json: schema: $ref: "#/components/schemas/RepoResponse" - /api/v1/stacks: + "404": + description: Repository not found or not visible to the caller. + /api/v1/repos/{owner}/{repo}/stacks: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" get: summary: List discovered stacks. operationId: listStacks @@ -41,7 +98,10 @@ paths: type: array items: $ref: "#/components/schemas/StackSummary" - /api/v1/stacks/{rootBranch}: + /api/v1/repos/{owner}/{repo}/stacks/{rootBranch}: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" get: summary: Get detailed stack by root branch. operationId: getStack @@ -60,7 +120,46 @@ paths: $ref: "#/components/schemas/StackDetail" "404": description: Stack not found. - /api/v1/branches: + /api/v1/repos/{owner}/{repo}/stacks/{rootBranch}/submit: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" + post: + summary: Submit a stack as stacked pull requests. + operationId: submitStack + parameters: + - in: path + name: rootBranch + required: true + schema: + type: string + responses: + "200": + description: Submit result. + "404": + description: Stack not found. + "405": + description: Server is read-only. + /api/v1/repos/{owner}/{repo}/sync: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" + post: + summary: Force an immediate refresh of one repository. + operationId: syncRepo + responses: + "204": + description: Refresh completed. + "404": + description: Repository not found or not visible to the caller. + "405": + description: Server is read-only. + "502": + description: Mirror fetch failed. + /api/v1/repos/{owner}/{repo}/branches: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" get: summary: List tracked branches. operationId: listBranches @@ -73,7 +172,10 @@ paths: type: array items: $ref: "#/components/schemas/BranchResponse" - /api/v1/branches/{branchName}: + /api/v1/repos/{owner}/{repo}/branches/{branchName}: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" get: summary: Get one tracked branch. operationId: getBranch @@ -92,13 +194,16 @@ paths: $ref: "#/components/schemas/BranchResponse" "404": description: Branch not found. - /api/v1/branch-diff: + /api/v1/repos/{owner}/{repo}/branch-diff/{branchName}: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" get: summary: Get raw patch diff for a tracked branch. operationId: getBranchDiff parameters: - - in: query - name: branch + - in: path + name: branchName required: true schema: type: string @@ -110,10 +215,13 @@ paths: schema: $ref: "#/components/schemas/BranchDiffResponse" "400": - description: Missing branch query parameter. + description: Missing branch. "404": description: Branch not found. - /api/v1/events: + /api/v1/repos/{owner}/{repo}/events: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" get: summary: Server-sent event stream for repository refresh events. operationId: streamEvents @@ -124,51 +232,66 @@ paths: text/event-stream: schema: type: string - /api/v1/repos: - get: - summary: List repositories the server is serving (filtered to the caller). - operationId: listRepos - responses: - "200": - description: Repository index. - content: - application/json: - schema: - $ref: "#/components/schemas/ReposListResponse" + /api/v1/webhooks/github: post: - summary: Onboard a GitHub repository (clone and start serving it). - description:> - Clones a GitHub repository the authenticated user can access and starts - serving it, recorded against that user so only they see it. Requires an - authenticated, writable, DB-backed server with a repos root configured; - read-only servers refuse with 405. - operationId: onboardRepo + summary: Receive a GitHub webhook delivery and trigger a repo refresh. + description:>- + Authenticated solely by the X-Hub-Signature-256 HMAC over the raw body; + it carries no session or CSRF token. Fails closed: when no webhook secret + is configured the endpoint returns 404. A verified push is acked + immediately and the matching managed repo is mirror-fetched off the + request path (a burst for one repo coalesces into a single fetch). It is + unaffected by read-only mode, since a refresh is a read-side operation. + operationId: receiveGitHubWebhook + parameters: + - in: header + name: X-Hub-Signature-256 + required: true + description: HMAC-SHA256 of the raw request body, keyed by the webhook secret. + schema: + type: string + - in: header + name: X-GitHub-Event + required: true + description: GitHub event name; only "push" triggers a refresh and "ping" is acked. + schema: + type: string requestBody: required: true content: application/json: schema: - $ref: "#/components/schemas/OnboardRepoRequest" + type: object + description: The GitHub webhook event payload (e.g. a push event). responses: - "201": - description: The newly served repository. - content: - application/json: - schema: - $ref: "#/components/schemas/RepoSummary" + "202": + description: Push accepted; a refresh is scheduled for the matching managed repo. + "204": + description:>- + Verified but no action taken — a ping, a non-push event, or a push + that did not resolve to a managed repo. "400": - description: Missing or invalid owner/name. + description: Could not read the request body. "401": - description: Authentication required. + description: Missing or invalid signature. "404": - description: Repository not found or not accessible to the caller. - "405": - description: Server is read-only. - "409": - description: Repository already onboarded. - "503": - description: Onboarding not available (no auth, database, or repos root). + description: Webhook receiver is disabled (no secret configured). components: + parameters: + Owner: + in: path + name: owner + required: true + description: GitHub repository owner (login), matched case-insensitively. + schema: + type: string + Repo: + in: path + name: repo + required: true + description: GitHub repository name, matched case-insensitively. + schema: + type: string schemas: ViewResponse: type: object @@ -373,6 +496,10 @@ components: properties: id: type: string + owner: + type: string + repo: + type: string displayName: type: string trunk: diff --git a/docs/deploy.md b/docs/deploy.md index 6f45cb8fa..e75bed2c9 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -230,7 +230,7 @@ clients refetch. Three things trigger it: (below). The reliable backstop. 2. **Webhooks** — an immediate, push-driven refresh of a single repo ([below](#evented-refresh-webhooks)). The low-latency path. -3. **Manual sync** — `POST /api/v1/repos/{repoID}/sync`, an on-demand refresh +3. **Manual sync** — `POST /api/v1/repos/{owner}/{repo}/sync`, an on-demand refresh (below). The fallback for local servers and for forcing a pull. The interval loop is the floor: webhooks and manual sync make refreshes faster @@ -284,7 +284,7 @@ point a GitHub webhook at the server: ### Manual sync -`POST /api/v1/repos/{repoID}/sync` forces a refresh of one repo on demand. It is +`POST /api/v1/repos/{owner}/{repo}/sync` forces a refresh of one repo on demand. It is session-gated like submit (and refused in read-only mode), so it is never an anonymous trigger. For a managed mirror it mirror-fetches then rebuilds; for a local `-cwd` working repo it only re-reads on-disk refs (it never mirror-fetches diff --git a/internal/api/auth/middleware_test.go b/internal/api/auth/middleware_test.go index 0250912be..87e1c36f7 100644 --- a/internal/api/auth/middleware_test.go +++ b/internal/api/auth/middleware_test.go @@ -72,7 +72,7 @@ func TestRequireCSRFHeader_BlocksMutatingMethodsWithoutHeader(t *testing.T) { })) for _, m := range []string{http.MethodPost, http.MethodPut, http.MethodPatch, http.MethodDelete} { - req := httptest.NewRequest(m, "/api/v1/repos/default/stacks/main/submit", nil) + req := httptest.NewRequest(m, "/api/v1/repos/acme/demo/stacks/main/submit", nil) rr := httptest.NewRecorder() h.ServeHTTP(rr, req) require.Equalf(t, http.StatusForbidden, rr.Code, "method %s without header should be 403", m) @@ -89,7 +89,7 @@ func TestRequireCSRFHeader_AllowsMutatingMethodsWithHeader(t *testing.T) { w.WriteHeader(http.StatusOK) })) - req := httptest.NewRequest(http.MethodPost, "/api/v1/repos/default/stacks/main/submit", nil) + req := httptest.NewRequest(http.MethodPost, "/api/v1/repos/acme/demo/stacks/main/submit", nil) req.Header.Set(CSRFHeader, "1") rr := httptest.NewRecorder() h.ServeHTTP(rr, req) diff --git a/internal/api/handlers/branch_diff.go b/internal/api/handlers/branch_diff.go index b66eedea2..a5a044ef6 100644 --- a/internal/api/handlers/branch_diff.go +++ b/internal/api/handlers/branch_diff.go @@ -46,9 +46,9 @@ func (h *BranchDiffHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - branchName := r.URL.Query().Get("branch") + branchName := r.PathValue("name") if branchName == "" { - http.Error(w, "missing branch query parameter", http.StatusBadRequest) + http.Error(w, "missing branch", http.StatusBadRequest) return } if !validateBranchName(w, branchName) { diff --git a/internal/api/handlers/branches_test.go b/internal/api/handlers/branches_test.go index d3445240f..b9f317ed0 100644 --- a/internal/api/handlers/branches_test.go +++ b/internal/api/handlers/branches_test.go @@ -24,7 +24,7 @@ func TestBranchesHandler_AllowsBranchNamedDiff(t *testing.T) { reg := singleEntryRegistry(t, s) handler := NewBranchesHandler(reg) - req := httptest.NewRequest(http.MethodGet, "/api/v1/branches/diff", nil) + req := withRepo(httptest.NewRequest(http.MethodGet, "/api/v1/repos/acme/demo/branches/diff", nil)) req.SetPathValue("name", "diff") rr := httptest.NewRecorder() @@ -44,7 +44,7 @@ func TestBranchesHandler_AllowsBranchEndingInDiffSuffix(t *testing.T) { reg := singleEntryRegistry(t, s) handler := NewBranchesHandler(reg) - req := httptest.NewRequest(http.MethodGet, "/api/v1/branches/"+url.PathEscape(branchName), nil) + req := withRepo(httptest.NewRequest(http.MethodGet, "/api/v1/repos/acme/demo/branches/"+url.PathEscape(branchName), nil)) req.SetPathValue("name", branchName) rr := httptest.NewRecorder() @@ -64,7 +64,8 @@ func TestBranchDiffHandler_ReturnsDiff(t *testing.T) { reg := singleEntryRegistry(t, s) handler := NewBranchDiffHandler(reg, 0) - req := httptest.NewRequest(http.MethodGet, "/api/v1/branch-diff?branch="+url.QueryEscape(branchName), nil) + req := withRepo(httptest.NewRequest(http.MethodGet, "/api/v1/repos/acme/demo/branch-diff/"+url.PathEscape(branchName), nil)) + req.SetPathValue("name", branchName) rr := httptest.NewRecorder() handler.ServeHTTP(rr, req) @@ -95,7 +96,8 @@ func TestBranchDiffHandler_ThrottleGateRespectsContext(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() - req := httptest.NewRequest(http.MethodGet, "/api/v1/branch-diff?branch="+url.QueryEscape(branchName), nil).WithContext(ctx) + req := withRepo(httptest.NewRequest(http.MethodGet, "/api/v1/repos/acme/demo/branch-diff/"+url.PathEscape(branchName), nil).WithContext(ctx)) + req.SetPathValue("name", branchName) rr := httptest.NewRecorder() handler.ServeHTTP(rr, req) @@ -104,20 +106,22 @@ func TestBranchDiffHandler_ThrottleGateRespectsContext(t *testing.T) { require.Empty(t, rr.Body.String(), "no diff should be computed when throttled and the client is gone") } -func TestBranchDiffHandler_RequiresBranchQuery(t *testing.T) { +func TestBranchDiffHandler_RequiresBranch(t *testing.T) { t.Parallel() s := scenario.NewScenario(t, testhelpers.BasicSceneSetup) reg := singleEntryRegistry(t, s) handler := NewBranchDiffHandler(reg, 0) - req := httptest.NewRequest(http.MethodGet, "/api/v1/branch-diff", nil) + // No {name} path value: the resource resolves to the repo but carries no + // branch, so the handler rejects it before touching git. + req := withRepo(httptest.NewRequest(http.MethodGet, "/api/v1/repos/acme/demo/branch-diff/", nil)) rr := httptest.NewRecorder() handler.ServeHTTP(rr, req) require.Equal(t, http.StatusBadRequest, rr.Code) - require.Contains(t, rr.Body.String(), "missing branch query parameter") + require.Contains(t, rr.Body.String(), "missing branch") } func TestBranchDiffHandler_RejectsUntrackedBranch(t *testing.T) { @@ -127,7 +131,8 @@ func TestBranchDiffHandler_RejectsUntrackedBranch(t *testing.T) { reg := singleEntryRegistry(t, s) handler := NewBranchDiffHandler(reg, 0) - req := httptest.NewRequest(http.MethodGet, "/api/v1/branch-diff?branch=main", nil) + req := withRepo(httptest.NewRequest(http.MethodGet, "/api/v1/repos/acme/demo/branch-diff/main", nil)) + req.SetPathValue("name", "main") rr := httptest.NewRecorder() handler.ServeHTTP(rr, req) @@ -136,15 +141,14 @@ func TestBranchDiffHandler_RejectsUntrackedBranch(t *testing.T) { require.Contains(t, rr.Body.String(), "branch not found or not tracked") } -func TestResolveRepo_Returns404ForUnknownID(t *testing.T) { +func TestResolveRepo_Returns404ForUnknownRepo(t *testing.T) { t.Parallel() reg := registry.New() - require.NoError(t, reg.Add(®istry.RepoEntry{ID: "default"})) + require.NoError(t, reg.Add(®istry.RepoEntry{ID: "default", Owner: "acme", Name: "demo"})) handler := NewBranchesHandler(reg) - req := httptest.NewRequest(http.MethodGet, "/api/v1/repos/missing/branches", nil) - req.SetPathValue("repoID", "missing") + req := withRepoCoords(httptest.NewRequest(http.MethodGet, "/api/v1/repos/acme/missing/branches", nil), "acme", "missing") rr := httptest.NewRecorder() handler.ServeHTTP(rr, req) @@ -167,15 +171,38 @@ func setupTrackedBranchScenario(t *testing.T, branchName string) *scenario.Scena return s } -// singleEntryRegistry builds a registry with one entry id="default" pointing -// at the scenario's engine. Mirrors the bootstrap behavior of the single-repo -// `-cwd` shortcut and lets tests skip path-value setup for repoID. +// testOwner and testRepo are the GitHub coordinates singleEntryRegistry +// registers its entry under. Direct handler calls (no router) resolve to it +// once withRepo sets the matching owner/repo path values resolveRepo reads. +const ( + testOwner = "acme" + testRepo = "demo" +) + +// singleEntryRegistry builds a registry with one entry pointing at the +// scenario's engine, indexed by testOwner/testRepo. Mirrors the bootstrap +// behavior of the single-repo `-cwd` shortcut. func singleEntryRegistry(t *testing.T, s *scenario.Scenario) *registry.Registry { t.Helper() reg := registry.New() require.NoError(t, reg.Add(®istry.RepoEntry{ ID: "default", + Owner: testOwner, + Name: testRepo, Engine: s.Engine, })) return reg } + +// withRepoCoords sets the owner/repo path values resolveRepo reads, standing in +// for the router's {owner}/{repo} match in a direct handler call. +func withRepoCoords(req *http.Request, owner, repo string) *http.Request { + req.SetPathValue("owner", owner) + req.SetPathValue("repo", repo) + return req +} + +// withRepo sets the path values for the singleEntryRegistry entry. +func withRepo(req *http.Request) *http.Request { + return withRepoCoords(req, testOwner, testRepo) +} diff --git a/internal/api/handlers/common.go b/internal/api/handlers/common.go index 8aac63d05..2b57e1b38 100644 --- a/internal/api/handlers/common.go +++ b/internal/api/handlers/common.go @@ -8,11 +8,6 @@ import ( "github.com/getstackit/stackit/internal/utils" ) -// defaultRepoID is the ID assigned to the single bootstrap repo when the -// server is started with `-cwd` (single-repo legacy shortcut). It's also -// substituted on the unscoped legacy routes that don't carry {repoID}. -const defaultRepoID = "default" - // Visibility controls whether a handler may expose operator- or // viewer-identifying fields in its response. A public (read-only) server // must not leak who is running it, so identity fields like currentUser are @@ -30,18 +25,13 @@ const ( VisibilityPublic ) -// resolveRepo looks up the repo entry for the {repoID} path value on r. -// If the path value is empty (legacy/unscoped route or direct test call) -// it falls back to defaultRepoID. Returns false and writes a 404 when the -// repoID does not exist in the registry or is not visible to the requesting -// user. A repo invisible to the caller 404s (not 403) so its existence is not -// disclosed. +// resolveRepo looks up the repo entry for the {owner}/{repo} path values on r, +// matched case-insensitively against the registry's GitHub coordinates. Returns +// false and writes a 404 when no such repo exists or it is not visible to the +// requesting user. A repo invisible to the caller 404s (not 403) so its +// existence is not disclosed. func resolveRepo(reg *registry.Registry, w http.ResponseWriter, r *http.Request) (*registry.RepoEntry, bool) { - id := r.PathValue("repoID") - if id == "" { - id = defaultRepoID - } - entry, ok := reg.Get(id) + entry, ok := reg.GetByOwnerRepo(r.PathValue("owner"), r.PathValue("repo")) if !ok || !visibleTo(entry, requestUser(r)) { http.NotFound(w, r) return nil, false diff --git a/internal/api/handlers/events_test.go b/internal/api/handlers/events_test.go index ae808612f..13659cddb 100644 --- a/internal/api/handlers/events_test.go +++ b/internal/api/handlers/events_test.go @@ -39,12 +39,13 @@ func TestEventsHandlerReturnsWhenBroadcasterCloses(t *testing.T) { broadcaster := registry.NewBroadcaster() require.NoError(t, reg.Add(®istry.RepoEntry{ ID: "default", + Owner: "acme", + Name: "demo", Broadcaster: broadcaster, })) handler := NewEventsHandler(reg, SSELimits{}) - req := httptest.NewRequest(http.MethodGet, "/api/v1/repos/default/events", nil) - req.SetPathValue("repoID", "default") + req := withRepo(httptest.NewRequest(http.MethodGet, "/api/v1/repos/acme/demo/events", nil)) recorder := newSignalingResponseWriter() done := make(chan struct{}) @@ -75,14 +76,15 @@ func TestEventsHandlerRejectsBeyondPerIPCap(t *testing.T) { broadcaster := registry.NewBroadcaster() require.NoError(t, reg.Add(®istry.RepoEntry{ ID: "default", + Owner: "acme", + Name: "demo", Broadcaster: broadcaster, })) handler := NewEventsHandler(reg, SSELimits{MaxPerIP: 1}) // First connection holds the only per-IP slot. Run it in the background; // it blocks streaming until the broadcaster closes. - firstReq := httptest.NewRequest(http.MethodGet, "/api/v1/repos/default/events", nil) - firstReq.SetPathValue("repoID", "default") + firstReq := withRepo(httptest.NewRequest(http.MethodGet, "/api/v1/repos/acme/demo/events", nil)) first := newSignalingResponseWriter() done := make(chan struct{}) go func() { @@ -98,8 +100,7 @@ func TestEventsHandlerRejectsBeyondPerIPCap(t *testing.T) { // Second connection from the same IP exceeds the cap and is rejected // without blocking. - secondReq := httptest.NewRequest(http.MethodGet, "/api/v1/repos/default/events", nil) - secondReq.SetPathValue("repoID", "default") + secondReq := withRepo(httptest.NewRequest(http.MethodGet, "/api/v1/repos/acme/demo/events", nil)) second := httptest.NewRecorder() handler.ServeHTTP(second, secondReq) require.Equal(t, http.StatusTooManyRequests, second.Code) @@ -119,12 +120,13 @@ func TestEventsHandlerReturns404ForUnknownRepo(t *testing.T) { reg := registry.New() require.NoError(t, reg.Add(®istry.RepoEntry{ ID: "default", + Owner: "acme", + Name: "demo", Broadcaster: registry.NewBroadcaster(), })) handler := NewEventsHandler(reg, SSELimits{}) - req := httptest.NewRequest(http.MethodGet, "/api/v1/repos/missing/events", nil) - req.SetPathValue("repoID", "missing") + req := withRepoCoords(httptest.NewRequest(http.MethodGet, "/api/v1/repos/acme/missing/events", nil), "acme", "missing") rr := httptest.NewRecorder() handler.ServeHTTP(rr, req) diff --git a/internal/api/handlers/sync_test.go b/internal/api/handlers/sync_test.go index 888ad5157..ed79d3983 100644 --- a/internal/api/handlers/sync_test.go +++ b/internal/api/handlers/sync_test.go @@ -22,10 +22,9 @@ func (f *fakeManagedSyncer) SyncRepo(_ context.Context, owner, name string) erro return f.err } -func syncRequest(repoID string) *http.Request { - req := httptest.NewRequest(http.MethodPost, "/api/v1/repos/"+repoID+"/sync", nil) - req.SetPathValue("repoID", repoID) - return req +func syncRequest(owner, repo string) *http.Request { + req := httptest.NewRequest(http.MethodPost, "/api/v1/repos/"+owner+"/"+repo+"/sync", nil) + return withRepoCoords(req, owner, repo) } func TestSyncHandler_ManagedRepoMirrorFetches(t *testing.T) { @@ -37,7 +36,7 @@ func TestSyncHandler_ManagedRepoMirrorFetches(t *testing.T) { h := NewSyncHandler(reg, sy) rr := httptest.NewRecorder() - h.ServeHTTP(rr, syncRequest("m")) + h.ServeHTTP(rr, syncRequest("octo", "widget")) require.Equal(t, http.StatusNoContent, rr.Code) require.Equal(t, []string{"octo/widget"}, sy.calls) @@ -52,7 +51,7 @@ func TestSyncHandler_ManagedRepoFetchErrorReturns502(t *testing.T) { h := NewSyncHandler(reg, sy) rr := httptest.NewRecorder() - h.ServeHTTP(rr, syncRequest("m")) + h.ServeHTTP(rr, syncRequest("octo", "widget")) require.Equal(t, http.StatusBadGateway, rr.Code) } @@ -63,13 +62,13 @@ func TestSyncHandler_LocalRepoRefreshesWithoutFetch(t *testing.T) { // Unmanaged repo with no engine: Refresh is a no-op, but the key assertion // is that the managed mirror-fetch path is never taken for a working repo // (which would detach its HEAD). - require.NoError(t, reg.Add(®istry.RepoEntry{ID: "default", Managed: false})) + require.NoError(t, reg.Add(®istry.RepoEntry{ID: "default", Managed: false, Owner: "acme", Name: "demo"})) sy := &fakeManagedSyncer{} h := NewSyncHandler(reg, sy) rr := httptest.NewRecorder() - h.ServeHTTP(rr, syncRequest("default")) + h.ServeHTTP(rr, syncRequest("acme", "demo")) require.Equal(t, http.StatusNoContent, rr.Code) require.Empty(t, sy.calls, "a local working repo must never be mirror-fetched") @@ -80,7 +79,7 @@ func TestSyncHandler_UnknownRepo404(t *testing.T) { h := NewSyncHandler(registry.New(), &fakeManagedSyncer{}) rr := httptest.NewRecorder() - h.ServeHTTP(rr, syncRequest("nope")) + h.ServeHTTP(rr, syncRequest("nope", "nope")) require.Equal(t, http.StatusNotFound, rr.Code) } @@ -91,8 +90,7 @@ func TestSyncHandler_RejectsNonPost(t *testing.T) { require.NoError(t, reg.Add(®istry.RepoEntry{ID: "m", Managed: true, Owner: "o", Name: "n"})) h := NewSyncHandler(reg, &fakeManagedSyncer{}) - req := httptest.NewRequest(http.MethodGet, "/api/v1/repos/m/sync", nil) - req.SetPathValue("repoID", "m") + req := withRepoCoords(httptest.NewRequest(http.MethodGet, "/api/v1/repos/o/n/sync", nil), "o", "n") rr := httptest.NewRecorder() h.ServeHTTP(rr, req) diff --git a/internal/api/handlers/validation_test.go b/internal/api/handlers/validation_test.go index 73a479f35..df217d836 100644 --- a/internal/api/handlers/validation_test.go +++ b/internal/api/handlers/validation_test.go @@ -31,27 +31,25 @@ var validationCases = []validationCase{ name: "BranchesHandler getBranch path value", build: func(reg *registry.Registry) http.Handler { return NewBranchesHandler(reg) }, method: http.MethodGet, - url: "/api/v1/branches/x", + url: "/api/v1/repos/acme/demo/branches/x", setRequest: func(req *http.Request, branch string) { req.SetPathValue("name", branch) }, }, { - name: "BranchDiffHandler branch query", + name: "BranchDiffHandler branch path value", build: func(reg *registry.Registry) http.Handler { return NewBranchDiffHandler(reg, 0) }, method: http.MethodGet, - url: "/api/v1/branch-diff?branch=x", + url: "/api/v1/repos/acme/demo/branch-diff/x", setRequest: func(req *http.Request, branch string) { - q := req.URL.Query() - q.Set("branch", branch) - req.URL.RawQuery = q.Encode() + req.SetPathValue("name", branch) }, }, { name: "SubmitHandler rootBranch path value", build: func(reg *registry.Registry) http.Handler { return NewSubmitHandler(reg) }, method: http.MethodPost, - url: "/api/v1/stacks/x/submit", + url: "/api/v1/repos/acme/demo/stacks/x/submit", setRequest: func(req *http.Request, branch string) { req.SetPathValue("rootBranch", branch) }, @@ -83,9 +81,9 @@ func TestHandlers_RejectMalformedBranchNames(t *testing.T) { t.Parallel() reg := registry.New() - require.NoError(t, reg.Add(®istry.RepoEntry{ID: "default"})) + require.NoError(t, reg.Add(®istry.RepoEntry{ID: "default", Owner: testOwner, Name: testRepo})) - req := httptest.NewRequest(vc.method, vc.url, nil) + req := withRepo(httptest.NewRequest(vc.method, vc.url, nil)) vc.setRequest(req, bad.input) rr := httptest.NewRecorder() diff --git a/internal/api/handlers/visibility_test.go b/internal/api/handlers/visibility_test.go index f1b4d65dc..d4c4f3cb4 100644 --- a/internal/api/handlers/visibility_test.go +++ b/internal/api/handlers/visibility_test.go @@ -96,7 +96,7 @@ func TestReposListFiltersByUser(t *testing.T) { func TestResolveRepoVisibility(t *testing.T) { t.Parallel() reg := registry.New() - require.NoError(t, reg.Add(®istry.RepoEntry{ID: "alice-repo", AddedBy: "alice"})) + require.NoError(t, reg.Add(®istry.RepoEntry{ID: "alice-repo", AddedBy: "alice", Owner: "alice", Name: "repo"})) // A tiny handler that 200s when resolveRepo succeeds. probe := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -105,16 +105,12 @@ func TestResolveRepoVisibility(t *testing.T) { } w.WriteHeader(http.StatusOK) }) - withRepoID := func(req *http.Request, id string) *http.Request { - req.SetPathValue("repoID", id) - return req - } t.Run("owner resolves", func(t *testing.T) { t.Parallel() gated, base := sessionFor(t, "alice", probe) rec := httptest.NewRecorder() - gated.ServeHTTP(rec, withRepoID(base, "alice-repo")) + gated.ServeHTTP(rec, withRepoCoords(base, "alice", "repo")) require.Equal(t, http.StatusOK, rec.Code) }) @@ -122,14 +118,14 @@ func TestResolveRepoVisibility(t *testing.T) { t.Parallel() gated, base := sessionFor(t, "bob", probe) rec := httptest.NewRecorder() - gated.ServeHTTP(rec, withRepoID(base, "alice-repo")) + gated.ServeHTTP(rec, withRepoCoords(base, "alice", "repo")) require.Equal(t, http.StatusNotFound, rec.Code) }) t.Run("anonymous gets 404", func(t *testing.T) { t.Parallel() rec := httptest.NewRecorder() - probe.ServeHTTP(rec, withRepoID(httptest.NewRequest(http.MethodGet, "/api/v1/repos/alice-repo/view", nil), "alice-repo")) + probe.ServeHTTP(rec, withRepoCoords(httptest.NewRequest(http.MethodGet, "/api/v1/repos/alice/repo/view", nil), "alice", "repo")) require.Equal(t, http.StatusNotFound, rec.Code) }) } diff --git a/internal/api/readonly_test.go b/internal/api/readonly_test.go index d6d2b359d..bcf37d480 100644 --- a/internal/api/readonly_test.go +++ b/internal/api/readonly_test.go @@ -66,7 +66,7 @@ func newTestAuthConfig(t *testing.T) *AuthConfig { // submitRequest builds a POST to the submit route with the CSRF header set, // so it clears RequireCSRFHeader and reaches the routed handler. func submitRequest() *http.Request { - req := httptest.NewRequest(http.MethodPost, "/api/v1/repos/default/stacks/main/submit", nil) + req := httptest.NewRequest(http.MethodPost, "/api/v1/repos/acme/demo/stacks/main/submit", nil) req.Header.Set(auth.CSRFHeader, "1") return req } diff --git a/internal/api/server.go b/internal/api/server.go index df4593081..eff1f882f 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -266,33 +266,22 @@ func (s *Server) buildHandler() (http.Handler, error) { apiMux.Handle("GET "+prefix+"/repos", reposListHandler) apiMux.Handle("POST "+prefix+"/repos", onboardHandler) - // New multi-repo routes. {repoID} resolves through the registry; - // unknown IDs return 404 from inside the handler. - apiMux.Handle("GET "+prefix+"/repos/{repoID}/view", viewHandler) - apiMux.Handle("GET "+prefix+"/repos/{repoID}/repo", repoHandler) - apiMux.Handle("GET "+prefix+"/repos/{repoID}/stacks", stacksHandler) - apiMux.Handle("GET "+prefix+"/repos/{repoID}/stacks/{name...}", stacksHandler) - apiMux.Handle("POST "+prefix+"/repos/{repoID}/stacks/{rootBranch}/submit", submitHandler) - apiMux.Handle("POST "+prefix+"/repos/{repoID}/sync", syncHandler) - apiMux.Handle("GET "+prefix+"/repos/{repoID}/branches", branchesHandler) - apiMux.Handle("GET "+prefix+"/repos/{repoID}/branches/{name...}", branchesHandler) - apiMux.Handle("GET "+prefix+"/repos/{repoID}/branch-diff", branchDiffHandler) - apiMux.Handle("GET "+prefix+"/repos/{repoID}/events", eventsHandler) - - // Legacy unscoped routes. With no {repoID} path value, handlers - // fall back to "default" (see defaultRepoID in handlers/common.go). - // These keep the existing web client working while the multi-repo - // migration is in progress and are removed once the frontend uses - // the /repos/{repoID}/ shape. - apiMux.Handle("GET "+prefix+"/view", viewHandler) - apiMux.Handle("GET "+prefix+"/repo", repoHandler) - apiMux.Handle("GET "+prefix+"/stacks", stacksHandler) - apiMux.Handle("GET "+prefix+"/stacks/{name...}", stacksHandler) - apiMux.Handle("POST "+prefix+"/stacks/{rootBranch}/submit", submitHandler) - apiMux.Handle("GET "+prefix+"/branches", branchesHandler) - apiMux.Handle("GET "+prefix+"/branches/{name...}", branchesHandler) - apiMux.Handle("GET "+prefix+"/branch-diff", branchDiffHandler) - apiMux.Handle("GET "+prefix+"/events", eventsHandler) + // GitHub-style per-repo routes. {owner}/{repo} resolves through the + // registry's owner/repo index (case-insensitive); unknown coordinates + // return 404 from inside the handler. Branch and stack names occupy a + // trailing {name...} wildcard so they can contain slashes; that forces + // branch-diff to be its own resource (the wildcard must be last, so + // .../branches/{name}/diff is not expressible). + apiMux.Handle("GET "+prefix+"/repos/{owner}/{repo}/view", viewHandler) + apiMux.Handle("GET "+prefix+"/repos/{owner}/{repo}/repo", repoHandler) + apiMux.Handle("GET "+prefix+"/repos/{owner}/{repo}/stacks", stacksHandler) + apiMux.Handle("GET "+prefix+"/repos/{owner}/{repo}/stacks/{name...}", stacksHandler) + apiMux.Handle("POST "+prefix+"/repos/{owner}/{repo}/stacks/{rootBranch}/submit", submitHandler) + apiMux.Handle("POST "+prefix+"/repos/{owner}/{repo}/sync", syncHandler) + apiMux.Handle("GET "+prefix+"/repos/{owner}/{repo}/branches", branchesHandler) + apiMux.Handle("GET "+prefix+"/repos/{owner}/{repo}/branches/{name...}", branchesHandler) + apiMux.Handle("GET "+prefix+"/repos/{owner}/{repo}/branch-diff/{name...}", branchDiffHandler) + apiMux.Handle("GET "+prefix+"/repos/{owner}/{repo}/events", eventsHandler) } // Apply session enforcement to /api/* only. /auth/* and static assets diff --git a/internal/api/sync_route_test.go b/internal/api/sync_route_test.go index ba188cebf..cf313ee95 100644 --- a/internal/api/sync_route_test.go +++ b/internal/api/sync_route_test.go @@ -13,7 +13,7 @@ import ( // syncRouteRequest builds a POST to the manual-sync route with the CSRF header // set so it clears RequireCSRFHeader and reaches the routed handler. func syncRouteRequest() *http.Request { - req := httptest.NewRequest(http.MethodPost, "/api/v1/repos/default/sync", nil) + req := httptest.NewRequest(http.MethodPost, "/api/v1/repos/acme/demo/sync", nil) req.Header.Set(auth.CSRFHeader, "1") return req }

AltStyle によって変換されたページ (->オリジナル) /