[Windows] Adding property in installer to support driver rollback (#2…

…6711)

Co-authored-by: clarkb7 <[email protected]>
Co-authored-by: derekwbrown <[email protected]>

.gitlab/kitchen_testing/new-e2e_testing/windows.yml

@@ -40,6 +40,7 @@
       - E2E_MSI_TEST: TestRepair
       - E2E_MSI_TEST: TestUpgrade
       - E2E_MSI_TEST: TestUpgradeRollback
+      - E2E_MSI_TEST: TestUpgradeRollbackWithoutCWS
       - E2E_MSI_TEST: TestUpgradeChangeUser
       - E2E_MSI_TEST: TestUpgradeFromV5
       - E2E_MSI_TEST: TestAgentUser/user_only
@@ -60,6 +61,7 @@
       - E2E_MSI_TEST: TestRepair
       - E2E_MSI_TEST: TestUpgrade
       - E2E_MSI_TEST: TestUpgradeRollback
+      - E2E_MSI_TEST: TestUpgradeRollbackWithoutCWS
       - E2E_MSI_TEST: TestUpgradeChangeUser
       - E2E_MSI_TEST: TestUpgradeFromV5
       - E2E_MSI_TEST: TestAgentUser/user_only

release.json

@@ -17,12 +17,12 @@
         "WINDOWS_DDNPM_SHASUM": "de6a2f437b906d1d0f3cfc9222c7f686b3d69726355c940476448a34535064c8",
         "SECURITY_AGENT_POLICIES_VERSION": "master",
         "WINDOWS_DDPROCMON_DRIVER": "release-signed",
-        "WINDOWS_DDPROCMON_VERSION": "1.0.1",
-        "WINDOWS_DDPROCMON_SHASUM": "7c13ba75f7a30704a6a4082e4cffc819c846fd6061c53372c8b9908fee11d621",
+        "WINDOWS_DDPROCMON_VERSION": "1.0.2",
+        "WINDOWS_DDPROCMON_SHASUM": "cf55e5163659dbbfac0c0cced6559a3042107da9e4df8140bea17067278061ab",
         "WINDOWS_APMINJECT_COMMENT": "The WINDOWS_APMINJECT entries below should NOT be added to the release targets",
         "WINDOWS_APMINJECT_MODULE": "release-signed",
