diff --git a/api/openapi/stackit.yaml b/api/openapi/stackit.yaml index bd7b9480..8bed904e 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 6f45cb8f..e75bed2c 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 0250912b..87e1c36f 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 b66eedea..a5a044ef 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 d3445240..b9f317ed 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 8aac63d0..2b57e1b3 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 ae808612..13659cdd 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 888ad515..ed79d398 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 73a479f3..df217d83 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 f1b4d65d..d4c4f3cb 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 d6d2b359..bcf37d48 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 df459308..eff1f882 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 ba188ceb..cf313ee9 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 によって変換されたページ (->オリジナル) /