diff --git a/README.md b/README.md index 55d6bbc..5b61283 100644 --- a/README.md +++ b/README.md @@ -170,6 +170,13 @@ What that does and does not promise: The [durability and clustering docs](https://skyoo2003.github.io/kvs/docs/clustering/) go through each of these, and what to do about them. +## Compatibility + +What `v1` promises not to break, and what it deliberately leaves out, is on the +[compatibility page](https://skyoo2003.github.io/kvs/docs/compatibility/). Read it before +pinning kvs, and before opening a port: kvs has no authorization and no TLS, only the RESP +listener can be given a password, and HTTP and gRPC listen on every interface by default. + ## Documentation Full documentation is available at [skyoo2003.github.io/kvs](https://skyoo2003.github.io/kvs). diff --git a/api_surface_test.go b/api_surface_test.go new file mode 100644 index 0000000..e6e3418 --- /dev/null +++ b/api_surface_test.go @@ -0,0 +1,167 @@ +package kvs + +import ( + "bytes" + "flag" + "fmt" + "go/ast" + "go/parser" + "go/printer" + "go/token" + "os" + "strings" + "testing" +) + +// goldenPath holds the exported surface of this package. It is the machine-readable half of +// content/docs/compatibility.md: the page says what is promised, this file says what is there. +const goldenPath = "testdata/api-surface.txt" + +// headerSep ends the human-facing preamble of the golden file. Everything after it is the +// surface itself, so the warning can be reworded without touching the comparison. +const headerSep = "# ---\n" + +const surfaceHeader = `# Exported API surface of this package - see content/docs/compatibility.md +# for how much of it v1 promises, and for the cluster plumbing it exempts by name. +# A line changed or removed below is a breaking change and needs a major version, unless the +# page exempts it. A line added is a new promise: it cannot be taken back within v1. +# Regenerate deliberately: go test -run TestPublicAPISurface . -update +` + headerSep + +var updateSurface = flag.Bool("update", false, "rewrite "+goldenPath+" from the current source") + +// TestPublicAPISurface fails when the exported surface of this package changes, so that +// widening or breaking the v1 promise is a deliberate act rather than a side effect of some +// other edit. Doc comments are deliberately not part of it: the parser is told to skip them, +// so rewording one costs nothing here. +func TestPublicAPISurface(t *testing.T) { + got := exportedSurface(t) + + if *updateSurface { + if err := os.MkdirAll("testdata", 0o750); err != nil { + t.Fatalf("create testdata: %v", err) + } + + if err := os.WriteFile(goldenPath, []byte(surfaceHeader+got), 0o600); err != nil { + t.Fatalf("write %s: %v", goldenPath, err) + } + + t.Logf("wrote %s", goldenPath) + + return + } + + raw, err := os.ReadFile(goldenPath) + if err != nil { + t.Fatalf("read %s: %v (regenerate with -update)", goldenPath, err) + } + + _, want, ok := strings.Cut(string(raw), headerSep) + if !ok { + t.Fatalf("%s has no %q separator; regenerate with -update", goldenPath, headerSep) + } + + if got != want { + t.Errorf("exported API surface changed.\n%s\n\n"+ + "A changed or removed line is a breaking change unless compatibility.md exempts it;\n"+ + "an added line is a new promise. See content/docs/compatibility.md. If deliberate:\n"+ + " go test -run TestPublicAPISurface . -update", + firstDifference(want, got)) + } +} + +// exportedSurface renders every exported declaration in the package directory, one block per +// file in name order so the output is stable across runs. +func exportedSurface(t *testing.T) string { + t.Helper() + + entries, err := os.ReadDir(".") + if err != nil { + t.Fatalf("read package directory: %v", err) + } + + fset := token.NewFileSet() + + var buf bytes.Buffer + + for _, entry := range entries { + name := entry.Name() + if entry.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") { + continue + } + + // Mode 0 leaves comments unparsed, which is what keeps a reworded doc comment from + // failing this test: only the declarations are the promise. + file, err := parser.ParseFile(fset, name, nil, 0) + if err != nil { + t.Fatalf("parse %s: %v", name, err) + } + + // FileExports drops unexported declarations, struct fields, and interface methods. + // It keeps the bodies of the exported functions, which is why signaturesOnly runs + // after it: how a method is written is not what callers depend on. + if !ast.FileExports(file) { + continue + } + + file.Decls = signaturesOnly(file.Decls) + + fmt.Fprintf(&buf, "==== %s ====\n", name) + + if err := printer.Fprint(&buf, fset, file); err != nil { + t.Fatalf("print %s: %v", name, err) + } + + buf.WriteString("\n\n") + } + + // One trailing newline, which is what the repository's end-of-file hook normalises the + // golden file to. Generating anything else makes committing it fail this test. + return strings.TrimRight(buf.String(), "\n") + "\n" +} + +// signaturesOnly drops the import block and every function body, leaving the declarations a +// caller can actually depend on. Without it an edit to the inside of an exported method would +// read as a change to the promise. +func signaturesOnly(decls []ast.Decl) []ast.Decl { + kept := decls[:0] + + for _, decl := range decls { + switch d := decl.(type) { + case *ast.GenDecl: + if d.Tok == token.IMPORT { + continue + } + case *ast.FuncDecl: + d.Body = nil + } + + kept = append(kept, decl) + } + + return kept +} + +// firstDifference points at the first line that does not match. The whole change is in +// `git diff` on the golden file; what a failing run needs is where to start looking. +func firstDifference(want, got string) string { + wantLines, gotLines := strings.Split(want, "\n"), strings.Split(got, "\n") + + for i := 0; i < len(wantLines) || i < len(gotLines); i++ { + wantLine, gotLine := lineAt(wantLines, i), lineAt(gotLines, i) + if wantLine != gotLine { + return fmt.Sprintf("first difference at line %d:\n promised: %q\n found: %q", + i+1, wantLine, gotLine) + } + } + + return "no line differs, but the text does (trailing whitespace?)" +} + +func lineAt(lines []string, i int) string { + if i >= len(lines) { + return "" + } + + return lines[i] +} diff --git a/changes/unreleased/Added-20260809-000000.yaml b/changes/unreleased/Added-20260809-000000.yaml new file mode 100644 index 0000000..13e6eba --- /dev/null +++ b/changes/unreleased/Added-20260809-000000.yaml @@ -0,0 +1,5 @@ +kind: Added +body: A compatibility page states what v1 promises not to break, what it leaves out, and the trust boundary kvs assumes; the exported Go surface is pinned in testdata/api-surface.txt and checked by a test, so it cannot widen or narrow unnoticed +time: 2026-08-09T00:00:00.000000+09:00 +custom: + Issue: "244" diff --git a/changes/unreleased/Fixed-20260810-100000.yaml b/changes/unreleased/Fixed-20260810-100000.yaml new file mode 100644 index 0000000..389cef8 --- /dev/null +++ b/changes/unreleased/Fixed-20260810-100000.yaml @@ -0,0 +1,5 @@ +kind: Fixed +body: An HTTP 405 now carries the same JSON error body as every other HTTP error, instead of an empty response the documented contract did not allow +time: 2026-08-10T10:00:00.000000+09:00 +custom: + Issue: "244" diff --git a/content/docs/cli.md b/content/docs/cli.md index 81719e9..417dddb 100644 --- a/content/docs/cli.md +++ b/content/docs/cli.md @@ -47,7 +47,7 @@ surfacing later. What a cluster does and does not promise is on the ### Configuration without flags -Every flag has a config file and environment equivalent. The RESP password has **only** +Every `serve` flag has a config file and environment equivalent. The RESP password has **only** those: a credential passed as an argument is visible to anything that can list processes. | Setting | Config key | Environment | diff --git a/content/docs/compatibility.md b/content/docs/compatibility.md new file mode 100644 index 0000000..dba7cf9 --- /dev/null +++ b/content/docs/compatibility.md @@ -0,0 +1,133 @@ +--- +title: "Compatibility" +weight: 6 +--- + +A `v1` tag is a promise: what this page lists will not break until `v2`, and what it does not +list may change in any release. This is the page to read before pinning kvs in something you +have to keep running. + +Versions follow [Semantic Versioning](https://semver.org/). Within `v1.x.y`, a new minor +release may add to the list below and a patch release may only fix behaviour that already +contradicts it. + +## What v1 covers + +### The wire protocols + +All three protocols share one keyspace, and that stays true for the type all three can carry: a +string written over HTTP is readable over RESP and gRPC, and the reverse. RESP's lists, hashes, +sets, and sorted sets have no HTTP or gRPC representation, so asking either for a key holding +one is an error rather than a value. + +| Protocol | Promised | Details | +|---|---|---| +| HTTP | `PUT`, `GET`, `DELETE` on `/v1/keys/{key}`, and `/healthz` — paths, methods, request and response bodies, status codes | [HTTP API](../http-api/) | +| gRPC | The service defined by the protobuf files under `api/kvsv1`, and the standard gRPC health service | `api/kvsv1` | +| RESP2 | The commands listed on the Redis API page, with the replies documented there | [Redis API](../redis-api/) | + +For gRPC the `.proto` file is the contract. The Go identifiers `protoc` generates from it +follow `protoc`'s own rules, so they are promised only insofar as the proto is. + +A RESP command that is not on the Redis API page is not promised, even if the server happens +to answer it. + +### The command line + +Every flag on the [CLI Usage](../cli/) page keeps its name, its meaning, and its default. Each +`kvs serve` setting keeps the config-file key and environment variable that set it too; +`--config`, `--version`, and `--help` have neither, because the CLI answers those itself rather +than passing them to the server. `kvs serve` with no flags +keeps asking for `:3456` for HTTP, `:3457` for gRPC, and `127.0.0.1:6379` for RESP — the +addresses are the promise, not that each one binds, because a default RESP port already taken +is logged and skipped rather than being fatal. `kvs version` keeps printing a version. + +New flags may be added. Existing ones will not change under you. + +### The Go library + +Importing `github.com/skyoo2003/kvs` gets you: + +- `Store` and its constructors `NewStore` and `Open`, with `Get`, `Put`, `Delete`, `Read`, + `Write`, `Snapshot`, `Speculate`, `Watch`, `SetCodec`, and `Close` +- The transaction types `ReadTx` and `Tx`, and `Watch` +- `Entry`, and the `Codec` interface with `StringCodec` +- The sentinel errors `ErrKeyNotFound`, `ErrNoCodec`, `ErrUnsupportedValue`, and `ErrNotLeader` + with `NotLeaderError` + +The exact signatures live in [`testdata/api-surface.txt`][surface], which is generated from +the source and compared against it by a test on every run. Nothing can join or leave that file +without the change showing up in review. + +[surface]: https://github.com/skyoo2003/kvs/blob/main/testdata/api-surface.txt + +## What v1 does not cover + +**Cluster plumbing on `Store`.** `SetReplicator`, `ReplaceWith`, and `ApplyReplicated` are +exported so `internal/cluster` can reach them across the package boundary, not for callers +importing this package. They may change or disappear in a minor release. + +**`github.com/skyoo2003/kvs/pkg/resp`.** It exists so the server can speak RESP2, not as a +RESP library for other programs. The protocol kvs answers on the wire is promised; this Go +package is not. + +**Anything under `internal/`.** The Go toolchain already stops you importing it; this is the +same statement in words. + +**The on-disk format.** The append log in `--data-dir`, and the Raft directory beside it, carry +no format version yet. Data written by one release is not promised to be readable by another. +Until that changes, treat an upgrade as: drain, start the new version against an empty +directory, reload. + +**Performance.** Throughput, latency, and memory are not part of the promise. kvs is built for +not losing writes, not for being fast, and a release may trade one for the other. + +**The Go version.** kvs builds with the Go release named in `go.mod`. A minor release may +require a newer one. + +## Breaking it anyway + +Something on this page can only be removed or changed in `v2`. Before that it gets deprecated +in a `v1.x` release — still working, marked in its documentation and in the release notes — +and stays that way until the next major. + +One exception: a fix for a security vulnerability may break a promise in a minor release. When +that happens the release notes say so explicitly, in those words. + +## Trust boundary + +**kvs has no authorization and no TLS, and only one of its three listeners can ask who you +are.** Set `resp_password` in the config file or `KVS_RESP_PASSWORD` in the environment — there +is deliberately no flag, because a flag puts the password in the process list — and the RESP +listener requires `AUTH` before it answers anything, cluster joins included, since `--join` goes +through that listener rather than a second port with its own credentials. HTTP and gRPC have +nothing of the kind: anyone who can open a connection to either can read, change, and delete +everything in the keyspace, and no listener encrypts anything, so a RESP password crosses the +wire in the clear. + +The defaults reflect that unevenly, and it is worth knowing which is which: + +| Listener | Default | Reachable from | Can require a password | +|---|---|---|---| +| HTTP | `:3456` | every interface | No | +| gRPC | `:3457` | every interface | No | +| RESP | `127.0.0.1:6379` | loopback only | Yes, with `resp_password` | + +RESP defaults to loopback because port 6379 is scanned continuously across the public +internet. HTTP and gRPC do not, so a kvs started with defaults on a machine with a public +address is exposed on two ports. + +Run kvs on a network you trust, or put something that authenticates in front of it. Do not put +a listener on a public address. + +This is a description of v1, not a permanent position. Giving the other two listeners the same +option adds to the promise rather than breaking it, so it can arrive in a `v1.x` release. + +## Further Reading + +- [Overview](../overview/) — installation and library usage +- [CLI Usage](../cli/) — every flag, config key, and environment variable +- [HTTP API](../http-api/) — REST endpoint details +- [Redis API](../redis-api/) — supported RESP commands and behaviour notes +- [Durability and Clustering](../clustering/) — what `--data-dir` and `--raft-addr` promise +- [Release Process](../release/) — how a version gets cut diff --git a/content/docs/contributing.md b/content/docs/contributing.md index fecc7b3..301b7c5 100644 --- a/content/docs/contributing.md +++ b/content/docs/contributing.md @@ -1,6 +1,6 @@ --- title: "Contributing" -weight: 6 +weight: 7 --- Contributions are welcome! See [`CONTRIBUTING.md`](https://github.com/skyoo2003/kvs/blob/main/CONTRIBUTING.md) for the full guide. diff --git a/content/docs/overview.md b/content/docs/overview.md index d955643..dc9a575 100644 --- a/content/docs/overview.md +++ b/content/docs/overview.md @@ -69,6 +69,11 @@ func main() { } ``` +## Compatibility + +What `v1` promises not to break, what it deliberately leaves out, and the trust boundary kvs +assumes are on the [Compatibility](../compatibility/) page. + ## License MIT License. See [LICENSE](https://github.com/skyoo2003/kvs/blob/main/LICENSE) for details. diff --git a/content/docs/release.md b/content/docs/release.md index 16a4ce6..af64dbf 100644 --- a/content/docs/release.md +++ b/content/docs/release.md @@ -1,6 +1,6 @@ --- title: "Release Process" -weight: 7 +weight: 8 --- The project uses [Changie](https://github.com/miniscruff/changie) for changelog management and [GoReleaser](https://goreleaser.com/) for automated releases. diff --git a/internal/server/http.go b/internal/server/http.go index 6b09df1..8e7576c 100644 --- a/internal/server/http.go +++ b/internal/server/http.go @@ -42,7 +42,7 @@ func NewHTTPHandler(store *kvs.Store) http.Handler { func (h *httpHandler) handleHealth(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet { - w.WriteHeader(http.StatusMethodNotAllowed) + writeJSONError(w, http.StatusMethodNotAllowed, "method not allowed") return } @@ -64,7 +64,7 @@ func (h *httpHandler) handleKey(w http.ResponseWriter, r *http.Request) { case http.MethodDelete: h.handleDelete(w, key) default: - w.WriteHeader(http.StatusMethodNotAllowed) + writeJSONError(w, http.StatusMethodNotAllowed, "method not allowed") } } diff --git a/internal/server/http_test.go b/internal/server/http_test.go index 811a917..c514b34 100644 --- a/internal/server/http_test.go +++ b/internal/server/http_test.go @@ -67,6 +67,34 @@ func TestHTTPHandlerPutGetDelete(t *testing.T) { } } +// TestHTTPHandlerRejectsUnsupportedMethod covers the promise the HTTP API page makes about every +// error carrying a JSON body, which a bare WriteHeader would break for 405 alone. +func TestHTTPHandlerRejectsUnsupportedMethod(t *testing.T) { + for _, target := range []string{"/healthz", "/v1/keys/language"} { + handler := NewHTTPHandler(kvs.NewStore()) + req := newTestRequest(t, http.MethodPost, target, http.NoBody) + res := httptest.NewRecorder() + + handler.ServeHTTP(res, req) + + if res.Code != http.StatusMethodNotAllowed { + t.Errorf("POST %s status = %d, want %d", target, res.Code, http.StatusMethodNotAllowed) + } + if got := res.Header().Get("Content-Type"); got != "application/json" { + t.Errorf("POST %s content type = %q, want application/json", target, got) + } + + var body errorResponse + if err := json.Unmarshal(res.Body.Bytes(), &body); err != nil { + t.Errorf("POST %s json.Unmarshal(%q) error = %v", target, res.Body.String(), err) + continue + } + if body.Error == "" { + t.Errorf("POST %s error message is empty", target) + } + } +} + func TestHTTPHandlerMissingKey(t *testing.T) { handler := NewHTTPHandler(kvs.NewStore()) req := newTestRequest(t, http.MethodGet, "/v1/keys/missing", http.NoBody) diff --git a/pkg/resp/reader.go b/pkg/resp/reader.go index 0329731..1b99414 100644 --- a/pkg/resp/reader.go +++ b/pkg/resp/reader.go @@ -1,4 +1,9 @@ // Package resp implements the RESP2 wire protocol spoken by Redis and Valkey clients. +// +// It exists for the server in this module to speak that protocol, not as a RESP library for +// other programs, and is outside the v1 compatibility promise: see +// content/docs/compatibility.md. The protocol kvs answers on the wire is promised; this Go +// package is not. package resp import ( diff --git a/replication.go b/replication.go index 4051e0d..9ece8af 100644 --- a/replication.go +++ b/replication.go @@ -33,6 +33,9 @@ func (e *NotLeaderError) Is(target error) bool { // // Set it before anything serves: it is read without the lock a write would take, on the // understanding that it is wired up once at startup. +// +// Exported for internal/cluster to reach across the package boundary, and outside the v1 +// compatibility promise: see content/docs/compatibility.md. func (s *Store) SetReplicator(replicate func(fn func(tx *Tx) error) error) { s.mu.Lock() defer s.mu.Unlock() @@ -60,6 +63,9 @@ func (s *Store) SetCodec(codec Codec) { // ReplaceWith throws the keyspace away and rebuilds it from snapshot, which is what a node // restored from a cluster snapshot needs: the agreed state is the only authority, so whatever the // node held before is worth keeping only until it arrives. +// +// Exported for internal/cluster to reach across the package boundary, and outside the v1 +// compatibility promise: see content/docs/compatibility.md. func (s *Store) ReplaceWith(snapshot [][]byte) error { s.mu.Lock() defer s.mu.Unlock() @@ -74,6 +80,9 @@ func (s *Store) ReplaceWith(snapshot [][]byte) error { // ApplyReplicated applies one frame the cluster has agreed on. A frame is one transaction on the // node that took the write, and applying it inside one transaction here is what keeps a MULTI // atomic on every node. +// +// Exported for internal/cluster to reach across the package boundary, and outside the v1 +// compatibility promise: see content/docs/compatibility.md. func (s *Store) ApplyReplicated(lines [][]byte) error { s.mu.Lock() defer s.mu.Unlock() diff --git a/testdata/api-surface.txt b/testdata/api-surface.txt new file mode 100644 index 0000000..ffc008b --- /dev/null +++ b/testdata/api-surface.txt @@ -0,0 +1,125 @@ +# Exported API surface of this package - see content/docs/compatibility.md +# for how much of it v1 promises, and for the cluster plumbing it exempts by name. +# A line changed or removed below is a breaking change and needs a major version, unless the +# page exempts it. A line added is a new promise: it cannot be taken back within v1. +# Regenerate deliberately: go test -run TestPublicAPISurface . -update +# --- +==== kvs.go ==== +package kvs + +var ( + ErrKeyNotFound = errors.New("key not found") + + ErrNoCodec = errors.New("no codec set") +) + +type Entry struct { + Value interface{} + ExpiresAt time.Time +} + +type Store struct { + // contains filtered or unexported fields +} + +func NewStore() *Store + +func Open(dir string, codec Codec) (*Store, error) + +func (s *Store) Close() error + +func (s *Store) Read(fn func(tx *ReadTx) error) error + +func (s *Store) Write(fn func(tx *Tx) error) error + +func (s *Store) Speculate(fn func(tx *Tx) error) ([][]byte, error) + +func (s *Store) Snapshot() ([][]byte, error) + +func (s *Store) Watch(keys ...string) *Watch + +func (s *Store) Get(key string) (interface{}, error) + +func (s *Store) Put(key string, value interface{}) error + +func (s *Store) Delete(key string) error + +type ReadTx struct { + // contains filtered or unexported fields +} + +func (tx *ReadTx) Now() time.Time + +func (tx *ReadTx) Get(key string) (Entry, bool) + +func (tx *ReadTx) Keys() []string + +func (tx *ReadTx) SortedKeys() []string + +func (tx *ReadTx) Len() int + +func (tx *ReadTx) Expiring() int + +type Tx struct { + ReadTx + // contains filtered or unexported fields +} + +func (tx *Tx) Get(key string) (Entry, bool) + +func (tx *Tx) Set(key string, entry Entry) + +func (tx *Tx) Delete(key string) bool + +func (tx *Tx) Flush() + +type Watch struct { + // contains filtered or unexported fields +} + +func (w *Watch) Conflicted() bool + +func (w *Watch) Close() + + +==== log.go ==== +package kvs + +var ErrUnsupportedValue = errors.New("unsupported value type") + +type Codec interface { + Encode(value interface{}) ([]byte, error) + Decode(data []byte) (interface{}, error) + + Clone(value interface{}) interface{} +} + +type StringCodec struct{} + +func (StringCodec) Encode(value interface{}) ([]byte, error) + +func (StringCodec) Decode(data []byte) (interface{}, error) + +func (StringCodec) Clone(value interface{}) interface{} + + +==== replication.go ==== +package kvs + +var ErrNotLeader = errors.New("not the leader") + +type NotLeaderError struct { + Leader string +} + +func (e *NotLeaderError) Error() string + +func (e *NotLeaderError) Is(target error) bool + +func (s *Store) SetReplicator(replicate func(fn func(tx *Tx) error) error) + +func (s *Store) SetCodec(codec Codec) + +func (s *Store) ReplaceWith(snapshot [][]byte) error + +func (s *Store) ApplyReplicated(lines [][]byte) error