diff --git a/internal/pkg/agent/install/uninstall.go b/internal/pkg/agent/install/uninstall.go index efcbafcaaff..f333e8c49a5 100644 --- a/internal/pkg/agent/install/uninstall.go +++ b/internal/pkg/agent/install/uninstall.go @@ -391,7 +391,7 @@ func killWatcher(pt *progressbar.ProgressBar) error { errs = errors.Join(errs, fmt.Errorf("failed to load watcher process with pid %d: %w", pid, err)) continue } - err = proc.Kill() + err = killNoneChildProcess(proc) if err != nil && !errors.Is(err, os.ErrProcessDone) { errs = errors.Join(errs, fmt.Errorf("failed to kill watcher process with pid %d: %w", pid, err)) continue diff --git a/internal/pkg/agent/install/uninstall_unix.go b/internal/pkg/agent/install/uninstall_unix.go index d63d4bcf2d4..2181a6db481 100644 --- a/internal/pkg/agent/install/uninstall_unix.go +++ b/internal/pkg/agent/install/uninstall_unix.go @@ -6,6 +6,8 @@ package install +import "os" + func isBlockingOnExe(_ error) bool { return false } @@ -17,3 +19,10 @@ func removeBlockingExe(_ error) error { func isRetryableError(_ error) bool { return false } + +// killNoneChildProcess provides a way of killing a process that is not started as a child of this process. +// +// On Unix systems it just calls the native golang kill. +func killNoneChildProcess(proc *os.Process) error { + return proc.Kill() +} diff --git a/internal/pkg/agent/install/uninstall_windows.go b/internal/pkg/agent/install/uninstall_windows.go index 5339cd376b3..07b6d2ddeaa 100644 --- a/internal/pkg/agent/install/uninstall_windows.go +++ b/internal/pkg/agent/install/uninstall_windows.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "io/fs" + "os" "syscall" "unsafe" @@ -160,3 +161,19 @@ type fileRenameInfo struct { type fileDispositionInfo struct { DeleteFile bool } + +// killNoneChildProcess provides a way of killing a process that is not started as a child of this process. +// +// On Windows when running in unprivileged mode the internal way that golang uses DuplicateHandle to perform the kill +// only works when the process is a child of this process. +func killNoneChildProcess(proc *os.Process) error { + h, e := syscall.OpenProcess(syscall.PROCESS_TERMINATE, false, uint32(proc.Pid)) + if e != nil { + return os.NewSyscallError("OpenProcess", e) + } + defer func() { + _ = syscall.CloseHandle(h) + }() + e = syscall.TerminateProcess(h, 1) + return os.NewSyscallError("TerminateProcess", e) +} diff --git a/testing/integration/upgrade_fleet_test.go b/testing/integration/upgrade_fleet_test.go index 3f4130d0fd4..185431cfdd5 100644 --- a/testing/integration/upgrade_fleet_test.go +++ b/testing/integration/upgrade_fleet_test.go @@ -215,14 +215,8 @@ func testUpgradeFleetManagedElasticAgent( require.NoError(t, err) if unprivileged { - if startParsedVersion.Less(*upgradetest.Version_8_13_0) { - t.Skipf("Starting version %s is less than 8.13 and doesn't support --unprivileged", startParsedVersion.String()) - } - if endParsedVersion.Less(*upgradetest.Version_8_13_0) { - t.Skipf("Ending version %s is less than 8.13 and doesn't support --unprivileged", endParsedVersion.String()) - } - if runtime.GOOS != define.Linux { - t.Skip("Unprivileged mode is currently only supported on Linux") + if !upgradetest.SupportsUnprivileged(startParsedVersion, endParsedVersion) { + t.Skipf("Either starting version %s or ending version %s doesn't support --unprivileged", startParsedVersion.String(), endParsedVersion.String()) } } diff --git a/testing/integration/upgrade_standalone_same_commit_test.go b/testing/integration/upgrade_standalone_same_commit_test.go index ae8511147d7..16d468bc0b9 100644 --- a/testing/integration/upgrade_standalone_same_commit_test.go +++ b/testing/integration/upgrade_standalone_same_commit_test.go @@ -18,7 +18,6 @@ import ( "os" "path" "path/filepath" - "runtime" "strings" "testing" "time" @@ -52,17 +51,10 @@ func TestStandaloneUpgradeSameCommit(t *testing.T) { t.Skipf("Minimum version for running this test is %q, current version: %q", *upgradetest.Version_8_13_0_SNAPSHOT, currentVersion) } - unprivilegedAvailable := true - if runtime.GOOS != define.Linux { - // only available on Linux at the moment - unprivilegedAvailable = false + unprivilegedAvailable := false + if upgradetest.SupportsUnprivileged(currentVersion) { + unprivilegedAvailable = true } - // This is probably redundant: see the skip statement above - if unprivilegedAvailable && currentVersion.Less(*upgradetest.Version_8_13_0) { - // only available if both versions are 8.13+ - unprivilegedAvailable = false - } - unPrivilegedString := "unprivileged" if !unprivilegedAvailable { unPrivilegedString = "privileged" diff --git a/testing/integration/upgrade_standalone_test.go b/testing/integration/upgrade_standalone_test.go index ca0432f7a6f..8e7d98e40d6 100644 --- a/testing/integration/upgrade_standalone_test.go +++ b/testing/integration/upgrade_standalone_test.go @@ -9,7 +9,6 @@ package integration import ( "context" "fmt" - "runtime" "testing" "time" @@ -36,14 +35,9 @@ func TestStandaloneUpgrade(t *testing.T) { require.NoError(t, err) for _, startVersion := range versionList { - unprivilegedAvailable := true - if runtime.GOOS != define.Linux { - // only available on Linux at the moment - unprivilegedAvailable = false - } - if unprivilegedAvailable && (startVersion.Less(*upgradetest.Version_8_13_0) || endVersion.Less(*upgradetest.Version_8_13_0)) { - // only available if both versions are 8.13+ - unprivilegedAvailable = false + unprivilegedAvailable := false + if upgradetest.SupportsUnprivileged(startVersion, endVersion) { + unprivilegedAvailable = true } t.Run(fmt.Sprintf("Upgrade %s to %s (privileged)", startVersion, define.Version()), func(t *testing.T) { testStandaloneUpgrade(t, startVersion, define.Version(), false) diff --git a/testing/upgradetest/upgrader.go b/testing/upgradetest/upgrader.go index 3acefff8f9f..8ed20dd71fd 100644 --- a/testing/upgradetest/upgrader.go +++ b/testing/upgradetest/upgrader.go @@ -15,13 +15,13 @@ import ( "strings" "time" + "github.com/hectane/go-acl" "github.com/otiai10/copy" "github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/details" v1client "github.com/elastic/elastic-agent/pkg/control/v1/client" v2proto "github.com/elastic/elastic-agent/pkg/control/v2/cproto" atesting "github.com/elastic/elastic-agent/pkg/testing" - "github.com/elastic/elastic-agent/pkg/testing/define" "github.com/elastic/elastic-agent/pkg/version" ) @@ -209,8 +209,7 @@ func PerformUpgrade( // in the unprivileged is unset we adjust it to use unprivileged when the version allows it // in the case that its explicitly set then we ensure the version supports it if upgradeOpts.unprivileged == nil { - if !startVersion.Less(*Version_8_13_0) && !endVersion.Less(*Version_8_13_0) && runtime.GOOS == define.Linux { - // both version support --unprivileged + if SupportsUnprivileged(startVersion, endVersion) { unprivileged := true upgradeOpts.unprivileged = &unprivileged logger.Logf("installation of Elastic Agent will use --unprivileged as both start and end version support --unprivileged mode") @@ -220,11 +219,8 @@ func PerformUpgrade( upgradeOpts.unprivileged = &unprivileged } } else if *upgradeOpts.unprivileged { - if startVersion.Less(*Version_8_13_0) { - return errors.New("cannot install starting version with --unprivileged (which is default) because the it is older than 8.13") - } - if endVersion.Less(*Version_8_13_0) { - return errors.New("cannot upgrade to ending version as end version doesn't support running with --unprivileged (which is default) because it is older than 8.13") + if !SupportsUnprivileged(startVersion, endVersion) { + return fmt.Errorf("cannot install with forced --unprivileged because either start version %s or end version %s doesn't support --unprivileged mode", startVersion.String(), endVersion.String()) } } @@ -574,7 +570,19 @@ func getSourceURI(ctx context.Context, f *atesting.Fixture, unprivileged bool) ( } if unprivileged { // move the file to temp directory - dir, err := os.MkdirTemp("", "agent-upgrade-*") + baseTmp := "" + if runtime.GOOS == "windows" { + // `elastic-agent-user` needs to have access to the file, default + // will place this in C:\Users\windows\AppData\Local\Temp\ which + // `elastic-agent-user` doesn't have access. + + // create C:\Temp with world read/write to use for temp directory + baseTmp, err = windowsBaseTemp() + if err != nil { + return "", fmt.Errorf("failed to create windows base temp path: %w", err) + } + } + dir, err := os.MkdirTemp(baseTmp, "agent-upgrade-*") if err != nil { return "", fmt.Errorf("failed to create temp directory: %w", err) } @@ -596,3 +604,22 @@ func getSourceURI(ctx context.Context, f *atesting.Fixture, unprivileged bool) ( } return "file://" + filepath.Dir(srcPkg), nil } + +func windowsBaseTemp() (string, error) { + baseTmp := "C:\\Temp" + _, err := os.Stat(baseTmp) + if err != nil { + if !errors.Is(err, os.ErrNotExist) { + return "", fmt.Errorf("failed to stat %s: %w", baseTmp, err) + } + err = os.Mkdir(baseTmp, 0777) + if err != nil { + return "", fmt.Errorf("failed to mkdir %s: %w", baseTmp, err) + } + } + err = acl.Chmod(baseTmp, 0777) + if err != nil { + return "", fmt.Errorf("failed to chmod %s: %w", baseTmp, err) + } + return baseTmp, nil +} diff --git a/testing/upgradetest/versions.go b/testing/upgradetest/versions.go index cdc66666fd7..ddb90097d5c 100644 --- a/testing/upgradetest/versions.go +++ b/testing/upgradetest/versions.go @@ -11,6 +11,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "sort" "strings" @@ -35,8 +36,8 @@ var ( Version_8_11_0_SNAPSHOT = version.NewParsedSemVer(8, 11, 0, "SNAPSHOT", "") // Version_8_13_0_SNAPSHOT is the minimum version for testing upgrading agent with the same hash Version_8_13_0_SNAPSHOT = version.NewParsedSemVer(8, 13, 0, "SNAPSHOT", "") - // Version_8_13_0 is the minimum version for proper unprivileged execution - Version_8_13_0 = version.NewParsedSemVer(8, 13, 0, "", "") + // Version_8_14_0_SNAPSHOT is the minimum version for proper unprivileged execution on all platforms + Version_8_14_0_SNAPSHOT = version.NewParsedSemVer(8, 14, 0, "SNAPSHOT", "") // ErrNoSnapshot is returned when a requested snapshot is not on the version list. ErrNoSnapshot = errors.New("failed to find a snapshot on the version list") @@ -254,3 +255,16 @@ func EnsureSnapshot(version string) string { } return version } + +// SupportsUnprivileged returns true when the version supports unprivileged mode. +func SupportsUnprivileged(versions ...*version.ParsedSemVer) bool { + for _, ver := range versions { + if ver.Less(*Version_8_13_0_SNAPSHOT) { + return false + } + if runtime.GOOS != define.Linux && ver.Less(*Version_8_14_0_SNAPSHOT) { + return false + } + } + return true +} diff --git a/testing/upgradetest/watcher.go b/testing/upgradetest/watcher.go index fd34dc82a7f..ab2312874fc 100644 --- a/testing/upgradetest/watcher.go +++ b/testing/upgradetest/watcher.go @@ -101,7 +101,7 @@ func WaitForNoWatcher(ctx context.Context, timeout time.Duration, interval time. for _, pid := range pids { proc, err := os.FindProcess(pid) if err == nil { - _ = proc.Kill() + _ = killNoneChildProcess(proc) } } // next interval ticker will check for no watcher and exit diff --git a/testing/upgradetest/watcher_unix.go b/testing/upgradetest/watcher_unix.go new file mode 100644 index 00000000000..858392145d2 --- /dev/null +++ b/testing/upgradetest/watcher_unix.go @@ -0,0 +1,16 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +//go:build !windows + +package upgradetest + +import "os" + +// killNoneChildProcess provides a way of killing a process that is not started as a child of this process. +// +// On Unix systems it just calls the native golang kill. +func killNoneChildProcess(proc *os.Process) error { + return proc.Kill() +} diff --git a/testing/upgradetest/watcher_windows.go b/testing/upgradetest/watcher_windows.go new file mode 100644 index 00000000000..15716f11bfd --- /dev/null +++ b/testing/upgradetest/watcher_windows.go @@ -0,0 +1,28 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +//go:build windows + +package upgradetest + +import ( + "os" + "syscall" +) + +// killNoneChildProcess provides a way of killing a process that is not started as a child of this process. +// +// On Windows when running in unprivileged mode the internal way that golang uses DuplicateHandle to perform the kill +// only works when the process is a child of this process. +func killNoneChildProcess(proc *os.Process) error { + h, e := syscall.OpenProcess(syscall.PROCESS_TERMINATE, false, uint32(proc.Pid)) + if e != nil { + return os.NewSyscallError("OpenProcess", e) + } + defer func() { + _ = syscall.CloseHandle(h) + }() + e = syscall.TerminateProcess(h, 1) + return os.NewSyscallError("TerminateProcess", e) +}