-        "WINDOWS_APMINJECT_VERSION": "1.1.1",
-        "WINDOWS_APMINJECT_SHASUM": "c5c228f6030ce2e19b8bfb28bd054dc246c7f1c799431e73018e6cb46ee59d2f"
+        "WINDOWS_APMINJECT_VERSION": "1.1.2",
+        "WINDOWS_APMINJECT_SHASUM": "27d85ab3a26c123b2655a838b0bec099268de2f2b86d2b8a74232e65f4f8f05f"
     },
     "nightly-a7": {
         "INTEGRATIONS_CORE_VERSION": "master",
@@ -36,12 +36,12 @@
         "WINDOWS_DDNPM_SHASUM": "de6a2f437b906d1d0f3cfc9222c7f686b3d69726355c940476448a34535064c8",
         "SECURITY_AGENT_POLICIES_VERSION": "master",
         "WINDOWS_DDPROCMON_DRIVER": "release-signed",
-        "WINDOWS_DDPROCMON_VERSION": "1.0.1",
-        "WINDOWS_DDPROCMON_SHASUM": "7c13ba75f7a30704a6a4082e4cffc819c846fd6061c53372c8b9908fee11d621",
+        "WINDOWS_DDPROCMON_VERSION": "1.0.2",
+        "WINDOWS_DDPROCMON_SHASUM": "cf55e5163659dbbfac0c0cced6559a3042107da9e4df8140bea17067278061ab",
         "WINDOWS_APMINJECT_COMMENT": "The WINDOWS_APMINJECT entries below should NOT be added to the release targets",
         "WINDOWS_APMINJECT_MODULE": "release-signed",
-        "WINDOWS_APMINJECT_VERSION": "1.1.1",
-        "WINDOWS_APMINJECT_SHASUM": "c5c228f6030ce2e19b8bfb28bd054dc246c7f1c799431e73018e6cb46ee59d2f"
+        "WINDOWS_APMINJECT_VERSION": "1.1.2",
+        "WINDOWS_APMINJECT_SHASUM": "27d85ab3a26c123b2655a838b0bec099268de2f2b86d2b8a74232e65f4f8f05f"
     },
     "release-a6": {
         "INTEGRATIONS_CORE_VERSION": "7.55.0-rc.3",

releasenotes/notes/windows-driver-rollback-36c4de3614d988b9.yaml

@@ -0,0 +1,11 @@
+# Each section from every release note are combined when the
+# CHANGELOG.rst is rendered. So the text needs to be worded so that
+# it does not depend on any information only available in another
+# section. This may mean repeating some details, but each section
+# must be readable independently of the other.
+#
+# Each section note must be formatted as reStructuredText.
+---
+fixes:
+  - |
+    Windows: Added driver rollback properties to ensure that all services and drivers are uninstalled or rolled back after an installation or upgrade failure.

test/new-e2e/tests/windows/install-test/base.go

@@ -116,59 +116,49 @@ func (s *baseAgentMSISuite) installAgentPackage(vm *components.RemoteHost, agent
 	return remoteMSIPath
 }
 
-func (s *baseAgentMSISuite) uninstallAgentAndRunUninstallTests(t *Tester) bool {
+func (s *baseAgentMSISuite) uninstallAgent() bool {
+	host := s.Env().RemoteHost
 	return s.T().Run("uninstall the agent", func(tt *testing.T) {
-		// Get config dir from registry before uninstalling
-		configDir, err := windowsAgent.GetConfigRootFromRegistry(t.host)
-		require.NoError(tt, err)
-		tt.Cleanup(func() {
-			// remove the agent config for a cleaner uninstall
-			tt.Logf("Removing agent configuration files")
-			err = t.host.RemoveAll(configDir)
-			require.NoError(tt, err)
-		})
-
 		if !tt.Run("uninstall", func(tt *testing.T) {
-			err := windowsAgent.UninstallAgent(t.host, filepath.Join(s.OutputDir, "uninstall.log"))
+			err := windowsAgent.UninstallAgent(host, filepath.Join(s.OutputDir, "uninstall.log"))
 			require.NoError(tt, err, "should uninstall the agent")
 		}) {
 			tt.Fatal("uninstall failed")
 		}
+	})
+}
+
+func (s *baseAgentMSISuite) uninstallAgentAndRunUninstallTests(t *Tester) bool {
+	host := s.Env().RemoteHost
+
+	if !s.uninstallAgent() {
+		return false
+	}
 
-		AssertDoesNotRemoveSystemFiles(s.T(), s.Env().RemoteHost, s.beforeInstall)
+	return s.T().Run("validate uninstall", func(tt *testing.T) {
+		AssertDoesNotRemoveSystemFiles(s.T(), host, s.beforeInstall)
 
 		s.Run("uninstall does not change system file permissions", func() {
-			AssertDoesNotChangePathPermissions(s.T(), s.Env().RemoteHost, s.beforeInstallPerms)
+			AssertDoesNotChangePathPermissions(s.T(), host, s.beforeInstallPerms)
 		})
 
 		t.TestUninstallExpectations(tt)
 	})
 }
 
-func (s *baseAgentMSISuite) installAndTestPreviousAgentVersion(vm *components.RemoteHost, agentPackage *windowsAgent.Package, options ...windowsAgent.InstallAgentOption) *Tester {
-	// create the tester
-	t := s.newTester(vm,
-		WithAgentPackage(agentPackage),
-		WithPreviousVersion(),
-	)
-
+func (s *baseAgentMSISuite) installAndTestPreviousAgentVersion(vm *components.RemoteHost, agentPackage *windowsAgent.Package, options ...windowsAgent.InstallAgentOption) {
 	_ = s.installAgentPackage(vm, agentPackage, options...)
-
-	// Ensure the agent is functioning properly to provide a proper foundation for the test
-	if !t.TestInstallExpectations(s.T()) {
-		s.T().FailNow()
-	}
-
-	return t
+	RequireAgentVersionRunningWithNoErrors(s.T(), s.NewTestClientForHost(vm), agentPackage.AgentVersion())
 }
 
 // installLastStable installs the last stable agent package on the VM, runs tests, and returns the Tester
-func (s *baseAgentMSISuite) installLastStable(vm *components.RemoteHost, options ...windowsAgent.InstallAgentOption) *Tester {
-
+func (s *baseAgentMSISuite) installAndTestLastStable(vm *components.RemoteHost, options ...windowsAgent.InstallAgentOption) *windowsAgent.Package {
 	previousAgentPackage, err := windowsAgent.GetLastStablePackageFromEnv()
 	s.Require().NoError(err, "should get last stable agent package from env")
 
-	return s.installAndTestPreviousAgentVersion(vm, previousAgentPackage, options...)
+	s.installAndTestPreviousAgentVersion(vm, previousAgentPackage, options...)
+
+	return previousAgentPackage
 }
 
 func (s *baseAgentMSISuite) readYamlConfig(host *components.RemoteHost, path string) (map[string]any, error) {

test/new-e2e/tests/windows/install-test/installtester.go

@@ -35,8 +35,7 @@ type Tester struct {
 	host              *components.RemoteHost
 	InstallTestClient *common.TestClient
 
-	agentPackage      *windowsAgent.Package
-	isPreviousVersion bool
+	agentPackage *windowsAgent.Package
 
 	expectedUserName   string
 	expectedUserDomain string
@@ -109,14 +108,6 @@ func WithAgentPackage(agentPackage *windowsAgent.Package) TesterOption {
 	}
 }
 
-// WithPreviousVersion sets the Tester to expect a previous version of the agent to be installed
-// and will not run all tests since expectations may have changed.
-func WithPreviousVersion() TesterOption {
-	return func(t *Tester) {
-		t.isPreviousVersion = true
-	}
-}
-
 // WithExpectedAgentUserName sets the expected user name the agent should run as
 // the domain remains the default for the host.
 func WithExpectedAgentUserName(user string) TesterOption {
@@ -245,12 +236,7 @@ func (t *Tester) TestUninstallExpectations(tt *testing.T) {
 	assert.NoError(tt, err, "uninstall should not remove agent user")
 
 	for _, serviceName := range servicetest.ExpectedInstalledServices() {
-		conf, err := windows.GetServiceConfig(t.host, serviceName)
-		if err == nil && windows.IsKernelModeServiceType(conf.ServiceType) {
-			// TODO WKINT-410: kernel mode services are not removed on install rollback
-			tt.Logf("WKINT-410: Skipping known failure, kernel mode service not removed: %s", serviceName)
-			continue
-		}
+		_, err := windows.GetServiceConfig(t.host, serviceName)
 		assert.Errorf(tt, err, "uninstall should remove service %s", serviceName)
 	}
 
@@ -268,11 +254,6 @@ func (t *Tester) TestUninstallExpectations(tt *testing.T) {
 	})
 }
 
-// Only do some basic checks on the agent since it's a previous version
-func (t *Tester) testPreviousVersionExpectations(tt *testing.T) {
-	RequireAgentRunningWithNoErrors(tt, t.InstallTestClient)
-}
-
 // More in depth checks on current version
 func (t *Tester) testCurrentVersionExpectations(tt *testing.T) {
 	common.CheckInstallation(tt, t.InstallTestClient)
@@ -588,10 +569,6 @@ func (t *Tester) TestInstallExpectations(tt *testing.T) bool {
 		}) {
 			tt.FailNow()
 		}
-		if t.isPreviousVersion {
-			t.testPreviousVersionExpectations(tt)
-		} else {
-			t.testCurrentVersionExpectations(tt)
-		}
+		t.testCurrentVersionExpectations(tt)
 	})
 }

test/new-e2e/tests/windows/install-test/npm_test.go

@@ -147,7 +147,7 @@ func (s *testNPMInstallSuite) TearDownSuite() {
 	s.cleanupOnSuccessInDevMode()
 }
 
-func (s *testNPMInstallSuite) installPreviousAgentVersion(vm *components.RemoteHost, options ...windowsAgent.InstallAgentOption) *Tester {
+func (s *testNPMInstallSuite) installPreviousAgentVersion(vm *components.RemoteHost, options ...windowsAgent.InstallAgentOption) {
 	if s.url == "" {
 		url, err := windowsAgent.GetStableMSIURL(s.previousVersion, "x86_64")
 		s.Require().NoError(err, "should get MSI URL for version %s", s.previousVersion)
@@ -157,7 +157,7 @@ func (s *testNPMInstallSuite) installPreviousAgentVersion(vm *components.RemoteH
 		Version: s.previousVersion,
 		URL:     s.url,
 	}
-	return s.baseAgentMSISuite.installAndTestPreviousAgentVersion(vm, previousAgentPackage, options...)
+	s.baseAgentMSISuite.installAndTestPreviousAgentVersion(vm, previousAgentPackage, options...)
 }
 
 func (s *testNPMInstallSuite) isNPMInstalled() bool {

test/new-e2e/tests/windows/install-test/upgrade_test.go

@@ -8,13 +8,17 @@ package installtest
 import (
 	"fmt"
 	"path/filepath"
+	"slices"
+	"strings"
 
 	windowsCommon "github.com/DataDog/datadog-agent/test/new-e2e/tests/windows/common"
 	windowsAgent "github.com/DataDog/datadog-agent/test/new-e2e/tests/windows/common/agent"
 	servicetest "github.com/DataDog/datadog-agent/test/new-e2e/tests/windows/install-test/service-test"
 
-	"github.com/stretchr/testify/require"
 	"testing"
+
+	"github.com/stretchr/testify/require"
+	"github.com/stretchr/testify/suite"
 )
 
 // TestUpgrade tests upgrading the agent from LAST_STABLE_VERSION to WINDOWS_AGENT_VERSION
@@ -35,7 +39,7 @@ func (s *testUpgradeSuite) TestUpgrade() {
 	vm := s.Env().RemoteHost
 
 	// install previous version
-	_ = s.installAndTestPreviousAgentVersion(vm, s.previousAgentPackge)
+	s.installAndTestPreviousAgentVersion(vm, s.previousAgentPackge)
 
 	// simulate upgrading from a version that didn't have the runtime-security.d directory
 	// to ensure upgrade places new config files.
@@ -81,7 +85,7 @@ func (s *testUpgradeRollbackSuite) TestUpgradeRollback() {
 	vm := s.Env().RemoteHost
 
 	// install previous version
-	previousTester := s.installLastStable(vm)
+	previousAgentPackage := s.installAndTestLastStable(vm)
 
 	// upgrade to the new version, but intentionally fail
 	if !s.Run(fmt.Sprintf("upgrade to %s with rollback", s.AgentPackage.AgentVersion()), func() {
@@ -101,11 +105,89 @@ func (s *testUpgradeRollbackSuite) TestUpgradeRollback() {
 	s.Require().NoError(err, "agent service should start after rollback")
 
 	// the previous version should be functional
-	if !previousTester.TestInstallExpectations(s.T()) {
+	RequireAgentVersionRunningWithNoErrors(s.T(), s.NewTestClientForHost(vm), previousAgentPackage.AgentVersion())
+
+	// Ensure services are still installed
+	// NOTE: will need to update this if we add or remove services
+	_, err = windowsCommon.GetServiceConfigMap(vm, servicetest.ExpectedInstalledServices())
+	s.Assert().NoError(err, "services should still be installed")
+
+	s.uninstallAgent()
+}
+
+// TestUpgradeRollbackWithoutCWS tests that when upgrading the agent from X.51 to WINDOWS_AGENT_VERSION
+// rolls back, that the ddprocmon service is not installed.
+func TestUpgradeRollbackWithoutCWS(t *testing.T) {
+	s := &testUpgradeRollbackWithoutCWSSuite{}
+	run(t, s)
+}
+
+type testUpgradeRollbackWithoutCWSSuite struct {
+	baseAgentMSISuite
+	previousAgentPackage *windowsAgent.Package
+}
+
+func (s *testUpgradeRollbackWithoutCWSSuite) SetupSuite() {
+	if setupSuite, ok := any(&s.baseAgentMSISuite).(suite.SetupAllSuite); ok {
+		setupSuite.SetupSuite()
+	}
+
+	// CWS was GA in X.52, so start by installing X.51
+	// match X to the major version from WINDOWS_AGENT_VERSION
+	var err error
+	majorVersion := strings.Split(s.AgentPackage.Version, ".")[0]
+	s.previousAgentPackage = &windowsAgent.Package{
+		Version: fmt.Sprintf("%s.51.0-1", majorVersion),
+		Arch:    "x86_64",
+	}
+	s.previousAgentPackage.URL, err = windowsAgent.GetStableMSIURL(s.previousAgentPackage.Version, s.previousAgentPackage.Arch)
+	s.Require().NoError(err, "should get stable agent package URL")
+}
+
+func (s *testUpgradeRollbackWithoutCWSSuite) TestUpgradeRollbackWithoutCWS() {
+	vm := s.Env().RemoteHost
+
+	// install previous version
+	s.installAndTestPreviousAgentVersion(vm, s.previousAgentPackage)
+
+	// upgrade to the new version, but intentionally fail
+	if !s.Run(fmt.Sprintf("upgrade to %s with rollback", s.AgentPackage.AgentVersion()), func() {
+		_, err := windowsAgent.InstallAgent(vm,
+			windowsAgent.WithPackage(s.AgentPackage),
+			windowsAgent.WithWixFailWhenDeferred(),
+			windowsAgent.WithInstallLogFile(filepath.Join(s.OutputDir, "upgrade.log")),
+		)
+		s.Require().Error(err, "should fail to install agent %s", s.AgentPackage.AgentVersion())
+	}) {
 		s.T().FailNow()
 	}
 
-	s.uninstallAgentAndRunUninstallTests(previousTester)
+	// TODO: we shouldn't have to start the agent manually after rollback
+	//       but the kitchen tests did too.
+	err := windowsCommon.StartService(vm, "DatadogAgent")
+	s.Require().NoError(err, "agent service should start after rollback")
+
+	// the previous version should be functional
+	RequireAgentVersionRunningWithNoErrors(s.T(), s.NewTestClientForHost(vm), s.previousAgentPackage.AgentVersion())
+
+	// Ensure CWS services are not installed, but other services are
+	cwsServices := []string{
+		"ddprocmon",
+		"datadog-security-agent",
+	}
+	// NOTE: will need to update this if we add or remove services
+	expectedServices := servicetest.ExpectedInstalledServices()
+	expectedServices = slices.DeleteFunc(expectedServices, func(s string) bool {
+		return slices.Contains(cwsServices, s)
+	})
+	_, err = windowsCommon.GetServiceConfigMap(vm, expectedServices)
+	s.Assert().NoError(err, "services should still be installed")
+	for _, service := range cwsServices {
+		_, err = windowsCommon.GetServiceConfig(vm, service)
+		s.Assert().Error(err, "service %s should not be installed", service)
+	}
+
+	s.uninstallAgent()
 }
 
 func TestUpgradeChangeUser(t *testing.T) {
@@ -125,7 +207,7 @@ func (s *testUpgradeChangeUserSuite) TestUpgradeChangeUser() {
 	s.Require().NotEqual(oldUserName, newUserName, "new user name should be different from the default")
 
 	// install previous version with defaults
-	_ = s.installLastStable(host)
+	s.installAndTestLastStable(host)
 
 	// upgrade to the new version
 	if !s.Run(fmt.Sprintf("upgrade to %s", s.AgentPackage.AgentVersion()), func() {

tools/windows/DatadogAgentInstaller/CustomActions.Tests/InstallState/InstallStateTestSetup.cs

@@ -15,6 +15,8 @@ public class InstallStateTestSetup : SessionTestBaseSetup
         public Mock<IRegistryServices> RegistryServices { get; } = new();
         public Mock<IServiceController> ServiceController { get; } = new();
 
+        public Mock<INativeMethods> NativeMethods { get; } = new();
+
         public InstallStateTestSetup()
         {
             ServiceController.SetupGet(s => s.Services).Returns(new WindowsService[] { });
@@ -25,7 +27,8 @@ public InstallStateCustomActions Create()
             return new InstallStateCustomActions(
                 Session.Object,
                 RegistryServices.Object,
-                ServiceController.Object);
+                ServiceController.Object,
+                NativeMethods.Object);
         }
 
         public InstallStateTestSetup WithRegistryKey(Registries registry, string path, Dictionary<string, object> keys)