`buildah build`: use the same overlay for the context directory for the whole build

[nalind]

What type of PR is this?

/kind feature

What this PR does / why we need it:

Change how we handle bind mounts of the build context directory from using a different upper for each read-write mount into it, to using one that lasts the duration of the full build.

We broke workflows which wrote content to the build context in one stage and then attempted to use archive or layout locations in the build context in a subsequent stage when we fixed CVE-2024-11218, and this should get them working again. Because that content was often addressed using relative path names which are no longer correct for the written location, we try to compensate by rewriting some types of references and straight-up rejecting some others.

How to verify it

New integration test! And some updated ones, too!

Which issue(s) this PR fixes:

Proposed fix for #5952.

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Changes "written" to the build context directory during `buildah build` `RUN --mount=type=bind` instructions are no longer discarded between instructions, but when the build completes, whether it completes successfully or not.

[cgwalters]

Trying to test this, but unrelated to the contents of this PR, I get the same problem trying git main:

$ ./bin/buildah from busybox
busybox-working-container
$ ./bin/buildah run busybox-working-container echo true
error running container: did not get container start message from parent: EOF
Error: setup network: pasta failed with exit code 1:
Couldn't open network namespace /proc/126984/ns/net: Permission denied

DEBU[0000] Running ["/usr/bin/crun" "create" "--bundle" "/var/tmp/buildah1984752272" "--pid-file" "/var/tmp/buildah1984752272/pid" "--no-new-keyring" "buildah-buildah1984752272"] 
DEBU[0000] waiting for parent start message             
DEBU[0000] pasta arguments: --config-net --dns-forward 169.254.1.1 -t none -u none -T none -U none --no-map-gw --quiet --netns /proc/126381/ns/net --map-guest-addr 169.254.1.2 
DEBU[0000] "/var/tmp/buildah1984752272/mnt/buildah-bind-target-10" is apparently not really mounted, skipping 
DEBU[0000] "/var/tmp/buildah1984752272/mnt/rootfs" is apparently not really mounted, skipping 
DEBU[0000] "/var/tmp/buildah1984752272/mnt" is apparently not really mounted, skipping 
error running container: did not get container start message from parent: EOF
DEBU[0000] Error building at step {Env:[PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin] Command:run Args:[touch /blah] Flags:[] Attrs:map[] Message:RUN touch /blah Heredocs:[] Original:RUN touch /blah}: setup network: pasta failed with exit code 1:
Couldn't open network namespace /proc/126381/ns/net: Permission denied 
Error: building at STEP "RUN touch /blah": setup network: pasta failed with exit code 1:
Couldn't open network namespace /proc/126381/ns/net: Permission denied

Is there something I need to do here to sync up the buildah binary with the default network config? Odd...

[cgwalters]

Nevermind of course after I posted my "wait these are weird denials" mental flag tripped and of course it's SELinux; a chcon --reference /usr/bin/buildah bin/buildah fixes it.

[cgwalters]

OK so this works for me:

FROM registry.access.redhat.com/ubi9/ubi:latest  as builder
RUN --mount=type=bind,src=.,rw,dst=/buildcontext \
  dnf -y install skopeo && skopeo copy docker://busybox oci:/buildcontext/out.oci

FROM oci:./out.oci
# Need to reference builder here to force ordering.
RUN --mount=type=bind,from=builder,src=.,target=/var/tmp true

So that's already a lot cleaner, thanks!

---

There is an important note here that this does break compatibility with the previous dockerfiles, I get:

Error: building at STEP "RUN --mount=type=bind,rw=true,src=.,dst=/buildcontext,bind-propagation=shared dnf -y install skopeo && skopeo copy docker://busybox oci:/buildcontext/out.oci": resolving mountpoints for container "7f7333013ba83fbe05c78aad0ef4edd17663317f4daa775e549ac11d28412a90": rw: must not provide an argument for option

Looks like changing rw=true to just rw fixes things. Not sure if that's intentional or not? It does look like there's no mention of rw=true in the upstream Dockerfile syntax.

---

