Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add unprivileged upgrade testing for Windows. #4642

Merged
2 changes: 1 addition & 1 deletion internal/pkg/agent/install/uninstall.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions internal/pkg/agent/install/uninstall_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

package install

import "os"

func isBlockingOnExe(_ error) bool {
return false
}
Expand All @@ -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()
}
17 changes: 17 additions & 0 deletions internal/pkg/agent/install/uninstall_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"errors"
"fmt"
"io/fs"
"os"
"syscall"
"unsafe"

Expand Down Expand Up @@ -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)
}
10 changes: 2 additions & 8 deletions testing/integration/upgrade_fleet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}

Expand Down
14 changes: 3 additions & 11 deletions testing/integration/upgrade_standalone_same_commit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"os"
"path"
"path/filepath"
"runtime"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -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"
Expand Down
12 changes: 3 additions & 9 deletions testing/integration/upgrade_standalone_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ package integration
import (
"context"
"fmt"
"runtime"
"testing"
"time"

Expand All @@ -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)
Expand Down
45 changes: 36 additions & 9 deletions testing/upgradetest/upgrader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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")
Expand All @@ -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())
}
}

Expand Down Expand Up @@ -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)
}
Expand All @@ -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
}
18 changes: 16 additions & 2 deletions testing/upgradetest/versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"fmt"
"os"
"path/filepath"
"runtime"
"sort"
"strings"

Expand All @@ -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")
Expand Down Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion testing/upgradetest/watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions testing/upgradetest/watcher_unix.go
Original file line number Diff line number Diff line change
@@ -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()
}
28 changes: 28 additions & 0 deletions testing/upgradetest/watcher_windows.go
Original file line number Diff line number Diff line change
@@ -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)
}
Loading