[WIP]Remove personal fork replace directive for filepath-securejoin#30889
[WIP]Remove personal fork replace directive for filepath-securejoin#30889wangke19 wants to merge 2 commits intoopenshift:mainfrom
Conversation
The replace directive pointing to github.com/wangke19/filepath-securejoin-1 was added in PR openshift#30746 as a workaround for opencontainers/runc v1.2.5 calling the removed MkdirAllHandle function from filepath-securejoin v0.6.0. However, go mod why shows that opencontainers/runc is not actually needed by the origin module: # github.com/opencontainers/runc (main module does not need package github.com/opencontainers/runc) Since runc is only a transitive dependency (not directly used), and the compatibility shim in the fork is not needed, we can safely remove the personal fork and use the official filepath-securejoin v0.6.0. This eliminates the supply chain risk of depending on a personal fork and follows Go module best practices. Note: PR openshift#30866 (K8s 1.35 rebase) will also remove this when it merges, as the rebase removes opencontainers/runc entirely from the dependency graph. This PR provides immediate cleanup for the current main branch.
Remove vendored fork code and use official upstream release. Key changes: - Removed mkdirall_compat_linux.go (compatibility shim from fork) - Reverted VERSION from 0.6.1 (fork) to 0.6.0 (official) - Updated pathrs-lite internal components to official v0.6.0 state Generated by: go mod vendor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (11)
📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
WalkthroughRemoved a replace directive in go.mod that mapped github.com/cyphar/filepath-securejoin to a patched fork at github.com/wangke19/filepath-securejoin-1. The change reverts dependency resolution to use the original module path directly. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wangke19 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@wangke19: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Closing this PR as CI testing revealed the personal fork is actually required. Root CauseThe personal fork
Without the compatibility shim, builds fail with: Why
|
Summary
This PR removes the personal fork replace directive for
github.com/cyphar/filepath-securejointhat was added in PR #30746.Problem
PR #30746 added a replace directive pointing to a personal fork (
github.com/wangke19/filepath-securejoin-1) as a workaround foropencontainers/runc v1.2.5calling the removedMkdirAllHandlefunction fromfilepath-securejoin v0.6.0.However, this has several issues:
go mod whyshows thatopencontainers/runcis not actually needed by the origin module:Solution
Remove the personal fork replace directive and use the official
filepath-securejoin v0.6.0.Since
runcis only a transitive dependency (not directly used by origin), the compatibility shim in the fork is not needed.Changes
Commit 1: Remove replace directive from go.mod
filepath-securejoinreplace directivegithub.com/cyphar/filepath-securejoin v0.6.0Commit 2: Update vendor directory
mkdirall_compat_linux.go(compatibility shim from fork)0.6.1(fork) to0.6.0(official)Testing
go mod tidy— no errorsgo mod vendor— updated successfullygo mod why github.com/opencontainers/runc— confirms runc is not needed by originRelated
opencontainers/runcentirely from the dependency graph. This PR provides immediate cleanup for the current main branch.Verification
The personal fork was only needed if
opencontainers/runcwas actively used by origin code. Since it's only a transitive dependency, we can safely remove it.