[DB] Production Ready #39
Conversation
WalkthroughThis update refactors database infrastructure by switching PostgreSQL credentials to Docker secrets, replacing host data mounts with a named volume, and introducing a dedicated migration service. Several new scripts and secrets files are added, SSL certificates and keys are regenerated, and Makefile targets are streamlined for improved maintainability and security. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Makefile as Makefile
participant Docker as Docker Compose
participant DB as api-db (PostgreSQL)
participant Migrate as api-db-migrate
Dev->>Makefile: make db:up
Makefile->>Docker: docker compose up api-db
Docker->>DB: Start with secrets, SSL, healthcheck
Dev->>Makefile: make db:migrate
Makefile->>Docker: docker compose run api-db-migrate
Docker->>Migrate: Run run-migration.sh with secrets
Migrate->>DB: Apply migrations
Dev->>Makefile: make db:down
Makefile->>Docker: docker compose down
Possibly related PRs
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
database/infra/secrets/postgres_db (1)
1-1: DB name probably isn’t secret – consider keeping it in versioned config instead ofdocker secretUnlike user/password, the database name rarely needs secrecy. Moving it back to plain env-var or compose file reduces secret sprawl and operator friction. If you keep it as a secret, at least convert this file to a placeholder to avoid diverging between environments.
database/infra/ssl/server.crt (1)
2-18: Certificate included – fine, but track renewal & trust chainStoring the public certificate is acceptable; just ensure you have an automated renewal process before
2026-06-27and that the corresponding private key is not version-controlled (see prior comment). Consider adding aREADMEnoting renewal steps.docker-compose.yml (1)
161-162: SSL certificate mount missing read-only flag.The server certificate is mounted as read-only (
:ro), but the private key mount on line 162 is missing the read-only flag for consistency and security.- - ./database/infra/ssl/server.key:/etc/ssl/private/server.key + - ./database/infra/ssl/server.key:/etc/ssl/private/server.key:ro
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.gitignore(0 hunks)config/makefile/db.mk(1 hunks)config/makefile/infra.mk(0 hunks)database/infra/docker-entrypoint-initdb.d/init-user-db.sh(0 hunks)database/infra/scripts/healthcheck.sh(1 hunks)database/infra/scripts/run-migration.sh(1 hunks)database/infra/secrets/postgres_db(1 hunks)database/infra/secrets/postgres_password(1 hunks)database/infra/secrets/postgres_user(1 hunks)database/infra/ssl/server.crt(1 hunks)database/infra/ssl/server.csr(1 hunks)database/infra/ssl/server.key(1 hunks)docker-compose.yml(3 hunks)
💤 Files with no reviewable changes (3)
- .gitignore
- config/makefile/infra.mk
- database/infra/docker-entrypoint-initdb.d/init-user-db.sh
🔇 Additional comments (19)
database/infra/ssl/server.csr (1)
1-16: SSL certificate infrastructure properly updated.The new CSR with CN=oullin-db-ssl aligns with the coordinated SSL infrastructure updates. The PEM format is correct and integrates properly with the Docker Compose SSL mount configuration.
database/infra/scripts/healthcheck.sh (1)
1-20: Excellent security and scripting practices implemented.The script demonstrates strong shell scripting fundamentals:
- Proper error handling with
set -e- Validation of secret file contents before use
- Safe variable quoting to prevent injection attacks
- Efficient process replacement with
exec- Clear error messaging for troubleshooting
The Docker secrets integration enhances security compared to environment variables.
database/infra/scripts/run-migration.sh (2)
14-16: Well-implemented migration script with proper argument forwarding.The script correctly forwards all arguments using
"$@"and follows good shell scripting practices with error handling and secure credential management.
11-12: Verify SSL mode configuration for database connections.The script correctly constructs the database URL using Docker secrets, but uses
sslmode=disable. Confirm this is intentional for internal container-to-container communication, or if SSL should be enabled for additional security.#!/bin/bash # Check if PostgreSQL SSL configuration is properly set up in the container # and whether SSL should be used for internal connections # Look for SSL configuration in PostgreSQL config cat database/infra/config/postgresql.conf | grep -i ssl || echo "No explicit SSL config found" # Check if SSL certificates are being mounted in docker-compose rg -A 5 -B 5 "server.crt\|server.key" docker-compose.ymldocker-compose.yml (9)
1-8: Docker secrets properly configured for credential security.The secrets configuration correctly uses file-based secrets with sensible default paths and environment variable overrides. This is a significant security improvement over plaintext environment variables.
13-16: Named volume implementation follows Docker best practices.Using a named volume (
oullin_db_data) instead of bind mounts provides better data portability and is managed entirely by Docker, reducing host filesystem dependencies.
77-77: Database host reference updated correctly.The environment variable correctly references the renamed database service
api-db, maintaining consistency with the service naming convention.
100-118: Migration service properly designed with correct dependencies.The dedicated
api-db-migrateservice follows good practices:
- Uses specific image version for reproducibility
- Properly depends on database health
- Mounts necessary scripts and migrations
- Uses Docker secrets for secure credential access
- Set to not restart to avoid unnecessary executions
136-139: Docker secrets integration properly implemented.Using
*_FILEenvironment variables allows PostgreSQL to read credentials from mounted secret files, enhancing security while maintaining compatibility with the official PostgreSQL image.
147-147: Port binding security enhancement implemented correctly.Binding to
127.0.0.1restricts database access to localhost only, preventing direct public internet access while allowing local application connections. This is a significant security improvement.
175-175: PostgreSQL startup command properly secures SSL key permissions.The command correctly sets ownership and permissions for the SSL private key before starting PostgreSQL with the custom configuration file. The use of
execensures proper signal handling.
180-180: Healthcheck implementation improved with dedicated script.Using a dedicated script instead of an inline command improves maintainability and allows for more sophisticated health checking logic. The CMD array format is more robust than SHELL format.
125-125: Verify PostgreSQL Alpine image compatibility.The switch from
postgres:17.4topostgres:17.3-alpineprovides a smaller attack surface and faster startup. Ensure all PostgreSQL extensions and features used by the application are available in the Alpine variant.Are there any known compatibility issues with PostgreSQL 17.3 Alpine image compared to the standard PostgreSQL image?config/makefile/db.mk (6)
1-2: PHONY targets properly updated for new workflow.The .PHONY declarations correctly reflect the new targets and removed obsolete ones, ensuring make targets always execute regardless of file existence.
5-7: Service name variables correctly align with Docker Compose configuration.The service names match exactly with those defined in
docker-compose.yml, ensuring consistency across the infrastructure management tools.
21-23: Script permission management properly implemented.The new
db:shtarget ensures the healthcheck and migration scripts have executable permissions, which is essential for their proper operation within the containers.
26-26: Database service management simplified and improved.Using
docker compose upwith the specific service name and detached mode is more efficient and cleaner than previous approaches.
44-48: SSL certificate generation standardized and simplified.The SSL generation process is now streamlined:
- Proper cleanup of existing certificates
- Standardized subject line matching the Docker setup
- Removed manual permission changes (handled by container startup)
This aligns well with the coordinated SSL infrastructure updates.
57-66: Migration commands properly refactored to use dedicated service.All migration commands now use
docker-compose run --rmwith the new migration service, which:
- Provides better isolation and consistency
- Uses the secure credential management
- Eliminates manual volume mounting
- Ensures proper cleanup with
--rm
| docker system prune -a --volumes -f && \ | ||
| docker ps -aq | xargs --no-run-if-empty docker stop && \ | ||
| docker ps -aq | xargs --no-run-if-empty docker rm && \ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix the order of Docker cleanup operations and reduce redundancy.
The current sequence has several issues:
-
Incorrect order: Lines 13-14 attempt to stop/remove containers AFTER
docker system prunehas already run, making them redundant or potentially problematic. -
Redundancy:
docker system prune -a --volumes -falready handles most cleanup that the individual prune commands (lines 8-11) perform. -
Safety concern: These commands affect ALL Docker resources system-wide, not just project-related ones.
Consider this improved sequence:
fresh:
docker compose down --remove-orphans && \
- docker container prune -f && \
- docker image prune -f && \
- docker volume prune -f && \
- docker network prune -f && \
- docker system prune -a --volumes -f && \
- docker ps -aq | xargs --no-run-if-empty docker stop && \
- docker ps -aq | xargs --no-run-if-empty docker rm && \
+ docker system prune -a --volumes -f && \
docker psThis simplified version:
- Removes redundant individual prune commands
- Eliminates the problematic container stop/remove operations that run after system prune
- Maintains the comprehensive cleanup while being more logical and safe
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| docker system prune -a --volumes -f && \ | |
| docker ps -aq | xargs --no-run-if-empty docker stop && \ | |
| docker ps -aq | xargs --no-run-if-empty docker rm && \ | |
| fresh: | |
| docker compose down --remove-orphans && \ | |
| docker system prune -a --volumes -f && \ | |
| docker ps |
🤖 Prompt for AI Agents
In config/makefile/app.mk around lines 12 to 14, reorder the Docker cleanup
commands to run container stop and remove operations before running docker
system prune, as the current order makes stopping/removing containers redundant
and ineffective. Remove redundant individual prune commands from earlier lines
since docker system prune -a --volumes -f already covers them. Adjust the
sequence to first stop and remove containers safely, then run system prune for
comprehensive cleanup, ensuring the commands target only project-related
resources if possible to avoid affecting the entire Docker system.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores