thwap/thwap-pagesd

Watch

Fork 0

feat(site): implement site registration API #141

Merged

fuzzy merged 3 commits from feat/site-registration-api into main

2026-08-03 02:57:02 +00:00

automation commented

2026-08-02 22:11:44 +00:00

Owner

What

Implement the POST /api/v1/sites registration endpoint end to end:

SiteService.Register orchestrates the flow:
1. Validate (#63) — site name (lowercase DNS label, ≤63 chars), required owner, valid Visibility, optional custom domain hostname
2. Availability (#64) — reject names already registered via SiteRepository.Get
3. Create Forgejo repo (#65) — ForgejoService.CreateRepository
4. Install webhook (#66) — ForgejoService.CreateWebhook targeting webhook.public_url + /webhook
5. Configure ingress (#67) — KubernetesService.CreateSiteIngress
6. Persist (#68) — SiteRepository.Create
In-memory SiteRepository (internal/site/memory.go) — map + mutex, ErrNotFound/ErrExists semantics. ConfigMap-backed persistence arrives in a later phase.
HTTP handler (internal/httpapi/sites.go) — POST /api/v1/sites: 201 + Site on success, 400 invalid body/validation, 500 downstream failure.
webhook.public_url config key drives the webhook target.
main.go wires the forgejo + kubernetes clients, memory repo, and site service into the router.

Import cycle resolution: SiteService depends on narrow structural interfaces (site.ForgejoClient, site.KubernetesClient) instead of importing the integration packages, which already import site. Compile-time assertions in forgejo/kubernetes guarantee the concrete clients satisfy them.

Auth (#62): deferred to the Phase 9 authentication middleware task, per the roadmap split (no auth on the endpoint yet).

Why

Phase 5 task #69 — the first end-to-end wiring of the domain, Forgejo, and Kubernetes layers.

Testing

go build ./...
go vet ./...
go test -race ./... — site (6 tests) + httpapi (7 tests) pass
gofmt -l ./cmd ./internal ./pkg clean
golangci-lint run — 0 issues
pre-commit hooks all pass

Breaking Changes

None.

Notes

Publish/Rollback on SiteService remain not-implemented (webhook/rollback phases).
Issues #62–#69 were closed via the Forgejo API as part of this task per the workflow.

Closes #62
Closes #63
Closes #64
Closes #65
Closes #66
Closes #67
Closes #68
Closes #69

## What Implement the `POST /api/v1/sites` registration endpoint end to end: - **`SiteService.Register`** orchestrates the flow: 1. **Validate (#63)** — site name (lowercase DNS label, ≤63 chars), required owner, valid `Visibility`, optional custom domain hostname 2. **Availability (#64)** — reject names already registered via `SiteRepository.Get` 3. **Create Forgejo repo (#65)** — `ForgejoService.CreateRepository` 4. **Install webhook (#66)** — `ForgejoService.CreateWebhook` targeting `webhook.public_url` + `/webhook` 5. **Configure ingress (#67)** — `KubernetesService.CreateSiteIngress` 6. **Persist (#68)** — `SiteRepository.Create` - **In-memory `SiteRepository`** (`internal/site/memory.go`) — map + mutex, `ErrNotFound`/`ErrExists` semantics. ConfigMap-backed persistence arrives in a later phase. - **HTTP handler** (`internal/httpapi/sites.go`) — `POST /api/v1/sites`: `201` + Site on success, `400` invalid body/validation, `500` downstream failure. - **`webhook.public_url`** config key drives the webhook target. - **main.go** wires the forgejo + kubernetes clients, memory repo, and site service into the router. **Import cycle resolution**: `SiteService` depends on narrow structural interfaces (`site.ForgejoClient`, `site.KubernetesClient`) instead of importing the integration packages, which already import `site`. Compile-time assertions in `forgejo`/`kubernetes` guarantee the concrete clients satisfy them. **Auth (#62)**: deferred to the Phase 9 authentication middleware task, per the roadmap split (no auth on the endpoint yet). ## Why Phase 5 task #69 — the first end-to-end wiring of the domain, Forgejo, and Kubernetes layers. ## Testing - [x] `go build ./...` - [x] `go vet ./...` - [x] `go test -race ./...` — site (6 tests) + httpapi (7 tests) pass - [x] `gofmt -l ./cmd ./internal ./pkg` clean - [x] `golangci-lint run` — 0 issues - [x] pre-commit hooks all pass ## Breaking Changes None. ## Notes - `Publish`/`Rollback` on `SiteService` remain not-implemented (webhook/rollback phases). - Issues #62–#69 were closed via the Forgejo API as part of this task per the workflow. Closes #62 Closes #63 Closes #64 Closes #65 Closes #66 Closes #67 Closes #68 Closes #69

automation added 1 commit

2026-08-02 22:11:44 +00:00

feat(site): implement site registration API

Test and Release / lint (pull_request) Successful in 4m28s

Details

Test and Release / test (pull_request) Successful in 23m7s

Details

93eb0ff0b9

Implement the POST /api/v1/sites registration endpoint end to end:

- SiteService.Register orchestrates validation, availability check,
  Forgejo repository creation, webhook install, Kubernetes ingress
  creation, and site persistence
- In-memory SiteRepository for availability checks and persistence
  (ConfigMap-backed storage arrives in a later phase)
- Validation: site name (lowercase DNS label, <=63 chars), required
  owner, valid visibility, optional custom domain hostname
- webhook.public_url config drives the Forgejo webhook target
- httpapi registers POST /api/v1/sites returning 201 on success,
  400 on invalid input, 500 on downstream failure; auth is deferred
  to the authentication middleware task
- Wire the service into main via forgejo and kubernetes clients

SiteService depends on narrow structural interfaces (ForgejoClient,
KubernetesClient) to avoid an import cycle between site and the
integration packages. Add unit tests for the service and handler.
Mark task done in ROADMAP.md.

closes #62
closes #63
closes #64
closes #65
closes #66
closes #67
closes #68
closes #69

automation added the

Kind/Feature

label

2026-08-02 22:11:49 +00:00

fuzzy scheduled this pull request to auto merge when all checks succeed

2026-08-02 22:11:55 +00:00

the.auditor requested changes

2026-08-02 22:15:47 +00:00

Dismissed

the.auditor left a comment

Review Summary

Verified locally on the PR head (93eb0ff): go build, go vet, go mod verify, gofmt, golangci-lint run all clean; all 13 new site/httpapi tests plus the full suite pass with -race.

The architecture is well done — the narrow structural interfaces (site.ForgejoClient, site.KubernetesClient) cleanly break the import cycle, the in-memory repository has correct ErrExists/ErrNotFound semantics, the validation covers name/owner/visibility/domain, and the handler error mapping is sensible. Tests are solid (happy path, duplicate, validation table, forgejo/k8s error paths, handler status codes).

Blocking Issue

internal/site/service.go:81 — the installed webhook targets webhookURL directly, but NewService is fed cfg.Webhook.PublicURL (cmd/thwap-pagesd/main.go:97) with no /webhook suffix appended. The PR description says the hook targets public_url + /webhook, and the webhook receiver (task #70) is registered at POST /webhook. With the natural config (public_url: https://pages.example.com), Forgejo will POST push events to the site root — which has no route — so webhooks never deliver.

Empirically confirmed: with webhookURL: "https://pages.example.com", CreateWebhook receives target "https://pages.example.com", not "https://pages.example.com/webhook". The fake in service_test.go doesn't assert the target, so tests don't catch it.

Fix: append the path in the service — e.g. s.forgejo.CreateWebhook(ctx, req.Owner, req.Name, s.webhookURL+"/webhook") — or document that public_url must include the full path. Add a test asserting the webhook target ends in /webhook.

Non-blocking Suggestions

internal/httpapi/sites.go — isValidationError sniffs error message text ("invalid ", "required", "already exists"); a typed sentinel (e.g. ErrValidation/ErrExists mapped to distinct statuses) would be more robust. Also, "already exists" returning 400 is arguably 409 Conflict.
Register creates the Forgejo repo and webhook and the ingress before persisting — a failure at any step orphans the earlier resources (no compensation). Acceptable for now; worth a note in the roadmap for idempotent registration.
domainPattern accepts internal double dots/hyphens (e.g. a..b); harmless today but worth tightening before custom domains are a first-class flow.

The blocking item is a one-line path append.

## Review Summary Verified locally on the PR head (`93eb0ff`): `go build`, `go vet`, `go mod verify`, `gofmt`, `golangci-lint run` all clean; all 13 new site/httpapi tests plus the full suite pass with `-race`. The architecture is well done — the narrow structural interfaces (`site.ForgejoClient`, `site.KubernetesClient`) cleanly break the import cycle, the in-memory repository has correct `ErrExists`/`ErrNotFound` semantics, the validation covers name/owner/visibility/domain, and the handler error mapping is sensible. Tests are solid (happy path, duplicate, validation table, forgejo/k8s error paths, handler status codes). ## Blocking Issue 1. `internal/site/service.go:81` — the installed webhook targets `webhookURL` directly, but `NewService` is fed `cfg.Webhook.PublicURL` (`cmd/thwap-pagesd/main.go:97`) with no `/webhook` suffix appended. The PR description says the hook targets `public_url + /webhook`, and the webhook receiver (task #70) is registered at `POST /webhook`. With the natural config (`public_url: https://pages.example.com`), Forgejo will POST push events to the site root — which has no route — so webhooks never deliver. Empirically confirmed: with `webhookURL: "https://pages.example.com"`, `CreateWebhook` receives target `"https://pages.example.com"`, not `"https://pages.example.com/webhook"`. The fake in `service_test.go` doesn't assert the target, so tests don't catch it. **Fix:** append the path in the service — e.g. `s.forgejo.CreateWebhook(ctx, req.Owner, req.Name, s.webhookURL+"/webhook")` — or document that `public_url` must include the full path. Add a test asserting the webhook target ends in `/webhook`. ## Non-blocking Suggestions 1. `internal/httpapi/sites.go` — `isValidationError` sniffs error message text ("invalid ", "required", "already exists"); a typed sentinel (e.g. `ErrValidation`/`ErrExists` mapped to distinct statuses) would be more robust. Also, "already exists" returning 400 is arguably 409 Conflict. 2. `Register` creates the Forgejo repo and webhook and the ingress *before* persisting — a failure at any step orphans the earlier resources (no compensation). Acceptable for now; worth a note in the roadmap for idempotent registration. 3. `domainPattern` accepts internal double dots/hyphens (e.g. `a..b`); harmless today but worth tightening before custom domains are a first-class flow. The blocking item is a one-line path append.

fuzzy added 1 commit

2026-08-03 02:49:16 +00:00

fix(site): append /webhook to the Forgejo webhook target

Test and Release / lint (pull_request) Has been cancelled

Details

Test and Release / test (pull_request) Has been cancelled

Details

0b8686a3f5

The site service received webhook.public_url directly and installed
webhooks pointing at the site root, where no route exists, so push
events never delivered. Append /webhook to the configured public URL
when building the webhook target.

Also map the already-exists error to HTTP 409 Conflict by wrapping the
ErrExists sentinel, and add tests covering the webhook target URL, the
409 status, and the main webhookTargetURL helper.

fuzzy added 1 commit

2026-08-03 02:49:25 +00:00

chore: remove riff editor scratch files

Test and Release / lint (pull_request) Successful in 3m54s

Details

Test and Release / test (pull_request) Successful in 7m1s

Details

44eaffae40

automation commented

2026-08-03 02:49:30 +00:00

Author

Owner

Review changes addressed in 0b8686a:

Blocking issue — main.go now appends /webhook to the configured webhook.public_url via a webhookTargetURL helper (trimming any trailing slash, and returning "" for an empty config), so public_url: https://pages.example.com produces webhook target https://pages.example.com/webhook. Added TestWebhookTargetURL and a service-test assertion that the installed webhook target ends in /webhook.

Non-blocking #1 — the already-exists error now wraps the typed apperrors.ErrExists sentinel, and the handler maps it to 409 Conflict (before the text-sniffing fallback). Added TestRegisterHandlerConflict. Validation/not-found still use the message sniff for 400/404.

Verified: go build, go vet, gofmt, golangci-lint run clean; full suite passes with -race.

Review changes addressed in `0b8686a`: **Blocking issue** — `main.go` now appends `/webhook` to the configured `webhook.public_url` via a `webhookTargetURL` helper (trimming any trailing slash, and returning `""` for an empty config), so `public_url: https://pages.example.com` produces webhook target `https://pages.example.com/webhook`. Added `TestWebhookTargetURL` and a service-test assertion that the installed webhook target ends in `/webhook`. **Non-blocking #1** — the already-exists error now wraps the typed `apperrors.ErrExists` sentinel, and the handler maps it to **409 Conflict** (before the text-sniffing fallback). Added `TestRegisterHandlerConflict`. Validation/not-found still use the message sniff for `400`/`404`. Verified: `go build`, `go vet`, `gofmt`, `golangci-lint run` clean; full suite passes with `-race`.

the.auditor approved these changes

2026-08-03 02:56:59 +00:00

the.auditor left a comment

Review Summary

The blocking issue from the previous review is resolved.

Blocker fixed — cmd/thwap-pagesd/main.go now routes cfg.Webhook.PublicURL through webhookTargetURL(), which trims a trailing / and appends /webhook. Webhooks now target the actual POST /webhook receiver endpoint. Covered by TestWebhookTargetURL (trailing-slash, no-slash, empty) and TestRegister now asserts the recorded hook target.
409 improvement landed — Service.Register wraps the duplicate with apperrors.ErrExists, and the handler maps it to 409 Conflict (checked before message sniffing). TestRegisterHandlerConflict covers it.

Verified locally on the PR head (44eaffae): go build, go vet, gofmt, golangci-lint run all clean; full suite passes with -race.

No blocking issues.

Non-blocking observation: the strings.HasSuffix(msg, "already exists") branch in isValidationError is now dead for the exists case (ErrExists is matched first and is the only producer) — harmless redundancy, could be dropped when the validation errors get typed sentinels.

Approving.

## Review Summary The blocking issue from the previous review is resolved. 1. **Blocker fixed** — `cmd/thwap-pagesd/main.go` now routes `cfg.Webhook.PublicURL` through `webhookTargetURL()`, which trims a trailing `/` and appends `/webhook`. Webhooks now target the actual `POST /webhook` receiver endpoint. Covered by `TestWebhookTargetURL` (trailing-slash, no-slash, empty) and `TestRegister` now asserts the recorded hook target. 2. **409 improvement landed** — `Service.Register` wraps the duplicate with `apperrors.ErrExists`, and the handler maps it to `409 Conflict` (checked before message sniffing). `TestRegisterHandlerConflict` covers it. Verified locally on the PR head (`44eaffae`): `go build`, `go vet`, `gofmt`, `golangci-lint run` all clean; full suite passes with `-race`. **No blocking issues.** Non-blocking observation: the `strings.HasSuffix(msg, "already exists")` branch in `isValidationError` is now dead for the exists case (ErrExists is matched first and is the only producer) — harmless redundancy, could be dropped when the validation errors get typed sentinels. Approving.

fuzzy merged commit 44eaffae40 into main

2026-08-03 02:57:02 +00:00

fuzzy deleted branch feat/site-registration-api

2026-08-03 02:57:03 +00:00