knative / build-templates

A library of build templates.
Apache License 2.0
183 stars 67 forks source link

add BuildKit build template #70

Closed AkihiroSuda closed 6 years ago

AkihiroSuda commented 6 years ago

Signed-off-by: Akihiro Suda suda.akihiro@lab.ntt.co.jp

cc @tonistiigi @vdemeester

AkihiroSuda commented 6 years ago

/assign @mattmoor

imjasonh commented 6 years ago

Thanks for contributing this, I'm looking forward to having more Dockerfile-based options for build templates.

There's a couple things about this template that are unusual compared to other build templates, but that's not necessarily a bad thing, it's just different and worth spelling out in the README at least.

  1. The build template assumes/requires that another service is also running on the cluster. That's not a deal-breaker, it's just not a model we've considered so far for build templates. All the ones we have today simply run some containers on the source and produce images directly from that source.

  2. Though the builder service is rootless, it runs as privileged: true -- I assume this is a hard requirement, correct? Can you elaborate on why that container needs privilege, and maybe link to documentation about how it works behind the scenes, to address any concerns about this requirement?

I'm a bit concerned that we'd be requiring users to run an image on their cluster with privileged: true, that you (or an attacker) could push updates to at any time to take control over the cluster. Is there any way to mitigate that concern, possibly by recommending users build their own buildkit-rootless image from source, as we do with the buildah template?

Thanks again for this contribution, and I think we'll be able to find a way to accept it into the repo, I'd just like to know a bit more about how it all works and how best to ensure it's secure for users to install on their cluster.

imjasonh commented 6 years ago

/ok-to-test

AkihiroSuda commented 6 years ago
  1. The build template assumes/requires that another service is also running on the cluster. That's not a deal-breaker, it's just not a model we've considered so far for build templates. All the ones we have today simply run some containers on the source and produce images directly from that source.

Having daemons would be useful for caching with (planned) cache-aware LB service.

We can also add @jessfraz 's img, which is daemonless version of BuildKit

  1. Though the builder service is rootless, it runs as privileged: true -- I assume this is a hard requirement, correct? Can you elaborate on why that container needs privilege, and maybe link to documentation about how it works behind the scenes, to address any concerns about this requirement?

Because Kubernetes prior to v1.12 does not allow runc in the Buildkit container to mount /proc: https://blog.jessfraz.com/post/building-container-images-securely-on-kubernetes/

On Kubernetes v1.12 and later (with latest Docker/containerd/cri-o), securityContext.procMount could be used instead of securityContext.privileged. https://github.com/kubernetes/kubernetes/commit/39004e852bb523d0497343705ee2bf42b4e9c3e3

I'm a bit concerned that we'd be requiring users to run an image on their cluster with privileged: true, that you (or an attacker) could push updates to at any time to take control over the cluster. Is there any way to mitigate that concern, possibly by recommending users build their own buildkit-rootless image from source, as we do with the buildah template?

Added image digest (akihirosuda/buildkit-rootless:20181002@sha256:e3cecbae5c6a979ce11048fca95c33b223a057e8a56a6ab1002f8cc5ceef524f) so that the image won't be updated. I also clarified how I built the image in the readme.

AkihiroSuda commented 6 years ago

Updated the image to moby/buildkit:v0.3.1-rootless@sha256:2407cc7f24e154a7b699979c7ced886805cac67920169dcebcca9166493ee2b6.

The image is now automatically built on Travis: https://github.com/moby/buildkit/blob/867bcd343f06228862a33643ae16e55c6a1e5fdb/.travis.yml#L26-L31

imjasonh commented 6 years ago

/lgtm /approve

Thanks for contributing this! 👍

knative-prow-robot commented 6 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AkihiroSuda, ImJasonH, vdemeester

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/knative/build-templates/blob/master/OWNERS)~~ [ImJasonH] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment