From bef7c9bcfbc319e451518ae460f38e3d032b5a97 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Wed, 19 Feb 2025 11:03:11 -0800 Subject: [PATCH 1/3] Fix windows permission error with self replace --- integration/autoupdate/tools/main_test.go | 1 + integration/autoupdate/tools/updater/modules.go | 2 +- integration/autoupdate/tools/updater_tsh_test.go | 2 ++ lib/utils/fs.go | 6 +++++- lib/utils/packaging/unarchive.go | 3 --- 5 files changed, 9 insertions(+), 5 deletions(-) diff --git a/integration/autoupdate/tools/main_test.go b/integration/autoupdate/tools/main_test.go index 50aeea81fea30..55878b58611fd 100644 --- a/integration/autoupdate/tools/main_test.go +++ b/integration/autoupdate/tools/main_test.go @@ -61,6 +61,7 @@ var ( func TestMain(m *testing.M) { modules.SetInsecureTestMode(true) + modules.SetModules(&modules.TestModules{TestBuildType: modules.BuildCommunity}) 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 017e5adb3cb70..a37e428aba9fd 100644 --- a/integration/autoupdate/tools/updater/modules.go +++ b/integration/autoupdate/tools/updater/modules.go @@ -72,7 +72,7 @@ func (p *TestModules) IsEnterpriseBuild() bool { // IsOSSBuild returns false for [TestModules]. func (p *TestModules) IsOSSBuild() bool { - return os.Getenv(TestBuild) == modules.BuildOSS + return true } // LicenseExpiry returns the expiry date of the enterprise license, if applicable. diff --git a/integration/autoupdate/tools/updater_tsh_test.go b/integration/autoupdate/tools/updater_tsh_test.go index 01b9ca7679a42..3ca2943468f9c 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 dbfd1b390d7b7..4124f6a7fd78d 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.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..400c77a241cec 100644 --- a/lib/utils/packaging/unarchive.go +++ b/lib/utils/packaging/unarchive.go @@ -129,9 +129,6 @@ 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. From c005fbf1c8400492f0510cf640bbe74291800616 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Mon, 24 Feb 2025 12:52:27 -0800 Subject: [PATCH 2/3] Aggregate errors --- lib/utils/fs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/utils/fs.go b/lib/utils/fs.go index 4124f6a7fd78d..0ddb0af13a1d9 100644 --- a/lib/utils/fs.go +++ b/lib/utils/fs.go @@ -433,7 +433,7 @@ func CopyFile(src, dest string, perm os.FileMode) (err error) { return trace.ConvertSystemError(err) } defer func() { - err = trace.Wrap(destFile.Close()) + err = trace.NewAggregate(err, trace.Wrap(destFile.Close())) }() _, err = destFile.ReadFrom(srcFile) if err != nil { From 506ba07094705f5ec6c2ab4845c53a43aa05fffc Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Tue, 25 Feb 2025 10:18:00 -0800 Subject: [PATCH 3/3] Update comments --- integration/autoupdate/tools/updater/modules.go | 6 +++--- lib/utils/packaging/unarchive.go | 7 ++++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/integration/autoupdate/tools/updater/modules.go b/integration/autoupdate/tools/updater/modules.go index a37e428aba9fd..6c1236393af78 100644 --- a/integration/autoupdate/tools/updater/modules.go +++ b/integration/autoupdate/tools/updater/modules.go @@ -65,14 +65,14 @@ 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 true + return os.Getenv(TestBuild) == modules.BuildOSS } // LicenseExpiry returns the expiry date of the enterprise license, if applicable. diff --git a/lib/utils/packaging/unarchive.go b/lib/utils/packaging/unarchive.go index 400c77a241cec..ed81a4b4b2d24 100644 --- a/lib/utils/packaging/unarchive.go +++ b/lib/utils/packaging/unarchive.go @@ -129,9 +129,10 @@ func replaceZip(toolsDir string, archivePath string, extractDir string, execName return trace.Wrap(err) } appPath := filepath.Join(toolsDir, baseName) - // 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) }