containers / bootc

Boot and upgrade via container images
https://containers.github.io/bootc/
Apache License 2.0
761 stars 82 forks source link

Add `bootc build commit` #216

Open cgwalters opened 11 months ago

cgwalters commented 11 months ago

This should take over the role of ostree container commit, however following up on https://github.com/ostreedev/ostree-rs-ext/pull/569 we can change our thoughts on /var.

vrothberg commented 11 months ago

Can you sketch out where bootc build commit would be called? Dockerfile/Buildah?

cgwalters commented 11 months ago

Yes as part of a RUN invocation

jmarrero commented 11 months ago

@cgwalters any reason for this not to be a pass-thru to ostree-ext for now?

cgwalters commented 11 months ago

One reason is we kind of just did a pivot around our handling of /var since https://github.com/ostreedev/ostree-rs-ext/pull/569 - and while this needs more testing and validation - we may want to degrade the /var bits to a warning for example.

A tricky thing about making it a literal re-exec style passthrough is right now ostree container is shipped by rpm-ostree, and ultimately we want to support bootc-only images, so we probably want to expose the container commit processing as a Rust API. Or possibly we split it out into a separate component/package.

cgwalters commented 11 months ago

This issue also relates to https://github.com/containers/bootc/pull/215 in that ultimately what we'd do here wouldn't involve ostree per se. But we'd need to then figure out something around the thorny issue of how we represent these xattrs (xref discussion in https://github.com/containers/storage/pull/1608 )

jmarrero commented 8 months ago

Just a heads up that we are working this issue on our mobbing sessions. Specially based on this comment: https://github.com/containers/bootc/pull/217#issuecomment-1864960639

I am not sure if this is intended to intersect with: #365

cgwalters commented 8 months ago

I am not sure if this is intended to intersect with: https://github.com/containers/bootc/issues/365

They're very different things, which argues for using different verbs.

cgwalters commented 8 months ago

I'm a bit uncertain about doing this versus offering a container image that can work as part of a multi-stage build and do this stuff. That's inherently maximally flexible and avoids shipping potentially all sorts of tools in smaller images.

i.e. we'd document:

FROM quay.io/someuser/useros:latest as rootfs
FROM quay.io/exampleos/bootc-image-tool
RUN --mount=type=bind,from=rootfs,target=/sysroot bootc-image-scan /sysroot

Now don't get me wrong, it does seem quite convenient to ship basic scanning stuff as part of the image itself so you can just stick a RUN bootc build-scan or whatever as part of your single image flow...but on the flip side, this isn't something that exists in the podman/docker ecosystem by default.

If we take the "bootc does one thing and does it well" philosophy then this may end up being outside it, right?

I could go either way...

BTW one thing that we should have a basic lint checker for is having a single kernel (like rpm-ostree does).

jmarrero commented 8 months ago

If we take the "bootc does one thing and does it well" philosophy then this may end up being outside it, right?

I can always be persuaded by the original Unix philosophy. I say we close this then.

cgwalters commented 8 months ago

I keep going back and forth honestly :smile:

One thing here is that while we have a stance that bootc is not a build system (you can use any container build system) there are in reality some rules it enforces specifically (such as the "one kernel" rule).

Even if we had an external tool, it could call out to bootc build-lint or whatever we call it for that set of rules.

But we should probably think about what other things we need to lint for. There are potentially a lot...but I don't know how much is bootc specific in the end, versus "generic Linux" stuff.

cgwalters commented 6 months ago

OK another one we hit recently is where a user has a restrictive umask 077, and is doing a container build from a git clone that has a Dockerfile that is doing

COPY usr /usr

Since the copy will preserve permissions (without --chmod) then we end up overwriting /usr with permissions 0700 which will just break running any code as non-root.

I think we should probably lint against this by default. There's not a really good use case for having /usr be permissions 0700.

As for a larger fix for this case...well, it's probably not going to be viable in reality to force everyone doing git clones to reset the umask to 022.

A COPY --chmod=0755 will work, but will also make all files unnecessarily executable. The simplest thing may be to show a pattern of splitting up data and executables via e.g.:

COPY --chmod=0755 usr-exe /usr
COPY --chmod=0644 usr-data /usr

Where usr-exe holds executables, and usr-data holds non-executables.

There is also https://docs.docker.com/reference/dockerfile/#copy---link - but we still need to canonicalize the permissions in that case.

Hmm...COPY should probably have something like COPY --mode-inherit that has a semantic like:

This would give us sane umask-independent semantics that hit the 95% case. There are of course use cases for things like setuid binaries being only executable by a specific user/group, but if you're doing that type of stuff you really should be explicitly setting the mode at install time for that specific binary.

vrothberg commented 5 months ago

@cgwalters, I feel this issue isn't actionable yet. Shall we groom it or is it a WIP?

travier commented 5 months ago

Looks like https://github.com/containers/bootc/pull/381 helps but does not fully fixes this.