This repository has been archived by the owner on Jan 8, 2024. It is now read-only.
plugin/docker: Adding a session when building with buildkit #1937
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
izaaklauer
force-pushed
the
bug/buildkitSession
branch
from
6acb024 to
cc483a6
Compare
izaaklauer
force-pushed
the
bug/buildkitSession
branch
from
cc483a6 to
b37cc37
Compare
krantzinator
approved these changes
Aug 2, 2021
mitchellh
approved these changes
Aug 2, 2021
briancain
suggested changes
Aug 2, 2021
briancain
approved these changes
Aug 3, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1056
Why this change?
We observed that we could replicate #1056 on most of the example repos by enabling buildkit. The failure seems to be governed by the FROM image, but isn't consistent - on my machine,
FROM nginx:stablewould be fine, but wouldn't work for @xiaolin-ninja. In any case, the error mentions not having a session, and this change fixes it.Future considerations
The docker CLI's usage of moby sessions is much more sophisticated than this. If you're curious, here's where to start reading. Things they are doing that we are not include:
Generating a session name based on the build context dir, not statically: https://github.com/docker/cli/blob/13e4a097ea4cd6df652c5f57f68771fe526fcb1d/cli/command/image/build_session.go#L34
Generating a sharedKey that includes randomness but is saved to disk and re-used: https://github.com/docker/cli/blob/13e4a097ea4cd6df652c5f57f68771fe526fcb1d/cli/command/image/build_session.go#L33
Adding session "Allow" methods, which it looks like turn into GRPC services, which seems to enable non-standard context paths, dockerfile paths, and docker daemon auth methods: (e.x. here and here)
I haven't found any documentation on what any of this session configuration is for, and would rather not dig deep into the moby code to figure it out at this moment. I'm comforted that img also uses a static session name and blank sharedKey, and because we only ever connect to a local docker daemon, I don't believe this hurts our security model.
I wouldn't be shocked if some advanced combination of buildkit parameters requires the fancy session configuration. At worst though, this change brings building with buildkit from mostly broken to mostly working, so I think it's worth merging.
Dependency for #1881