Local exporter creates incorrect symlinks with multi-platform

[jedevc]

Contributing guidelines and issue reporting guide

• I've read the contributing guidelines and wholeheartedly agree. I've also read the issue reporting guide.

Well-formed report checklist

• I have found a bug that the documentation does not mention anything about my problem
• I have found a bug that there are no open or closed issues that are related to my problem
• I have provided version/information about my environment and done my best to provide a reproducer

Description of bug

Bug description

When doing a multi-platform build, the link destinations are retargeted to the root, I'd argue potentially incorrectly.

Likely, the links are already invalid - especially if they're absolute links, I'd be unpacking the local export to a subdir, not the root filesystem. Iff we want to rewrite the links at all, I think the correct behavior would be to instead rewrite the absolute links to relative ones, so that they just work. But I think the correct thing to do for local exports + tar exports is to preserve the link destinations as they were, and not attempt to rewrite them.

Reproduction

# The debian:bookworm-slim image contains symlinks like /etc/alternatives/awk -> /usr/bin/mawk
FROM debian:bookworm-slim

# also create a test symlink to make it easier to inspect
RUN ln -s /usr/bin/bash /test-symlink

Build using single-platform (good case):

$ buildctl build \
  --frontend dockerfile.v0 \
  --local context=. \
  --local dockerfile=. \
  --output type=local,dest=./good \
  --opt platform=linux/amd64

$ readlink ./good/etc/alternatives/awk ./good/test-symlink
/usr/bin/mawk
/usr/bin/bash

Build using multi-platform (bad case):

$ buildctl build \                                                                                                                           
  --frontend dockerfile.v0 \
  --local context=. \
  --local dockerfile=. \
  --output type=local,dest=./bad \
  --opt platform=linux/amd64,linux/arm64

$ ❯ readlink ./bad/linux_amd64/etc/alternatives/awk ./good/linux_amd64/test-symlink
/linux_amd64/usr/bin/mawk
/linux_amd64/usr/bin/bash

Version information

Buildkit v0.29.0.

[tonistiigi]

It does indeed look wrong for local exporter. I guess for the tar as well as even though it makes symlinks broken they would only be correct anyway if extracted to / destination directory.

For the SubDirFS struct the rewrite does make sense though so I think we should make it configurable and make sure the exporter doesn't use it.

[jedevc]

For the SubDirFS struct the rewrite does make sense though so I think we should make it configurable and make sure the exporter doesn't use it.

Just to check what you mean here. We should add an opt to SubDirFS to enable link rewriting (and regardless of the default), we would just set it to false for the exporters, so that link targets aren't rewritten at all.

In your opinion would this be a breaking change? Definitely needs changelogging, and I don't think anyone would be relying on this behavior, but curious what you think.

[tonistiigi]

Just to check what you mean here. We should add an opt to SubDirFS to enable link rewriting (and regardless of the default), we would just set it to false for the exporters, so that link targets aren't rewritten at all.

yes

In your opinion would this be a breaking change? Definitely needs changelogging, and I don't think anyone would be relying on this behavior, but curious what you think.

I think we can consider it a bugfix for exporters.

[jedevc]

As a note for the implementer (likely me), we should also add a capability to allow checking for this fix.

If the server doesn't support this, the client should be able to error out, instead of generating a bad result.