Update software titles cron to include browser field (#15491)

cmd/fleetctl/get_test.go

@@ -667,6 +667,7 @@ spec:
   id: 0
   name: foo
   source: chrome_extensions
+  browser: ""
   versions:
   - id: 0
     version: 0.0.1
@@ -686,6 +687,7 @@ spec:
   id: 0
   name: bar
   source: deb_packages
+  browser: ""
   versions:
   - id: 0
     version: 0.0.3
@@ -701,6 +703,7 @@ spec:
       "id": 0,
       "name": "foo",
       "source": "chrome_extensions",
+	  "browser": "",
       "hosts_count": 2,
       "versions_count": 3,
       "versions": [
@@ -732,6 +735,7 @@ spec:
       "id": 0,
       "name": "bar",
       "source": "deb_packages",
+	  "browser": "",
       "hosts_count": 0,
       "versions_count": 1,
       "versions": [

server/datastore/mysql/migrations/tables/20231207102320_AlterSoftwareTitlesAddBrowser.go

@@ -0,0 +1,35 @@
+package tables
+
+import (
+	"database/sql"
+	"fmt"
+)
+
+func init() {
+	MigrationClient.AddMigration(Up_20231207102320, Down_20231207102320)
+}
+
+func Up_20231207102320(tx *sql.Tx) error {
+	_, err := tx.Exec((`DELETE FROM software_titles;`)) // delete all software titles, it will be repopulated on the next cron
+	if err != nil {
+		return fmt.Errorf("failed to delete software titles: %w", err)
+	}
+
+	_, err = tx.Exec(`
+		ALTER TABLE software_titles 
+		ADD COLUMN browser varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '';`)
+	if err != nil {
+		return fmt.Errorf("failed to add browser column to software titles table: %w", err)
+	}
+
+	_, err = tx.Exec(`ALTER TABLE software_titles DROP KEY idx_software_titles_name_source;`)
+	if err != nil {
+		return fmt.Errorf("failed to drop name-source key from software titles table: %w", err)
+	}
+
+	return nil
+}
+
+func Down_20231207102320(tx *sql.Tx) error {
+	return nil
+}

server/datastore/mysql/migrations/tables/20231207102320_AlterSoftwareTitlesAddBrowser_test.go

@@ -0,0 +1,74 @@
+package tables
+
+import (
+	"context"
+	"testing"
+
+	"github.com/jmoiron/sqlx"
+	"github.com/stretchr/testify/require"
+)
+
+func TestUp_20231207102320(t *testing.T) {
+	db := applyUpToPrev(t)
+
+	insertStmt := "INSERT INTO software_titles (name, source) VALUES (?, ?)"
+
+	_, err := db.Exec(insertStmt, "test-name", "test-source")
+	require.NoError(t, err)
+
+	selectStmt := "SELECT id, name, source FROM software_titles"
+	var rows []struct {
+		ID     uint   `db:"id"`
+		Name   string `db:"name"`
+		Source string `db:"source"`
+	}
+	err = sqlx.SelectContext(context.Background(), db, &rows, selectStmt)
+	require.NoError(t, err)
+	require.Len(t, rows, 1)
+
+	applyNext(t, db)
+
+	selectStmt = "SELECT id, name, source, browser FROM software_titles"
+	type newRow struct {
+		ID      uint   `db:"id"`
+		Name    string `db:"name"`
+		Source  string `db:"source"`
+		Browser string `db:"browser"`
+	}
+	var newRows []newRow
+
+	// migration should delete all rows
+	err = sqlx.SelectContext(context.Background(), db, &newRows, selectStmt)
+	require.NoError(t, err)
+	require.Len(t, newRows, 0)
+
+	// re-insert the old row
+	_, err = db.Exec(insertStmt, "test-name", "test-source")
+	require.NoError(t, err)
+	err = sqlx.SelectContext(context.Background(), db, &newRows, selectStmt)
+	require.NoError(t, err)
+	require.Len(t, newRows, 1)
+	require.Equal(t, "test-name", newRows[0].Name)
+	require.Equal(t, "test-source", newRows[0].Source)
+	require.Equal(t, "", newRows[0].Browser) // default browser is empty string
+
+	insertStmt = "INSERT INTO software_titles (name, source, browser) VALUES (?, ?, ?)"
+
+	_, err = db.Exec(insertStmt, "test-name", "test-source", "test-browser")
+	require.NoError(t, err)
+
+	newRows = []newRow{}
+	err = sqlx.SelectContext(context.Background(), db, &newRows, selectStmt)
+	require.NoError(t, err)
+	require.Len(t, newRows, 2)
+	var found bool
+	for _, row := range newRows {
+		if row.Browser == "test-browser" {
+			require.False(t, found)
+			found = true
+		} else {
+			// browser should be empty for existing rows
+			require.Equal(t, "", row.Browser)
+		}
+	}
+}

server/datastore/mysql/migrations/tables/20231207102321_AddIndexSoftwareNameSourceBrowser.go

@@ -0,0 +1,27 @@
+package tables
+
+import (
+	"database/sql"
+	"fmt"
+)
+
+func init() {
+	MigrationClient.AddMigration(Up_20231207102321, Down_20231207102321)
+}
+
+func Up_20231207102321(tx *sql.Tx) error {
+	_, err := tx.Exec(`ALTER TABLE software_titles ADD UNIQUE INDEX idx_sw_titles (name, source, browser);`)
+	if err != nil {
+		return fmt.Errorf("failed to add unique index to software titles table: %w", err)
+	}
+
+	_, err = tx.Exec(`ALTER TABLE software ADD INDEX idx_sw_name_source_browser (name, source, browser);`)
+	if err != nil {
+		return fmt.Errorf("failed to add name-source-browser index to software table: %w", err)
+	}
+	return nil
+}
+
+func Down_20231207102321(tx *sql.Tx) error {
+	return nil
+}

server/datastore/mysql/migrations/tables/20231207102321_AddIndexSoftwareNameSourceBrowser_test.go

@@ -0,0 +1,39 @@
+package tables
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestUp_20231207102321(t *testing.T) {
+	db := applyUpToPrev(t)
+
+	insertStmt := "INSERT INTO software_titles (name, source, browser) VALUES (?, ?, ?)"
+	_, err := db.Exec(insertStmt, "test-name", "test-source", "")
+	require.NoError(t, err)
+
+	_, err = db.Exec(insertStmt, "test-name2", "test-source", "")
+	require.NoError(t, err)
+
+	applyNext(t, db)
+
+	// unique constraint applies to name+source+browser
+	_, err = db.Exec(insertStmt, "test-name", "test-source", "")
+	require.ErrorContains(t, err, "Duplicate entry")
+
+	_, err = db.Exec(insertStmt, "test-name", "test-source", "test-browser")
+	require.NoError(t, err)
+
+	_, err = db.Exec(insertStmt, "test-name2", "test-source", "test-browser")
+	require.NoError(t, err)
+
+	_, err = db.Exec(insertStmt, "test-name2", "test-source2", "test-browser")
+	require.NoError(t, err)
+
+	_, err = db.Exec(insertStmt, "test-name2", "test-source2", "test-browser2")
+	require.NoError(t, err)
+
+	_, err = db.Exec(insertStmt, "test-name2", "test-source2", "test-browser2")
+	require.ErrorContains(t, err, "Duplicate entry")
+}

server/datastore/mysql/software.go

@@ -310,7 +310,7 @@ SELECT
     s.name,
     s.version,
     s.source,
-	s.browser,
+    s.browser,
     s.bundle_identifier,
     s.release,
     s.vendor,
@@ -1311,14 +1311,15 @@ func (ds *Datastore) ReconcileSoftwareTitles(ctx context.Context) error {
 
 	// ensure all software titles are in the software_titles table
 	upsertTitlesStmt := `
-INSERT INTO software_titles (name, source)
+INSERT INTO software_titles (name, source, browser)
 SELECT DISTINCT
 	name,
-	source
+	source,
+	browser
 FROM
 	software s
 WHERE 
-	NOT EXISTS (SELECT 1 FROM software_titles st WHERE (s.name, s.source) = (st.name, st.source)) 
+	NOT EXISTS (SELECT 1 FROM software_titles st WHERE (s.name, s.source, s.browser) = (st.name, st.source, st.browser)) 
 ON DUPLICATE KEY UPDATE software_titles.id = software_titles.id`
 	// TODO: consider the impact of on duplicate key update vs. risk of insert ignore
 	// or performing a select first to see if the title exists and only inserting
@@ -1339,15 +1340,15 @@ UPDATE
 SET
 	s.title_id = st.id
 WHERE
-	(s.name, s.source) = (st.name, st.source)
+	(s.name, s.source, s.browser) = (st.name, st.source, st.browser)
 	AND (s.title_id IS NULL OR s.title_id != st.id)`
 
 	res, err = ds.writer(ctx).ExecContext(ctx, updateSoftwareStmt)
 	if err != nil {
-		return ctxerr.Wrap(ctx, err, "update software titles")
+		return ctxerr.Wrap(ctx, err, "update software title_id")
 	}
 	n, _ = res.RowsAffected()
-	level.Debug(ds.logger).Log("msg", "update software titles", "rows_affected", n)
+	level.Debug(ds.logger).Log("msg", "update software title_id", "rows_affected", n)
 
 	// clean up orphaned software titles
 	cleanupStmt := `

server/datastore/mysql/software_test.go

@@ -2588,7 +2588,7 @@ func TestReconcileSoftwareTitles(t *testing.T) {
 	host3 := test.NewHost(t, ds, "host3", "", "host3key", "host3uuid", time.Now())
 
 	expectedSoftware := []fleet.Software{
-		{Name: "foo", Version: "0.0.1", Source: "chrome_extensions"},
+		{Name: "foo", Version: "0.0.1", Source: "chrome_extensions", Browser: "chrome"},
 		{Name: "foo", Version: "v0.0.2", Source: "chrome_extensions"},
 		{Name: "foo", Version: "0.0.3", Source: "chrome_extensions"},
 		{Name: "bar", Version: "0.0.3", Source: "deb_packages"},
@@ -2608,7 +2608,7 @@ func TestReconcileSoftwareTitles(t *testing.T) {
 
 	getSoftware := func() ([]fleet.Software, error) {
 		var sw []fleet.Software
-		err := ds.writer(ctx).SelectContext(ctx, &sw, `SELECT * FROM software ORDER BY name, version`)
+		err := ds.writer(ctx).SelectContext(ctx, &sw, `SELECT * FROM software ORDER BY name, source, browser, version`)
 		if err != nil {
 			return nil, err
 		}
@@ -2617,32 +2617,32 @@ func TestReconcileSoftwareTitles(t *testing.T) {
 
 	getTitles := func() ([]fleet.SoftwareTitle, error) {
 		var swt []fleet.SoftwareTitle
-		err := ds.writer(ctx).SelectContext(ctx, &swt, `SELECT * FROM software_titles ORDER BY name, source`)
+		err := ds.writer(ctx).SelectContext(ctx, &swt, `SELECT * FROM software_titles ORDER BY name, source, browser`)
 		if err != nil {
 			return nil, err
 		}
 		return swt, nil
 	}
 
-	expectedTitlesByNS := map[string]fleet.SoftwareTitle{}
+	expectedTitlesByNSB := map[string]fleet.SoftwareTitle{}
 	assertSoftware := func(t *testing.T, wantSoftware []fleet.Software, wantNilTitleID []fleet.Software) {
 		gotSoftware, err := getSoftware()
 		require.NoError(t, err)
 		require.Len(t, gotSoftware, len(wantSoftware))
 
-		byNSV := map[string]fleet.Software{}
+		byNSBV := map[string]fleet.Software{}
 		for _, s := range wantSoftware {
-			byNSV[s.Name+s.Source+s.Version] = s
+			byNSBV[s.Name+s.Source+s.Browser+s.Version] = s
 		}
 
 		for _, r := range gotSoftware {
-			_, ok := byNSV[r.Name+r.Source+r.Version]
+			_, ok := byNSBV[r.Name+r.Source+r.Browser+r.Version]
 			require.True(t, ok)
 
 			if r.TitleID == nil {
 				var found bool
 				for _, s := range wantNilTitleID {
-					if s.Name == r.Name && s.Source == r.Source && s.Version == r.Version {
+					if s.Name == r.Name && s.Source == r.Source && s.Browser == r.Browser && s.Version == r.Version {
 						found = true
 						break
 					}
@@ -2650,12 +2650,13 @@ func TestReconcileSoftwareTitles(t *testing.T) {
 				require.True(t, found)
 			} else {
 				require.NotNil(t, r.TitleID)
-				swt, ok := expectedTitlesByNS[r.Name+r.Source]
+				swt, ok := expectedTitlesByNSB[r.Name+r.Source+r.Browser]
 				require.True(t, ok)
 				require.NotNil(t, r.TitleID)
 				require.Equal(t, swt.ID, *r.TitleID)
 				require.Equal(t, swt.Name, r.Name)
 				require.Equal(t, swt.Source, r.Source)
+				require.Equal(t, swt.Browser, r.Browser)
 			}
 		}
 	}
@@ -2665,11 +2666,12 @@ func TestReconcileSoftwareTitles(t *testing.T) {
 			if len(expectMissing) > 0 {
 				require.NotContains(t, expectMissing, r.Name)
 			}
-			e, ok := expectedTitlesByNS[r.Name+r.Source]
+			e, ok := expectedTitlesByNSB[r.Name+r.Source+r.Browser]
 			require.True(t, ok)
 			require.Equal(t, e.ID, r.ID)
 			require.Equal(t, e.Name, r.Name)
 			require.Equal(t, e.Source, r.Source)
+			require.Equal(t, e.Browser, r.Browser)
 		}
 	}
 
@@ -2680,19 +2682,27 @@ func TestReconcileSoftwareTitles(t *testing.T) {
 	require.NoError(t, ds.ReconcileSoftwareTitles(ctx))
 	swt, err := getTitles()
 	require.NoError(t, err)
-	require.Len(t, swt, 3)
+	require.Len(t, swt, 4)
 
 	require.Equal(t, swt[0].Name, "bar")
 	require.Equal(t, swt[0].Source, "deb_packages")
-	expectedTitlesByNS[swt[0].Name+swt[0].Source] = swt[0]
+	require.Equal(t, swt[0].Browser, "")
+	expectedTitlesByNSB[swt[0].Name+swt[0].Source+swt[0].Browser] = swt[0]
 
 	require.Equal(t, swt[1].Name, "baz")
 	require.Equal(t, swt[1].Source, "deb_packages")
-	expectedTitlesByNS[swt[1].Name+swt[1].Source] = swt[1]
+	require.Equal(t, swt[1].Browser, "")
+	expectedTitlesByNSB[swt[1].Name+swt[1].Source+swt[1].Browser] = swt[1]
 
 	require.Equal(t, swt[2].Name, "foo")
 	require.Equal(t, swt[2].Source, "chrome_extensions")
-	expectedTitlesByNS[swt[2].Name+swt[2].Source] = swt[2]
+	require.Equal(t, swt[2].Browser, "")
+	expectedTitlesByNSB[swt[2].Name+swt[2].Source+swt[2].Browser] = swt[2]
+
+	require.Equal(t, swt[3].Name, "foo")
+	require.Equal(t, swt[3].Source, "chrome_extensions")
+	require.Equal(t, swt[3].Browser, "chrome")
+	expectedTitlesByNSB[swt[3].Name+swt[3].Source+swt[3].Browser] = swt[3]
 
 	// title_id is now populated for all software entries
 	assertSoftware(t, expectedSoftware, nil)
@@ -2706,7 +2716,7 @@ func TestReconcileSoftwareTitles(t *testing.T) {
 	require.NoError(t, ds.ReconcileSoftwareTitles(context.Background()))
 	gotTitles, err := getTitles()
 	require.NoError(t, err)
-	require.Len(t, gotTitles, 2)
+	require.Len(t, gotTitles, 3)
 	assertTitles(t, gotTitles, []string{"bar"})
 
 	// add bar to host 3
@@ -2720,20 +2730,20 @@ func TestReconcileSoftwareTitles(t *testing.T) {
 	// bar isn't added back to software titles until we reconcile software titles
 	gotTitles, err = getTitles()
 	require.NoError(t, err)
-	require.Len(t, gotTitles, 2)
+	require.Len(t, gotTitles, 3)
 	assertTitles(t, gotTitles, []string{"bar"})
 
 	// reconcile software titles
 	require.NoError(t, ds.ReconcileSoftwareTitles(ctx))
 	gotTitles, err = getTitles()
 	require.NoError(t, err)
-	require.Len(t, gotTitles, 3)
+	require.Len(t, gotTitles, 4)
 
 	// bar was added back to software titles with a new ID
-	require.Equal(t, gotTitles[0].Name, "bar")
+	require.Equal(t, "bar", gotTitles[0].Name)
-	require.Equal(t, gotTitles[0].Source, "deb_packages")
+	require.Equal(t, "deb_packages", gotTitles[0].Source)
-	require.NotEqual(t, expectedTitlesByNS[gotTitles[0].Name+gotTitles[0].Source], gotTitles[0].ID)
+	require.NotEqual(t, expectedTitlesByNSB[gotTitles[0].Name+gotTitles[0].Source], gotTitles[0].ID)
-	expectedTitlesByNS[gotTitles[0].Name+gotTitles[0].Source] = gotTitles[0]
+	expectedTitlesByNSB[gotTitles[0].Name+gotTitles[0].Source] = gotTitles[0]
 	assertTitles(t, gotTitles, nil)
 
 	// title_id is now populated for bar
@@ -2751,7 +2761,7 @@ func TestReconcileSoftwareTitles(t *testing.T) {
 	require.NoError(t, ds.ReconcileSoftwareTitles(ctx))
 	gotTitles, err = getTitles()
 	require.NoError(t, err)
-	require.Len(t, gotTitles, 3)
+	require.Len(t, gotTitles, 4)
 	assertTitles(t, gotTitles, nil)
 
 	// title_id is now populated for new version of foo
@@ -2769,10 +2779,11 @@ func TestReconcileSoftwareTitles(t *testing.T) {
 	require.NoError(t, ds.ReconcileSoftwareTitles(ctx))
 	gotTitles, err = getTitles()
 	require.NoError(t, err)
-	require.Len(t, gotTitles, 4)
+	require.Len(t, gotTitles, 5)
-	require.Equal(t, gotTitles[3].Name, "foo")
+	require.Equal(t, "foo", gotTitles[4].Name)
-	require.Equal(t, gotTitles[3].Source, "rpm_packages")
+	require.Equal(t, "rpm_packages", gotTitles[4].Source)
-	expectedTitlesByNS[gotTitles[3].Name+gotTitles[3].Source] = gotTitles[3]
+	require.Equal(t, "", gotTitles[4].Browser)
+	expectedTitlesByNSB[gotTitles[4].Name+gotTitles[4].Source+gotTitles[4].Browser] = gotTitles[4]
 	assertTitles(t, gotTitles, nil)
 
 	// title_id is now populated for new source of foo

server/fleet/software.go

@@ -136,6 +136,8 @@ type SoftwareTitle struct {
 	Name string `json:"name" db:"name"`
 	// Source is the source reported by osquery.
 	Source string `json:"source" db:"source"`
+	// Browser is the browser type (e.g., "chrome", "firefox", "safari")
+	Browser string `json:"browser" db:"browser"`
 	// HostsCount is the number of hosts that use this software title.
 	HostsCount uint `json:"hosts_count" db:"hosts_count"`
 	// VesionsCount is the number of versions that have the same title.