Uninstalling software in a host also updates software table (#10540)

https://github.com/fleetdm/confidential/issues/1968

It's ready for review but I still need to load test this.

- [X] Changes file added for user-visible changes in `changes/` or
`orbit/changes/`.
See [Changes
files](https://fleetdm.com/docs/contributing/committing-changes#changes-files)
for more information.
- ~[ ] Documented any API changes (docs/Using-Fleet/REST-API.md or
docs/Contributing/API-for-contributors.md)~
- ~[ ] Documented any permissions changes~
- [X] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)
- [X] Added support on fleet's osquery simulator `cmd/osquery-perf` for
new osquery data ingestion features.
- [X] Added/updated tests
- [X] Manual QA for all new/changed functionality
  - ~For Orbit and Fleet Desktop changes:~
- ~[ ] Manual QA must be performed in the three main OSs, macOS, Windows
and Linux.~
- ~[ ] Auto-update manual QA, from released version of component to new
version (see [tools/tuf/test](../tools/tuf/test/README.md)).~

changes/1968-remove-software-entries-if-uninstalled-by-all

@@ -0,0 +1 @@
+* Uninstalling applications from hosts will remove the corresponding entry in `software` if no more hosts have the application installed.

cmd/osquery-perf/agent.go

@@ -228,6 +228,10 @@ type softwareEntityCount struct {
 	vulnerable                   int
 	withLastOpened               int
 	lastOpenedProb               float64
+	commonSoftwareUninstallCount int
+	commonSoftwareUninstallProb  float64
+	uniqueSoftwareUninstallCount int
+	uniqueSoftwareUninstallProb  float64
 }
 
 func newAgent(
@@ -751,6 +755,12 @@ func (a *agent) softwareMacOS() []map[string]string {
 			"last_opened_at":    lastOpenedAt,
 		}
 	}
+	if a.softwareCount.commonSoftwareUninstallProb > 0.0 && rand.Float64() <= a.softwareCount.commonSoftwareUninstallProb {
+		rand.Shuffle(len(commonSoftware), func(i, j int) {
+			commonSoftware[i], commonSoftware[j] = commonSoftware[j], commonSoftware[i]
+		})
+		commonSoftware = commonSoftware[:a.softwareCount.common-a.softwareCount.commonSoftwareUninstallCount]
+	}
 	uniqueSoftware := make([]map[string]string, a.softwareCount.unique)
 	for i := 0; i < len(uniqueSoftware); i++ {
 		var lastOpenedAt string
@@ -765,6 +775,12 @@ func (a *agent) softwareMacOS() []map[string]string {
 			"last_opened_at":    lastOpenedAt,
 		}
 	}
+	if a.softwareCount.uniqueSoftwareUninstallProb > 0.0 && rand.Float64() <= a.softwareCount.uniqueSoftwareUninstallProb {
+		rand.Shuffle(len(uniqueSoftware), func(i, j int) {
+			uniqueSoftware[i], uniqueSoftware[j] = uniqueSoftware[j], uniqueSoftware[i]
+		})
+		uniqueSoftware = uniqueSoftware[:a.softwareCount.unique-a.softwareCount.uniqueSoftwareUninstallCount]
+	}
 	randomVulnerableSoftware := make([]map[string]string, a.softwareCount.vulnerable)
 	for i := 0; i < len(randomVulnerableSoftware); i++ {
 		sw := vulnerableSoftware[rand.Intn(len(vulnerableSoftware))]
@@ -1201,8 +1217,15 @@ func main() {
 		queryInterval       = flag.Duration("query_interval", 10*time.Second, "Interval for live query requests")
 		onlyAlreadyEnrolled = flag.Bool("only_already_enrolled", false, "Only start agents that are already enrolled")
 		nodeKeyFile         = flag.String("node_key_file", "", "File with node keys to use")
+
 		commonSoftwareCount          = flag.Int("common_software_count", 10, "Number of common installed applications reported to fleet")
-		uniqueSoftwareCount         = flag.Int("unique_software_count", 10, "Number of unique installed applications reported to fleet")
+		commonSoftwareUninstallCount = flag.Int("common_software_uninstall_count", 1, "Number of common software to uninstall")
+		commonSoftwareUninstallProb  = flag.Float64("common_software_uninstall_prob", 0.1, "Probability of uninstalling common_software_uninstall_count unique software/s")
+
+		uniqueSoftwareCount          = flag.Int("unique_software_count", 10, "Number of uninstalls ")
+		uniqueSoftwareUninstallCount = flag.Int("unique_software_uninstall_count", 1, "Number of unique software to uninstall")
+		uniqueSoftwareUninstallProb  = flag.Float64("unique_software_uninstall_prob", 0.1, "Probability of uninstalling unique_software_uninstall_count common software/s")
+
 		vulnerableSoftwareCount     = flag.Int("vulnerable_software_count", 10, "Number of vulnerable installed applications reported to fleet")
 		withLastOpenedSoftwareCount = flag.Int("with_last_opened_software_count", 10, "Number of applications that may report a last opened timestamp to fleet")
 		lastOpenedChangeProb        = flag.Float64("last_opened_change_prob", 0.1, "Probability of last opened timestamp to be reported as changed [0, 1]")
@@ -1225,6 +1248,13 @@ func main() {
 		*orbitProb = 0
 	}
 
+	if *commonSoftwareUninstallCount >= *commonSoftwareCount {
+		log.Fatalf("Argument common_software_uninstall_count cannot be bigger than common_software_count")
+	}
+	if *uniqueSoftwareUninstallCount >= *uniqueSoftwareCount {
+		log.Fatalf("Argument unique_software_uninstall_count cannot be bigger than unique_software_count")
+	}
+
 	var tmpls []*template.Template
 	requestedTemplates := strings.Split(*osTemplates, ",")
 	for _, nm := range requestedTemplates {
@@ -1265,6 +1295,10 @@ func main() {
 				vulnerable:                   *vulnerableSoftwareCount,
 				withLastOpened:               *withLastOpenedSoftwareCount,
 				lastOpenedProb:               *lastOpenedChangeProb,
+				commonSoftwareUninstallCount: *commonSoftwareUninstallCount,
+				commonSoftwareUninstallProb:  *commonSoftwareUninstallProb,
+				uniqueSoftwareUninstallCount: *uniqueSoftwareUninstallCount,
+				uniqueSoftwareUninstallProb:  *uniqueSoftwareUninstallProb,
 			}, entityCount{
 				common: *commonUserCount,
 				unique: *uniqueUserCount,

server/datastore/mysql/software.go

@@ -200,25 +200,42 @@ func deleteUninstalledHostSoftwareDB(
 	currentMap map[string]fleet.Software,
 	incomingMap map[string]fleet.Software,
 ) error {
-	var deletesHostSoftware []interface{}
-	deletesHostSoftware = append(deletesHostSoftware, hostID)
-
+	var deletesHostSoftware []uint
 	for currentKey, curSw := range currentMap {
 		if _, ok := incomingMap[currentKey]; !ok {
 			deletesHostSoftware = append(deletesHostSoftware, curSw.ID)
 		}
 	}
-	if len(deletesHostSoftware) <= 1 {
+	if len(deletesHostSoftware) == 0 {
 		return nil
 	}
-	sql := fmt.Sprintf(
-		`DELETE FROM host_software WHERE host_id = ? AND software_id IN (%s)`,
-		strings.TrimSuffix(strings.Repeat("?,", len(deletesHostSoftware)-1), ","),
-	)
-	if _, err := tx.ExecContext(ctx, sql, deletesHostSoftware...); err != nil {
+
+	stmt := `DELETE FROM host_software WHERE host_id = ? AND software_id IN (?);`
+	stmt, args, err := sqlx.In(stmt, hostID, deletesHostSoftware)
+	if err != nil {
+		return ctxerr.Wrap(ctx, err, "build delete host software query")
+	}
+	if _, err := tx.ExecContext(ctx, stmt, args...); err != nil {
 		return ctxerr.Wrap(ctx, err, "delete host software")
 	}
 
+	// Cleanup the software table when no more hosts have the deleted host_software
+	// table entries.
+	// Otherwise the software will be listed by ds.ListSoftware but ds.SoftwareByID,
+	// ds.CountHosts and ds.ListHosts will return a *notFoundError error for such
+	// software.
+	stmt = `DELETE FROM software WHERE id IN (?) AND 
+	NOT EXISTS (
+		SELECT 1 FROM host_software hsw WHERE hsw.software_id = software.id
+	)`
+	stmt, args, err = sqlx.In(stmt, deletesHostSoftware)
+	if err != nil {
+		return ctxerr.Wrap(ctx, err, "build delete software query")
+	}
+	if _, err := tx.ExecContext(ctx, stmt, args...); err != nil {
+		return ctxerr.Wrap(ctx, err, "delete software")
+	}
+
 	return nil
 }
 

server/datastore/mysql/software_test.go

@@ -38,6 +38,7 @@ func TestSoftware(t *testing.T) {
 		{"HostsByCVE", testHostsByCVE},
 		{"HostsBySoftwareIDs", testHostsBySoftwareIDs},
 		{"UpdateHostSoftware", testUpdateHostSoftware},
+		{"UpdateHostSoftwareUpdatesSoftware", testUpdateHostSoftwareUpdatesSoftware},
 		{"ListSoftwareByHostIDShort", testListSoftwareByHostIDShort},
 		{"ListSoftwareVulnerabilitiesByHostIDsSource", testListSoftwareVulnerabilitiesByHostIDsSource},
 		{"InsertSoftwareVulnerability", testInsertSoftwareVulnerability},
@@ -147,10 +148,7 @@ func testSoftwareCPE(t *testing.T, ds *Datastore) {
 		{Name: "zoo", Version: "0.0.5", Source: "rpm_packages", BundleIdentifier: ""},               // non-empty -> empty
 	}
 
-	err := ds.UpdateHostSoftware(context.Background(), host1.ID, software1)
-	require.NoError(t, err)
-
-	err = ds.UpdateHostSoftware(context.Background(), host1.ID, software2)
+	err := ds.UpdateHostSoftware(context.Background(), host1.ID, append(software1, software2...))
 	require.NoError(t, err)
 
 	q := fleet.SoftwareIterQueryOptions{ExcludedSources: oval.SupportedSoftwareSources}
@@ -1324,6 +1322,131 @@ func testHostsBySoftwareIDs(t *testing.T, ds *Datastore) {
 	require.Equal(t, hosts[1].Hostname, "host2")
 }
 
+// testUpdateHostSoftwareUpdatesSoftware tests that uninstalling applications
+// from hosts (ds.UpdateHostSoftware) will remove the corresponding entry in
+// `software` if no more hosts have the application installed.
+func testUpdateHostSoftwareUpdatesSoftware(t *testing.T, ds *Datastore) {
+	ctx := context.Background()
+
+	h1 := test.NewHost(t, ds, "host", "", "hostkey", "hostuuid", time.Now())
+	h2 := test.NewHost(t, ds, "host2", "", "hostkey2", "hostuuid2", time.Now())
+
+	// Set the initial software list.
+	sw1 := []fleet.Software{
+		{Name: "foo", Version: "0.0.1", Source: "test", GenerateCPE: "cpe_foo"},
+		{Name: "bar", Version: "0.0.2", Source: "test", GenerateCPE: "cpe_bar"},
+		{Name: "baz", Version: "0.0.3", Source: "test", GenerateCPE: "cpe_baz"},
+	}
+	err := ds.UpdateHostSoftware(ctx, h1.ID, sw1)
+	require.NoError(t, err)
+	sw2 := []fleet.Software{
+		{Name: "foo", Version: "0.0.1", Source: "test", GenerateCPE: "cpe_foo"},
+		{Name: "bar", Version: "0.0.2", Source: "test", GenerateCPE: "cpe_bar"},
+		{Name: "baz", Version: "0.0.3", Source: "test", GenerateCPE: "cpe_baz"},
+		{Name: "baz2", Version: "0.0.3", Source: "test", GenerateCPE: "cpe_baz"},
+	}
+	err = ds.UpdateHostSoftware(ctx, h2.ID, sw2)
+	require.NoError(t, err)
+
+	// ListSoftware uses host_software_counts table.
+	err = ds.SyncHostsSoftware(ctx, time.Now())
+	require.NoError(t, err)
+
+	// Check the returned software.
+	cmpNameVersionCount := func(expected, got []fleet.Software) {
+		cmp := make([]fleet.Software, len(got))
+		for i, sw := range got {
+			cmp[i] = fleet.Software{Name: sw.Name, Version: sw.Version, HostsCount: sw.HostsCount}
+		}
+		require.ElementsMatch(t, expected, cmp)
+	}
+	opts := fleet.SoftwareListOptions{WithHostCounts: true}
+	software := listSoftwareCheckCount(t, ds, 4, 4, opts, false)
+	expectedSoftware := []fleet.Software{
+		{Name: "foo", Version: "0.0.1", HostsCount: 2},
+		{Name: "bar", Version: "0.0.2", HostsCount: 2},
+		{Name: "baz", Version: "0.0.3", HostsCount: 2},
+		{Name: "baz2", Version: "0.0.3", HostsCount: 1},
+	}
+	cmpNameVersionCount(expectedSoftware, software)
+
+	// Update software for the two hosts.
+	//
+	//	- foo is still present in both hosts
+	//	- new is added to h1.
+	//	- baz is removed from h2.
+	//	- baz2 is removed from h2.
+	//	- bar is removed from both hosts.
+	sw1Updated := []fleet.Software{
+		{Name: "foo", Version: "0.0.1", Source: "test", GenerateCPE: "cpe_foo"},
+		{Name: "baz", Version: "0.0.3", Source: "test", GenerateCPE: "cpe_baz"},
+		{Name: "new", Version: "0.0.4", Source: "test", GenerateCPE: "cpe_new"},
+	}
+	err = ds.UpdateHostSoftware(ctx, h1.ID, sw1Updated)
+	require.NoError(t, err)
+	sw2Updated := []fleet.Software{
+		{Name: "foo", Version: "0.0.1", Source: "test", GenerateCPE: "cpe_foo"},
+	}
+	err = ds.UpdateHostSoftware(ctx, h2.ID, sw2Updated)
+	require.NoError(t, err)
+
+	var (
+		bazSoftwareID  uint
+		barSoftwareID  uint
+		baz2SoftwareID uint
+	)
+	for _, s := range software {
+		if s.Name == "baz" {
+			bazSoftwareID = s.ID
+		}
+		if s.Name == "baz2" {
+			baz2SoftwareID = s.ID
+		}
+		if s.Name == "bar" {
+			barSoftwareID = s.ID
+		}
+	}
+	require.NotZero(t, bazSoftwareID)
+	require.NotZero(t, barSoftwareID)
+	require.NotZero(t, baz2SoftwareID)
+
+	// "new" is not returned until ds.SyncHostsSoftware is executed.
+	// "baz2" is gone from the software list.
+	// "baz" still has the wrong count because ds.SyncHostsSoftware hasn't run yet.
+	//
+	// So... counts are "off" until ds.SyncHostsSoftware is run.
+	software = listSoftwareCheckCount(t, ds, 2, 2, opts, false)
+	expectedSoftware = []fleet.Software{
+		{Name: "foo", Version: "0.0.1", HostsCount: 2},
+		{Name: "baz", Version: "0.0.3", HostsCount: 2},
+	}
+	cmpNameVersionCount(expectedSoftware, software)
+
+	hosts, err := ds.HostsBySoftwareIDs(ctx, []uint{bazSoftwareID})
+	require.NoError(t, err)
+	require.Len(t, hosts, 1)
+	require.Equal(t, hosts[0].ID, h1.ID)
+
+	hosts, err = ds.HostsBySoftwareIDs(ctx, []uint{barSoftwareID})
+	require.NoError(t, err)
+	require.Empty(t, hosts)
+	hosts, err = ds.HostsBySoftwareIDs(ctx, []uint{baz2SoftwareID})
+	require.NoError(t, err)
+	require.Empty(t, hosts)
+
+	// ListSoftware uses host_software_counts table.
+	err = ds.SyncHostsSoftware(ctx, time.Now())
+	require.NoError(t, err)
+
+	software = listSoftwareCheckCount(t, ds, 3, 3, opts, false)
+	expectedSoftware = []fleet.Software{
+		{Name: "foo", Version: "0.0.1", HostsCount: 2},
+		{Name: "baz", Version: "0.0.3", HostsCount: 1},
+		{Name: "new", Version: "0.0.4", HostsCount: 1},
+	}
+	cmpNameVersionCount(expectedSoftware, software)
+}
+
 func testUpdateHostSoftware(t *testing.T, ds *Datastore) {
 	ctx := context.Background()