Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require (
github.com/sahilm/fuzzy v0.1.1
github.com/spf13/cobra v1.10.1
github.com/spf13/pflag v1.0.10
golang.org/x/sys v0.31.0
k8s.io/apimachinery v0.34.1
k8s.io/client-go v0.34.1
k8s.io/klog/v2 v2.140.0
Expand All @@ -30,7 +31,6 @@ require (
github.com/x448/float16 v0.8.4 // indirect
go.yaml.in/yaml/v2 v2.4.2 // indirect
golang.org/x/net v0.38.0 // indirect
golang.org/x/sys v0.31.0 // indirect
golang.org/x/text v0.23.0 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
k8s.io/utils v0.0.0-20250604170112-4c0f3b243397 // indirect
Expand Down
26 changes: 13 additions & 13 deletions internal/installation/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func Uninstall(p environment.Paths, name string) error {
symlinkPath := filepath.Join(p.BinPath(), pluginNameToBin(name, IsWindows()))
klog.V(3).Infof("Unlink %q", symlinkPath)
if err := removeLink(symlinkPath); err != nil {
return errors.Wrap(err, "could not uninstall symlink of plugin")
return errors.Wrap(err, "could not uninstall link of plugin")
}

pluginInstallPath := p.PluginInstallPath(name)
Expand All @@ -196,39 +196,39 @@ func createOrUpdateLink(binDir, binary, plugin string) error {
dst := filepath.Join(binDir, pluginNameToBin(plugin, IsWindows()))

if err := removeLink(dst); err != nil {
return errors.Wrap(err, "failed to remove old symlink")
return errors.Wrap(err, "failed to remove old link")
}
if _, err := os.Stat(binary); os.IsNotExist(err) {
return errors.Wrapf(err, "can't create symbolic link, source binary (%q) cannot be found in extracted archive", binary)
return errors.Wrapf(err, "can't create link, source binary (%q) cannot be found in extracted archive", binary)
}

// Create new
klog.V(2).Infof("Creating symlink to %q at %q", binary, dst)
if err := os.Symlink(binary, dst); err != nil {
return errors.Wrapf(err, "failed to create a symlink from %q to %q", binary, dst)
klog.V(2).Infof("Creating link to %q at %q", binary, dst)
if err := createSymlink(binary, dst); err != nil {
return errors.Wrapf(err, "failed to create a link from %q to %q", binary, dst)
}
klog.V(2).Infof("Created symlink at %q", dst)
klog.V(2).Infof("Created link at %q", dst)

return nil
}

// removeLink removes a symlink reference if exists.
// removeLink removes a symlink or directory junction if it exists.
func removeLink(path string) error {
fi, err := os.Lstat(path)
if os.IsNotExist(err) {
klog.V(3).Infof("No file found at %q", path)
return nil
} else if err != nil {
return errors.Wrapf(err, "failed to read the symlink in %q", path)
return errors.Wrapf(err, "failed to read the link in %q", path)
}

if fi.Mode()&os.ModeSymlink == 0 {
return errors.Errorf("file %q is not a symlink (mode=%s)", path, fi.Mode())
if !isLink(fi) {
return errors.Errorf("file %q is not a link (mode=%s)", path, fi.Mode())
}
if err := os.Remove(path); err != nil {
return errors.Wrapf(err, "failed to remove the symlink in %q", path)
return errors.Wrapf(err, "failed to remove the link in %q", path)
}
klog.V(3).Infof("Removed symlink from %q", path)
klog.V(3).Infof("Removed link from %q", path)
return nil
}

Expand Down
253 changes: 253 additions & 0 deletions internal/installation/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,259 @@ func Test_applyDefaults(t *testing.T) {
}
}

func Test_bug_osSymlink_doesNotResolvePlatformSymlinks(t *testing.T) {
tmpDir := t.TempDir()

resolvedTmpDir, err := filepath.EvalSymlinks(tmpDir)
if err != nil {
t.Fatal(err)
}
if tmpDir == resolvedTmpDir {
t.Skip("no platform symlinks detected (e.g., macOS /var -> /private/var)")
}

binaryPath := filepath.Join(tmpDir, "kubectl-foo")
if err := os.WriteFile(binaryPath, []byte("#!/bin/sh\necho foo"), 0o755); err != nil {
t.Fatal(err)
}

linkPath := filepath.Join(resolvedTmpDir, "kubectl-foo-link")
if err := os.Symlink(binaryPath, linkPath); err != nil {
t.Fatalf("os.Symlink() error = %v", err)
}
defer os.Remove(linkPath)

target, err := os.Readlink(linkPath)
if err != nil {
t.Fatalf("os.Readlink() error = %v", err)
}

resolvedBinary, err := filepath.EvalSymlinks(binaryPath)
if err != nil {
t.Fatal(err)
}

if target == resolvedBinary {
t.Fatal("UNEXPECTED: os.Symlink resolved the path; bug may be fixed at the OS level")
}
if target != binaryPath {
t.Fatalf("expected os.Symlink to store unresolved path %q, got %q", binaryPath, target)
}
}

func Test_createSymlink_exists(t *testing.T) {
tmpDir := testutil.NewTempDir(t)
src := filepath.Join(testdataPath(t), "plugin-foo", "kubectl-foo")
dst := filepath.Join(tmpDir.Root(), "kubectl-test_plugin")

if err := createSymlink(src, dst); err != nil {
t.Fatalf("createSymlink() error = %v", err)
}

fi, err := os.Lstat(dst)
if err != nil {
t.Fatalf("os.Lstat() error = %v", err)
}
if fi.Mode()&os.ModeSymlink == 0 {
t.Error("expected link to have ModeSymlink set")
}
}

func Test_createOrUpdateLink_usesCreateSymlink(t *testing.T) {
tmpDir := testutil.NewTempDir(t)
binary := filepath.Join(testdataPath(t), "plugin-foo", "kubectl-foo")

if err := createOrUpdateLink(tmpDir.Root(), binary, "test-plugin"); err != nil {
t.Fatalf("createOrUpdateLink() error = %v", err)
}

linkPath := filepath.Join(tmpDir.Root(), pluginNameToBin("test-plugin", IsWindows()))
fi, err := os.Lstat(linkPath)
if err != nil {
t.Fatalf("failed to lstat link: %v", err)
}
if fi.Mode()&os.ModeSymlink == 0 {
t.Fatal("expected link to have ModeSymlink set")
}

target, err := os.Readlink(linkPath)
if err != nil {
t.Fatalf("failed to readlink: %v", err)
}
wantResolved, err := filepath.EvalSymlinks(binary)
if err != nil {
t.Fatalf("filepath.EvalSymlinks() error = %v", err)
}
if target != wantResolved {
t.Errorf("link target = %q; want %q", target, wantResolved)
}
}

func Test_createOrUpdateLink_resolvesSymlinks(t *testing.T) {
tmpDir := testutil.NewTempDir(t)
resolvedRoot, err := filepath.EvalSymlinks(tmpDir.Root())
if err != nil {
t.Fatal(err)
}
if tmpDir.Root() == resolvedRoot {
t.Skip("no platform symlinks detected (e.g., macOS /var -> /private/var)")
}

binDir := filepath.Join(resolvedRoot, "bin")
if err := os.MkdirAll(binDir, 0o755); err != nil {
t.Fatal(err)
}

binaryDir := filepath.Join(tmpDir.Root(), "store", "plugin-foo", "v1.0.0")
if err := os.MkdirAll(binaryDir, 0o755); err != nil {
t.Fatal(err)
}
binaryPath := filepath.Join(binaryDir, "kubectl-foo")
if err := os.WriteFile(binaryPath, []byte("#!/bin/sh\necho foo"), 0o755); err != nil {
t.Fatal(err)
}

if err := createOrUpdateLink(binDir, binaryPath, "foo"); err != nil {
t.Fatalf("createOrUpdateLink() error = %v", err)
}

linkPath := filepath.Join(binDir, pluginNameToBin("foo", IsWindows()))
target, err := os.Readlink(linkPath)
if err != nil {
t.Fatalf("os.Readlink() error = %v", err)
}

resolvedBinary, err := filepath.EvalSymlinks(binaryPath)
if err != nil {
t.Fatalf("filepath.EvalSymlinks() error = %v", err)
}

if target != resolvedBinary {
t.Errorf("link target = %q; want resolved %q (unresolved was %q)", target, resolvedBinary, binaryPath)
}
}

func Test_reproduce_osSymlink_vs_createSymlink(t *testing.T) {
tmpDir := t.TempDir()
resolved, err := filepath.EvalSymlinks(tmpDir)
if err != nil {
t.Fatal(err)
}
if tmpDir == resolved {
t.Skip("no platform symlinks detected (e.g., macOS /var -> /private/var)")
}

binary := filepath.Join(tmpDir, "kubectl-repro")
if err := os.WriteFile(binary, []byte("#!/bin/sh\necho repro"), 0o755); err != nil {
t.Fatal(err)
}

resolvedBinary, err := filepath.EvalSymlinks(binary)
if err != nil {
t.Fatal(err)
}

oldLink := filepath.Join(resolved, "old-link")
if err := os.Symlink(binary, oldLink); err != nil {
t.Fatal(err)
}
defer os.Remove(oldLink)

oldTarget, _ := os.Readlink(oldLink)
if oldTarget == resolvedBinary {
t.Fatal("UNEXPECTED: os.Symlink resolved the path; cannot reproduce bug")
}
if oldTarget != binary {
t.Fatalf("os.Symlink target = %q; want unresolved %q", oldTarget, binary)
}

newLink := filepath.Join(resolved, "new-link")
if err := createSymlink(binary, newLink); err != nil {
t.Fatal(err)
}
newTarget, _ := os.Readlink(newLink)
if newTarget != resolvedBinary {
t.Fatalf("createSymlink target = %q; want resolved %q", newTarget, resolvedBinary)
}
}

// Test_reproduce_oldCodePath_osSymlink_unresolved demonstrates the bug in the
// old code path: os.Symlink() stores the raw (unresolved) path, which breaks
// when platform symlinks are involved (e.g., macOS /var -> /private/var).
// On Windows, os.Symlink() additionally requires elevated privileges.
func Test_reproduce_oldCodePath_osSymlink_unresolved(t *testing.T) {
tmpDir := t.TempDir()

resolvedTmpDir, err := filepath.EvalSymlinks(tmpDir)
if err != nil {
t.Fatal(err)
}
if tmpDir == resolvedTmpDir {
t.Skip("no platform symlinks detected (e.g., macOS /var -> /private/var); symlink resolution bug not reproducible on this platform")
}

// Simulate a plugin binary inside an install directory that uses the
// unresolved path (as TempDir returns on macOS).
binaryPath := filepath.Join(tmpDir, "kubectl-bugdemo")
if err := os.WriteFile(binaryPath, []byte("#!/bin/sh\necho bugdemo"), 0o755); err != nil {
t.Fatal(err)
}

resolvedBinary, err := filepath.EvalSymlinks(binaryPath)
if err != nil {
t.Fatal(err)
}

// OLD CODE PATH: os.Symlink stores the unresolved path verbatim.
binDir := filepath.Join(resolvedTmpDir, "bin")
if err := os.Mkdir(binDir, 0o755); err != nil {
t.Fatal(err)
}
oldLink := filepath.Join(binDir, "kubectl-bugdemo_old")
if err := os.Symlink(binaryPath, oldLink); err != nil {
t.Fatal(err)
}
defer os.Remove(oldLink)

oldTarget, err := os.Readlink(oldLink)
if err != nil {
t.Fatal(err)
}

// BUG: the old code stores the unresolved path, which contains the
// platform symlink prefix (e.g., /var/folders/... instead of
// /private/var/folders/...). This means the link target and the
// resolved binary path don't match.
if oldTarget == resolvedBinary {
t.Fatal("UNEXPECTED: os.Symlink resolved the path; bug not reproducible")
}

// Prove the mismatch exists — this IS the bug.
t.Logf("BUG CONFIRMED: os.Symlink target = %q (unresolved)", oldTarget)
t.Logf(" expected resolved target = %q", resolvedBinary)
if oldTarget != binaryPath {
t.Fatalf("expected os.Symlink to use unresolved path %q, got %q", binaryPath, oldTarget)
}

// NEW CODE PATH: createSymlink resolves the path first.
newLink := filepath.Join(binDir, "kubectl-bugdemo_new")
if err := createSymlink(binaryPath, newLink); err != nil {
t.Fatal(err)
}
defer os.Remove(newLink)

newTarget, err := os.Readlink(newLink)
if err != nil {
t.Fatal(err)
}

// The fix: createSymlink stores the RESOLVED path.
if newTarget != resolvedBinary {
t.Errorf("createSymlink target = %q; want resolved %q", newTarget, resolvedBinary)
}
t.Logf("FIX VERIFIED: createSymlink target = %q (resolved)", newTarget)
}

func TestCleanupStaleKrewInstallations(t *testing.T) {
dir := testutil.NewTempDir(t)

Expand Down
Loading
Loading