Add unprivileged upgrade testing for Windows. (#4642)

* Add unprivileged upgrade testing for Windows.

* Adjust to 8.14.0-SNAPSHOT as minimum.

* Add SeCreateSymbolicLinkPrivilege

* Simplify if unprivileged mode is supported.

* Fix file access for upgrade.

* Fix kill watcher process on Windows in unprivileged mode.

* Fix imports.

* Fix kill non child process in tests.

* Fix lint.

Author: blakerouse

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

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()
+}

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)
+}

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())
 		}
 	}
 

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"

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)

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
+}

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
+}

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

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()
+}

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)
+}