Fix stale cache when using bind mount with build stage

[ekedaigle]

What type of PR is this?

/kind bug

What this PR does / why we need it:

This fixes a bug where stale images would be used when using a previous build stage as a bind mount source

How to verify it

Trivial example to reproduce:

FROM ubi8/ubi as base

FROM base as builder
RUN mkdir /build && echo "1" > /build/foo.txt

FROM base as prod
RUN --mount=type=bind,source=/build,from=builder,target=/build \
     cp /build/foo.txt bar.txt

1. Build this file with buildah build -f <dockerfile> --layers.
2. Change the "1" to a "2" and build again, observing the final image hash changes
3. Change the "2" back to a "1" and observe the hash not change as it should

Which issue(s) this PR fixes:

Fixes #6609

Special notes for your reviewer:

I'm not particularly familiar with this code base, this was just an annoyance I wanted to fix, so comments welcome.

Does this PR introduce a user-facing change?

Fixes bug where stale images could be used when using a previous build stage as a bind mount source

[TomSweeneyRedHat]

@ekedaigle thanks for the PR!
The imagebuildah/stage_executor.go has format issues. Please `gofmt -s -w imagebuildah/stage_executor.go" and add it back in.

@nalind PTAL

[nalind]

Well spotted, and it's pretty close to what I think we'd want to be doing for that case. The wrinkle is that the value being used in the current PR is the previous stage's image's ID (the hex part of the digest of its configuration blob), when for other source locations we use the digest of the image's manifest (including the algorithm part) or a digest over a directory tree. The image ID's still unique enough that it should cause affect cache evaluation the way we desire, though.

The way I see that being sorted would be:

• add a new map field (imageDigestMap?) alongside the current imageMap field in the executor structure
• also set a value in the new map to r.CommitResults.Digest.String() when the corresponding value in the imageMap is set to r.ImageID in the current version
• use the value from the new map where the imageMap value is currently being consulted in the new block added by the PR

Would you be up for making those changes, or would you prefer for one of us prepare a follow-up PR? Either way, thanks for catching this!

[ekedaigle]

Would you be up for making those changes, or would you prefer for one of us prepare a follow-up PR? Either way, thanks for catching this!

If one of you wants to take it don't let me stop you, I'd have to get more familiar with the internals here before making the changes myself.

[nalind]

Alright, then.
LGTM

[nalind]

Can you squash these together, using something like git rebase -i?

[ekedaigle]

Sure thing, should be set now.

[nalind]

Thanks!