From e92867e6623bc6046a13340fa28a26f48bfae16e Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Wed, 26 Feb 2025 11:28:27 -0800 Subject: [PATCH] Fix Windows permission error with self remove (#52316) (#52498) * Fix windows permission error with self replace * Aggregate errors * Update comments --- integration/autoupdate/tools/main_test.go | 2 ++ integration/autoupdate/tools/updater/modules.go | 4 ++-- integration/autoupdate/tools/updater_tsh_test.go | 2 ++ lib/utils/fs.go | 6 +++++- lib/utils/packaging/unarchive.go | 10 ++++------ 5 files changed, 15 insertions(+), 9 deletions(-) diff --git a/integration/autoupdate/tools/main_test.go b/integration/autoupdate/tools/main_test.go index db203f84c0fb5..5c28e1043ca8d 100644 --- a/integration/autoupdate/tools/main_test.go +++ b/integration/autoupdate/tools/main_test.go @@ -39,6 +39,7 @@ import ( "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/integration/helpers/archive" + "github.com/gravitational/teleport/lib/modules" ) const ( @@ -59,6 +60,7 @@ var ( ) func TestMain(m *testing.M) { + modules.SetModules(&modules.TestModules{TestBuildType: "CLI"}) ctx := context.Background() tmp, err := os.MkdirTemp(os.TempDir(), testBinaryName) if err != nil { diff --git a/integration/autoupdate/tools/updater/modules.go b/integration/autoupdate/tools/updater/modules.go index e02c14501ac87..523ad4a974f14 100644 --- a/integration/autoupdate/tools/updater/modules.go +++ b/integration/autoupdate/tools/updater/modules.go @@ -64,12 +64,12 @@ func (p *TestModules) BuildType() string { return "CLI" } -// IsEnterpriseBuild returns false for [TestModules]. +// IsEnterpriseBuild returns true if `UPDATER_TEST_BUILD` env is set `ent` for [TestModules]. func (p *TestModules) IsEnterpriseBuild() bool { return os.Getenv(TestBuild) == modules.BuildEnterprise } -// IsOSSBuild returns false for [TestModules]. +// IsOSSBuild returns true if `UPDATER_TEST_BUILD` env is set `oss` for [TestModules]. func (p *TestModules) IsOSSBuild() bool { return os.Getenv(TestBuild) == modules.BuildOSS } diff --git a/integration/autoupdate/tools/updater_tsh_test.go b/integration/autoupdate/tools/updater_tsh_test.go index b2372683b6c5a..17816e34e4458 100644 --- a/integration/autoupdate/tools/updater_tsh_test.go +++ b/integration/autoupdate/tools/updater_tsh_test.go @@ -1,3 +1,5 @@ +//go:build !windows + /* * Teleport * Copyright (C) 2024 Gravitational, Inc. diff --git a/lib/utils/fs.go b/lib/utils/fs.go index 741bb8e8cf87e..b71bdcd1c8cfd 100644 --- a/lib/utils/fs.go +++ b/lib/utils/fs.go @@ -422,15 +422,19 @@ func RecursiveChown(dir string, uid, gid int) error { return nil } -func CopyFile(src, dest string, perm os.FileMode) error { +func CopyFile(src, dest string, perm os.FileMode) (err error) { srcFile, err := os.Open(src) if err != nil { return trace.ConvertSystemError(err) } + defer srcFile.Close() destFile, err := os.OpenFile(dest, os.O_RDWR|os.O_CREATE|os.O_TRUNC, perm) if err != nil { return trace.ConvertSystemError(err) } + defer func() { + err = trace.NewAggregate(err, trace.Wrap(destFile.Close())) + }() _, err = destFile.ReadFrom(srcFile) if err != nil { return trace.ConvertSystemError(err) diff --git a/lib/utils/packaging/unarchive.go b/lib/utils/packaging/unarchive.go index 6496afbc182c3..ed81a4b4b2d24 100644 --- a/lib/utils/packaging/unarchive.go +++ b/lib/utils/packaging/unarchive.go @@ -129,12 +129,10 @@ func replaceZip(toolsDir string, archivePath string, extractDir string, execName return trace.Wrap(err) } appPath := filepath.Join(toolsDir, baseName) - if err := os.Remove(appPath); err != nil && !os.IsNotExist(err) { - return trace.Wrap(err) - } - // For the Windows build we have to copy binary to be able - // to do this without administrative access as it required - // for symlinks. + // For the Windows build, we need to copy the binary to perform updates without requiring + // administrative access, which would otherwise be needed for creating symlinks. + // Since symlinks are not used on the Windows platform, there's no need to remove appPath + // before copying the new binary — it will simply be replaced. if err := utils.CopyFile(dest, appPath, 0o755); err != nil { return trace.Wrap(err) }