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

new: add host bind mounts #5402

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

new: add host bind mounts #5402

wants to merge 1 commit into from

Conversation

vaab
Copy link

@vaab vaab commented Oct 6, 2024

Many thanks for buildkit ! This PR adds the ability to mount local host directory.

Which could help solve #3310 #4508 ...

and I'm aware of https://github.com/moby/buildkit/blob/master/PROJECT.md#client

Especially:

Host

    The BuildKit API with default daemon configuration does not allow changes to the host filesystem or reading the host filesystem outside of the BuildKit state directory.
    Application and frontend containers are not allowed to read or write to the host system, run privileged system calls, or access external devices directly. Monitoring the load of the system is allowed.

Client

    Buildctl does not allow access to any directories or file paths that are not explicitly set by the user with command line arguments. The untrusted BuildKit daemon does not have any way to access files that were not listed.
    When extracting build results to a directory specified with --output or --cache-to, no subfile can escape to the outside directory (e.g. via symlinks)

Let me explain why I feel this is useful : There are cases where we control everything (daemon, production of DAG, launching build) and don't really care about these security features but want to use DAG solver, caching of buildkit. I see buildkit as... a build kit. I can understand the security features in other environments.

My use case here is that I need to provide caching data that is produced by other external tools before buildkit comes into play, and no, I don't want to copy this into the states of buildkitd because it is utterly inefficient (the mount data is huge and always changing = invalidating cache).

Yes, I'm aware that it breaks reproducibility garantees big time, and I value reproducibility. But not this time, because I have my own garantees that what I plan to mount-bind is not breaking reproducibility. And to be honest, reproducibility is broken in most end-user (I thinking of most Dockerfile) usage of buildkit to my knowledge.

Would something approaching this working implementation be of any interests ? I was thinking of additional safeguards (like command line args on the buildkitd like --allow-bind-host to allow the behavior only if specified), and I'll gladly add tests of course if the idea is accepted.

With this implementation, the following works:

    run := st.Run( 
        llb.Shlex("sh -c 'touch /host_data/c;  ls -al /host_data > /tmp/output.txt'"),
        llb.AddMount("/tmp", output),
        llb.AddHostBindMount(
            "/host_data", // path in the container running the current step 
            "/tmp/toto", // path on the host
        ),
    )

A final note: I'm very new to the go language, to buildkit concepts and history, so I might have overlooked simple concepts or solutions, made terrible mistakes in my implementation. So any feedback is more than welcome.

@AkihiroSuda
Copy link
Member

AkihiroSuda commented Oct 9, 2024

command line args on the buildkitd like --allow-bind-host to allow the behavior only if specified

This needs further clarification.
Will --allow-bind-host=/foo allow bind mounting /foo/bar too? (My suggestion is no)
What if /foo/bar contains a symlink?

@vaab
Copy link
Author

vaab commented Oct 14, 2024

It needs more than clarification, it needs specification ;-) . But before moving forward, as it could go against some of the basic rules of this project, I'd like to know if it has any chance to be accepted.

That said, your suggestion with --allow-bind-host=/foo allowing bind mounting /foo and not /foo/bar seem sane to me and your case with /foo/bar being a symlink makes sense.

For now, this current PR doesn't care about security and allows any bind-host, because many possible usage of buildkit (through go api) are not in a context where these security concerns are pertinent (using it daemon-less as a local build resolver for instance, leveraging the cache and DAG optimizations).

Signed-off-by: Valentin Lab <[email protected]>
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.

2 participants