diff --git a/changes/30565-cron-errors b/changes/30565-cron-errors new file mode 100644 index 0000000000..8b13f66687 --- /dev/null +++ b/changes/30565-cron-errors @@ -0,0 +1 @@ +* Fixed cases where the uninstall script population job introduced in Fleet 4.57.0 would attempt to extract package IDs on software that we don't generate uninstall scripts for, causing errors in logs and retries of the job. diff --git a/ee/server/service/software_installers.go b/ee/server/service/software_installers.go index e96dddf7f7..bb7a1a3f45 100644 --- a/ee/server/service/software_installers.go +++ b/ee/server/service/software_installers.go @@ -2365,10 +2365,10 @@ func UninstallSoftwareMigration( softwareInstallStore fleet.SoftwareInstallerStore, logger kitlog.Logger, ) error { - // Find software installers without package_id - idMap, err := ds.GetSoftwareInstallersWithoutPackageIDs(ctx) + // Find software installers that should have their uninstall script populated + idMap, err := ds.GetSoftwareInstallersPendingUninstallScriptPopulation(ctx) if err != nil { - return ctxerr.Wrap(ctx, err, "getting software installers without package_id") + return ctxerr.Wrap(ctx, err, "getting software installers to modufy") } if len(idMap) == 0 { return nil diff --git a/server/datastore/mysql/software_installers.go b/server/datastore/mysql/software_installers.go index cfe4cbaa98..571250edf7 100644 --- a/server/datastore/mysql/software_installers.go +++ b/server/datastore/mysql/software_installers.go @@ -2353,10 +2353,9 @@ func (ds *Datastore) GetDetailsForUninstallFromExecutionID(ctx context.Context, return result.Name, result.SelfService, nil } -func (ds *Datastore) GetSoftwareInstallersWithoutPackageIDs(ctx context.Context) (map[uint]string, error) { - query := ` - SELECT id, storage_id FROM software_installers WHERE package_ids = '' - ` +func (ds *Datastore) GetSoftwareInstallersPendingUninstallScriptPopulation(ctx context.Context) (map[uint]string, error) { + query := `SELECT id, storage_id FROM software_installers WHERE package_ids = '' + AND extension NOT IN ('exe', 'tar.gz', 'dmg', 'zip')` type result struct { ID uint `db:"id"` StorageID string `db:"storage_id"` diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index 3b6e802576..69ea7b232d 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -1901,8 +1901,10 @@ type Datastore interface { // (if set) post-install scripts, otherwise those fields are left empty. GetSoftwareInstallerMetadataByTeamAndTitleID(ctx context.Context, teamID *uint, titleID uint, withScriptContents bool) (*SoftwareInstaller, error) - // GetSoftwareInstallersWithoutPackageIDs returns a map of software installers to storage ids that do not have a package ID. - GetSoftwareInstallersWithoutPackageIDs(ctx context.Context) (map[uint]string, error) + // GetSoftwareInstallersPendingUninstallScriptPopulation returns a map of software installers to storage IDs that: + // 1. need uninstall scripts populated + // 2. can have uninstall scripts auto-generated by Fleet + GetSoftwareInstallersPendingUninstallScriptPopulation(ctx context.Context) (map[uint]string, error) // GetMSIInstallersWithoutUpgradeCode returns a map of MSI software installers to storage ids that do not have an upgrade code set. GetMSIInstallersWithoutUpgradeCode(ctx context.Context) (map[uint]string, error) diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index b83568f23a..6056923a59 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -7700,7 +7700,7 @@ func (s *DataStore) GetSoftwareInstallerMetadataByTeamAndTitleID(ctx context.Con return s.GetSoftwareInstallerMetadataByTeamAndTitleIDFunc(ctx, teamID, titleID, withScriptContents) } -func (s *DataStore) GetSoftwareInstallersWithoutPackageIDs(ctx context.Context) (map[uint]string, error) { +func (s *DataStore) GetSoftwareInstallersPendingUninstallScriptPopulation(ctx context.Context) (map[uint]string, error) { s.mu.Lock() s.GetSoftwareInstallersWithoutPackageIDsFuncInvoked = true s.mu.Unlock() diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 9378f97de8..ac65494b4c 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -11350,6 +11350,38 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareInstallerUploadDownloadAndD err = eeservice.UninstallSoftwareMigration(context.Background(), s.ds, s.softwareInstallStore, logger) require.NoError(t, err) + // Update DB by clearing package ids and swapping extension to one we skip + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + if _, err = q.ExecContext(context.Background(), `UPDATE software_installers SET package_ids = '', extension = 'tar.gz' WHERE id = ?`, + installerID); err != nil { + return err + } + return nil + }) + + // Running the migration again causes no issues. + err = eeservice.UninstallSoftwareMigration(context.Background(), s.ds, s.softwareInstallStore, logger) + require.NoError(t, err) + + // Package ID and extension should not have been modified + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + var packageIDs string + if err := sqlx.GetContext(context.Background(), q, &packageIDs, `SELECT package_ids FROM software_installers WHERE id = ?`, + installerID); err != nil { + return err + } + assert.Empty(t, packageIDs) + + var extension string + if err := sqlx.GetContext(context.Background(), q, &extension, `SELECT extension FROM software_installers WHERE id = ?`, + installerID); err != nil { + return err + } + assert.Equal(t, "tar.gz", extension) + + return nil + }) + // delete the installer s.Do("DELETE", fmt.Sprintf("/api/latest/fleet/software/titles/%d/available_for_install", titleID), nil, http.StatusNoContent, "team_id", fmt.Sprintf("%d", *payload.TeamID))