fix(docker): make compose flow work as documented#247
Conversation
The server service had `environment:` for inline overrides but no
`env_file:` entry, so values from .env never reached the running
process inside the container. Compose only loaded .env to expand
${VAR} inside the YAML itself.
Add `env_file: [.env]` so .env remains the single source of truth
for container configuration.
docker-compose.postgres.yml only declared the postgres service, so combining it with the main compose left the server built without the `postgres` Cargo feature and with no DATABASE_URL pointing at the postgres service. The "use instead" README pointer was therefore broken: running the file alone gave users a lonely postgres with no Guardian, and running both files combined gave a server that silently used the filesystem backend (DATABASE_URL reached the container but the binary had no code to consume it). Changes: - docker-compose.postgres.yml: add a `server` override that rebuilds with GUARDIAN_SERVER_FEATURES=postgres, waits for the postgres healthcheck via depends_on, and sets DATABASE_URL using the `postgres` service hostname. - README.md: fix the absolute path link to be relative, replace the "use instead" wording with the combined compose command, and note that --build is required to switch backend. - .env.example: split the commented DATABASE_URL into the in-compose variant (host `postgres`) and the host-run variant (host `localhost`), and mark POSTGRES_PASSWORD as required for the postgres compose path. After these changes the documented postgres flow works end-to-end: postgres comes up healthy, the server is rebuilt with the postgres feature, diesel migrations run on startup, and the server reads and writes through postgres.
The /var/ directory accumulates filesystem-backend runtime state (storage, metadata, keystore) when GUARDIAN_*_PATH is pointed at ./var/... — a common bare-metal convention to avoid writing to the absolute /var/guardian default. Mirror the existing /keystore/ rule so this state stays out of git.
The active GUARDIAN_OPERATOR_PUBLIC_KEYS_JSON line suggested a general configuration option, but the server code does not read this variable. It is consumed only by scripts/aws-deploy.sh, which translates it into a Secrets Manager secret and sets the server's GUARDIAN_OPERATOR_PUBLIC_KEYS_SECRET_ID accordingly. For local or docker setups the server reads GUARDIAN_OPERATOR_PUBLIC_KEYS_FILE. Replace the misleading active line with a commented block that shows both options and notes the docker-specific need to mount the keys file into the container.
env_file: [.env] required the .env file to exist or compose would
fail at config/up time, even though the server has working
defaults and can boot without it. Use the `required: false` variant
so .env is loaded if present and silently ignored when absent.
This preserves the original Compose behavior of "no .env is fine"
while keeping the F1 fix (variables propagate to the container
when .env exists).
POSTGRES_PASSWORD was documented as required for the postgres
override but only commented in .env.example, so a user who forgot
to set it would see the postgres container start with an empty
password and the server fail to connect with a generic interpolation
result. Use ${POSTGRES_PASSWORD:?...} on the DATABASE_URL so compose
errors immediately with a clear remediation hint.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR enables local Docker Compose development with Postgres by adding an override-based service configuration, updating environment variable examples, and documenting the combined Docker Compose invocation for users setting up the development environment. ChangesDocker Compose Postgres Integration
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thank you for your contribution to guardian. Before being able to integrate those changes, we would like you to sign our Contributor License Agreement. You can sign the CLA by just posting a Pull Request Comment with the sentence below. Thanks. I confirm that I have read and hereby agree to the OpenZeppelin Contributor License Agreement You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
There was a problem hiding this comment.
Pull request overview
This PR improves the Docker Compose developer workflow so it works end-to-end as documented, particularly around .env propagation and the Postgres-backed setup.
Changes:
- Pass
.envvariables into theservercontainer in the default Compose flow (while allowing.envto be absent). - Wire up a Postgres-backed Compose flow by overriding server build features and setting
DATABASE_URL, and document the override usage. - Clarify
.env.exampleguidance and ignore a localvar/directory.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates Compose docs to describe using the Postgres override file. |
| docker-compose.yml | Adds optional .env injection into the server container. |
| docker-compose.postgres.yml | Adds server overrides for Postgres feature + DATABASE_URL and waits for Postgres health. |
| .gitignore | Ignores var/ directory. |
| .env.example | Clarifies Postgres connection examples and operator allowlist variable usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| build: | ||
| args: | ||
| GUARDIAN_SERVER_FEATURES: postgres |
| env_file: | ||
| - path: .env | ||
| required: false |
|
Thanks a lot for your contribution! It looks good! Before merging maybe to make docker-compose.postgres.yml independent as per #247 (comment). |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #247 +/- ##
==========================================
+ Coverage 73.18% 75.28% +2.09%
==========================================
Files 122 131 +9
Lines 21565 23314 +1749
==========================================
+ Hits 15782 17551 +1769
+ Misses 5783 5763 -20 Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
docker-compose.postgres.yml now uses `include:` to pull in docker-compose.yml, so the postgres flow can start with a single `-f docker-compose.postgres.yml` instead of layering two files. The base server config and top-level volumes are inherited; the postgres-specific bits layer on top. Verified `docker compose config` output is byte-identical to the previous two-file override approach.
Hi! Implementing the copilot suggestion as-is won't make it fully independent because I experimented with many alternatives to find the best in order to make it independent and I pivoted to use the include feature, which I think is a cleaner approach than the one I previously implemented. The change is in cc76874 Info about the alternatives I analyzed and my thoughts on each (not a necessary read)
...
extends:
file: docker-compose.yml
service: server
...
volumes:
guardian-postgres: # Specific from this file
# Duplicated from the other file
guardian-storage:
guardian-metadata:
guardian-keystore:
include:
- docker-compose.yml |
Hey! At LambdaClass we are experimenting with deploying our own guardian instance and we found a few things that we thought were worth changing, mostly aimed towards improving the docker compose flow.
Fixes
.envwas being ignored by the container, it now reaches the server (or skipped cleanly if absent).env.examplethat theGUARDIAN_OPERATOR_PUBLIC_KEYS_JSONvariable is not read by the server but by a script that is for AWS deploy.POSTGRES_PASSWORDsilently broke postgres later, now it fails immediately with a clear hintSummary by CodeRabbit
Documentation
Chores
.gitignoreto exclude temporary directory.