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

[fix] Enable (most) wrapping when seeing USER directive #34

Merged
merged 10 commits into from
Oct 6, 2023
Merged

Conversation

viega
Copy link
Contributor

@viega viega commented Oct 4, 2023

Issue

Closes #33

Description

Implements changes discussed in #33 that make wrapping in the face of a USER directory work in most cases. In particular:

  1. Whenever Buildkit is used and the docker version is 20 or higher, use the --chmod flag to COPY.

  2. If # 1 doesn't apply, and the last section has a USER directive, then we switch the USER to root before the copy/chmod, then we switch it back at the end.

If neither apply and there is a user inherited from a base image, then wrapping will generally still fail.

Testing

This is still in draft b/c there's no test yet. Ideally, we would do all three of:

  1. Create a basic container with a USER in the last section, see if it works. Should build via chalk.
  2. Use the same Dockerfile, but try it with BUILDKIT disabled. Should build via chalk.
  3. Create a two-section dockerfile, with the USER directive in the first section. Should kick to the failsafe path.

viega added 2 commits October 3, 2023 21:00
… --chmod is not available to us, and the last section has a USER section, then we wrap a manual chmod with USER root / USER <whatever they provided>
@viega viega requested a review from miki725 October 4, 2023 02:10
@viega viega marked this pull request as ready for review October 6, 2023 01:17
Copy link
Contributor

@miki725 miki725 left a comment

Choose a reason for hiding this comment

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

👍 this has edge cases however this is prolly good enough for most people as buildkit is pretty popular and we documented current limitation in release notes

@viega viega merged commit a5a5d48 into main Oct 6, 2023
2 checks passed
@viega viega deleted the jtv/dockeruser branch October 6, 2023 01:40
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.

Wrapping is currently incompatible with the USER directive
3 participants