feat: multi-stage docker image, migrations container, 96.35% smaller#871
feat: multi-stage docker image, migrations container, 96.35% smaller#871MrgSub merged 7 commits intoMail-0:stagingfrom
Conversation
|
@adiologydev is attempting to deploy a commit to the Zero Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis set of changes introduces a new multi-stage Docker build system for a Bun-based Next.js application, replacing the previous Dockerfile with a more modular approach. The new Dockerfiles are organized under Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Docker as Docker Build System
participant App as Bun Next.js App
participant DB as Database (Postgres)
participant Migrate as Migrations Service
Dev->>Docker: docker-compose up
Docker->>DB: Start Postgres container
Docker->>DB: Run healthcheck (pg_isready)
Docker->>Migrate: Start migrations container (runs bun run db:migrate)
Migrate->>DB: Apply migrations
Docker->>App: Build App image (multi-stage Dockerfile)
App->>App: Run build (bun run build, output standalone)
Docker->>App: Start App container
App->>DB: Connect to database
App->>App: Expose port 3000, serve requests
Possibly related PRs
Suggested reviewers
Poem
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. 🪧 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: 5
🧹 Nitpick comments (7)
docker/db/Dockerfile (1)
17-34: Well-structured runner stage that follows Docker best practicesThe runner stage correctly copies only the necessary files from the deps stage and the source repository. Setting DATABASE_URL as a build argument provides flexibility during image build.
Consider adding a HEALTHCHECK instruction to ensure the migration service can properly report its status:
WORKDIR /app/packages/db + +# Add healthcheck to indicate successful migration run +HEALTHCHECK --interval=5s --timeout=3s --start-period=10s --retries=3 \ + CMD [ -f /tmp/migration_success ] || exit 1apps/mail/next.config.ts (1)
6-6: Critical optimization for Docker buildsThe conditional
output: 'standalone'setting is key to achieving the 96.35% image size reduction mentioned in the PR objectives. This generates an optimized Next.js output that includes only the necessary runtime files when building for Docker.Consider adding a comment explaining why this setting is important for Docker builds:
- output: process.env.DOCKER_BUILD ? 'standalone' : undefined, + // Use standalone output for Docker builds to dramatically reduce image size + output: process.env.DOCKER_BUILD ? 'standalone' : undefined,docker/app/Dockerfile (1)
24-76: Effective builder stage with proper environment handlingThe builder stage correctly sets up all necessary build arguments and environment variables. Setting DOCKER_BUILD=true on line 69 is particularly important as it activates the standalone output in next.config.ts.
Consider moving the sharp installation to the deps stage to improve build caching:
# Install turbo globally RUN bun install -g turbo COPY package.json bun.lock turbo.json ./ RUN mkdir -p apps packages COPY apps/*/package.json ./apps/ +COPY apps/mail/package.json ./apps/mail/ COPY packages/*/package.json ./packages/ COPY packages/tsconfig/ ./packages/tsconfig/ -RUN bun install +# Install dependencies and sharp for Next.js image optimization +RUN bun install && cd apps/mail && bun install sharpdocker-compose.yaml (4)
22-26: Ensure migrations complete before starting the app
depends_onwithout health conditions only waits for container start, not for migrations to finish. Thezeroservice may begin before migrations complete, leading to startup errors. Consider adding a healthcheck to themigrationsservice or introducing an orchestration step (e.g., an entrypoint script) that waits for migrations to exit successfully before launching the main app.
27-34: Tighten healthcheck configurationA 90 s interval delays failure detection. Reducing it to around 30 s will catch issues sooner. Additionally, you can explicitly check for a 2xx HTTP status code to make the probe more robust.
57-61: Use dynamic variables in healthcheckThe healthcheck currently hardcodes the database name
zerodotemail. IfPOSTGRES_DBis overridden, the probe will still point to the default. Reference the Compose variables for consistency:healthcheck: test: ['CMD', 'pg_isready', '-U', '${POSTGRES_USER:-postgres}', '-d', '${POSTGRES_DB:-zerodotemail}']
81-87: Reevaluate container_name and env naming in Upstash proxy
- A hard-coded
container_namecan collide in multi-env setups; consider removing it to let Compose manage naming automatically.- The
SRH_TOKENfallback usesREDIS_TOKEN, which may confuse readers—unify on one variable.- Since no
portsare published, thecurlhealthcheck runs inside the container (OK), but if external access is ever needed, add aports:block.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.dockerignore(1 hunks)Dockerfile(0 hunks)apps/mail/next.config.ts(1 hunks)docker-compose.yaml(1 hunks)docker/app/Dockerfile(1 hunks)docker/db/Dockerfile(1 hunks)
💤 Files with no reviewable changes (1)
- Dockerfile
🔇 Additional comments (4)
.dockerignore (1)
1-48: Well-structured .dockerignore file that properly excludes unnecessary filesThis comprehensive .dockerignore file will help reduce Docker build context size and improve build times by excluding unnecessary files like node_modules, build artifacts, and development files. This directly supports the PR's goal of creating smaller, more efficient Docker images.
docker/db/Dockerfile (1)
1-16: Excellent use of multi-stage build with minimal dependenciesThe deps stage properly isolates dependency installation and installs only the necessary packages for database operations. Using Alpine-based images aligns with the PR goal of reducing image size.
docker/app/Dockerfile (1)
1-22: Well-structured base and dependencies stagesGood use of Alpine-based images and proper dependency installation strategy. Breaking the build into distinct stages follows Docker best practices and contributes to the smaller image size goal.
docker-compose.yaml (1)
70-74: Approve Redis healthcheckThe
valkeyservice healthcheck correctly verifies container liveness withredis-cli pingand appropriate intervals/retries. This ensures dependent services only start once Redis is ready.
docker/app/Dockerfile
Outdated
| # ======================================== | ||
| # Runner Stage: Production Environment | ||
| # ======================================== | ||
| FROM base AS runner | ||
| WORKDIR /app | ||
|
|
||
| # Define build arguments | ||
| ARG NEXT_PUBLIC_APP_URL \ | ||
| DATABASE_URL \ | ||
| BETTER_AUTH_SECRET \ | ||
| BETTER_AUTH_URL \ | ||
| BETTER_AUTH_TRUSTED_ORIGINS \ | ||
| GOOGLE_CLIENT_ID \ | ||
| GOOGLE_CLIENT_SECRET \ | ||
| REDIS_URL \ | ||
| REDIS_TOKEN \ | ||
| RESEND_API_KEY \ | ||
| OPENAI_API_KEY \ | ||
| AI_SYSTEM_PROMPT \ | ||
| GROQ_API_KEY \ | ||
| GOOGLE_GENERATIVE_AI_API_KEY \ | ||
| NEXT_TELEMETRY_DISABLED | ||
|
|
||
| # Set environment variables for build | ||
| ENV NEXT_PUBLIC_APP_URL=${NEXT_PUBLIC_APP_URL} \ | ||
| DATABASE_URL=${DATABASE_URL} \ | ||
| BETTER_AUTH_SECRET=${BETTER_AUTH_SECRET} \ | ||
| BETTER_AUTH_URL=${BETTER_AUTH_URL} \ | ||
| BETTER_AUTH_TRUSTED_ORIGINS=${BETTER_AUTH_TRUSTED_ORIGINS} \ | ||
| GOOGLE_CLIENT_ID=${GOOGLE_CLIENT_ID} \ | ||
| GOOGLE_CLIENT_SECRET=${GOOGLE_CLIENT_SECRET} \ | ||
| REDIS_URL=${REDIS_URL} \ | ||
| REDIS_TOKEN=${REDIS_TOKEN} \ | ||
| RESEND_API_KEY=${RESEND_API_KEY} \ | ||
| OPENAI_API_KEY=${OPENAI_API_KEY} \ | ||
| AI_SYSTEM_PROMPT=${AI_SYSTEM_PROMPT} \ | ||
| GROQ_API_KEY=${GROQ_API_KEY} \ | ||
| GOOGLE_GENERATIVE_AI_API_KEY=${GOOGLE_GENERATIVE_AI_API_KEY} \ | ||
| NEXT_TELEMETRY_DISABLED=${NEXT_TELEMETRY_DISABLED} \ | ||
| NODE_ENV=production | ||
|
|
||
| RUN addgroup -S -g 1001 bunjs && \ | ||
| adduser -S -u 1001 nextjs -G bunjs | ||
|
|
||
| # Copy public assets | ||
| COPY --from=builder --chown=nextjs:bunjs /app/apps/mail/public ./apps/mail/public | ||
|
|
||
| # Leverage output traces to reduce image size (standalone output) | ||
| COPY --from=builder --chown=nextjs:bunjs /app/apps/mail/.next/standalone ./ | ||
| COPY --from=builder --chown=nextjs:bunjs /app/apps/mail/.next/static ./apps/mail/.next/static | ||
|
|
||
| # Switch to non-root user | ||
| USER nextjs | ||
|
|
||
| # Set server port and host | ||
| EXPOSE 3000 | ||
| ENV PORT=3000 | ||
| ENV HOSTNAME="0.0.0.0" | ||
|
|
||
| # Start the server | ||
| CMD ["bun", "apps/mail/server.js"] No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Secure and optimized runner stage
Excellent security practice by creating and using a non-root user (lines 119-130). The use of the Next.js standalone output (lines 126-127) is key to achieving the 96.35% size reduction mentioned in the PR objectives.
Consider using a reusable environment variable setup to avoid duplication between builder and runner stages:
-# Define build arguments
+# Define build arguments with ARG directive
ARG NEXT_PUBLIC_APP_URL \
DATABASE_URL \
BETTER_AUTH_SECRET \
[...]
NEXT_TELEMETRY_DISABLED
-# Set environment variables for build
-ENV NEXT_PUBLIC_APP_URL=${NEXT_PUBLIC_APP_URL} \
- DATABASE_URL=${DATABASE_URL} \
- BETTER_AUTH_SECRET=${BETTER_AUTH_SECRET} \
- [...]
- NEXT_TELEMETRY_DISABLED=${NEXT_TELEMETRY_DISABLED} \
- NODE_ENV=production
+# Use a shell script to set all environment variables from ARGs
+COPY --from=builder /app/docker/app/set-env-vars.sh /set-env-vars.sh
+RUN chmod +x /set-env-vars.sh && \
+ /set-env-vars.sh > /app/.env && \
+ echo "NODE_ENV=production" >> /app/.env
+
+# Load environment variables from file
+ENV $(cat /app/.env | xargs)This approach requires creating a set-env-vars.sh script in your repository, but would simplify environment variable management and reduce Dockerfile duplication.
Committable suggestion skipped: line range outside the PR's diff.
docker/app/Dockerfile
Outdated
| RUN bun run build | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add healthcheck for production container
To ensure the container monitoring system can properly report service health status:
# Start the server
CMD ["bun", "apps/mail/server.js"]
+
+# Add healthcheck
+HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \
+ CMD wget -q -O - http://localhost:3000/api/health || exit 1This assumes you have or will implement a health endpoint at /api/health. If you use a different health endpoint path, adjust accordingly.
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
We perform the health check in the Docker compose file, thus, this is redundant and not required.
There was a problem hiding this comment.
@adiologydev You're absolutely right. I see that the healthcheck is indeed already implemented in the Docker Compose file for the zero service. Thank you for clarifying this approach - having the health check in the compose file is a clean way to manage it without duplicating it in the Dockerfile. I appreciate the correction.
✏️ Learnings added
Learnt from: adiologydev
PR: Mail-0/Zero#871
File: docker/app/Dockerfile:76-77
Timestamp: 2025-05-04T19:55:38.542Z
Learning: The Zero project implements container healthchecks in the Docker Compose file rather than in individual Dockerfiles to avoid redundancy.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| migrations: | ||
| build: | ||
| context: . | ||
| dockerfile: docker/db/Dockerfile | ||
| args: | ||
| DATABASE_URL: postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@db:5432/${POSTGRES_DB:-zerodotemail} | ||
| depends_on: | ||
| - db | ||
| command: ['bun', 'run', 'db:migrate'] | ||
| restart: 'no' | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize build context for migrations
Using the repo root (.) as the build context bakes unnecessary files into the migrations image and slows builds. Scope the context to the docker/db directory (where your migration Dockerfile and assets live) to speed up builds and shrink context size.
Proposed diff:
migrations:
build:
- context: .
+ context: docker/db
dockerfile: docker/db/Dockerfile📝 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.
| migrations: | |
| build: | |
| context: . | |
| dockerfile: docker/db/Dockerfile | |
| args: | |
| DATABASE_URL: postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@db:5432/${POSTGRES_DB:-zerodotemail} | |
| depends_on: | |
| - db | |
| command: ['bun', 'run', 'db:migrate'] | |
| restart: 'no' | |
| migrations: | |
| build: | |
| context: docker/db | |
| dockerfile: docker/db/Dockerfile | |
| args: | |
| DATABASE_URL: postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@db:5432/${POSTGRES_DB:-zerodotemail} | |
| depends_on: | |
| - db | |
| command: ['bun', 'run', 'db:migrate'] | |
| restart: 'no' |
There was a problem hiding this comment.
The original context is required since our Docker files access contents throughout the root of the monorepo, changing this would require significant unnecessary manoeuvring.
There was a problem hiding this comment.
@adiologydev I understand completely. In a monorepo setup, keeping the root context is often necessary to access files across packages. Your approach makes sense given that your Dockerfiles need access to various parts of the project structure.
The good news is you've already addressed the main downside (large context size) by adding a comprehensive .dockerignore file, which is the correct way to optimize build context when you need to maintain access to the full repository structure.
✏️ Learnings added
Learnt from: adiologydev
PR: Mail-0/Zero#871
File: docker-compose.yaml:36-46
Timestamp: 2025-05-04T20:00:52.172Z
Learning: The project uses a monorepo structure where Docker builds require the root context to access files throughout the repository. Changing build contexts for services like migrations would require significant restructuring.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| image: postgres:17-alpine | ||
| restart: unless-stopped | ||
| environment: | ||
| POSTGRES_USER: postgres | ||
| POSTGRES_PASSWORD: postgres | ||
| POSTGRES_DB: zerodotemail | ||
| POSTGRES_USER: ${POSTGRES_USER:-postgres} | ||
| POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-postgres} | ||
| POSTGRES_DB: ${POSTGRES_DB:-zerodotemail} |
There was a problem hiding this comment.
Fix indentation of environment variables
The POSTGRES_USER, POSTGRES_PASSWORD, and POSTGRES_DB entries are under environment:, but the PGDATA variable (line 54) is not indented into the same block and will be ignored. It should be nested under environment:.
Proposed diff:
db:
image: postgres:17-alpine
restart: unless-stopped
environment:
- POSTGRES_USER: ${POSTGRES_USER:-postgres}
- POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-postgres}
- POSTGRES_DB: ${POSTGRES_DB:-zerodotemail}
- PGDATA: /var/lib/postgresql/data/pgdata
+ POSTGRES_USER: ${POSTGRES_USER:-postgres}
+ POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-postgres}
+ POSTGRES_DB: ${POSTGRES_DB:-zerodotemail}
+ PGDATA: /var/lib/postgresql/data/pgdata📝 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.
| image: postgres:17-alpine | |
| restart: unless-stopped | |
| environment: | |
| POSTGRES_USER: postgres | |
| POSTGRES_PASSWORD: postgres | |
| POSTGRES_DB: zerodotemail | |
| POSTGRES_USER: ${POSTGRES_USER:-postgres} | |
| POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-postgres} | |
| POSTGRES_DB: ${POSTGRES_DB:-zerodotemail} | |
| db: | |
| image: postgres:17-alpine | |
| restart: unless-stopped | |
| environment: | |
| POSTGRES_USER: ${POSTGRES_USER:-postgres} | |
| POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-postgres} | |
| POSTGRES_DB: ${POSTGRES_DB:-zerodotemail} | |
| PGDATA: /var/lib/postgresql/data/pgdata |
There was a problem hiding this comment.
This is an incorrect suggestion. It's properly indented.
There was a problem hiding this comment.
@adiologydev You're absolutely right, and I apologize for my incorrect suggestion. Looking at the full file content again, I can see that PGDATA is indeed properly indented under the environment: block. Thank you for the correction!
✏️ Learnings added
Learnt from: adiologydev
PR: Mail-0/Zero#871
File: docker-compose.yaml:48-53
Timestamp: 2025-05-04T19:53:46.722Z
Learning: In docker-compose.yaml, the PGDATA environment variable for PostgreSQL containers should be properly indented under the environment block. The current indentation in the project is correct.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
docker-compose.yaml
Outdated
| zero: | ||
| build: | ||
| context: . | ||
| dockerfile: docker/app/Dockerfile | ||
| args: | ||
| NEXT_PUBLIC_APP_URL: ${NEXT_PUBLIC_APP_URL:-http://localhost:3000} | ||
| DATABASE_URL: postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@db:5432/${POSTGRES_DB:-zerodotemail} | ||
| BETTER_AUTH_SECRET: ${BETTER_AUTH_SECRET:-my-better-auth-secret} | ||
| BETTER_AUTH_URL: ${BETTER_AUTH_URL:-http://localhost:3000} | ||
| BETTER_AUTH_TRUSTED_ORIGINS: ${BETTER_AUTH_TRUSTED_ORIGINS:-http://localhost:3000} | ||
| GOOGLE_CLIENT_ID: ${GOOGLE_CLIENT_ID} | ||
| GOOGLE_CLIENT_SECRET: ${GOOGLE_CLIENT_SECRET} | ||
| REDIS_URL: http://upstash-proxy:80 | ||
| REDIS_TOKEN: ${REDIS_TOKEN:-upstash-local-token} | ||
| RESEND_API_KEY: ${RESEND_API_KEY} | ||
| OPENAI_API_KEY: ${OPENAI_API_KEY} | ||
| AI_SYSTEM_PROMPT: ${AI_SYSTEM_PROMPT} | ||
| GROQ_API_KEY: ${GROQ_API_KEY} | ||
| GOOGLE_GENERATIVE_AI_API_KEY: ${GOOGLE_GENERATIVE_AI_API_KEY} | ||
| NEXT_TELEMETRY_DISABLED: ${NEXT_TELEMETRY_DISABLED:-0} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid baking secrets into the build image
Build args are stored in Docker image metadata and can leak sensitive information (API keys, auth secrets, etc.). It's safer to pass these values at container runtime via the environment section rather than as args. Also, consider narrowing the build context to docker/app to reduce build payload.
Proposed diff:
services:
zero:
build:
- context: .
+ context: docker/app
dockerfile: docker/app/Dockerfile
- args:
- NEXT_PUBLIC_APP_URL: ${NEXT_PUBLIC_APP_URL:-http://localhost:3000}
- DATABASE_URL: postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@db:5432/${POSTGRES_DB:-zerodotemail}
- BETTER_AUTH_SECRET: ${BETTER_AUTH_SECRET:-my-better-auth-secret}
- BETTER_AUTH_URL: ${BETTER_AUTH_URL:-http://localhost:3000}
- BETTER_AUTH_TRUSTED_ORIGINS: ${BETTER_AUTH_TRUSTED_ORIGINS:-http://localhost:3000}
- GOOGLE_CLIENT_ID: ${GOOGLE_CLIENT_ID}
- GOOGLE_CLIENT_SECRET: ${GOOGLE_CLIENT_SECRET}
- REDIS_URL: http://upstash-proxy:80
- REDIS_TOKEN: ${REDIS_TOKEN:-upstash-local-token}
- RESEND_API_KEY: ${RESEND_API_KEY}
- OPENAI_API_KEY: ${OPENAI_API_KEY}
- AI_SYSTEM_PROMPT: ${AI_SYSTEM_PROMPT}
- GROQ_API_KEY: ${GROQ_API_KEY}
- GOOGLE_GENERATIVE_AI_API_KEY: ${GOOGLE_GENERATIVE_AI_API_KEY}
- NEXT_TELEMETRY_DISABLED: ${NEXT_TELEMETRY_DISABLED:-0}
+ args:
+ NEXT_TELEMETRY_DISABLED: ${NEXT_TELEMETRY_DISABLED:-0}
+ environment:
+ NEXT_PUBLIC_APP_URL: ${NEXT_PUBLIC_APP_URL:-http://localhost:3000}
+ DATABASE_URL: postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@db:5432/${POSTGRES_DB:-zerodotemail}
+ BETTER_AUTH_SECRET: ${BETTER_AUTH_SECRET}
+ BETTER_AUTH_URL: ${BETTER_AUTH_URL}
+ BETTER_AUTH_TRUSTED_ORIGINS: ${BETTER_AUTH_TRUSTED_ORIGINS}
+ GOOGLE_CLIENT_ID: ${GOOGLE_CLIENT_ID}
+ GOOGLE_CLIENT_SECRET: ${GOOGLE_CLIENT_SECRET}
+ REDIS_URL: http://upstash-proxy:80
+ REDIS_TOKEN: ${REDIS_TOKEN:-upstash-local-token}
+ RESEND_API_KEY: ${RESEND_API_KEY}
+ OPENAI_API_KEY: ${OPENAI_API_KEY}
+ AI_SYSTEM_PROMPT: ${AI_SYSTEM_PROMPT}
+ GROQ_API_KEY: ${GROQ_API_KEY}
+ GOOGLE_GENERATIVE_AI_API_KEY: ${GOOGLE_GENERATIVE_AI_API_KEY}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Although we could let some runtime variables out of the arguments, nextjs still require some during static generation at build time. How would you like to address this?
There was a problem hiding this comment.
@adiologydev You're absolutely right. Next.js has specific requirements for environment variables during build time, especially for static site generation. Let's take a hybrid approach:
services:
zero:
build:
context: .
dockerfile: docker/app/Dockerfile
args:
# Keep variables needed during build time
NEXT_PUBLIC_APP_URL: ${NEXT_PUBLIC_APP_URL:-http://localhost:3000}
DATABASE_URL: postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@db:5432/${POSTGRES_DB:-zerodotemail}
BETTER_AUTH_URL: ${BETTER_AUTH_URL:-http://localhost:3000}
BETTER_AUTH_TRUSTED_ORIGINS: ${BETTER_AUTH_TRUSTED_ORIGINS:-http://localhost:3000}
NEXT_TELEMETRY_DISABLED: ${NEXT_TELEMETRY_DISABLED:-0}
# Remove sensitive values from build args
- BETTER_AUTH_SECRET: ${BETTER_AUTH_SECRET:-my-better-auth-secret}
- GOOGLE_CLIENT_ID: ${GOOGLE_CLIENT_ID}
- GOOGLE_CLIENT_SECRET: ${GOOGLE_CLIENT_SECRET}
- REDIS_TOKEN: ${REDIS_TOKEN:-upstash-local-token}
- RESEND_API_KEY: ${RESEND_API_KEY}
- OPENAI_API_KEY: ${OPENAI_API_KEY}
- AI_SYSTEM_PROMPT: ${AI_SYSTEM_PROMPT}
- GROQ_API_KEY: ${GROQ_API_KEY}
- GOOGLE_GENERATIVE_AI_API_KEY: ${GOOGLE_GENERATIVE_AI_API_KEY}
REDIS_URL: http://upstash-proxy:80
# Add all variables as runtime environment variables
environment:
NEXT_PUBLIC_APP_URL: ${NEXT_PUBLIC_APP_URL:-http://localhost:3000}
DATABASE_URL: postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@db:5432/${POSTGRES_DB:-zerodotemail}
BETTER_AUTH_SECRET: ${BETTER_AUTH_SECRET:-my-better-auth-secret}
BETTER_AUTH_URL: ${BETTER_AUTH_URL:-http://localhost:3000}
BETTER_AUTH_TRUSTED_ORIGINS: ${BETTER_AUTH_TRUSTED_ORIGINS:-http://localhost:3000}
GOOGLE_CLIENT_ID: ${GOOGLE_CLIENT_ID}
GOOGLE_CLIENT_SECRET: ${GOOGLE_CLIENT_SECRET}
REDIS_URL: http://upstash-proxy:80
REDIS_TOKEN: ${REDIS_TOKEN:-upstash-local-token}
RESEND_API_KEY: ${RESEND_API_KEY}
OPENAI_API_KEY: ${OPENAI_API_KEY}
AI_SYSTEM_PROMPT: ${AI_SYSTEM_PROMPT}
GROQ_API_KEY: ${GROQ_API_KEY}
GOOGLE_GENERATIVE_AI_API_KEY: ${GOOGLE_GENERATIVE_AI_API_KEY}
NEXT_TELEMETRY_DISABLED: ${NEXT_TELEMETRY_DISABLED:-0}This approach:
- Keeps essential build-time variables as build args (NEXT_PUBLIC_* and those needed for SSG)
- Moves sensitive credentials to runtime-only environment variables
- Ensures all variables are available at runtime through the environment section
The Dockerfile would need to be updated to use these environment variables at runtime instead of build args for the sensitive values.
Would this compromise work for your Next.js build requirements while improving security?
✏️ Learnings added
Learnt from: adiologydev
PR: Mail-0/Zero#871
File: docker-compose.yaml:2-21
Timestamp: 2025-05-04T23:13:26.797Z
Learning: Next.js requires certain environment variables during static site generation at build time, particularly those with the NEXT_PUBLIC_ prefix. When using Docker, these should be passed as build args, while sensitive values like API keys and secrets should ideally only be passed at runtime as environment variables.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
And what if we want to completely disable SSG project wide so that everything's a runtime variable? Any suggestions on that?
There was a problem hiding this comment.
@adiologydev Yes, it's possible to disable SSG project-wide and use only runtime variables. Here's how you can approach it:
-
Force Server-Side Rendering (SSR) for all pages:
In yournext.config.ts, add:const nextConfig = { // Existing config... output: 'standalone', runtime: 'nodejs', experimental: { // Existing experimental features... serverComponentsExternalPackages: [], // If using App Router } };
-
Convert all static pages to dynamic:
For Pages Router, usegetServerSidePropsin all pages
For App Router, setexport const dynamic = 'force-dynamic'in layout/page files -
Update Docker setup:
services: zero: build: context: . dockerfile: docker/app/Dockerfile args: - NEXT_PUBLIC_APP_URL: ${NEXT_PUBLIC_APP_URL:-http://localhost:3000} - DATABASE_URL: postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@db:5432/${POSTGRES_DB:-zerodotemail} - BETTER_AUTH_SECRET: ${BETTER_AUTH_SECRET:-my-better-auth-secret} - BETTER_AUTH_URL: ${BETTER_AUTH_URL:-http://localhost:3000} - BETTER_AUTH_TRUSTED_ORIGINS: ${BETTER_AUTH_TRUSTED_ORIGINS:-http://localhost:3000} - GOOGLE_CLIENT_ID: ${GOOGLE_CLIENT_ID} - GOOGLE_CLIENT_SECRET: ${GOOGLE_CLIENT_SECRET} - REDIS_URL: http://upstash-proxy:80 - REDIS_TOKEN: ${REDIS_TOKEN:-upstash-local-token} - RESEND_API_KEY: ${RESEND_API_KEY} - OPENAI_API_KEY: ${OPENAI_API_KEY} - AI_SYSTEM_PROMPT: ${AI_SYSTEM_PROMPT} - GROQ_API_KEY: ${GROQ_API_KEY} - GOOGLE_GENERATIVE_AI_API_KEY: ${GOOGLE_GENERATIVE_AI_API_KEY} - NEXT_TELEMETRY_DISABLED: ${NEXT_TELEMETRY_DISABLED:-0} + DOCKER_BUILD: "true" + NEXT_TELEMETRY_DISABLED: ${NEXT_TELEMETRY_DISABLED:-0} environment: + NEXT_PUBLIC_APP_URL: ${NEXT_PUBLIC_APP_URL:-http://localhost:3000} + DATABASE_URL: postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@db:5432/${POSTGRES_DB:-zerodotemail} + BETTER_AUTH_SECRET: ${BETTER_AUTH_SECRET:-my-better-auth-secret} + BETTER_AUTH_URL: ${BETTER_AUTH_URL:-http://localhost:3000} + BETTER_AUTH_TRUSTED_ORIGINS: ${BETTER_AUTH_TRUSTED_ORIGINS:-http://localhost:3000} + GOOGLE_CLIENT_ID: ${GOOGLE_CLIENT_ID} + GOOGLE_CLIENT_SECRET: ${GOOGLE_CLIENT_SECRET} + REDIS_URL: http://upstash-proxy:80 + REDIS_TOKEN: ${REDIS_TOKEN:-upstash-local-token} + RESEND_API_KEY: ${RESEND_API_KEY} + OPENAI_API_KEY: ${OPENAI_API_KEY} + AI_SYSTEM_PROMPT: ${AI_SYSTEM_PROMPT} + GROQ_API_KEY: ${GROQ_API_KEY} + GOOGLE_GENERATIVE_AI_API_KEY: ${GOOGLE_GENERATIVE_AI_API_KEY} + NEXT_TELEMETRY_DISABLED: ${NEXT_TELEMETRY_DISABLED:-0}
Trade-offs to consider:
- ✅ Better security (no secrets in build)
- ✅ Simpler Docker setup with all variables at runtime
- ❌ Potentially slower initial page loads
- ❌ Higher server resource usage
The standalone output option still provides optimal container size while enabling runtime environment variables to work properly.
✏️ Learnings added
Learnt from: adiologydev
PR: Mail-0/Zero#871
File: docker-compose.yaml:2-21
Timestamp: 2025-05-04T23:21:21.374Z
Learning: To completely disable SSG (Static Site Generation) in Next.js projects for Docker deployments, use 'force-dynamic' for App Router or getServerSideProps for Pages Router in all pages, and set output: 'standalone' in next.config.ts. This allows all environment variables to be passed at runtime instead of build time, improving security for sensitive values.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: adiologydev
PR: Mail-0/Zero#871
File: docker-compose.yaml:2-21
Timestamp: 2025-05-04T23:13:26.797Z
Learning: Next.js requires certain environment variables during static site generation at build time, particularly those with the NEXT_PUBLIC_ prefix. When using Docker, these should be passed as build args, while sensitive values like API keys and secrets should ideally only be passed at runtime as environment variables.
|
Can you separate out the deploy docker-compose from the db and redis, |
|
Hey @adiologydev I just tested it! The Dockerfile is working fine but compose is not. The app services depends on upstash-proxy, and the upstash-proxy is failing the healthcheck due to Could you please update the CMD on upstash-proxy service on the compose (or do something to make the health check pass)?
|
adiologydev
force-pushed
the
feat/docker-improvements
branch
from
18e046c to
44b0bdb
Compare
|
All resolved, ready for merge. Had an old upstream merge before causing conflict. |
|
Pushing an update with seperate backend soon |
4 participants
Description
This PR introduces an optimised multi-stage Docker image for the main app and an additional image + container to run migrations every time a new version is deployed.
All the images are built as context in the Docker Compose file to avoid requiring external registries.
Type of Change
Please delete options that are not relevant.
Areas Affected
Please check all that apply:
Testing Done
Describe the tests you've done:
Checklist
Screenshots/Recordings
By submitting this pull request, I confirm that my contribution is made under the terms of the project's license.
Summary by CodeRabbit
New Features
Chores