Now that I dig in a bit more, one unfortunate thing here is that the need to use the RUN to force ordering...has meant this whole time we're subject to all the "injected content" bugs like https://github.com/containers/buildah/issues/4242 and https://github.com/containers/buildah/issues/5950 etc.

When I unpack and inspect the resulting image, we have this final layer with

drwxr-xr-x 0/0               0 2025-02-06 16:44 etc/
-rwx------ 0/0               0 2025-02-06 16:44 etc/hostname
-rwx------ 0/0               0 2025-02-06 16:44 etc/hosts
-rwx------ 0/0               0 2025-02-06 16:44 etc/resolv.conf
drwxr-xr-x 0/0               0 2025-02-06 16:44 proc/
drwxr-xr-x 0/0               0 2025-02-06 16:44 run/
drwxr-xr-x 0/0               0 2025-02-06 16:44 sys/
drwxr-xr-x 0/0               0 2025-02-06 16:44 var/
drwxr-xr-t 0/0               0 2025-02-06 16:44 var/tmp/

with floating timestamps. That said hmm, actually in this case actually it works to do buildah build --timestamp=<pinned> and that won't also change all the timestamps on the inherited layers. So that's a good mitigation.

But the sad thing is that these things just keep piling on...now for me we have a new one of these in Konflux which is injecting content_sets which also has no attempt made to generate timestamp-reproducible data right now either (and it uses buildah too, so it inherits that, but we still need to ensure the timestamps of the files that we actually write and aren't synthetic container runtime artifacts are canonicalized too)

So in the end, I think we'll definitely try to

[nalind]

There is an important note here that this does break compatibility with the previous dockerfiles, I get:

Error: building at STEP "RUN --mount=type=bind,rw=true,src=.,dst=/buildcontext,bind-propagation=shared dnf -y install skopeo && skopeo copy docker://busybox oci:/buildcontext/out.oci": resolving mountpoints for container "7f7333013ba83fbe05c78aad0ef4edd17663317f4daa775e549ac11d28412a90": rw: must not provide an argument for option

Looks like changing rw=true to just rw fixes things. Not sure if that's intentional or not? It does look like there's no mention of rw=true in the upstream Dockerfile syntax.

Not being strict about the arguments for --mount contributed to CVE-2024-9407. I added more checks in that area in #5925, and they would have landed in v1.39.0.

[nalind]

@containers/buildah-maintainers PTAL

[cevich]

Would this address #5988 also or is that something different?

[TomSweeneyRedHat]

LGTM @nalind Looks like the branch needs an update.

[nalind]

This is going to exacerbate #5988 for people who are running into it. /hold

[dustymabe]

hey @nalind is there any way to unblock this? this is still an important use case for us on the CoreOS side.

[nalind]

I'm working through adding something like https://github.com/containers/buildah/issues/5988#issuecomment-2718307144, but other things have been clamoring for attention in the meanwhile.

[packit-as-a-service[bot]]

Ephemeral COPR build failed. @containers/packit-build please check.

[nalind]

Would like to get #6126 merged along with this. /unhold

[dustymabe]

Is this just blocked on code review at this point? Or something else?

[nalind]

@dustymabe I'd say that's accurate.

[openshift-ci[bot]]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, nalind

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~~ [flouthoc,nalind]

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

[TomSweeneyRedHat]

@nalind this has conflicts now, and I'm not sure you have addressed the lst bits of comments from @mtrmac

[nalind]

@mtrmac I don't think there's an ordering dependency between the two, but admittedly, it's been a while.

[mtrmac]

/lgtm

then, this one has had more approving reviews.

[openshift-ci[bot]]

@mtrmac: changing LGTM is restricted to collaborators

In response to this:

/lgtm

then, this one has had more approving reviews.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[TomSweeneyRedHat]

LGTM

[TomSweeneyRedHat]

/lgtm

[jlebon]

Awesome to see this merge! Looking forward to be able to drop the -v $PWD:/buildcontext hack everywhere. Hmm, I think this also allows dropping --security-opt=label=disable in cases where it was added just for that.

Anyway, it looks like this was a lot of non-trivial work, but it's very much appreciated! (And hopefully in the future we can lean more into this and have nicer semantics for the pattern described in https://github.com/containers/buildah/issues/5952.)