Don't skip symlinks during local source upload

[dollierp]

Changes

Don't skip symlinks during local source upload

/kind bug

Submitter Checklist

• Includes tests if functionality changed/was added
• Includes docs if changes are user-facing
• Set a kind label on this PR
• Release notes block has been filled in, or marked NONE

See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.

Release Notes

Don't skip symlinks during local source upload

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign heavywombat for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[IrvingMg]

Thanks for the PR!

Overall, LGTM. I left a couple of comments. It would also be nice to have unit tests for at least basic cases, as the changes are related to the core functionality of the upload command.

[dollierp]

Hi @IrvingMg,

Thanks for your review.

I updated the PR with new changes trying to address your remarks.

Regards,

[Copilot]

Pull request overview

This PR fixes a bug where symlinks were being skipped during local source uploads. Previously, the tar helper would skip any file that wasn't a regular file (including symlinks, directories, and other special file types). The changes now allow symlinks to be included in the tar archive while still skipping unsupported file types.

Changes:

• Modified skipPath function to exclude symlinks from being skipped (allowing them to be processed)
• Added symlink handling in writeFileToTar to read symlink targets, detect if they point outside the source directory, and properly set the tar header linkname
• Added isSymlinkTargetOutsideOfDir function to check if symlink targets escape the source directory and warn appropriately
• Updated test to verify that symlinks are captured in the tar archive
• Fixed documentation from "buildrun" to "build" command references

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pkg/shp/streamer/tar.go	Modified skipPath condition to allow symlinks through filtering
pkg/shp/streamer/util.go	Added symlink handling logic with safety check for targets pointing outside source directory
pkg/shp/streamer/tar_test.go	Enhanced test to verify symlinks are included in tar output
pkg/shp/cmd/build/upload.go	Fixed documentation typo from "buildrun" to "build"
docs/shp_build_upload.md	Updated documentation examples to match corrected command

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[IrvingMg]

Hi @dollierp, thanks for the changes.

There’s one more Copilot comment, could you take a look?

[dollierp]

Hi @IrvingMg,

I updated the PR to make Copilot happy.

Let me know if something else is missing.

Regards.

[IrvingMg]

Thanks again for the changes. I left a couple more comments.

[dollierp]

Hi @IrvingMg,

I tried to address your latest remarks and updated the PR.

Regards,

[IrvingMg]

Thanks for the update, looks good overall. Just a few final comments from my side.

[dollierp]

Hi @IrvingMg,

I've tried to incorporate your latest feedback and have updated the pull request.

Regards,

[IrvingMg]

Thanks for the changes! Overall /lgtm

@shipwright-io/approvers PTAL, this PR has been open for a while.

[IrvingMg]

/lgtm