copier.Put: check for is-not-a-directory using lstat, not stat

[nalind]

What type of PR is this?

/kind bug

What this PR does / why we need it:

When checking if something that we want to overwrite with a directory is already a directory or not, use lstat instead of stat. If it's a symbolic link, it's not a directory.

This is a subtle behavior change, but it's in line with docker build.

How to verify it

A slightly longer unit test!
New conformance test!

Which issue(s) this PR fixes:

Follow up to #3655 and #3656.

Special notes for your reviewer:

Does this PR introduce a user-facing change?

When COPYing or ADDing a directory to a container where the location of the to-be-added directory is a symbolic link to a different directory, the contents of the directory being added are no longer merged with the contents of the directory to which the link points.  The symbolic link will be replaced by the new directory and its contents.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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 [nalind]

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

[vrothberg]

LGTM

Thank you, @nalind. You're impressively fast!

[nalind]

(added to the release note)

[rhatdan]

/lgtm
/hold

[rhatdan]

/hold cancel

[srcshelton]

Hey there - I've noticed a change between podman-3.4.4 and podman-4.0.0-rc3 (likely in other RCs too) which bumped buildah from 1.23.1 to 1.24.0, and I assume it's this fix… although in this case it seems to be the reverse of the description.

I have a multi-stage Container-file which imports a couple of images, then creates a final image from scratch - from the first of these images, a particular filesystem object was a directory, and then the second image which was COPY'd over the top had a symlink in the same destination location. With 1.23.1 and all previous, the result was the symlink being present in the final image. WIth 1.24.0, the directory is there instead, and the symlink has seemingly been entirely skipped.

The description says:

When COPYing or ADDing a directory to a container where the location of the to-be-added directory is a symbolic link to a different directory, the contents of the directory being added are no longer merged with the contents of the directory to which the link points. The symbolic link will be replaced by the new directory and its contents.

… which I read as COPYing a directory over a symlink - but this use-case instead copies a symlink over a directory, apparently resulting in the directory being maintained but the symlink lost.

It's an easy fix (… once I'd figured out the problem, which took a while!) but I just wanted to check this was an intentional effect of this change, rather than a missed edge-case?

(… should the copy fail if there's ambiguity such as this? Arguably, the new behaviour silently fails part of the COPY?)

[rhatdan]

@nalind ^^

[nalind]

Can you elaborate a bit more on the case? How did you copy a symlink over a directory? If the source specified for COPY is a symbolic link, it's dereferenced, so I'm having trouble reproducing what you're describing.

[srcshelton]

Here's the commit which fixed the issue:

srcshelton/docker-gentoo-build@e76b152

The first base-image (FROM "${zxtm_image_name}:${zxtm_service_version}" AS zxtm) has a /usr/local/zeus/zxtm/conf relative symlink (to conf_A in the same directory), and this the copied to /opt/zeus in the image being built from scratch.

The second base-image (FROM "localhost/${image_name}:${service_version}" AS base) has an /opt/zeus/zxtm/conf directory.

Up to buildah-1.23.1, the result of this process in this order was that the scratch image had a /opt/zeus/zxtm/conf relative symlink (still to conf_A). With buildah-1.24.0, the result is instead /opt/zeus/zxtm/conf being the directory from the second base-image.

I've ben able to fix this change be swapping the order of the COPY statements, I was simply curious as to whether this was expected?

[nalind]

Yes, if a path in the first set of files copied was a symbolic link, and in the second set of files copied it was a directory, the expectation is that the symlink is replaced with the directory.

It looks like the version before the commit copied a directory tree from the first ("zxtm", where zxtm/conf in that tree is a symbolic link), and then copied a directory tree from the second stage ("base", the one where zxtm/conf in that tree is a directory) after that, so after this change I'd expect to find a directory at that location in the built image.