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

Build --secret with buildkit #1288

Merged
merged 2 commits into from
Aug 17, 2018
Merged

Conversation

tiborvass
Copy link
Collaborator

@tiborvass tiborvass commented Aug 14, 2018

This patch implements docker build --secret id=mysecret,src=/secret/file
for buildkit frontends that request the mysecret secret.

It is currently implemented in the tonistiigi/dockerfile:secrets20180808
frontend via RUN --mount=type=secret,id=mysecret

Signed-off-by: Tibor Vass [email protected]

Also revendors buildkit and docker/docker

@tiborvass
Copy link
Collaborator Author

@vdemeester @cpuguy83 :) this is pretty dope. This is just wiring stuff together, the heavy lifting was done in buildkit repo.

Copy link
Collaborator

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐯
Small lint failure to fix though 😉

cli/command/image/build_buildkit.go:1::warning: file is not gofmted with -s (gofmt)
cli/command/image/build_buildkit.go:1::warning: file is not goimported (goimports)

@codecov-io
Copy link

codecov-io commented Aug 14, 2018

Codecov Report

Merging #1288 into master will increase coverage by 0.01%.
The diff coverage is 60.46%.

@@            Coverage Diff             @@
##           master    #1288      +/-   ##
==========================================
+ Coverage   54.03%   54.05%   +0.01%     
==========================================
  Files         272      272              
  Lines       18072    18114      +42     
==========================================
+ Hits         9766     9792      +26     
- Misses       7690     7706      +16     
  Partials      616      616

@tiborvass
Copy link
Collaborator Author

@vdemeester fixed

Copy link
Collaborator

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

Weird that it leaves an empty file in the container image where you mount a secret. Is this an issue with the frontend or with buildkit?

return secretsprovider.NewSecretProvider(store), nil
}

func parseSecret(value string) (*secretsprovider.FileSource, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we can add some test for these two new functions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

@thaJeztah
Copy link
Member

Trying to get this to work, but I'm probably doing it wrong (running against Docker 18.06)

printf "hello secret" > ./mysecret.txt

export DOCKER_BUILDKIT=1

docker build --no-cache --console=false --secret id=mysecret,src=$(pwd)/mysecret.txt -f - . <<EOF
# syntax = tonistiigi/dockerfile:runmount20180618
FROM busybox
RUN echo "hello world"
RUN --mount=type=secret,id=mysecret echo "anything here"
RUN --mount=type=secret,id=mysecret,dst=/foobar cat /foobar
EOF

Whatever I try to do with --mount=type=secret.... seems to give me an exit code 2

@cpuguy83
Copy link
Collaborator

It doesn't work on 18.06, missing some daemon stuff.

@AkihiroSuda
Copy link
Collaborator

Is # syntax = still needed?

@tiborvass
Copy link
Collaborator Author

tiborvass commented Aug 17, 2018

@thaJeztah you're using the runmount flavored dockefile frontend, instead of secrets.

Note that --secret is now guarded by API version 1.39.

@cpuguy83

Weird that it leaves an empty file in the container image where you mount a secret. Is this an issue with the frontend or with buildkit?

Agreed it's weird, will debug it but shouldn't be a blocker for this PR. I added a couple of tests.

@AkihiroSuda

Is # syntax = still needed?

Yes, this is not part of the stable compiled-in default frontend.

PTAL :)

This patch implements `docker build --secret id=mysecret,src=/secret/file`
for buildkit frontends that request the mysecret secret.

It is currently implemented in the tonistiigi/dockerfile:secrets20180808
frontend via RUN --mount=type=secret,id=mysecret

Signed-off-by: Tibor Vass <[email protected]>
vendors github.com/docker/docker to a7ff19d69a90dfe152abd146221c8b9b46a0903d

Signed-off-by: Tibor Vass <[email protected]>
@thaJeztah
Copy link
Member

Tried this again, and looks good :)

printf "hello secret" > ./mysecret.txt

export DOCKER_BUILDKIT=1

docker build --no-cache --progress=plain --secret id=mysecret,src=$(pwd)/mysecret.txt -f - . <<EOF
# syntax = tonistiigi/dockerfile:secrets20180808
FROM busybox
RUN --mount=type=secret,id=mysecret cat /run/secrets/mysecret
RUN --mount=type=secret,id=mysecret,dst=/foobar cat /foobar
EOF
#8 [2/3] RUN --mount=type=secret,id=mysecret cat /run/secrets/mysecret
#8       digest: sha256:ab7659590ba7fcf4ec338708fcf824ea76ee7b48b13dce4b9c01faf19395c39d
#8         name: "[2/3] RUN --mount=type=secret,id=mysecret cat /run/secrets/mysecret"
#8      started: 2018-08-17 16:08:28.0910424 +0000 UTC
#8 0.303 hello secret#8    completed: 2018-08-17 16:08:28.6508443 +0000 UTC
#8     duration: 559.8019ms
#8 0.303 hello secret

#9 [3/3] RUN --mount=type=secret,id=mysecret,dst=/foobar cat /foobar
#9       digest: sha256:67234c5c8ff0bbd7d6c342980f66f9e5ac75405d2860bbd15747e44291b5cb1d
#9         name: "[3/3] RUN --mount=type=secret,id=mysecret,dst=/foobar cat /foobar"
#9      started: 2018-08-17 16:08:28.6597585 +0000 UTC
#9 0.343 hello secret#9    completed: 2018-08-17 16:08:29.2602578 +0000 UTC
#9     duration: 600.4993ms
#9 0.343 hello secret

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Member

/cc @albers for bash completion 😅

@thaJeztah thaJeztah merged commit cb142fa into docker:master Aug 17, 2018
@GordonTheTurtle GordonTheTurtle added this to the 18.09.0 milestone Aug 17, 2018
@tiborvass
Copy link
Collaborator Author

Big thanks to y'all for the review and to Tonis for the implementation!

chrishiestand added a commit to chrishiestand/docker-image-resource that referenced this pull request Aug 22, 2018
It is inaccurate to say that build args will not persist in the final image. They are visible with `docker history` and `docker inspect`.

As soon as possible, we should upgrade to docker 18.09 to support `--secret` - a way to securely pass credentials into the build context.

docker/cli#1288
@haizaar
Copy link

haizaar commented Nov 4, 2018

@thaJeztah
Can you please elaborate on export DOCKER_BUILDKIT=1?
Will we have to define this env var when this feature becomes available as part of docker-ce 18.09?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants