Updated BatchSetSoftwareInstallers to use bundle_identifier (#26252)

The software titles unique key was changed to include bundle identifier
in #25794. This caused an issue
when running gitops - installing a software with the same bundle
identifier but different names.

#26226

- [x] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)
- [x] Added/updated automated tests
- [x] A detailed QA plan exists on the associated ticket (if it isn't
there, work with the product group's QA engineer to add it)
- [x] Manual QA for all new/changed functionality

Author: ksykulev

server/datastore/mysql/software_installers.go

@@ -1071,21 +1071,22 @@ func (ds *Datastore) CleanupUnusedSoftwareInstallers(ctx context.Context, softwa
 func (ds *Datastore) BatchSetSoftwareInstallers(ctx context.Context, tmID *uint, installers []*fleet.UploadSoftwareInstallerPayload) error {
 	const upsertSoftwareTitles = `
 INSERT INTO software_titles
-  (name, source, browser)
+  (name, source, browser, bundle_identifier)
 VALUES
   %s
 ON DUPLICATE KEY UPDATE
   name = VALUES(name),
   source = VALUES(source),
-  browser = VALUES(browser)
+  browser = VALUES(browser),
+  bundle_identifier = VALUES(bundle_identifier)
 `
 
 	const loadSoftwareTitles = `
 SELECT
   id
 FROM
   software_titles
-WHERE (name, source, browser) IN (%s)
+WHERE (unique_identifier, source, browser) IN (%s)
 `
 
 	const unsetAllInstallersFromPolicies = `
@@ -1190,7 +1191,7 @@ COALESCE(post_install_script_content_id != ? OR
 	(post_install_script_content_id IS NULL AND ? IS NOT NULL) OR
 	(? IS NULL AND post_install_script_content_id IS NOT NULL)
 , FALSE) is_metadata_modified FROM software_installers
-WHERE global_or_team_id = ?	AND title_id IN (SELECT id FROM software_titles WHERE name = ? AND source = ? AND browser = '')
+WHERE global_or_team_id = ?	AND title_id IN (SELECT id FROM software_titles WHERE unique_identifier = ? AND source = ? AND browser = '')
 `
 
 	const insertNewOrEditedInstaller = `
@@ -1216,7 +1217,7 @@ INSERT INTO software_installers (
 	install_during_setup
 ) VALUES (
   ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?,
-  (SELECT id FROM software_titles WHERE name = ? AND source = ? AND browser = ''),
+  (SELECT id FROM software_titles WHERE unique_identifier = ? AND source = ? AND browser = ''),
   ?, (SELECT name FROM users WHERE id = ?), (SELECT email FROM users WHERE id = ?), ?, ?, COALESCE(?, false)
 )
 ON DUPLICATE KEY UPDATE
@@ -1245,7 +1246,7 @@ FROM
 WHERE
 	global_or_team_id = ?	AND
 	-- this is guaranteed to select a single title_id, due to unique index
-	title_id IN (SELECT id FROM software_titles WHERE name = ? AND source = ? AND browser = '')
+	title_id IN (SELECT id FROM software_titles WHERE unique_identifier = ? AND source = ? AND browser = '')
 `
 
 	const deleteInstallerLabelsNotInList = `
@@ -1330,18 +1331,43 @@ WHERE
 
 		var args []any
 		for _, installer := range installers {
-			args = append(args, installer.Title, installer.Source, "")
+			args = append(
+				args,
+				installer.Title,
+				installer.Source,
+				"",
+				func() *string {
+					if strings.TrimSpace(installer.BundleIdentifier) != "" {
+						return &installer.BundleIdentifier
+					}
+					return nil
+				}(),
+			)
 		}
 
 		values := strings.TrimSuffix(
-			strings.Repeat("(?,?,?),", len(installers)),
+			strings.Repeat("(?,?,?,?),", len(installers)),
 			",",
 		)
 		if _, err := tx.ExecContext(ctx, fmt.Sprintf(upsertSoftwareTitles, values), args...); err != nil {
 			return ctxerr.Wrap(ctx, err, "insert new/edited software title")
 		}
 
 		var titleIDs []uint
+		args = []any{}
+		for _, installer := range installers {
+			args = append(
+				args,
+				BundleIdentifierOrName(installer.BundleIdentifier, installer.Title),
+				installer.Source,
+				"",
+			)
+		}
+		values = strings.TrimSuffix(
+			strings.Repeat("(?,?,?),", len(installers)),
+			",",
+		)
+
 		if err := sqlx.SelectContext(ctx, tx, &titleIDs, fmt.Sprintf(loadSoftwareTitles, values), args...); err != nil {
 			return ctxerr.Wrap(ctx, err, "load existing titles")
 		}
@@ -1441,7 +1467,7 @@ WHERE
 				postInstallScriptID,
 				// WHERE clause
 				globalOrTeamID,
-				installer.Title,
+				BundleIdentifierOrName(installer.BundleIdentifier, installer.Title),
 				installer.Source,
 			}
 
@@ -1472,7 +1498,7 @@ WHERE
 				postInstallScriptID,
 				installer.Platform,
 				installer.SelfService,
-				installer.Title,
+				BundleIdentifierOrName(installer.BundleIdentifier, installer.Title),
 				installer.Source,
 				installer.UserID,
 				installer.UserID,
@@ -1495,7 +1521,7 @@ WHERE
 			// ID (cannot use res.LastInsertID due to the upsert statement, won't
 			// give the id in case of update)
 			var installerID uint
-			if err := sqlx.GetContext(ctx, tx, &installerID, loadSoftwareInstallerID, globalOrTeamID, installer.Title, installer.Source); err != nil {
+			if err := sqlx.GetContext(ctx, tx, &installerID, loadSoftwareInstallerID, globalOrTeamID, BundleIdentifierOrName(installer.BundleIdentifier, installer.Title), installer.Source); err != nil {
 				return ctxerr.Wrapf(ctx, err, "load id of new/edited installer with name %q", installer.Filename)
 			}
 

server/datastore/mysql/software_installers_test.go

@@ -753,18 +753,19 @@ func testBatchSetSoftwareInstallers(t *testing.T, ds *Datastore) {
 	tfr0, err := fleet.NewTempFileReader(ins0File, t.TempDir)
 	require.NoError(t, err)
 	err = ds.BatchSetSoftwareInstallers(ctx, &team.ID, []*fleet.UploadSoftwareInstallerPayload{{
-		InstallScript:   "install",
-		InstallerFile:   tfr0,
-		StorageID:       ins0,
-		Filename:        "installer0",
-		Title:           "ins0",
-		Source:          "apps",
-		Version:         "1",
-		PreInstallQuery: "foo",
-		UserID:          user1.ID,
-		Platform:        "darwin",
-		URL:             "https://example.com",
-		ValidatedLabels: &fleet.LabelIdentsWithScope{},
+		InstallScript:    "install",
+		InstallerFile:    tfr0,
+		StorageID:        ins0,
+		Filename:         "installer0",
+		Title:            "ins0",
+		Source:           "apps",
+		Version:          "1",
+		PreInstallQuery:  "foo",
+		UserID:           user1.ID,
+		Platform:         "darwin",
+		URL:              "https://example.com",
+		ValidatedLabels:  &fleet.LabelIdentsWithScope{},
+		BundleIdentifier: "com.example.ins0",
 	}})
 	require.NoError(t, err)
 	softwareInstallers, err = ds.GetSoftwareInstallers(ctx, team.ID)
@@ -950,6 +951,54 @@ func testBatchSetSoftwareInstallers(t *testing.T, ds *Datastore) {
 		{Name: ins1, Source: "apps", Browser: ""},
 	})
 
+	// Add software installer with same name different bundle id
+	err = ds.BatchSetSoftwareInstallers(ctx, &team.ID, []*fleet.UploadSoftwareInstallerPayload{{
+		InstallScript:    "install",
+		InstallerFile:    tfr0,
+		StorageID:        ins0,
+		Filename:         "installer0",
+		Title:            "ins0",
+		Source:           "apps",
+		Version:          "1",
+		PreInstallQuery:  "foo",
+		UserID:           user1.ID,
+		Platform:         "darwin",
+		URL:              "https://example.com",
+		ValidatedLabels:  &fleet.LabelIdentsWithScope{},
+		BundleIdentifier: "com.example.different.ins0",
+	}})
+	require.NoError(t, err)
+	softwareInstallers, err = ds.GetSoftwareInstallers(ctx, team.ID)
+	require.NoError(t, err)
+	require.Len(t, softwareInstallers, 1)
+	assertSoftware([]fleet.SoftwareTitle{
+		{Name: ins0, Source: "apps", Browser: "", BundleIdentifier: ptr.String("com.example.different.ins0")},
+	})
+
+	// Add software installer with the same bundle id but different name
+	err = ds.BatchSetSoftwareInstallers(ctx, &team.ID, []*fleet.UploadSoftwareInstallerPayload{{
+		InstallScript:    "install",
+		InstallerFile:    tfr0,
+		StorageID:        ins0,
+		Filename:         "installer0",
+		Title:            "ins0-different",
+		Source:           "apps",
+		Version:          "1",
+		PreInstallQuery:  "foo",
+		UserID:           user1.ID,
+		Platform:         "darwin",
+		URL:              "https://example.com",
+		ValidatedLabels:  &fleet.LabelIdentsWithScope{},
+		BundleIdentifier: "com.example.ins0",
+	}})
+	require.NoError(t, err)
+	softwareInstallers, err = ds.GetSoftwareInstallers(ctx, team.ID)
+	require.NoError(t, err)
+	require.Len(t, softwareInstallers, 1)
+	assertSoftware([]fleet.SoftwareTitle{
+		{Name: "ins0-different", Source: "apps", Browser: "", BundleIdentifier: ptr.String("com.example.ins0")},
+	})
+
 	// remove everything
 	err = ds.BatchSetSoftwareInstallers(ctx, &team.ID, []*fleet.UploadSoftwareInstallerPayload{})
 	require.NoError(t, err)