-
Notifications
You must be signed in to change notification settings - Fork 24
feat: Add multistage conversion for single-stage Dockerfiles #94
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Rajiv Singh <rajivperfect007@gmail.com>
|
hi @jdolitsky, could you please review this? I have tested it locally. I created a simplified Dockerfile like this: FROM ubuntu:20.04
RUN apt-get update && apt-get install -y \
python3 \
python3-pip \
curl
COPY app.py /app/app.py
COPY requirements.txt /app/requirements.txt
WORKDIR /app
RUN pip3 install -r requirements.txt
CMD ["python3", "app.py"]
Then I ran the command and It converted the Dockerfile into a multistage build, like this: ➜ dfc git:(mult) ✗ go run main.go --multistage -i test_single_stage.dockerfile
time=2025-06-06T03:47:34.609+05:30 level=INFO msg="Saving dockerfile backup" path=test_single_stage.dockerfile.bak
time=2025-06-06T03:47:34.609+05:30 level=INFO msg="Overwriting dockerfile" path=test_single_stage.dockerfileThe resulting Dockerfile looks like: FROM cgr.dev/ORG/chainguard-base:latest AS builder
USER root
RUN apk add --no-cache curl py3-pip python-3
# Runtime stage with minimal image
FROM cgr.dev/ORG/chainguard-base:latest
COPY --from=builder app.py /app/app.py
COPY requirements.txt /app/requirements.txt
WORKDIR /app
RUN pip3 install -r requirements.txt
CMD ["python3", "app.py"] |
|
@iamrajiv woah wild! Thanks for submitting. Would you be able to throw the full example files into the testdata/ directory and somehow test them via unit tests (may break on Convert if multi is not enabled). Let me know if need any help with that. Excited to see this |
|
Thanks @jdolitsky yeah sure let me push the tests. You want unit tests only right? Because I can see the existing ones are written in a table driven format so I should follow the same approach? |
|
Hi @jdolitsky, I have added the test. Could you please review it once? |
jdolitsky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to spend a little bit more time looking at this, but its not clear this is working as expected just looking at the testdata files (Dockerfiles before less functional).
The way you've split the tests looks good to me
Signed-off-by: Rajiv Singh <rajivperfect007@gmail.com>
Signed-off-by: Rajiv Singh <rajivperfect007@gmail.com>
|
Hi @jdolitsky, I made the required changes in the testdata Dockerfile to include the npm part. I also refactored the code a bit to introduce a generic MultistageOptions struct. It now allows us to control the build and runtime stage aliases, decide whether to preserve the original aliases, and configure how to handle them and all those stuffs. |
|
@iamrajiv sorry for delay here. I'm hesitant to go forward with this PR at the moment. The testdata before/after files do not (IMO) show a real-world scenario where somebody would want to convert to use multistage. The original idea for this issue is to enable people to get closer to a "distroless" build. The final stage shouldnt really be running commands like "pip install" or "npm install" and instead just copying files from the previous stage. An interesting case would be seeing gcc-glibc -> static or go -> static. There are some examples here: https://edu.chainguard.dev/chainguard/chainguard-images/about/getting-started-distroless/ I'm not necessarily saying the code here is not functional, but based on the test data doesn't appear to provide end-user with a much better Dockerfile than before. Let me know if that makes sense or need further clarification. Thanks again for looking into this issue. |
|
it almost makes sense to start with a PR that adds the before/after testdata for how we wish to see this work, then add the code implementation to get there |
Signed-off-by: Rajiv Singh <rajivperfect007@gmail.com>
Signed-off-by: Rajiv Singh <rajivperfect007@gmail.com>
Signed-off-by: Rajiv Singh <rajivperfect007@gmail.com>
|
yeah sure @jdolitsky i have added test data for distroless can u review once if all good |
Fixes: #39