Skip to content

fix(docker): default to serve and move the database to /data - #133

Open
blueagledev wants to merge 1 commit into
szaffarano:masterfrom
blueagledev:fix/docker-serve-cmd-and-data-path
Open

blueagledev wants to merge 1 commit into
szaffarano:masterfrom
blueagledev:fix/docker-serve-cmd-and-data-path

Conversation

@blueagledev

@blueagledev blueagledev commented Aug 15, 2026

Copy link
Copy Markdown

Two Dockerfile fixes plus the README lines that go with them. Found while evaluating
korrosync as the sync server for my own ebook setup — I built the image and ran it, though I
am not running korrosync day to day — and independent of the timestamp PR (#132). The analysis and the patch were produced with AI assistance and directed by me;
what was executed rather than asserted is under Checks.

1. The image has no runnable default command

Cli::command is a required subcommand
(cli.rs#L10-L11),
and the Dockerfile sets ENTRYPOINT with no CMD
(Dockerfile#L50),
so the README's own run line
(README.md#L77-L82)
exits 2 with a usage dump. Built from master:

$ docker run --rm -v $(pwd)/data:/data korrosync:master
KOReader synchronization server

Usage: korrosync [OPTIONS] <COMMAND>

Commands:
  serve  Start the sync server
  user   User management commands
  db     Database maintenance commands
  help   Print this message or the help of the given subcommand(s)
...
exit=2

CMD ["serve"] supplies the default. Arguments replace CMD and not ENTRYPOINT, so the
subcommands keep working. Built from this branch:

$ docker run -d --name ks -v $(pwd)/data:/data korrosync:branch
f35b06cbd156ad4971561180637208f50b4245475266aeb1a5389809bd8748bd
$ docker logs ks
2026-08-15T20:05:35.224814Z  INFO korrosync: Server listening on 0.0.0.0:3000

$ docker run --rm -v $(pwd)/data:/data korrosync:branch db info
Database path: /data/db.redb
Database size: 1056768 bytes
Users: 0

2. The default database path is inside the container layer

KORROSYNC_DB_PATH defaulted to /db.redb
(Dockerfile#L44),
which is in the writable container layer. docker rm plus a recreate drops it, which shows
up as 401 Invalid credentials and reads like a password problem rather than an empty
database. From the master image:

$ docker run --name ksm korrosync:master user create -u alice -p secret
User 'alice' created successfully
$ docker rm ksm
ksm
$ docker run --name ksm korrosync:master user list
No users found

On this branch the same sequence against a mounted /data keeps the user, with no -e
flag:

$ docker run --rm -v $(pwd)/data:/data korrosync:branch user create -u alice -p secret
User 'alice' created successfully
$ docker run --rm -v $(pwd)/data:/data korrosync:branch user list
USERNAME             LAST ACTIVITY
----------------------------------------
alice                never

Total: 1 user(s)

Anyone following your instructions was never at risk: README.md#L77-L82 passes both
-v $(pwd)/data:/data and -e KORROSYNC_DB_PATH=/data/db.redb, which puts the database on
the bind mount. The problem is that the default only works if you copy that line verbatim,
and that the -e flag exists solely to compensate for a default pointing somewhere unsafe.
Making /data/db.redb the image default removes the trap, lets the -e line go, and lines
the image up with the application default of data/db.redb
(config.rs#L28).

No VOLUME declaration

My first draft added VOLUME ["/data"] and I dropped it. Checked with a throwaway image
that appends to /data/db on every run: a plain docker run attaches a fresh anonymous
volume, so the file was empty again after docker rm and a recreate, and the two cycles
left two dangling volumes behind. It also cannot be undeclared downstream. The default path
plus one README sentence is the whole fix.

Breaking change

Moving the default strands the database of anyone who relied on /db.redb, and it presents
as exactly the 401 Invalid credentials this PR is fixing. The commit therefore carries a
BREAKING CHANGE: footer, and the README has the upgrade line:

docker cp korrosync:/db.redb ./data/db.redb

That footer will make release-plz cut a minor bump rather than a patch, for a change that
touches no Rust. If you would rather not have that, drop the footer and I will fold the
warning into the release notes by hand instead.

Checks

No .rs files change, so the Rust CI path filters skip this PR. Run anyway in nix develop, on this branch:

  • cargo test: 43 unit, 39 integration and 19 doc tests, all pass
  • cargo fmt --check, cargo clippy -- -D warnings, cargo machete: clean
  • cargo deny check advisories: fails, and fails identically on master, with
    RUSTSEC-2026-0233, RUSTSEC-2026-0234 and RUSTSEC-2026-0235 against rkyv 0.8.16 plus a
    yanked spin

Both images were built from this repo and exercised locally, master and this branch; every
transcript above is from those runs.

`Commands` is a required subcommand, so the image had no runnable
default: the README's own `docker run ... korrosync` exits 2 with a usage
dump. `CMD ["serve"]` supplies one and still leaves `docker run ...
korrosync db info` and friends working, since arguments replace CMD and
not ENTRYPOINT.

The database also defaulted to /db.redb, inside the writable container
layer, where `docker rm` plus a recreate drops every user and reading
position. The README already says to pass
`-e KORROSYNC_DB_PATH=/data/db.redb`, so nobody following it was exposed;
the trap was for anyone who did not, and that `-e` line existed only to
compensate for the default. Making /data the default removes both, and
matches the application default of `data/db.redb`.

No VOLUME declaration: a plain `docker run` attaches a fresh anonymous
volume, so it does not make the data survive a recreate, it leaves a
dangling volume behind on every cycle, and a downstream image cannot
undeclare it. The default path plus a README sentence is the whole fix.

BREAKING CHANGE: the image no longer sets KORROSYNC_DB_PATH=/db.redb, it
defaults to /data/db.redb. A deployment that relied on the old default
comes back up against an empty database and answers 401 Invalid
credentials, which is the same symptom this commit fixes. Copy the file
across before recreating the container:
docker cp korrosync:/db.redb ./data/db.redb

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 15, 2026 20:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts the container UX defaults for Korrosync so the published Docker image runs successfully without requiring an explicit subcommand, and so the default database location aligns with a persistent mount point (/data) rather than the ephemeral container layer.

Changes:

  • Add CMD ["serve"] so docker run korrosync starts the server by default while preserving subcommand behavior.
  • Change the image default KORROSYNC_DB_PATH to /data/db.redb (from /db.redb) to encourage persistence via a bind mount.
  • Update README Docker run examples, add a subcommand example, and document the upgrade path for older images that used /db.redb.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
README.md Removes the redundant KORROSYNC_DB_PATH env var from run examples, adds subcommand usage example, and documents upgrade guidance for the old /db.redb default.
Dockerfile Sets safer default DB path (/data/db.redb) and adds CMD ["serve"] so the image has a runnable default command.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@blueagledev
blueagledev force-pushed the fix/docker-serve-cmd-and-data-path branch from 07ee28a to ac45b9b Compare August 30, 2026 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants