Clear pending (un)installs when installers are deleted (#23427)

For unreleased bug #23350, introduced because we're no longer
cascade-deleting _all_ related installs (related to #22996, #21654,
#22087)

# Checklist for submitter

- [x] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality

frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/DeleteSoftwareModal/DeleteSoftwareModal.tsx

@@ -18,13 +18,15 @@ const DELETE_SW_INSTALLED_DURING_SETUP_ERROR_MSG =
 interface IDeleteSoftwareModalProps {
   softwareId: number;
   teamId: number;
+  softwarePackageName?: string;
   onExit: () => void;
   onSuccess: () => void;
 }
 
 const DeleteSoftwareModal = ({
   softwareId,
   teamId,
+  softwarePackageName,
   onExit,
   onSuccess,
 }: IDeleteSoftwareModalProps) => {
@@ -51,7 +53,23 @@ const DeleteSoftwareModal = ({
   return (
     <Modal className={baseClass} title="Delete software" onExit={onExit}>
       <>
-        <p>Software won&apos;t be uninstalled from existing hosts.</p>
+        <p>
+          Software won&apos;t be uninstalled from existing hosts, but any
+          pending pending installs and uninstalls{" "}
+          {softwarePackageName ? (
+            <>
+              for <b> {softwarePackageName}</b>{" "}
+            </>
+          ) : (
+            ""
+          )}
+          will be canceled.
+        </p>
+        <p>
+          Installs or uninstalls currently running on a host will still
+          complete, but results won’t appear in Fleet.
+        </p>
+        <p>You cannot undo this action.</p>
         <div className="modal-cta-wrap">
           <Button variant="alert" onClick={onDeleteSoftware}>
             Delete

frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/SoftwarePackageCard/SoftwarePackageCard.tsx

@@ -395,6 +395,7 @@ const SoftwarePackageCard = ({
       {showDeleteModal && (
         <DeleteSoftwareModal
           softwareId={softwareId}
+          softwarePackageName={softwarePackage?.name}
           teamId={teamId}
           onExit={() => setShowDeleteModal(false)}
           onSuccess={onDeleteSuccess}

server/datastore/mysql/activities.go

@@ -261,12 +261,12 @@ func (ds *Datastore) ListHostUpcomingActivities(ctx context.Context, hostID uint
 		`SELECT
 			COUNT(*) c
 			FROM host_software_installs hsi
-			WHERE hsi.host_id = :host_id AND
+			WHERE hsi.host_id = :host_id AND hsi.software_installer_id IS NOT NULL AND
 				hsi.status = :software_status_install_pending`,
 		`SELECT
 			COUNT(*) c
 			FROM host_software_installs hsi
-			WHERE hsi.host_id = :host_id AND
+			WHERE hsi.host_id = :host_id AND hsi.software_installer_id IS NOT NULL AND
 				hsi.status = :software_status_uninstall_pending`,
 		`
 		SELECT

server/datastore/mysql/activities_test.go

@@ -417,10 +417,23 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) {
 		UserID:        u.ID,
 	})
 	require.NoError(t, err)
+	sw3, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{
+		InstallScript: "install to delete",
+		InstallerFile: installer,
+		StorageID:     uuid.NewString(),
+		Filename:      "todelete.pkg",
+		Title:         "todelete",
+		Source:        "apps",
+		Version:       "0.0.3",
+		UserID:        u.ID,
+	})
+	require.NoError(t, err)
 	sw1Meta, err := ds.GetSoftwareInstallerMetadataByID(ctx, sw1)
 	require.NoError(t, err)
 	sw2Meta, err := ds.GetSoftwareInstallerMetadataByID(ctx, sw2)
 	require.NoError(t, err)
+	sw3Meta, err := ds.GetSoftwareInstallerMetadataByID(ctx, sw3)
+	require.NoError(t, err)
 
 	// insert a VPP app
 	vppCommand1, vppCommand2 := "vpp-command-1", "vpp-command-2"
@@ -523,7 +536,15 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) {
 	h2SelfService, err := ds.InsertSoftwareInstallRequest(noUserCtx, h2.ID, sw1Meta.InstallerID, true, nil)
 	require.NoError(t, err)
 
-	// nothing for h3
+	// create pending install and uninstall requests for h3 that will be deleted
+	_, err = ds.InsertSoftwareInstallRequest(ctx, h3.ID, sw3Meta.InstallerID, false, nil)
+	require.NoError(t, err)
+	err = ds.InsertSoftwareUninstallRequest(ctx, "uninstallRun", h3.ID, sw3Meta.InstallerID)
+	require.NoError(t, err)
+
+	// delete installer (should clear pending requests)
+	err = ds.DeleteSoftwareInstaller(ctx, sw3Meta.InstallerID)
+	require.NoError(t, err)
 
 	// force-set the order of the created_at timestamps
 	endTime := SetOrderedCreatedAtTimestamps(t, ds, time.Now(), "host_script_results", "execution_id", h1A, h1B)

server/datastore/mysql/software_installers.go

@@ -414,38 +414,45 @@ var (
 )
 
 func (ds *Datastore) DeleteSoftwareInstaller(ctx context.Context, id uint) error {
-	// allow delete only if install_during_setup is false
-	res, err := ds.writer(ctx).ExecContext(ctx, `DELETE FROM software_installers WHERE id = ? AND install_during_setup = 0`, id)
-	if err != nil {
-		if isMySQLForeignKey(err) {
-			// Check if the software installer is referenced by a policy automation.
-			var count int
-			if err := sqlx.GetContext(ctx, ds.reader(ctx), &count, `SELECT COUNT(*) FROM policies WHERE software_installer_id = ?`, id); err != nil {
-				return ctxerr.Wrapf(ctx, err, "getting reference from policies")
-			}
-			if count > 0 {
-				return errDeleteInstallerWithAssociatedPolicy
-			}
+	return ds.withTx(ctx, func(tx sqlx.ExtContext) error {
+		err := ds.runInstallerUpdateSideEffectsInTransaction(ctx, tx, id, true, true)
+		if err != nil {
+			return ctxerr.Wrap(ctx, err, "clean up related installs and uninstalls")
 		}
-		return ctxerr.Wrap(ctx, err, "delete software installer")
-	}
 
-	rows, _ := res.RowsAffected()
-	if rows == 0 {
-		// could be that the software installer does not exist, or it is installed
-		// during setup, do additional check.
-		var installDuringSetup bool
-		if err := sqlx.GetContext(ctx, ds.reader(ctx), &installDuringSetup,
-			`SELECT install_during_setup FROM software_installers WHERE id = ?`, id); err != nil && !errors.Is(err, sql.ErrNoRows) {
-			return ctxerr.Wrap(ctx, err, "check if software installer is installed during setup")
+		// allow delete only if install_during_setup is false
+		res, err := tx.ExecContext(ctx, `DELETE FROM software_installers WHERE id = ? AND install_during_setup = 0`, id)
+		if err != nil {
+			if isMySQLForeignKey(err) {
+				// Check if the software installer is referenced by a policy automation.
+				var count int
+				if err := sqlx.GetContext(ctx, tx, &count, `SELECT COUNT(*) FROM policies WHERE software_installer_id = ?`, id); err != nil {
+					return ctxerr.Wrapf(ctx, err, "getting reference from policies")
+				}
+				if count > 0 {
+					return errDeleteInstallerWithAssociatedPolicy
+				}
+			}
+			return ctxerr.Wrap(ctx, err, "delete software installer")
 		}
-		if installDuringSetup {
-			return errDeleteInstallerInstalledDuringSetup
+
+		rows, _ := res.RowsAffected()
+		if rows == 0 {
+			// could be that the software installer does not exist, or it is installed
+			// during setup, do additional check.
+			var installDuringSetup bool
+			if err := sqlx.GetContext(ctx, tx, &installDuringSetup,
+				`SELECT install_during_setup FROM software_installers WHERE id = ?`, id); err != nil && !errors.Is(err, sql.ErrNoRows) {
+				return ctxerr.Wrap(ctx, err, "check if software installer is installed during setup")
+			}
+			if installDuringSetup {
+				return errDeleteInstallerInstalledDuringSetup
+			}
+			return notFound("SoftwareInstaller").WithID(id)
 		}
-		return notFound("SoftwareInstaller").WithID(id)
-	}
 
-	return nil
+		return nil
+	})
 }
 
 func (ds *Datastore) InsertSoftwareInstallRequest(ctx context.Context, hostID uint, softwareInstallerID uint, selfService bool, policyID *uint) (string, error) {
@@ -847,13 +854,59 @@ WHERE
   team_id = ?
 `
 
+	const deleteAllPendingUninstallScriptExecutions = `
+		DELETE FROM host_script_results WHERE execution_id IN (
+			SELECT execution_id FROM host_software_installs WHERE status = 'pending_uninstall'
+				AND software_installer_id IN (
+					SELECT id FROM software_installers WHERE global_or_team_id = ?
+			   )
+		)
+`
+	const deleteAllPendingSoftwareInstalls = `
+		DELETE FROM host_software_installs
+		   WHERE status IN('pending_install', 'pending_uninstall')
+				AND software_installer_id IN (
+					SELECT id FROM software_installers WHERE global_or_team_id = ?
+			   )
+`
+	const markAllSoftwareInstallsAsRemoved = `
+		UPDATE host_software_installs SET removed = TRUE
+			WHERE status IS NOT NULL AND host_deleted_at IS NULL
+				AND software_installer_id IN (
+					SELECT id FROM software_installers WHERE global_or_team_id = ?
+			   )
+`
+
 	const deleteAllInstallersInTeam = `
 DELETE FROM
   software_installers
 WHERE
   global_or_team_id = ?
 `
 
+	const deletePendingUninstallScriptExecutionsNotInList = `
+		DELETE FROM host_script_results WHERE execution_id IN (
+			SELECT execution_id FROM host_software_installs WHERE status = 'pending_uninstall'
+				AND software_installer_id IN (
+					SELECT id FROM software_installers WHERE global_or_team_id = ? AND title_id NOT IN (?)
+			   )
+		)
+`
+	const deletePendingSoftwareInstallsNotInList = `
+		DELETE FROM host_software_installs
+		   WHERE status IN('pending_install', 'pending_uninstall')
+				AND software_installer_id IN (
+					SELECT id FROM software_installers WHERE global_or_team_id = ? AND title_id NOT IN (?)
+			   )
+`
+	const markSoftwareInstallsNotInListAsRemoved = `
+		UPDATE host_software_installs SET removed = TRUE
+			WHERE status IS NOT NULL AND host_deleted_at IS NULL
+				AND software_installer_id IN (
+					SELECT id FROM software_installers WHERE global_or_team_id = ? AND title_id NOT IN (?)
+			   )
+`
+
 	const unsetInstallersNotInListFromPolicies = `
 UPDATE
   policies
@@ -963,9 +1016,23 @@ ON DUPLICATE KEY UPDATE
 			if _, err := tx.ExecContext(ctx, unsetAllInstallersFromPolicies, globalOrTeamID); err != nil {
 				return ctxerr.Wrap(ctx, err, "unset all obsolete installers in policies")
 			}
+
+			if _, err := tx.ExecContext(ctx, deleteAllPendingUninstallScriptExecutions, globalOrTeamID); err != nil {
+				return ctxerr.Wrap(ctx, err, "delete all pending uninstall script executions")
+			}
+
+			if _, err := tx.ExecContext(ctx, deleteAllPendingSoftwareInstalls, globalOrTeamID); err != nil {
+				return ctxerr.Wrap(ctx, err, "delete all pending host software install records")
+			}
+
+			if _, err := tx.ExecContext(ctx, markAllSoftwareInstallsAsRemoved, globalOrTeamID); err != nil {
+				return ctxerr.Wrap(ctx, err, "mark all host software installs as removed")
+			}
+
 			if _, err := tx.ExecContext(ctx, deleteAllInstallersInTeam, globalOrTeamID); err != nil {
 				return ctxerr.Wrap(ctx, err, "delete obsolete software installers")
 			}
+
 			return nil
 		}
 
@@ -1010,6 +1077,30 @@ ON DUPLICATE KEY UPDATE
 			}
 		}
 
+		stmt, args, err = sqlx.In(deletePendingUninstallScriptExecutionsNotInList, globalOrTeamID, titleIDs)
+		if err != nil {
+			return ctxerr.Wrap(ctx, err, "build statement to delete pending uninstall script executions")
+		}
+		if _, err := tx.ExecContext(ctx, stmt, args...); err != nil {
+			return ctxerr.Wrap(ctx, err, "delete obsolete pending uninstall script executions")
+		}
+
+		stmt, args, err = sqlx.In(deletePendingSoftwareInstallsNotInList, globalOrTeamID, titleIDs)
+		if err != nil {
+			return ctxerr.Wrap(ctx, err, "build statement to delete pending software installs")
+		}
+		if _, err := tx.ExecContext(ctx, stmt, args...); err != nil {
+			return ctxerr.Wrap(ctx, err, "delete obsolete pending host software install records")
+		}
+
+		stmt, args, err = sqlx.In(markSoftwareInstallsNotInListAsRemoved, globalOrTeamID, titleIDs)
+		if err != nil {
+			return ctxerr.Wrap(ctx, err, "build statement to mark obsolete host software installs as removed")
+		}
+		if _, err := tx.ExecContext(ctx, stmt, args...); err != nil {
+			return ctxerr.Wrap(ctx, err, "mark obsolete host software installs as removed")
+		}
+
 		stmt, args, err = sqlx.In(deleteInstallersNotInList, globalOrTeamID, titleIDs)
 		if err != nil {
 			return ctxerr.Wrap(ctx, err, "build statement to delete obsolete installers")

server/datastore/mysql/software_installers_test.go

@@ -548,21 +548,23 @@ func testGetSoftwareInstallResult(t *testing.T, ds *Datastore) {
 			require.NoError(t, err)
 			require.Equal(t, swFilename, res.SoftwarePackage)
 
-			// delete installer to confirm that we can still access the install record
+			// delete installer to confirm that we can still access the install record (unless pending)
 			err = ds.DeleteSoftwareInstaller(ctx, installerID)
 			require.NoError(t, err)
 
+			if tc.expectedStatus == fleet.SoftwareInstallPending { // expect pending to be deleted
+				_, err = ds.GetSoftwareInstallResults(ctx, installUUID)
+				require.Error(t, err, notFound("HostSoftwareInstallerResult"))
+				return
+			}
+
 			ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
 				// ensure version is not changed, though we don't expose it yet
 				var version string
 				err := sqlx.GetContext(ctx, q, &version, `SELECT "version" FROM host_software_installs WHERE execution_id = ?`, installUUID)
 				require.NoError(t, err)
 				require.Equal(t, "1.11", version)
 
-				// let's also set the removed flag to ensure that the status we're pulling doesn't change
-				_, err = q.ExecContext(ctx, `UPDATE host_software_installs SET removed = 1 WHERE execution_id = ?`, installUUID)
-				require.NoError(t, err)
-
 				return nil
 			})
 

server/service/integration_enterprise_test.go

@@ -11172,6 +11172,26 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallersSideEffec
 	orbitKey := setOrbitEnrollment(t, h, s.ds)
 	h.OrbitNodeKey = &orbitKey
 
+	// create another host that doesn't have fleetd installed
+	h2, err := s.ds.NewHost(context.Background(), &fleet.Host{
+		DetailUpdatedAt: time.Now(),
+		LabelUpdatedAt:  time.Now(),
+		PolicyUpdatedAt: time.Now(),
+		SeenTime:        time.Now().Add(-1 * time.Minute),
+		OsqueryHostID:   ptr.String(t.Name() + uuid.New().String()),
+		NodeKey:         ptr.String(t.Name() + uuid.New().String()),
+		Hostname:        fmt.Sprintf("%sbar.local", t.Name()),
+		Platform:        "linux",
+	})
+	require.NoError(t, err)
+	err = s.ds.AddHostsToTeam(context.Background(), &tm.ID, []uint{h2.ID})
+	require.NoError(t, err)
+	h2.TeamID = &tm.ID
+
+	// host installs fleetd
+	orbitKey2 := setOrbitEnrollment(t, h2, s.ds)
+	h2.OrbitNodeKey = &orbitKey2
+
 	// install software
 	installResp := installSoftwareResponse{}
 	s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/software/%d/install", h.ID, titlesResp.SoftwareTitles[0].ID), nil, http.StatusAccepted, &installResp)
@@ -11295,6 +11315,38 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallersSideEffec
 	installDetailsResp := getSoftwareInstallResultsResponse{}
 	s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/software/install/%s/results", installUUID), nil, http.StatusOK, &installDetailsResp)
 	require.Equal(t, fleet.SoftwareInstalled, installDetailsResp.Results.Status)
+
+	// queue another install before we delete the installer via batch
+	pendingResp := installSoftwareResponse{}
+	s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/software/%d/install", h.ID, titlesResp.SoftwareTitles[0].ID), nil, http.StatusAccepted, &pendingResp)
+
+	// install should show as pending
+	s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/software", h.ID), nil, http.StatusOK, &afterPreinstallHostResp)
+	require.Equal(t, fleet.SoftwareInstallPending, *afterPreinstallHostResp.Software[0].Status)
+	installUUID = afterPreinstallHostResp.Software[0].SoftwarePackage.LastInstall.InstallUUID
+
+	// queue an uninstall on another host
+	uninstallResp := installSoftwareResponse{}
+	s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/software/%d/uninstall", h2.ID, titlesResp.SoftwareTitles[0].ID), nil, http.StatusAccepted, &uninstallResp)
+
+	// uninstall should show as pending
+	s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/software", h2.ID), nil, http.StatusOK, &afterPreinstallHostResp)
+	require.Equal(t, fleet.SoftwareUninstallPending, *afterPreinstallHostResp.Software[0].Status)
+
+	// delete all installers
+	s.DoJSON("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: []fleet.SoftwareInstallerPayload{}}, http.StatusOK, &batchResponse, "team_name", tm.Name)
+	packages = waitBatchSetSoftwareInstallersCompleted(t, s, tm.Name, batchResponse.RequestUUID)
+	require.Len(t, packages, 0)
+
+	// software should no longer exist on either host
+	s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/software", h.ID), nil, http.StatusOK, &afterPreinstallHostResp)
+	require.Len(t, afterPreinstallHostResp.Software, 0)
+	s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/software", h2.ID), nil, http.StatusOK, &afterPreinstallHostResp)
+	require.Len(t, afterPreinstallHostResp.Software, 0)
+
+	// pending install record should not exist
+	installDetailsResp = getSoftwareInstallResultsResponse{}
+	s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/software/install/%s/results", installUUID), nil, http.StatusNotFound, &installDetailsResp)
 }
 
 func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallersWithPoliciesAssociated() {