Skip to content
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.

Sign up for GitHub

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

Fix production build broken by missing env DATABASE_URL at build time #41

Merged
merged 1 commit into from
Jul 22, 2024
Merged

Fix production build broken by missing env DATABASE_URL at build time #41

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .dockerignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
Dockerfile
.dockerignore
node_modules
npm-debug.log
.pnpm-store
out
dist
*.md
.git
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were inserting a lot of shit in the image.

.next
!.next/static
!.next/standalone
44 changes: 28 additions & 16 deletions apps/web/docker/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ COPY . .

RUN turbo prune @latitude-data/web --docker

# COMMON stage (installer)
# BUILDER stage
# ------------------------------------------------------
FROM base AS installer
FROM base AS builder
WORKDIR /app

COPY --from=base /app/out/json/ .
COPY --from=base /app/out/pnpm-lock.yaml ./pnpm-lock.yaml
COPY --from=base /app/out/pnpm-workspace.yaml ./pnpm-workspace.yaml
Expand All @@ -17,38 +19,48 @@ RUN --mount=type=cache,id=pnpm,target=/pnpm/store pnpm install \
--frozen-lockfile \
--filter "@latitude-data/web..."

# PRODUCTION stage
# ------------------------------------------------------
FROM installer AS builder
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

builder and installer can be merged I think.


RUN --mount=type=cache,id=pnpm,target=/pnpm/store \
BUILDING_CONTAINER=true \
pnpm turbo build --filter='@latitude-data/web...'

FROM base AS runner
FROM base as prod-deps
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is worth it to have a stage only to install production deps. That then are copied in production stage

WORKDIR /app
RUN --mount=type=cache,id=pnpm,target=/pnpm/store pnpm install \
--prod \
--frozen-lockfile \
--filter "@latitude-data/web..."

# PRODUCTION
FROM base AS production
WORKDIR /app-prod

# Don't run production as root
# Public files doesn't have the unpriviledged user
COPY --from=base /app/apps/web/public ./apps/web/public

# Unpriviledged user
RUN addgroup --system --gid 1001 nodejs
RUN adduser --system --uid 1001 nextjs
USER nextjs

COPY --from=installer /app/apps/web/next.config.mjs .
COPY --from=installer /app/apps/web/package.json .
COPY --from=installer /app/node_modules ./node_modules
COPY --from=installer /app/apps/web/package.json ./apps/web/package.json
andresgutgon marked this conversation as resolved.
Show resolved Hide resolved
COPY --from=builder /app/apps/web/next.config.mjs .
COPY --from=builder /app/apps/web/package.json .
COPY --from=prod-deps /app/node_modules ./node_modules

# Automatically leverage output traces to reduce image size
# https://nextjs.org/docs/advanced-features/output-file-tracing
COPY --from=prod-builder --chown=nextjs:nodejs /app/apps/web/.next/standalone ./apps
COPY --from=prod-builder --chown=nextjs:nodejs /app/apps/web/.next/static ./apps/web/.next/static
COPY --from=prod-builder --chown=nextjs:nodejs /app/apps/web/public ./apps/web/public
COPY --from=builder --chown=nextjs:nodejs /app/apps/web/.next/standalone ./apps
COPY --from=builder --chown=nextjs:nodejs /app/apps/web/.next/static ./apps/web/.next/static

ARG PORT=3000
ENV PORT $PORT

# Configure host name. Important for setting the domain.
ARG HOSTNAME="0.0.0.0"
ENV HOSTNAME $HOSTNAME

ENV NODE_ENV=production
ENV NEXT_TELEMETRY_DISABLED=1

EXPOSE 3000

CMD HOSTNAME="0.0.0.0" node apps/web/server.js -p $PORT
CMD node apps/web/server.js -p $PORT
21 changes: 21 additions & 0 deletions docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ services:
context: .
dockerfile: docker/Dockerfile.base
restart: 'none'

elastic:
image: elasticsearch:8.14.1
environment:
- 'discovery.type=single-node'
- 'ELASTIC_USERNAME=latitude'
- 'ELASTIC_PASSWORD=secret'

db:
image: postgres
environment:
Expand All @@ -21,7 +23,26 @@ services:
volumes:
- ./docker/init-db.sh:/docker-entrypoint-initdb.d/init-db.sh
- ./docker/pgdata:/var/lib/postgresql/data

redis:
image: redis
ports:
- '6379:6379'

web:
build:
context: .
target: production
dockerfile: apps/web/docker/Dockerfile
# This enables to have this service for testing production build
# It will not run when doing docker compose up
profiles: [building]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docker compose allow to have profiles. So no profile runs when docker compose up which is perfect for this case. I only want for the combinience of developing Dockerfile production and run docker compose build web

volumes:
- ./packages/env:/app/packages/env
- ./packages/core:/app/packages/core
- ./packages/jobs:/app/packages/jobs
- ./packages/web-ui:/app/packages/web-ui
- ./apps/web:/app/apps/web
depends_on:
- base
- db
1 change: 1 addition & 0 deletions packages/core/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
},
"dependencies": {
"@latitude-data/env": "workspace:^",
"@t3-oss/env-core": "^0.10.1",
Copy link
Contributor Author

@andresgutgon andresgutgon Jul 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the perfect excuse for this sorry. Explained in the next comment.

Copy link
Collaborator

@geclos geclos Jul 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plz... if (process.env.BUILDING_CONTAINER == 'true') return let's not fight more over this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why you don't want a tool built for this propose

Copy link
Collaborator

@geclos geclos Jul 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually like it for NextJS for the client-side safety measures that you mention ☝🏼 . I hate that we use it in api because it brings 0 value there, and adding a dependency is just overhead.

Copy link
Contributor Author

@andresgutgon andresgutgon Jul 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this package is type save. When you do env.SOMETHING you're sure that's a number, string, enum,... Doing the old way you still use untyped process.env

"bcrypt": "^5.1.1",
"drizzle-orm": "0.31.4",
"lodash-es": "^4.17.21",
Expand Down
19 changes: 8 additions & 11 deletions packages/core/src/env.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
import { createEnv } from '@t3-oss/env-core'
import z from 'zod'

import '@latitude-data/env'

const envvars = z.object({
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing this breaks production build because it tries to find DATABASE_URL at build time. With t3-env this is easy fixable by using skipValidation with BUILDING_CONTAINER flag as we did in web.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why? I don't get it why is this code evaluated at build time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it's imported by nextjs bundler. And because is imported zod parse function is run

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

next bundler runs the code of its dependencies at compile time? :drunk:

NODE_ENV: z.string(),
DATABASE_URL: z.string(),
export default createEnv({
skipValidation: process.env.BUILDING_CONTAINER == 'true',
server: {
NODE_ENV: z.string(),
DATABASE_URL: z.string().url(),
},
runtimeEnv: process.env,
})

export default envvars.parse(process.env)

declare global {
namespace NodeJS {
interface ProcessEnv extends z.infer<typeof envvars> {}
}
}
1 change: 1 addition & 0 deletions packages/core/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"extends": "@latitude-data/typescript-config/base.json",
"compilerOptions": {
"moduleResolution": "Bundler",
"baseUrl": ".",
"rootDir": "./src",
"outDir": "./dist",
Expand Down
3 changes: 3 additions & 0 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading