Skip to content

installation: Windows hard link for plugin shims, then symlink (EvalSymlinks fallback)#904

Closed
carterpewpew wants to merge 2 commits into
kubernetes-sigs:masterfrom
carterpewpew:fix/os-symlink-requires
Closed

installation: Windows hard link for plugin shims, then symlink (EvalSymlinks fallback)#904
carterpewpew wants to merge 2 commits into
kubernetes-sigs:masterfrom
carterpewpew:fix/os-symlink-requires

Conversation

@carterpewpew

@carterpewpew carterpewpew commented Apr 26, 2026

Copy link
Copy Markdown

[WIP]
What this PR does

On Windows, creating plugin shims in .krew/bin no longer depends only on os.Symlink. The code tries os.Link first (same volume, no extra symlink privilege in the common case), then falls back to os.Symlink. The source path is passed through filepath.EvalSymlinks before the symlink path so the fallback does not use an unresolved path if the install path goes through a junction or directory symlink.

Uninstall and upgrades use removeLink with an isLink helper on Windows that also allows removing hard-linked bin entries, not only symlinks.

On non-Windows, behavior is unchanged: createSymlink is a plain os.Symlink and isLink is symlink-only, matching the old logic.

Fixes #886

carterpewpew and others added 2 commits April 27, 2026 00:40
Extract createSymlink() into build-tagged files (symlink_unix.go,
symlink_windows.go) to isolate platform-specific symlink logic.
On Windows, this enables elevated-privilege symlink creation with
fallback to directory junctions.

Add comprehensive symlink tests covering creation, target
verification via os.Readlink/filepath.EvalSymlinks, platform-level
path resolution, and dangling symlink behavior on Unix.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add Test_reproduce_oldCodePath_osSymlink_unresolved to demonstrate that
os.Symlink stores raw unresolved paths (e.g., /var/folders/... on macOS)
while createSymlink correctly resolves platform symlinks first. The test
confirms the bug in the old code path and verifies the fix.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@k8s-ci-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: carterpewpew
Once this PR has been reviewed and has the lgtm label, please assign soltysh 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:

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

@k8s-ci-robot
k8s-ci-robot requested review from ahmetb and soltysh April 26, 2026 19:36
@linux-foundation-easycla

Copy link
Copy Markdown

CLA Missing ID CLA Not Signed

One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via:

Co-authored-by: name <email>

Supported Co-authored-by: formats include:

  1. Anything <id+login@users.noreply.github.com> - it will locate your GitHub user by id part.
  2. Anything <login@users.noreply.github.com> - it will locate your GitHub user by login part.
  3. Anything <public-email> - it will locate your GitHub user by public-email part. Note that this email must be made public on Github.
  4. Anything <other-email> - it will locate your GitHub user by other-email part but only if that email was used before for any other CLA as a main commit author.
  5. login <any-valid-email> - it will locate your GitHub user by login part, note that login part must be at least 3 characters long.

Alternatively, if the co-author should not be included, remove the Co-authored-by: line from the commit message.

Please update your commit message(s) by doing git commit --amend and then git push [--force] and then request re-running CLA check via commenting on this pull request:

/easycla

@k8s-ci-robot

Copy link
Copy Markdown
Contributor

Welcome @carterpewpew!

It looks like this is your first PR to kubernetes-sigs/krew 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/krew has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 26, 2026
@carterpewpew

carterpewpew commented Apr 26, 2026

Copy link
Copy Markdown
Author

Sorry, accidentally opened, please ignore

@carterpewpew carterpewpew changed the title Use directory junctions on Windows to eliminate privilege requirements installation: Windows hard link for plugin shims, then symlink (EvalSymlinks fallback) Apr 27, 2026
@ahmetb

ahmetb commented Apr 27, 2026

Copy link
Copy Markdown
Member

Looks promising, especially if you could figure out how not to break existing installations that already installed some plugins via symlinks (it should work, right?). Thanks for taking this on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants