Wired up local development for S3 adapter #25436
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds object-storage support: a new MinIO setup script at Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-09-10T21:24:49.363ZApplied to files:
📚 Learning: 2025-05-27T18:08:00.458ZApplied to files:
📚 Learning: 2025-05-27T18:08:00.458ZApplied to files:
🪛 Shellcheck (0.11.0).docker/minio/setup.sh[warning] 2-2: In POSIX sh, set option pipefail is undefined. (SC3040) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (3)
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 |
![cursor[bot]](https://web.archive.org/web/20251204203126im_/https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on December 3
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
![coderabbitai[bot]](https://web.archive.org/web/20251204203126im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
package.json (1)
42-42: Verify that hardcoding the profile doesn't conflict with profile flexibility.The script explicitly uses
--profile object-storageand bypasses theCOMPOSE_PROFILESenvironment variable mechanism used by other docker:dev:* scripts (e.g.,docker:devat line 41). This forces the object-storage profile and makes it incompatible with theCOMPOSE_PROFILESenv var pattern.Consider whether this is intentional or if the script should support the profile flexibility pattern:
- "docker:dev:object-storage": "docker compose -f compose.yml -f compose.object-storage.yml --profile object-storage up --attach=ghost --force-recreate --no-log-prefix", + "docker:dev:object-storage": "COMPOSE_PROFILES=${COMPOSE_PROFILES:-object-storage} docker compose -f compose.yml -f compose.object-storage.yml up --attach=ghost --force-recreate --no-log-prefix --wait",Additionally, consider adding
--waitfor consistency withdocker:dev:analytics(line 43), which waits for services to be ready.compose.object-storage.yml (1)
14-14: Consider documenting hardcoded credentials and verifying the prefix path.The following values are hardcoded in the configuration:
- S3 prefix path:
ab/ab1234567890abcdef1234567890abcd(line 14) — appears to be a placeholder or example. Verify this is intentional.- MinIO credentials:
minio-user/minio-pass(lines 19–20, 36–37) — acceptable for local development but should be documented or made environment-driven for flexibility across development environments.Consider whether these should reference environment variables or be documented in a developer guide to prevent confusion.
Also applies to: 19-20, 36-37
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.docker/minio/setup.sh(1 hunks)compose.object-storage.yml(1 hunks)package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-09T17:25:12.439Z
Learning: Applies to ghost/core/config.development.json : Add Tinybird configuration to ghost/core/config.development.json for local development
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 24862
File: .github/workflows/ci-docker.yml:320-324
Timestamp: 2025-09-10T21:24:49.363Z
Learning: In GitHub Actions workflows using docker compose, the `docker compose pull` command works correctly with fork PRs even when some images (like the Ghost development image) are only loaded locally and not available in remote registries. The command doesn't fail when it encounters locally-loaded images during the pull process.
📚 Learning: 2025-05-27T18:08:00.458Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: The Ghost Docker Compose setup has two independent profiles: `ghost` profile (v0, runs all apps in a single container) and `split` profile (work in progress, runs Ghost server, admin, and frontend apps in separate services). The `split` profile will eventually replace `ghost` as the default.
Applied to files:
package.jsoncompose.object-storage.yml
📚 Learning: 2025-05-27T18:08:00.458Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: Services that are dependencies for both Ghost Docker Compose profiles (`ghost` and `split`) need to include both profiles in their `profiles` configuration to ensure they start correctly under either profile.
Applied to files:
package.jsoncompose.object-storage.yml
🪛 Shellcheck (0.11.0)
.docker/minio/setup.sh
[warning] 2-2: In POSIX sh, set option pipefail is undefined.
(SC3040)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Ghost-CLI tests
- GitHub Check: Acceptance tests (Node 22.13.1, mysql8)
- GitHub Check: Acceptance tests (Node 22.13.1, sqlite3)
- GitHub Check: Comments-UI tests
- GitHub Check: Signup-form tests
- GitHub Check: Legacy tests (Node 22.13.1, sqlite3)
- GitHub Check: Admin-X Settings tests
- GitHub Check: Legacy tests (Node 22.13.1, mysql8)
- GitHub Check: ActivityPub tests
- GitHub Check: Unit tests (Node 22.13.1)
- GitHub Check: Admin tests - Chrome
- GitHub Check: Build & Push
- GitHub Check: Cursor Bugbot
🔇 Additional comments (5)
.docker/minio/setup.sh (2)
1-2: Note: Shell portability warning from static analysis.The shebang
#!/bin/shcombined withset -euo pipefail(line 2) triggers ShellCheck SC3040 becausepipefailis a bash extension, not POSIX shell standard. While this works in most modern Linux environments where/bin/shis symlinked to bash, it may not be portable to strict POSIX systems.If portability is a concern, consider switching to
#!/bin/bash. Otherwise, this is acceptable for Docker-based development environments.
6-14: Script structure and bucket initialization logic look solid.The idempotent bucket creation (line 10 with
--ignore-existing), policy configuration (line 13), and environment-driven bucket naming (line 4) are well-designed for the development setup. Error handling viaset -euo pipefailensures the script fails fast if any step encounters an issue.compose.object-storage.yml (3)
2-27: Service dependencies and Ghost configuration look correct.The ghost service correctly extends the base compose.yml service and applies the object-storage profile isolation. The dependency chain is well-structured: Ghost depends on minio being healthy AND minio-setup completing successfully, which ensures proper initialization order before Ghost attempts to use S3 storage.
The S3Storage configuration (lines 8–22) correctly separates internal service communication (
http://minio:9000) from external URLs (http://127.0.0.1:9000), which is appropriate for a local development setup.
29-47: MinIO service configuration with healthcheck is sound.The MinIO service includes proper health checks (line 44), volume persistence (line 42), and a pinned version (line 31), which are all good practices for development environments. The restart policy ensures the service stays running for the development session.
48-61: MinIO setup service correctly orchestrates initialization.The minio-setup service uses the minio/mc image to run the initialization script (.docker/minio/setup.sh), depends on minio being healthy, and uses
restart: 'no'to prevent repeated re-runs. This is the correct pattern for one-shot setup tasks in Docker Compose.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #25436 +/- ##
=======================================
Coverage 71.97% 71.97%
=======================================
Files 1528 1528
Lines 116466 116466
Branches 13884 13884
=======================================
+ Hits 83821 83822 +1
+ Misses 31630 31629 -1
Partials 1015 1015
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
allouis
force-pushed
the
push-snrzqlzuuvtr
branch
from
fe9b7cd to
add2a54
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.docker/minio/setup.sh (1)
1-3: POSIX sh compatibility:pipefailis a bash extension.The script uses
#!/bin/shwithset -euo pipefail, butpipefailis not defined in strict POSIX sh (SC3040 warning). While widely supported on modern systems, you may preferset -o pipefailfor explicit option syntax. For local development purposes, the current approach is acceptable unless you have strict POSIX compliance requirements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.docker/minio/setup.sh(1 hunks)compose.object-storage.yml(1 hunks)package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 24862
File: .github/workflows/ci-docker.yml:320-324
Timestamp: 2025-09-10T21:24:49.363Z
Learning: In GitHub Actions workflows using docker compose, the `docker compose pull` command works correctly with fork PRs even when some images (like the Ghost development image) are only loaded locally and not available in remote registries. The command doesn't fail when it encounters locally-loaded images during the pull process.
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: The Ghost Docker Compose setup has two independent profiles: `ghost` profile (v0, runs all apps in a single container) and `split` profile (work in progress, runs Ghost server, admin, and frontend apps in separate services). The `split` profile will eventually replace `ghost` as the default.
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: Services that are dependencies for both Ghost Docker Compose profiles (`ghost` and `split`) need to include both profiles in their `profiles` configuration to ensure they start correctly under either profile.
📚 Learning: 2025-05-27T18:08:00.458Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: The Ghost Docker Compose setup has two independent profiles: `ghost` profile (v0, runs all apps in a single container) and `split` profile (work in progress, runs Ghost server, admin, and frontend apps in separate services). The `split` profile will eventually replace `ghost` as the default.
Applied to files:
package.jsoncompose.object-storage.yml
📚 Learning: 2025-05-27T18:08:00.458Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: Services that are dependencies for both Ghost Docker Compose profiles (`ghost` and `split`) need to include both profiles in their `profiles` configuration to ensure they start correctly under either profile.
Applied to files:
package.jsoncompose.object-storage.yml
🪛 Shellcheck (0.11.0)
.docker/minio/setup.sh
[warning] 2-2: In POSIX sh, set option pipefail is undefined.
(SC3040)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Setup
- GitHub Check: Build & Push
- GitHub Check: Setup
- GitHub Check: Cursor Bugbot
🔇 Additional comments (3)
package.json (1)
42-42: LGTM! Consistent with existing Docker development scripts.The npm script correctly chains compose files, specifies the object-storage profile, and uses standard options for local development. The pattern aligns with other
docker:dev:*commands.compose.object-storage.yml (2)
8-22: Hardcoded S3Storage configuration acceptable for local development.The ghost service includes hardcoded bucket (ghost-dev), region (us-east-1), prefix, path-style setting, endpoint, and credentials. These defaults are appropriate for local zero-config development and don't pose a security concern since this is a local-only setup. Consider documenting these defaults in a README or inline comment for developers who may want to customize them.
21-22: Verify: Media and files URLs point to identical paths.Both
urls__mediaandurls__filesare set to the same path:http://127.0.0.1:9000/ghost-dev/ab/ab1234567890abcdef1234567890abcd. Confirm whether Ghost expects different URL prefixes for media vs. files, or if sharing the same path is the intended configuration.
allouis
force-pushed
the
push-snrzqlzuuvtr
branch
from
add2a54 to
f0cf27b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.docker/minio/setup.sh(1 hunks)compose.object-storage.yml(1 hunks)package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- compose.object-storage.yml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 24862
File: .github/workflows/ci-docker.yml:320-324
Timestamp: 2025-09-10T21:24:49.363Z
Learning: In GitHub Actions workflows using docker compose, the `docker compose pull` command works correctly with fork PRs even when some images (like the Ghost development image) are only loaded locally and not available in remote registries. The command doesn't fail when it encounters locally-loaded images during the pull process.
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: The Ghost Docker Compose setup has two independent profiles: `ghost` profile (v0, runs all apps in a single container) and `split` profile (work in progress, runs Ghost server, admin, and frontend apps in separate services). The `split` profile will eventually replace `ghost` as the default.
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: Services that are dependencies for both Ghost Docker Compose profiles (`ghost` and `split`) need to include both profiles in their `profiles` configuration to ensure they start correctly under either profile.
🪛 Shellcheck (0.11.0)
.docker/minio/setup.sh
[warning] 2-2: In POSIX sh, set option pipefail is undefined.
(SC3040)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Unit tests (Node 22.13.1)
- GitHub Check: Ghost-CLI tests
- GitHub Check: Acceptance tests (Node 22.13.1, mysql8)
- GitHub Check: Admin-X Settings tests
- GitHub Check: Acceptance tests (Node 22.13.1, sqlite3)
- GitHub Check: Signup-form tests
- GitHub Check: Comments-UI tests
- GitHub Check: Legacy tests (Node 22.13.1, sqlite3)
- GitHub Check: ActivityPub tests
- GitHub Check: Legacy tests (Node 22.13.1, mysql8)
- GitHub Check: Admin tests - Chrome
- GitHub Check: Build & Push
ref https://linear.app/ghost/issue/PRO-1534 The idea behind this approach is to be non-intrusive to existing development flows, but allow a zero-config approach to run locally. We're using MinIO as the S3 compatible backend, and all of the object storage related services and config is in a new compose file.
allouis
force-pushed
the
push-snrzqlzuuvtr
branch
from
f0cf27b to
f63925f
Compare
ref https://linear.app/ghost/issue/PRO-1534 In order to use object storage locally we need a local S3 compatible service running, but we didn't want to make that mandatory, or break existing development setups, so we've tried to be non-intrusive to existing flows but allow a zero-config approach to run locally. We're using MinIO as the S3 compatible backend, and all of the object storage related services and config is in a new compose file, which keeps everything separate for now. We do need to eventually converge on how to run Ghost with all the different possible options, but for now this massively improves DX for object storage project and we can continue to improve this in future.
ref https://linear.app/ghost/issue/PRO-1534
The idea behind this approach is to be non-intrusive to existing development flows, but allow a zero-config approach to run locally.
We're using MinIO as the S3 compatible backend, and all of the object storage related services and config is in a new compose file.
Note
Introduces a MinIO-backed object storage profile with a setup script and compose file, wiring Ghost to the S3 storage adapter and adding a helper Docker script.
compose.object-storage.ymlwithminioservice (ports 9000/9001, healthcheck, volume) andminio-setupinit job.ghostservice to useS3Storagevia env vars (bucket,region,endpoint,cdnUrl, credentials, URL prefixes,forcePathStyle)..docker/minio/setup.shto configuremcalias, ensureghost-devbucket, and enable anonymous download.package.jsonscriptdocker:dev:object-storageto start Ghost with the object-storage profile.Written by Cursor Bugbot for commit f63925f. This will update automatically on new commits. Configure here.