Update buildah, but ensure flatpak and bootc builds continue working

[chmeliik]

After merging #121, users that build Flatpaks reported (internally) that their builds stopped working. We reverted the update in #126 to undo the regression until we could investigate more. In #126 (comment), @cgwalters pointed out that bootc builds likewise depend on the older version of buildah and linked the relevant issues.

After going through containers/buildah#5952 and containers/buildah#5988, this is my current understanding of the situation:

• Flatpak and bootc builds both depend on the ability to bind-mount the context directory and write to it using something like

  RUN --mount=type=bind,rw,src=.,dst=/buildcontext ...
  

• This stopped working after containers/buildah@2c70035 and was apparently never intended to work
• It may start working again after this is merged? buildah build: use the same overlay for the context directory for the whole build containers/buildah#5975

Even without waiting for the PR on the buildah side, maybe we could make flatpak and bootc builds work like this:

• Give the Konflux buildah task a MOUNT_BUILD_CONTEXT parameter. When enabled, add --volume $context_dir:/buildcontext to the buildah build invocation. The mount destination path can be configurable. The source can be configurable too, as long as it's within the context dir.
• Flatpak and bootc Containerfiles can drop the --mount=... from RUN instructions and instead write to the pre-mounted context directory.

[chmeliik]

@cgwalters @owtaylor what do you think, would the suggested workaround/solution be acceptable?

[chmeliik]

Thinking about the Task param a bit more, it should probably look like this:

- name: BUILD_CONTEXT_MOUNTS
  value:
    - ".:/buildcontext"
    - "export:/export"

i.e. it would be an array of arguments for --volume, except the left-side paths would be relative and the buildah task would make them absolute

[cgwalters]

Thanks, this is a very obvious-in-retrospect solution indeed. I have verified that my trivial reproducer works with modifications to do this:

> cat Dockerfile.oci
FROM registry.access.redhat.com/ubi9/ubi:latest  as builder
RUN dnf -y install skopeo && skopeo copy docker://busybox oci:/buildcontext/out.oci

FROM oci:./out.oci
# Need to reference builder here to force ordering. But since we have to run
# something anyway, we might as well cleanup after ourselves.
RUN --mount=type=bind,from=builder,src=.,target=/var/tmp rm /buildcontext/out.oci -rf
> podman build -v $(pwd):/buildcontext -t localhost/testoci -f Dockerfile.oci
...
Successfully tagged localhost/toci:latest
f883e191440c95011c92c20358eaae0bd618e6206c4b757d5658cc216bcdb57e

However...there is one downside of this, which is that it doesn't work with podman-remote aka podman-machine, whereas the other approach does. But...that's not fatal for Konflux, just logistically annoying for those of us using that (e.g. on MacOS, or even on Linux as a way to handle privileges for disk image building, ref containers/podman-bootc#9 etc).

[cgwalters]

The source can be configurable too, as long as it's within the context dir.

Actually, we don't need the context directory at all - we just need an empty, writable temporary directory on the host, scoped to the lifetime of the build. There's basically zero security concerns with that. It's the equivalent of Kubernetes emptydir.

So this could be like

- name: EMPTYDIR_MOUNTS
  value:
    - "/buildcontext"

or so? (Tekton has some builtin support for things like this but I don't think that applies to the buildah-oci-remote-ta type thing)

[chmeliik]

Yeah an empty dir would be even better. Though it may be un-ergonomic, because as the Containerfile author, you'll need to know the path on the host:

FROM registry.access.redhat.com/ubi9/ubi:latest  as builder
# this /buildcontext is the path mounted in the container
RUN dnf -y install skopeo && skopeo copy docker://busybox oci:/buildcontext/out.oci

# this /tmp/buildcontext is the path on the host (the buildah Pod)
FROM oci:/tmp/buildcontext/out.oci

Some ways we could make it better:

• Make sure the host path is always the same as the user-specified mount destination (e.g. /buildcontext). This would require the buildah Task to run as root to create the directory. Currently, the task does run as root.
• Same as above but have only one hardcoded emptydir, don't give the option to choose the path. Then we can pre-create that volume on the Task level using https://tekton.dev/docs/pipelines/tasks/#specifying-volumes and don't tie ourselves to root

Or stick to mounting the context directory, since that one can be accessed with . without having to know the host path

[cgwalters]

Although actually...sorry, it is messier than that. The problem is the FROM oci:./out.oci part. The ./ there is referring to I believe the current working directory of podman/buildah (as distinct from the context directory, though they're very commonly the same of course).

And the complexity here is that at the time of doing the FROM here, the code isn't using the mount namespace of the container, it's using the mount namespace of the host.

...EDIT, I see you're saying the same thing.

[cgwalters]

Or stick to mounting the context directory, since that one can be accessed with . without having to know the host path

Yeah this is by far the simplest thing that will work in a straightforward way both in podman build directly and in the buildah tasks, let's just directly support this like

- name: WORKINGDIR_MOUNT
  value: /buildcontext

or so? There's no need to support mounting anything else than . and there's no sane use case for mounting . in multiple places, so let's just have a direct option in the buildah task to mount . at the place of the user's choosing.

[chmeliik]

Yeah this is by far the simplest thing that will work in a straightforward way both in podman build directly and in the buildah tasks, let's just directly support this like

- name: WORKINGDIR_MOUNT
  value: /buildcontext

or so? There's no need to support mounting anything else than . and there's no sane use case for mounting . in multiple places, so let's just have a direct option in the buildah task to mount . at the place of the user's choosing.

Yeah, that sounds reasonable 👍

[chmeliik]

We'll add the option to the buildah task ASAP. Afterwards, we'll need to make sure all the bootc and flatpak builds are updated to use that, and then we should finally be able to update buildah.