canonical / canonical/microceph
Redesign replication REST API: resource-scoped paths, drop body-encoded request type
- Dominant language
- Go
- Stars
- 396
- Forks
- 74
- Avg merge
- 2d 20h
- Merged PRs (30d)
- 7
Description
Parent: #726
Depends on: #727 (per-workload FSM and site-wide-action bypass must land first).
## Background
The current replication REST surface encodes operation semantics in two places:
- HTTP method + URL path
- A `RequestType` field on the JSON request body (`api/types/replication_rbd.go:107`, `api/types/replication_cephfs.go:36`)
Both are needed today because the URL `PUT /ops/replication/{wl}` is overloaded: it serves both site-wide actions (promote, demote) and would collide with a resource-scoped path if the resource name happened to be `promote`, `demote`, or anything placed at the action position. The body field exists to disambiguate.
This produces several artifacts:
- `WorkloadReplicationRequest = ""` sentinel constant (`api/types/replication.go:30`).
- `OverwriteRequestType` server-side method that conditionally overwrites the client-supplied value (`api/types/replication_rbd.go:158`, `api/types/replication_cephfs.go:82`) — empty overwrite preserves client intent (workload-level), non-empty replaces it (resource-level).
- `GetAPIRequestType` / `GetWorkloadRequestType` parsers that string-split a hyphen-encoded value carrying both the HTTP method and the FSM event (`api/types/replication.go:53,64`).
- Client-side HTTP method dispatch reads `RequestType` to choose verb (`client/replication.go:24,31`).
The schema is internally consistent but reflects the URL ambiguity rather than fixing it.
## Proposed redesign
URL layout where path identifies the resource, HTTP method identifies the operation, and a separate `/actions/` namespace holds site-wide ops so resource names cannot collide:
```
# Remotes
GET /v1/remotes
POST /v1/remotes
GET /v1/remotes/{name}
DELETE /v1/remotes/{name}
# RBD mirrors
GET /v1/mirrors/rbd
GET /v1/mirrors/rbd/pools/{pool}
POST /v1/mirrors/rbd/pools/{pool}
PATCH /v1/mirrors/rbd/pools/{pool}
DELETE /v1/mirrors/rbd/pools/{pool}
GET /v1/mirrors/rbd/pools/{pool}/images/{image}
POST /v1/mirrors/rbd/pools/{pool}/images/{image}
DELETE /v1/mirrors/rbd/pools/{pool}/images/{image}
# CephFS mirrors
GET /v1/mirrors/cephfs/volumes/{vol}
POST /v1/mirrors/cephfs/volumes/{vol}
DELETE /v1/mirrors/cephfs/volumes/{vol}
# Site-wide actions (separate namespace — cannot collide with resource names)
POST /v1/mirrors/rbd/actions/promote
POST /v1/mirrors/rbd/actions/demote
POST /v1/mirrors/rbd/actions/resync
```
### Properties
- Path identifies the resource. HTTP method identifies the operation. No body field carries that information.
- `/actions/` is a fixed sub-namespace under the workload root. A pool named `actions` lives at `pools/actions`, not at `actions`. No collision.
- `POST` on a resource = enable. `DELETE` = disable. `PATCH` = reconfigure. `GET` = status or list.
- Site actions are `POST` under `/actions/` because they are imperative invocations, not resource lifecycle transitions.
- `/v1/` prefix gives a versioning anchor.
### What this kills
- `RequestType` field on the wire (becomes server-internal, derived from `(method, path)`, or replaced by direct dispatch at the API entry).
- `WorkloadReplicationRequest` sentinel.
- `OverwriteRequestType` method.
- `GetAPIRequestType` / `GetWorkloadRequestType` string-split parsing.
### What stays
- FSM internals untouched (assuming #727 has landed). Event constants still drive handler dispatch. The mapping from `(method, path)` to event happens once at the API entry.
- Handler interface unchanged.
- Database / remote storage unchanged.
## Migration
The replication API is consumed by the in-tree CLI (`cmd/microceph/replication_*.go`). Audit needed: enumerate any callers outside the microceph CLI.
- No external consumer: internal rename is feasible — single PR updates server routes, client library, and CLI together.
- External consumers exist: ship `/v1/` alongside legacy paths and deprecate legacy in a follow-up release.
## Out of scope
- FSM internals and per-workload transition tables (#727).
- ProxyTarget setting for mutating endpoints (handled by concurrency issue under #726).
- Concurrency safety on mutating requests (handled by concurrency issue under #726).
## Acceptance
- Replication API responds at `/v1/mirrors/...` and `/v1/remotes/...`.
- Body schemas no longer carry a request-type discriminator.
- CLI subcommands construct URLs by resource path; HTTP method is picked directly without consulting a body field.
- Resource names that overlap with action verbs (`promote`, `demote`, `resync`) work correctly.
- Legacy paths either removed or marked deprecated, depending on the migration audit.
Contributor guide
Research direction
Start by reading api/types/replication.go, api/types/replication_rbd.go, api/types/replication_cephfs.go, client/replication.go, and cmd/microceph/replication_*.go; first verify that #727 has landed and audit callers beyond the CLI. Trace the current routes and request-type handling before changing them. Done means the /v1/ resource and action paths work, body discriminators and obsolete parsers are removed, CLI dispatch uses paths and methods, and the migration choice is documented.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api, backend, cli, distributed-systems
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100