-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
dockerfile (labs): implement ADD --checksum=<checksum> <http src> <dest>
#3093
Conversation
1d3fd70
to
3ee3cb2
Compare
Oh, this is great! Thanks for working on this! I was just discussing this feature with @crazy-max last week. Following the related ticket, I was thinking about this, and I can see potential use-cases that would also want to use this for local files (so also for A local Dockerfile used to build from a build-context elsewhere; in that case the Dockerfile wants to check that the file in the "elsewhere" context has the expected checksum; docker build -f ./Dockerfile /path/to/build-context/
docker build -f ./Dockerfile https://github.com/foo/bar.git
docker build -f- https://github.com/foo/bar.git <EOF
FROM foo
ADD --checksum=$CHECKSUM file.tar.gz .
EOF Second thought (but perhaps feature creep / YAGNI); do we expect other verification methods to be added? If so, do we want this to be more generic ( Thinking of; FROM scratch
# verify checksum
ADD --verify=checksum=sha256:<checksum> some-file .
# verify GPG signed
ADD --verify=gpg=<GPG key> some-file . |
Yes, but checksum verification is currently not implemented in the FileOp.
I think that will be in the source policy engine (OPA/Rego?) #2943 (comment) |
I'm not too familiar with all the internals, so my comment is purely "user perspective";
|
Yes, but probably in a separate PR.
Returns |
Thanks! Expanding in future is fine (as long as we indeed have a good error message before that). I'll let others give their input as well on |
3ee3cb2
to
ee850b9
Compare
This could be handy but I wonder if we could not put instead the chesksum as suffix after FROM scratch
COPY --from=moby/buildkit:latest@sha256:67c9251f9f2e103e1ee489b6cead518b6d82607ef485d3f1505fc4095a55ebeb /usr/bin/buildkitd /usr/bin/ So it would not require an extra flag?: # syntax=docker/dockerfile-upstream:master
# ↑ Will be included in `syntax=docker/dockerfile:1.5`
FROM scratch
ADD https://mirrors.edge.kernel.org/pub/linux/kernel/Historic/linux-0.01.tar.gz@sha256:24454f830cdb571e2c4ad15481119c43b3cafd48dd869a9b2945d1036d1dc68d / Looking at the RFC
|
What about the cases like:
Is there some pattern, how checksums could be validated for this case? |
This can be misinterpreted like
|
This should work? FROM scratch AS foo-linux-amd64
ADD --checksum=sha256:deadbeef https://..../releases/release-linux-amd64.tar.gz /
FROM scratch AS foo-freebsd-riscv64
ADD --checksum=sha256:cafebabe https://..../releases/release-freebsd-riscv64.tar.gz /
ARG TARGETOS
ARG TARGETARCH
FROM foo-$TARGETOS-$TARGETARCH AS foo |
ee850b9
to
26b6ef9
Compare
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.
LGTM but maybe we should keep it in labs
for a release in case there is a need for something like --verify
as well.
Do you think it should be |
If we adopt this syntax, can we skip the |
I would prefer not. I don't actually see how the |
If we don't expect other options to be added, then @AkihiroSuda any reason you put this one back in draft? |
For moving this to the labs channel. |
26b6ef9
to
8a87e58
Compare
Moved to the labs. |
@AkihiroSuda check the linter |
e.g., ADD --checksum=sha256:24454f830cdb571e2c4ad15481119c43b3cafd48dd869a9b2945d1036d1dc68d https://mirrors.edge.kernel.org/pub/linux/kernel/Historic/linux-0.01.tar.gz / Fix issue 975 Signed-off-by: Akihiro Suda <[email protected]>
Fixed |
87186e4
to
a83fbb9
Compare
ADD --checksum=<checksum> <http src> <dest>
ADD --checksum=<checksum> <http src> <dest>
I tried to get this to work with I was wanting to rewrite this Dockerfile with this new syntax. I was able to with |
This isn't as easy as this validation isn't just checking the downloaded bytes but deeply integrated with the caching system. Eg. if you have a build that does: |
Got it. Thanks. Makes sense. I appreciate the extra insight. |
e.g.,
Fix #975