OnCallReady

Lesson 10.50 · Images & Builds · 12 min read

Reviewing a Dockerfile in five minutes

In plain words

Imagine checking a friend's recipe before they cook for a hundred people. You do not taste every ingredient. You run down a short list: is the flour fresh and labelled, are the steps in an order that does not waste time, is anything cooked twice, who is in charge of the kitchen, and is the secret family sauce written on the card they are about to photocopy for everyone?

Reviewing a Dockerfile is the same short list: the base (FROM, pinned, slim), the order of steps and the cache, the layers, what runs and as whom (ENTRYPOINT, USER), and secrets and labels. Each point comes from an earlier lesson in this chapter, and each has a consequence you can name in the review comment.

The problem

A teammate opens a pull request (PR: a proposed change that others review before it is merged) that adds or changes a Dockerfile. It builds, so it looks fine. But every problem in this chapter - slow builds, huge images, lost SIGTERMs, leaked keys - builds fine too. You need a fast, repeatable way to spot them by reading.

Everything in this chapter compresses into a five-part checklist. Run it top to bottom on any Dockerfile in a PR.

What you need to know already: the whole chapter so far - tags and digests (10.5), the cache and its order (10.13), layers and docker history (10.10), the context and .dockerignore (10.21), multi-stage (10.24), ENTRYPOINT and PID 1 (10.28), base images (10.38), non-root and secrets (10.40), HEALTHCHECK (10.44).

1. The base

FROM node:latest                      # moving tag, 1.1GB, full toolchain
FROM node:22-slim@sha256:9b1c...      # versioned, slim, pinned

2. Order and the cache

COPY . .                              # before the install: every edit re-downloads
RUN npm ci

3. Layers

RUN apt-get update
RUN apt-get install -y curl
RUN rm -rf /var/lib/apt/lists/*       # frees nothing
RUN chown -R app /app                 # duplicates /app

4. What runs, and as whom

ENTRYPOINT java -jar app.jar          # sh is PID 1, SIGTERM lost
USER root                             # or no USER at all

5. Secrets and metadata

ARG NPM_TOKEN
ENV DB_PASSWORD=...
COPY .env .

Writing review comments

A useful review comment names the consequence, not just the rule:

- "COPY . . before npm ci: every source change re-downloads ~400MB of deps;
   copy package*.json first."
- "Shell-form ENTRYPOINT: sh becomes PID 1, SIGTERM never reaches java, every
   deploy waits the grace period and SIGKILLs in-flight requests."
- "ENV DB_PASSWORD: visible to anyone who can pull the image
   (docker inspect). Inject at runtime; rotate this one."

Then ask for evidence: docker history before and after, docker images for the size, and a time docker stop for anything touching the entrypoint.

Two linters (programs that read a file and warn about known mistakes) help: BuildKit's own docker build --check . catches a few of these, and hadolint (a Dockerfile linter, also available as an image) catches more:

docker run --rm -i hadolint/hadolint < Dockerfile

(-i keeps standard input open, so the Dockerfile you redirect in reaches the linter.) Worth running both in CI.

What you can now do

Why it helps

Platform engineers review Dockerfiles constantly: every new service, every "why is our CI slow" ticket, every security finding. This checklist is how you do it in five minutes and leave comments that change things. "COPY . . before npm ci" becomes "every source change re-downloads 400MB of deps". "Shell-form ENTRYPOINT" becomes "SIGTERM never reaches java, every deploy waits 30s and kills in-flight requests". Naming the consequence is what gets a comment accepted by a team that does not care about rules. It also prepares you for the interview task "here is a Dockerfile, what would you change?", which appears in many platform and DevOps loops, and for adding hadolint and docker build --check to a pipeline so the obvious findings are caught before a human looks.

FAQ

What are the most important things to check first?

In order of damage: secrets in the image (ARG/ENV values, copied key files), because they need rotation, not just a fix. Then the entrypoint and user: shell form means lost SIGTERM, root means a bigger blast radius. Then the base: latest or an unpinned full image makes builds unreproducible and large. Then cache order and layers, which cost build time and size but not correctness.

Is hadolint enough, or do I still need to review by hand?

Use both. hadolint and docker build --check catch mechanical issues well: missing --no-install-recommends, unpinned packages, apt-get update in its own RUN, shell-form CMD, secrets in ARG or ENV. They cannot know that EXPOSE 8080 does not match the port the app listens on, that a cache-busting ARG is above an expensive step, or that the app ignores SIGTERM. Let the linter clear the noise so the review is about behaviour.

Why is a cleanup-only RUN line a problem?

Every RUN creates a layer, and layers only add. RUN rm -rf /var/lib/apt/lists/* in its own step writes whiteouts that hide the files, but the bytes are still in the earlier layer, pulled by every server. The image size does not shrink at all. The cleanup has to happen in the same RUN that created the files, or the files should live in a cache mount or an earlier build stage.

Why ask for evidence in a Dockerfile review?

Because claims like "this makes it smaller" or "this fixes shutdown" are easy to test and often wrong. docker images before and after shows the size, docker history shows which layer holds the bytes, a timed docker stop shows whether SIGTERM is handled (a 10s stop means SIGKILL), and docker run --rm image id shows the user. Evidence also teaches the author how to check it next time.

What labels should an image carry, and why?

The OCI annotations: org.opencontainers.image.source (the repo URL), .revision (the commit SHA) and .version. With them, anyone looking at a running container can find the exact commit it came from with docker inspect, which matters in incidents and in security scans that report a CVE against an image. Set the revision from CI with a build arg in the final stage, near the end so it does not bust the cache.

In an interview Junior

Here is a Dockerfile with FROM node:latest, COPY . ., RUN npm install and CMD npm start. What would you change?

Go through it in five passes - base, order, layers, runtime, secrets - and say the cost of each problem:

Then ask for evidence: docker history, docker images, time docker stop.

Also asked: How would you enforce Dockerfile quality across many teams? · What does hadolint check? · What makes a good code review comment on a Dockerfile?

Practise this lesson in the terminal Free, in your browser - a real Ubuntu terminal to try it in, with missions that check your work.