Add Docker setup and CI/CD improvements #107
Conversation
TatevikGr
force-pushed
the
dev
branch
4 times, most recently
from
43f9c45 to
af6df68
Compare
WalkthroughAdds containerization (Dockerfile, docker-compose, .dockerignore), Git attributes, CI update to ubuntu-22.04 with explicit DB env exports, composer dependency and repository changes, README Docker deployment docs, a shortened PR template, and a CodeRabbit config enabling auto-review. Changes
Sequence Diagram(s)sequenceDiagram
participant Compose as Docker Compose
participant App as app (container)
participant MySQL as db (MySQL 8.0)
participant PG as postgres (Postgres 15)
Note over Compose: docker-compose up
Compose->>App: build image (Dockerfile)
Compose->>MySQL: start container (db_data)
Compose->>PG: start container (pg_data)
rect rgb(220,240,255)
Note over App,MySQL: Primary DB path (MySQL)
MySQL->>MySQL: healthcheck & init
MySQL-->>App: healthy (port exposed)
end
rect rgb(240,220,255)
Note over App,PG: Alternative DB path (Postgres)
PG->>PG: healthcheck & init
PG-->>App: healthy (port exposed)
end
App->>App: read env PHPLIST_DATABASE_*, DATABASE_DRIVER
App->>App: start Apache (DocumentRoot /public)
sequenceDiagram
participant Runner as GitHub Actions Runner
participant Env as Job Environment
participant PHPUnit as vendor/bin/phpunit
Note over Runner: CI system tests step
Runner->>Env: export PHPLIST_DATABASE_NAME
Runner->>Env: export PHPLIST_DATABASE_USER
Runner->>Env: export PHPLIST_DATABASE_PASSWORD
Runner->>Env: export PHPLIST_DATABASE_PORT (from MySQL service)
Runner->>Env: set PHPLIST_DATABASE_HOST = 127.0.0.1
Runner->>PHPUnit: vendor/bin/phpunit tests/System/
PHPUnit->>Env: read PHPLIST_DATABASE_*
PHPUnit->>PHPUnit: run system tests
PHPUnit-->>Runner: test results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (2)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.coderabbit.yaml(1 hunks).dockerignore(1 hunks).gitattributes(1 hunks).github/workflows/ci.yml(2 hunks)Dockerfile(1 hunks)README.md(1 hunks)composer.json(2 hunks)docker-compose.yml(1 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
docker-compose.yml
[medium] 67-68: Basic Auth Credentials
(CKV_SECRET_4)
🪛 markdownlint-cli2 (0.18.1)
README.md
188-188: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (13)
.coderabbit.yaml (1)
1-9: Configuration is valid and well-aligned with project needs.The CodeRabbit configuration is schema-compliant and appropriately configured. The "chill" review profile, enabled auto-review with
.*regex for all branches, and high-level summary generation align well with the PR objectives to expand automated review coverage. All values conform to the configuration schema..gitattributes (1)
1-3: Good use of export-ignore to exclude non-production files.This aligns well with the expanded .dockerignore and overall build optimization efforts in this PR.
composer.json (3)
40-43: Verify the VCS repository is stable and maintained.The new VCS repository
https://github.com/tatevikgr/rss-bundle.gitpoints to a personal GitHub account. Consider whether this introduces maintenance risks or if it should be managed in an organization account to ensure long-term availability and collaborative management.
49-52: Clarify unusual dependency constraints.The following patterns are atypical:
- Line 49:
phplist/web-frontendchanged fromdev-mastertodev-main(branch rename—likely intentional).- Line 51:
tatevikgr/rest-api-clientusesdev-ISSUE-357(issue-specific branch).- Line 52:
tatevikgr/rss-feedusesdev-main as 0.1.0(mixing a dev branch with a semver alias).Verify these constraints are intentional and document if they are temporary workarounds or permanent patterns.
112-116: Verify plugin allowlist is necessary.The new
allow-pluginsconfiguration enablesphp-http/discovery. Confirm this is required and document the reason in a comment or PR description..github/workflows/ci.yml (2)
6-6: Verify system tests actually read PHPLIST_ environment variables.*The workflow now exports
PHPLIST_DATABASE_*andPHPLIST_DATABASE_HOSTenvironment variables (lines 67–71) before running system tests. Confirm that the test code intests/System/is configured to read these variables from the environment rather than fromconfig/parameters.yml. If not, these exports will have no effect.Also applies to: 66-72
6-6: Ubuntu 22.04 runner is a good upgrade.The shift from ubuntu-20.04 to ubuntu-22.04 is appropriate for modern CI/CD pipelines and aligns with PHP 8.1 support.
.dockerignore (1)
1-19: Good build context optimization.The .dockerignore comprehensively excludes development files, caches, and unnecessary artifacts. This reduces build context size and speeds up Docker builds. The
vendor/exclusion is appropriate since dependencies are installed fresh within the Dockerfile viacomposer install.Dockerfile (3)
38-38: Review the order of config directory copy vs. composer install.Line 38 copies
config ./configbefore line 41 runscomposer install. This is atypical because:
- Composer scripts may expect the full project structure already in place.
- The config directory may be generated/modified by composer install.
Verify this ordering doesn't cause issues. Consider moving the
COPY config ./configline after the composer install, or clarify why it must come before.Also applies to: 41-41
48-49: Document or reconsider silent cache warmup failures.Lines 48–49 use
|| trueto suppress errors fromcache:clearandcache:warmup. While this prevents build failures if the database is unavailable at build time, it may hide real configuration or dependency issues.Consider:
- Adding a comment explaining when these commands might fail gracefully.
- Alternatively, remove
|| trueif the database is expected to be available (e.g., in a multi-stage build or build service).- Validate that cache warmup actually succeeds in CI and production deployments.
51-54: Permission handling is sound.Explicit
chownandchmodforvar/andpublic/directories ensure the Apache process (www-data) has proper write access for caching and logs.docker-compose.yml (2)
10-19: Clarify database driver configuration strategy.The app service environment shows:
- Lines 10–12: MySQL config (commented out)
- Lines 17–19: PostgreSQL config (active)
This suggests PostgreSQL is the default, but the comments may confuse users. Add documentation or clarify intent:
- In docker-compose.yml: Add a comment explaining the override strategy (e.g., "PostgreSQL is default; uncomment MySQL config and comment out PostgreSQL to switch").
- In README: Document how to switch between MySQL and PostgreSQL.
This is especially important since
depends_on: [db](line 23–24) refers to the MySQL service, but the app defaults to PostgreSQL (line 17). Verify this is intentional.
31-47: Database services are well-configured with healthchecks.The MySQL and PostgreSQL services include appropriate healthchecks, volume persistence, and environment variable configuration. Separate volumes (
db_data,pg_data) ensure data persists across container restarts.Also applies to: 49-65
Summary
Unit test
Are your changes covered with unit tests, and do they not break anything?
You can run the existing unit tests using this command:
Code style
Have you checked that you code is well-documented and follows the PSR-2 coding
style?
You can check for this using this command:
Other Information
If there's anything else that's important and relevant to your pull
request, mention that information here. This could include benchmarks,
or other information.
If you are updating any of the CHANGELOG files or are asked to update the
CHANGELOG files by reviewers, please add the CHANGELOG entry at the top of the file.
Thanks for contributing to phpList!
Summary by CodeRabbit
New Features
Documentation
Chores