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

update best practices #22169

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

update best practices #22169

wants to merge 4 commits into from

Conversation

adyanth
Copy link

@adyanth adyanth commented Mar 6, 2025

Description

Update the ADD recommendation to be clear on the behavior over RUN curl and counterparts.

Related issues or tickets

Closes #22168
Related to #18582

Reviews

Initially changed as a result of discussion in moby/buildkit#4125

@github-actions github-actions bot added the area/build Relates to Dockerfiles or docker build command label Mar 6, 2025
Copy link

netlify bot commented Mar 6, 2025

Deploy Preview for docsdocker ready!

Name Link
🔨 Latest commit bef2c8e
🔍 Latest deploy log https://app.netlify.com/sites/docsdocker/deploys/67c927522d0f360008c4a4c0
😎 Deploy Preview https://deploy-preview-22169--docsdocker.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

something like `wget` and `tar`, because it ensures a more precise build cache.
`ADD` also has built-in support for checksum validation of the remote
resources, and a protocol for parsing branches, tags, and subdirectories from
[Git URLs](/reference/cli/docker/buildx/build.md#git-repositories).

> [!NOTE]
>
> `ADD` redownloads the file every time the image is built to verify the checksum
Copy link
Member

Choose a reason for hiding this comment

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

This is not correct for servers that support etags. Only HEAD request is needed to know the file has not changed if there is local cache.

Copy link
Author

Choose a reason for hiding this comment

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

Is this better?


If the file being downloaded is supposed to be part of the image and is okay to be redownloaded on each build (to verify changes), using `ADD` as part of the image build is more suitable.

If the file is an archive being extracted, or not supposed to be part of the final image, using `ADD` by itself would add an additional layer and subsequently removing it using `RUN rm` will not decrease the image size. In this case, look at the below example to use a multi stage build with a `scratch` image to download the file using `ADD` and bind mounting it where needed in the final image.
Copy link
Member

Choose a reason for hiding this comment

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

I don't see how "supposed to be part of final image" matters in here. Layers in the target stage end up in the exported image, and RUN rm does not release data from the parent layers. This is true for all instructions and nothing specific about ADD or RUN.

Copy link
Author

@adyanth adyanth Mar 6, 2025

Choose a reason for hiding this comment

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

It was to clarify that always using ADD in your image build process is not the recommendation, since it adds a layer when you might not want one, which is how saying ADD is always better than curl/wget in the best practice reads to me. Is there a better way to phrase that using ADD without a multi stage build to bring it in when the artifact is not needed in the final image is not a best practice?

@adyanth adyanth requested a review from tonistiigi March 6, 2025 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build Relates to Dockerfiles or docker build command
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ADD over curl documentation does not cover all cases
2 participants