-
Notifications
You must be signed in to change notification settings - Fork 5
Add local MinIO S3 service to Docker Compose and update related scripts #129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdded local S3-compatible object storage (MinIO) to dev infra, updated docker compose and npm scripts for bring-up/tear-down, enhanced start/stop scripts with MinIO bootstrap and bucket provisioning, and refactored lib/r2.ts to detect endpoints and initialize S3 client for local or Cloudflare R2 usage. Changes
Sequence DiagramsequenceDiagram
participant App as Application Code
participant R2Lib as lib/r2
participant Env as Env vars
participant S3 as S3 Client (MinIO or R2)
participant Bucket as Bucket ops
App->>R2Lib: call getS3() / putObject()/deleteObjects()
R2Lib->>Env: read R2_ENDPOINT, R2_ACCOUNT_ID, keys
alt explicit local endpoint present
R2Lib->>S3: create client with endpoint + forcePathStyle
note right of S3 #DFF2E1: MinIO / S3-compatible
else Cloudflare R2 (no explicit endpoint)
R2Lib->>S3: create client with account ID, region="auto"
note right of S3 #FFF4D1: Cloudflare R2 (virtual-hosted)
end
S3-->>R2Lib: client ready
R2Lib->>Bucket: perform putObject / deleteObjects (batched)
Bucket-->>App: operation result (with logged errors if any)
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/r2.ts (2)
70-85: Critical: Missing bucket validation in putObject.Line 78 uses
process.env.R2_BUCKETwithout validation, which will passundefinedto the S3Client, causing runtime errors during object uploads.Apply this diff to add validation:
export async function putObject(options: { key: string; body: Buffer | Uint8Array; contentType: string; cacheControl?: string; }): Promise<void> { + if (!process.env.R2_BUCKET) { + throw new Error("R2_BUCKET is required for object storage operations"); + } + const s3 = getS3(); const cmd = new PutObjectCommand({ Bucket: process.env.R2_BUCKET,
99-137: Critical bucket validation issue, but error handling is excellent.Line 105 uses
process.env.R2_BUCKETwithout validation (same issue asputObject). However, the batching logic and per-key error handling are well-implemented.Apply this diff to add bucket validation:
export async function deleteObjects(keys: string[]): Promise<DeleteResult> { const results: DeleteResult = []; if (!keys.length) return results; + if (!process.env.R2_BUCKET) { + throw new Error("R2_BUCKET is required for object storage operations"); + } + const s3 = getS3(); const MAX_PER_BATCH = 1000;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
README.md(3 hunks)docker-compose.yml(2 hunks)lib/r2.ts(3 hunks)package.json(1 hunks)scripts/start-dev-infra.sh(2 hunks)scripts/stop-dev-infra.sh(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
package.json
📄 CodeRabbit inference engine (AGENTS.md)
Require Node.js >= 22 via package.json engines
Files:
package.json
**/*.{ts,tsx,css,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation across the codebase
Files:
package.jsonlib/r2.ts
lib/**
📄 CodeRabbit inference engine (AGENTS.md)
lib/ holds domain utilities and caching (lib/cache); import these via @/... aliases
Files:
lib/r2.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript only; keep modules small, pure, and roughly ≤300 LOC
Consolidate imports using @/... path aliases
Files:
lib/r2.ts
🧠 Learnings (1)
📚 Learning: 2025-10-20T02:11:42.179Z
Learnt from: CR
PR: jakejarvis/domainstack.io#0
File: AGENTS.md:0-0
Timestamp: 2025-10-20T02:11:42.179Z
Learning: Applies to **/*.test.{ts,tsx} : For R2 storage, mock aws-sdk/client-s3 (S3Client with mocked send) and set R2_* env vars with vi.stubEnv in suites touching uploads/deletes
Applied to files:
lib/r2.ts
🪛 Shellcheck (0.11.0)
scripts/start-dev-infra.sh
[warning] 13-13: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
🔇 Additional comments (12)
package.json (1)
16-17: LGTM! Script naming is clearer and more intuitive.The rename from
dev:start-dockertodocker:upand the addition ofdocker:downfollow standard Docker CLI command patterns, making the developer experience more consistent.docker-compose.yml (3)
28-29: LGTM! Logging configuration balances verbosity and debugging.Disabling traffic logging while keeping connection info is a reasonable tradeoff for local development, reducing noise without losing important diagnostic information.
68-69: Document that default credentials are for local development only.The
minioadmin/minioadmincredentials are standard MinIO defaults, but ensure developers understand these are for local development only and should never be used in production or committed to version control if changed.Verify that the README.md explicitly mentions these credentials are for local development only and provides guidance on securing production deployments.
78-78: LGTM! Volume declaration is correct.The
minio_datavolume is properly declared and matches the MinIO service volume mount, ensuring data persistence across container restarts.scripts/stop-dev-infra.sh (1)
1-27: LGTM! Clean and well-structured teardown script.The script properly handles:
- Error handling with
set -euo pipefail- Environment detection for Docker and Docker Compose v2
- User-friendly error messages
- Consistent structure with the start script
README.md (3)
66-72: LGTM! Clear and comprehensive environment variable documentation.The MinIO/R2 configuration is well-documented with:
- Local endpoint configuration
- Default credentials matching docker-compose.yml
- Public base URL for browser access to stored assets
89-102: LGTM! Command documentation is clear and includes platform-specific guidance.The updated commands and Linux networking guidance help developers on different platforms set up the local environment correctly. The addition of
docker:downprovides a clean way to tear down services.
127-132: LGTM! Commands list is clear and up-to-date.The commands section accurately reflects the updated script names and provides a quick reference for common development tasks.
lib/r2.ts (1)
15-17: LGTM! Explicit credential checks prevent silent failures.The upfront validation of required credentials provides clear error messages and prevents cryptic failures downstream.
scripts/start-dev-infra.sh (3)
8-15: LGTM! Environment loading is correctly implemented.The script properly loads
.env.localif present, usingset -a/set +ato export all variables. The ShellCheck warning about non-constant source is a known false positive for environment file loading patterns.
72-73: LGTM! MinIO readiness check is consistent with other services.The wait logic for MinIO S3 API on port 9000 follows the same pattern as other services, ensuring the service is ready before proceeding with bucket provisioning.
113-124: LGTM! Clear success message and helpful log streaming.The success message clearly lists all services with their endpoints, and using
execwithlogs -fprovides a seamless development experience by keeping logs streaming in the foreground.
224c2c5 to
d0d6003
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/start-dev-infra.sh (1)
34-36: Don’t fully suppresspullerrors; warn and continue.Keep non-fatal behavior but surface issues.
-# Pull missing images (no-op if already pulled), then start detached -$DOCKER_COMPOSE pull --ignore-pull-failures || true +# Pull missing images (no-op if already pulled), then start detached +if ! $DOCKER_COMPOSE pull --ignore-pull-failures; then + echo "⚠️ Warning: 'docker compose pull' had errors; continuing…" >&2 +fi $DOCKER_COMPOSE up -d
♻️ Duplicate comments (2)
docker-compose.yml (1)
65-65: Pin the MinIO image to a specific release.Avoid
latestto ensure reproducible dev environments.- image: minio/minio:latest + # Pin to a known-good MinIO release for reproducibility + image: minio/minio:RELEASE.2025-09-07T15-34-20Zlib/r2.ts (1)
50-54: TrimR2_REGIONto avoid passing empty strings.Prevents accidental empty region when env var is set but blank.
- region: process.env.R2_REGION || "us-east-1", + region: (process.env.R2_REGION?.trim() || "us-east-1"),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
README.md(3 hunks)docker-compose.yml(2 hunks)lib/r2.ts(4 hunks)package.json(1 hunks)scripts/start-dev-infra.sh(2 hunks)scripts/stop-dev-infra.sh(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
lib/**
📄 CodeRabbit inference engine (AGENTS.md)
lib/ holds domain utilities and caching (lib/cache); import these via @/... aliases
Files:
lib/r2.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript only; keep modules small, pure, and roughly ≤300 LOC
Consolidate imports using @/... path aliases
Files:
lib/r2.ts
**/*.{ts,tsx,css,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation across the codebase
Files:
lib/r2.tspackage.json
package.json
📄 CodeRabbit inference engine (AGENTS.md)
Require Node.js >= 22 via package.json engines
Files:
package.json
🪛 Shellcheck (0.11.0)
scripts/start-dev-infra.sh
[warning] 13-13: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
🔇 Additional comments (3)
package.json (1)
115-117: Engines constraint looks good.Node >= 22 requirement matches guidelines. No changes needed.
As per coding guidelines.
scripts/stop-dev-infra.sh (1)
1-27: LGTM for graceful teardown.Checks for Docker/Compose v2 and clean
downare solid.lib/r2.ts (1)
44-56: Confirm intent: treat any non-R2 endpoint as “local.”If a custom Cloudflare domain (not ending in
.r2.cloudflarestorage.com) is ever used, this path forces path-style and non-autoregion, which may be incompatible.Would you like a small flag (e.g., R2_PROVIDER=r2|minio) to disambiguate and avoid misclassification?
Also applies to: 86-105
| # Local MinIO S3 (TCP on 9000) | ||
| minio: | ||
| image: minio/minio:latest | ||
| command: server /data --console-address ":9001" | ||
| environment: | ||
| MINIO_ROOT_USER: minioadmin | ||
| MINIO_ROOT_PASSWORD: minioadmin | ||
| ports: | ||
| - "9000:9000" # S3 API endpoint | ||
| - "9001:9001" # Web console | ||
| volumes: | ||
| - minio_data:/data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Add a healthcheck for MinIO.
Lets up --wait (or external waits) detect readiness without custom logic.
minio:
image: minio/minio:RELEASE.2025-09-07T15-34-20Z
command: server /data --console-address ":9001"
+ healthcheck:
+ test: ["CMD", "curl", "-f", "http://localhost:9000/minio/health/ready"]
+ interval: 10s
+ timeout: 5s
+ retries: 5
environment:
MINIO_ROOT_USER: minioadmin
MINIO_ROOT_PASSWORD: minioadmin📝 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.
| # Local MinIO S3 (TCP on 9000) | |
| minio: | |
| image: minio/minio:latest | |
| command: server /data --console-address ":9001" | |
| environment: | |
| MINIO_ROOT_USER: minioadmin | |
| MINIO_ROOT_PASSWORD: minioadmin | |
| ports: | |
| - "9000:9000" # S3 API endpoint | |
| - "9001:9001" # Web console | |
| volumes: | |
| - minio_data:/data | |
| # Local MinIO S3 (TCP on 9000) | |
| minio: | |
| image: minio/minio:RELEASE.2025-09-07T15-34-20Z | |
| command: server /data --console-address ":9001" | |
| healthcheck: | |
| test: ["CMD", "curl", "-f", "http://localhost:9000/minio/health/ready"] | |
| interval: 10s | |
| timeout: 5s | |
| retries: 5 | |
| environment: | |
| MINIO_ROOT_USER: minioadmin | |
| MINIO_ROOT_PASSWORD: minioadmin | |
| ports: | |
| - "9000:9000" # S3 API endpoint | |
| - "9001:9001" # Web console | |
| volumes: | |
| - minio_data:/data |
| - "9000:9000" # S3 API endpoint | ||
| - "9001:9001" # Web console |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Limit MinIO exposure to loopback.
Binding to all interfaces exposes the S3 API and console to your LAN. Prefer loopback in dev.
- - "9000:9000" # S3 API endpoint
- - "9001:9001" # Web console
+ - "127.0.0.1:9000:9000" # S3 API endpoint
+ - "127.0.0.1:9001:9001" # Web console📝 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.
| - "9000:9000" # S3 API endpoint | |
| - "9001:9001" # Web console | |
| - "127.0.0.1:9000:9000" # S3 API endpoint | |
| - "127.0.0.1:9001:9001" # Web console |
🤖 Prompt for AI Agents
In docker-compose.yml around lines 71 to 72, the MinIO ports are currently bound
to all interfaces ("9000:9000" and "9001:9001"); change the host bindings to
loopback (e.g. "127.0.0.1:9000:9000" and "127.0.0.1:9001:9001") so the S3 API
and web console are only accessible from the local machine in development, and
keep a comment noting this is intentional for dev-only exposure.
| ```bash | ||
| pnpm dev:start-docker | ||
| pnpm docker:up | ||
| ``` | ||
|
|
||
| > On Linux, if `host.docker.internal` isn’t available, add `extra_hosts` to the Inngest service in `docker-compose.yml`: | ||
| > On Linux, if `host.docker.internal` isn’t available, add `extra_hosts` to the Inngest and MinIO services in `docker-compose.yml`: | ||
| > | ||
| > ```yaml | ||
| > extra_hosts: ["host.docker.internal:host-gateway"] | ||
| > ``` | ||
| To stop everything cleanly: | ||
| ```bash | ||
| pnpm docker:down | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Docs align; consider a short safety tip.
Looks great. Optional: note that MinIO ports can be bound to 127.0.0.1 in docker-compose to avoid LAN exposure.
🤖 Prompt for AI Agents
In README.md around lines 88 to 102, add a short safety tip that recommends
binding MinIO ports to localhost to avoid exposing the service on the LAN;
update the docs to suggest setting the MinIO service ports to 127.0.0.1 in
docker-compose (or equivalent) and briefly state the security benefit and that
this is optional for local-only access.
| if [ -f "$ENV_FILE" ]; then | ||
| echo "💉 Loading $ENV_FILE" | ||
| set -a | ||
| . "$ENV_FILE" | ||
| set +a | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Silence ShellCheck for dynamic env sourcing.
Avoid false positives on . "$ENV_FILE" by annotating.
if [ -f "$ENV_FILE" ]; then
echo "💉 Loading $ENV_FILE"
set -a
+ # shellcheck disable=SC1090
. "$ENV_FILE"
set +a
fi📝 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.
| if [ -f "$ENV_FILE" ]; then | |
| echo "💉 Loading $ENV_FILE" | |
| set -a | |
| . "$ENV_FILE" | |
| set +a | |
| fi | |
| if [ -f "$ENV_FILE" ]; then | |
| echo "💉 Loading $ENV_FILE" | |
| set -a | |
| # shellcheck disable=SC1090 | |
| . "$ENV_FILE" | |
| set +a | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 13-13: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
🤖 Prompt for AI Agents
In scripts/start-dev-infra.sh around lines 10 to 15, ShellCheck raises a false
positive for the dynamic env file sourcing ". \"$ENV_FILE\""; add a ShellCheck
annotation immediately above the sourcing line to silence it by either adding "#
shellcheck source=/dev/null" or "# shellcheck disable=SC1090" so the dynamic
source is allowed while keeping the rest of the checks intact.
Summary by CodeRabbit
New Features
Documentation
Chores