From 8aa268b1b5aa99b96f27e4f1b996e37cf458dbd3 Mon Sep 17 00:00:00 2001 From: Jahziel Villasana-Espinoza Date: Fri, 16 May 2025 11:27:13 -0400 Subject: [PATCH] prevent panic if installer is removed from s3 (#29189) > for #28815 # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Added/updated automated tests - [x] Manual QA for all new/changed functionality - [x] For unreleased bug fixes in a release candidate, confirmed that the fix is not expected to adversely impact load test results or alerted the release DRI if additional load testing is needed. --- ee/server/service/software_installers.go | 11 ++++++- server/service/integration_enterprise_test.go | 31 ++++++++++++++++--- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/ee/server/service/software_installers.go b/ee/server/service/software_installers.go index 61a8365759..857ff204bb 100644 --- a/ee/server/service/software_installers.go +++ b/ee/server/service/software_installers.go @@ -1817,6 +1817,7 @@ func (svc *Service) softwareBatchUpload( case ok: // Perfect match: existing installer on the same team installer.StorageID = p.SHA256 + if foundInstaller.Extension == "exe" || foundInstaller.Extension == "tar.gz" { if p.InstallScript == "" { return fmt.Errorf("Couldn't edit. Install script is required for .%s packages.", foundInstaller.Extension) @@ -1882,8 +1883,16 @@ func (svc *Service) softwareBatchUpload( } } + var installerBytesExist bool + if p.SHA256 != "" { + installerBytesExist, err = svc.softwareInstallStore.Exists(ctx, installer.StorageID) + if err != nil { + return err + } + } + // no accessible matching installer was found, so attempt to download it from URL. - if installer.StorageID == "" { + if installer.StorageID == "" || !installerBytesExist { if p.SHA256 != "" && p.URL == "" { return fmt.Errorf("package not found with hash %s", p.SHA256) } diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 5641f03ea2..68b844e329 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -17382,14 +17382,14 @@ func (s *integrationEnterpriseTestSuite) TestBatchSoftwareUploadWithSHAs() { require.True(t, hitDebURL) hitDebURL = false - var installerHash string + var debHash string mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { - return sqlx.GetContext(ctx, q, &installerHash, "SELECT storage_id FROM software_installers WHERE title_id = ?", packages[0].TitleID) + return sqlx.GetContext(ctx, q, &debHash, "SELECT storage_id FROM software_installers WHERE title_id = ?", packages[0].TitleID) }) - require.NotEmpty(t, installerHash) + require.NotEmpty(t, debHash) // add the hash to the payload - softwareToInstall[0].SHA256 = installerHash + softwareToInstall[0].SHA256 = debHash // dry run shouldn't hit the download endpoint since we included the SHA s.DoJSON("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: softwareToInstall}, http.StatusAccepted, &batchResponse, "team_name", team1.Name, "dry_run", "true") @@ -17570,7 +17570,7 @@ func (s *integrationEnterpriseTestSuite) TestBatchSoftwareUploadWithSHAs() { require.True(t, hitPkgURL) hitPkgURL = false - // attempt to add the exe to team 1 without scripts. Even though the admin has access to + // Attempt to add the exe to team 1 without scripts. Even though the admin has access to // both teams, it should fail because scripts are required for exes. // Check without either script first. s.token = s.getTestAdminToken() @@ -17634,6 +17634,27 @@ done require.Equal(t, pkgURL, stResp.SoftwareTitle.SoftwarePackage.URL) require.Equal(t, file.GetInstallScript("pkg"), stResp.SoftwareTitle.SoftwarePackage.InstallScript) require.Equal(t, expectedUninstallScript, stResp.SoftwareTitle.SoftwarePackage.UninstallScript) + + // Clean up all installers from the store. We don't care about how many installers are + // removed in this test, so just check that the cleanup succeeded. + _, err = s.softwareInstallStore.Cleanup(ctx, nil, time.Now()) + require.NoError(t, err) + + // At this point, the exe payload has no URL. Batch set should fail because the + // installer bytes don't exist anymore and there is no URL provided. + s.DoJSON("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: softwareToInstall}, http.StatusAccepted, &batchResponse, "team_name", team2.Name) + errMsg = waitBatchSetSoftwareInstallersFailed(t, s, team2.Name, batchResponse.RequestUUID) + require.Contains(t, errMsg, fmt.Sprintf("package not found with hash %s", exeHash)) + + // Add the URL back and do the batch set. Should succeed and re-download all installers. + softwareToInstall[1].URL = exeURL + hitDebURL, hitExeURL, hitPkgURL = false, false, false + s.DoJSON("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: softwareToInstall}, http.StatusAccepted, &batchResponse, "team_name", team2.Name) + packages = waitBatchSetSoftwareInstallersCompleted(t, s, team2.Name, batchResponse.RequestUUID) + require.Len(t, packages, 3) + for _, v := range []bool{hitDebURL, hitExeURL, hitPkgURL} { + require.True(t, v) + } } func (s *integrationEnterpriseTestSuite) TestBatchSoftwareInstallerAndFMACategories() {