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.

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

build: use a base image for Docker builds #6515

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

eatyourgreens
Copy link
Contributor

@eatyourgreens eatyourgreens commented Dec 5, 2024

Split the Docker build into three stages:

  1. base image: Node 20 Alpine with the build environment, package files, Yarn and Lerna config.
  2. builder image: bootstraps the project and root apps from the base image.
  3. runner image: installs only production dependencies and runs the built apps from the builder image. This is the image that's deployed to Kubernetes.

Run the Next.js apps as the node user, to secure the apps in production.

Please request review from @zooniverse/frontend team or an individual member of that team.

Linked Issue and/or Talk Post

How to Review

This shouldn't change the final, deployed Docker image. App containers should still run as usual (though with the apps running as node, not root. See #6514.) The base image should cache some common build layers so that they don't run twice during a build and deploy (eg. setting NODE_ENV or setting up the user and group.) It might be easiest to check this by running a branch deploy and checking that everything works as usual.

In the running production container, everything in /usr/src should be owned/run by node.

jimodonnell@Jims-MBP-2 front-end-monorepo % docker compose run --rm prod-shell
[+] Creating 1/0
 ✔ Network front-end-monorepo_default  Created                                                                                                                   0.0s 
/usr/src $ ls -al
total 900
drwxr-xr-x    1 node     node          4096 Dec  5 11:35 .
drwxr-xr-x    1 root     root          4096 Dec  5 11:29 ..
drwxr-xr-x    3 node     node          4096 Nov 26 13:25 .yarn
-rw-r--r--    1 node     node           176 Nov 26 13:25 .yarnrc
-rw-r--r--    1 node     node           145 Oct 12 07:57 lerna.json
drwxr-xr-x  663 node     node         32768 Dec  5 11:36 node_modules
-rw-r--r--    1 node     node          1602 Nov 26 13:25 package.json
drwxr-xr-x    1 node     node          4096 Oct 30 12:32 packages
-rw-r--r--    1 node     node        840981 Dec  5 08:26 yarn.lock
/usr/src $ whoami
node
/usr/src $ ps -a
PID   USER     TIME  COMMAND
    1 node      0:00 /bin/sh
   10 node      0:00 ps -a

With docker compose, you can use the prod-shell service to check that the production image is set up correctly. eg. check the production deploy environment:

jimodonnell@Jims-MBP-2 front-end-monorepo % docker compose run --rm prod-shell 
[+] Creating 1/0
 ✔ Network front-end-monorepo_default  Created                                                                                                      0.0s 
/usr/src $ printenv
NODE_VERSION=20.18.1
SENTRY_RELEASE=
PANOPTES_ENV=production
HOSTNAME=6840f5fa7a1a
YARN_VERSION=1.22.22
SHLVL=2
HOME=/home/node
COMMIT_ID=
TERM=xterm
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
NEXT_TELEMETRY_DISABLED=1
GITHUB_REF_NAME=
APP_ENV=development
PWD=/usr/src
NODE_ENV=production

Checklist

PR Creator - Please cater the checklist to fit the review needed for your code changes.
PR Reviewer - Use the checklist during your review. Each point should be checkmarked or discussed before PR approval.

General

  • Tests are passing locally and on Github
  • Documentation is up to date and changelog has been updated if appropriate
  • You can yarn panic && yarn bootstrap or docker-compose up --build and FEM works as expected
  • FEM works in all major desktop browsers: Firefox, Chrome, Edge, Safari (Use Browserstack account as needed)
  • FEM works in a mobile browser

General UX

Example Staging Project: i-fancy-cats

  • All pages of a FEM project load: Home Page, Classify Page, and About Pages
  • Can submit a classification
  • Can sign-in and sign-out
  • The component is accessible

Refactoring

  • The PR creator has described the reason for refactoring
  • The refactored component(s) continue to work as expected

@coveralls
Copy link

coveralls commented Dec 5, 2024

Coverage Status

coverage: 77.572% (-0.005%) from 77.577%
when pulling 55e08c7 on eatyourgreens:docker-base-image
into d168739 on zooniverse:master.

@goplayoutside3
Copy link
Contributor

@eatyourgreens could you use this PR opportunity to include code comments in the Dockerfile like # comment? As a team we've ended up in a spot where FEM's Docker config has changed a lot over the years without documented decisions other than sporadic PRs. FEM's Docker config is also very different from every other Zooniverse service. When any dev reads the Dockerfile, they should be able to read a basic understanding of its strategy.

@eatyourgreens
Copy link
Contributor Author

could you use this PR opportunity to include code comments in the Dockerfile

Sure! I've added comments, and questions where I don't understand what the Dockerfile is doing.

@eatyourgreens eatyourgreens force-pushed the docker-base-image branch 2 times, most recently from 1b95940 to 645f823 Compare December 6, 2024 10:56
@eatyourgreens
Copy link
Contributor Author

eatyourgreens commented Dec 6, 2024

Looking back through the Dockerfile history for this PR, I discovered that the monorepo originally built and ran on Node 8!
#595

That explains why it still has packages that were written as CommonJS modules, using require and module.exports. Not an issue here but that is relevant for:

Comment on lines +56 to +62
# Add build secrets.
# NB. GitHub warns that ARG should not be used for sensitive data.
ARG CONTENTFUL_ACCESS_TOKEN

ADD yarn.lock /usr/src/
ARG CONTENTFUL_SPACE_ID

RUN chown -R node:node .
ARG SENTRY_AUTH_TOKEN
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the production image, change the owner and group of all the app files to `node:node`, then run the Next.js apps as `node`.
Split the Docker build into three stages:
1. base image: Node 20 Alpine with the build environment, package files, Yarn and Lerna config.
1. builder image: bootstraps the project and root apps from the base image.
1. runner image: installs only production dependencies and runs the built apps from the `builder` image. This is the image that's deployed to Kubernetes.

Run the Next.js apps as the `node` user, to secure the apps in production.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants