From 45abf8c9adc463a78d49b9768a7b2152fc199a91 Mon Sep 17 00:00:00 2001 From: Jonathan Katz <44128041+jkatz01@users.noreply.github.com> Date: Fri, 31 Jul 2026 21:36:44 -0400 Subject: [PATCH] Add software installer upload/download progress to GitOps runs (#50250) **Related issue:** Resolves #45728 Changes: - Adds a new redis key to keep track of downloaded packages. It starts out with an empty list and gets filled with each download. Each update writes the entire struct at once to the key. - Adds logging in the fleetctl gitops client to show which packages were downloaded - Fixes the categories key potentially expiring # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [ ] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - :x: Timeouts are implemented and retries are limited to avoid infinite loops - Right now the batch will write the whole slice of all packages to a single redis key for every package in the loop. Looks like performance is acceptable for now (500 packages), but maybe this will need to be limited. - [ ] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [ ] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [x] QA'd all new/changed functionality manually ## Summary by CodeRabbit ## New Features - Added per-package software download progress in fleetctl GitOps. - Progress now reports downloading, completed, skipped, and failed packages during real and dry runs. - Installation output now distinguishes applying and applied stages. ## Bug Fixes - Improved download error messages and cached-package handling. - Prevented duplicate progress messages and ensured tracking issues do not interrupt successful software batches. ## Tests - Expanded coverage for progress reporting, failures, dry runs, package types, and authorization scenarios. --- changes/45728-gitops-software-upload-progress | 1 + .../testdata/gitops/lib/dummy_installer.pkg | Bin 0 -> 644 bytes .../gitops/team_software_script_package.yml | 19 ++ .../gitops_enterprise_integration_test.go | 15 +- .../integrationtest/gitops/software_test.go | 70 ++++++ ee/server/service/software_installers.go | 177 ++++++++++++---- ee/server/service/software_installers_test.go | 21 +- server/fleet/service.go | 8 +- server/fleet/software_installer.go | 32 +++ server/mock/service/service_mock.go | 4 +- server/service/client.go | 27 ++- server/service/client_software.go | 62 +++++- server/service/client_teams.go | 9 +- server/service/client_test.go | 106 ++++++++++ server/service/integration_enterprise_test.go | 76 ++++++- server/service/software_installers.go | 19 +- server/service/software_installers_test.go | 199 +++++++++++++++++- 17 files changed, 761 insertions(+), 84 deletions(-) create mode 100644 changes/45728-gitops-software-upload-progress create mode 100644 cmd/fleetctl/fleetctl/testdata/gitops/lib/dummy_installer.pkg create mode 100644 cmd/fleetctl/fleetctl/testdata/gitops/team_software_script_package.yml diff --git a/changes/45728-gitops-software-upload-progress b/changes/45728-gitops-software-upload-progress new file mode 100644 index 0000000000..c897f6077f --- /dev/null +++ b/changes/45728-gitops-software-upload-progress @@ -0,0 +1 @@ +- Added software upload progress logging to fleetctl GitOps. diff --git a/cmd/fleetctl/fleetctl/testdata/gitops/lib/dummy_installer.pkg b/cmd/fleetctl/fleetctl/testdata/gitops/lib/dummy_installer.pkg new file mode 100644 index 0000000000000000000000000000000000000000..41aa77748b189402122c0cff0c809554fccea101 GIT binary patch literal 644 zcmV-~0(<>h=KBKi7a2P#ErPaf~CXLiS{4%4|oR~Sauwfls=lLSF)I_uig zKKVL+rXo2MZ)(y*@rIP|%zPka2GD+fk$QKAf)SRbWSbIxWPF#=9pMT;m&8XknD6m& zMx(zquusNX(Z)o4Ul4^C6r-cN2XciY2o!kbh_}i$fSjG%UTCcLIwGVRXivT_ zSS}P^@FTGZuxG7BEDzyFS5d>Es7(JlOQ_IrAK7DU8@q zv0>WEiZXzVam@u(kTYhj=3G9;NuV+sa5NN*qo`i^n|K}Tpm^GKQOjQCXz5qcZY%9&Vf*87D4`c}3 zv|L$(72I%uL%n0tj8mo17lZK91)`;C9E#JQ6x2I1ek{nKu>Ocz-^kbf7Nhfcrik83 zfkRw9=n7It4q9c z?G%=?&r@?#cRYr^0&neOO7= 400 { return nil, nil, fleet.NewInvalidArgumentError( "software.url", - fmt.Sprintf("Couldn't edit software. URL (%q) received response status code %d.", downloadURL, resp.StatusCode), + fmt.Sprintf("URL (%q) received response status code %d.", downloadURL, resp.StatusCode), ) } @@ -3001,7 +3006,7 @@ func downloadInstallerURL(ctx context.Context, downloadURL string, ifNoneMatch s if errors.Is(err, fleethttp.ErrMaxSizeExceeded) || errors.As(err, &maxBytesErr) { return nil, nil, fleet.NewInvalidArgumentError( "software.url", - fmt.Sprintf("Couldn't edit software. URL (%q). The maximum file size is %s", downloadURL, installersize.Human(maxInstallerSize)), + fmt.Sprintf("URL (%q). The maximum file size is %s", downloadURL, installersize.Human(maxInstallerSize)), ) } return nil, nil, fmt.Errorf("reading installer %q contents: %w", downloadURL, err) @@ -3010,6 +3015,23 @@ func downloadInstallerURL(ctx context.Context, downloadURL string, ifNoneMatch s return resp, tfr, nil } +func softwarePackageProgressName(payload *fleet.SoftwareInstallerPayload) string { + switch { + case payload.DisplayName != "": + return payload.DisplayName + case payload.MaintainedApp != nil && payload.MaintainedApp.Name != "": + return payload.MaintainedApp.Name + } + + filename := file.ExtractFilenameFromURLPath(payload.URL, "") + // A url path with no extension comes back with a trailing dot. + filename = strings.TrimSuffix(filename, ".") + if filename == "" { + return payload.URL + } + return filename +} + func (svc *Service) softwareBatchUpload( requestUUID string, teamID *uint, @@ -3062,6 +3084,11 @@ func (svc *Service) softwareBatchUpload( } }(time.Now()) + // Every write marshals the whole slice, so writing only your own index isn't enough if + // the download goroutine limit is ever raised. The Redis write stays outside the lock. + downloadProgress := make([]fleet.SoftwarePackageDownloadProgress, len(payloads)) + var downloadProgressMutex sync.Mutex + // Periodically refresh the expiration on the batch install process so that, even when downloading/uploading // large installers, we ensure the server doesn't lose track of the batch. This way, the only time a batch times // out is if the server goes offline during running the batch. @@ -3077,6 +3104,20 @@ func (svc *Service) softwareBatchUpload( return case <-ticker.C: _ = svc.keyValueStore.Set(ctx, batchSoftwarePrefix+requestUUID, batchSetProcessing, keyExpireTime) + + progressKey := batchSoftwarePrefix + requestUUID + batchSoftwareDownloadedSuffix + progressJSON, err := svc.keyValueStore.Get(ctx, progressKey) + if err == nil && progressJSON != nil { + _ = svc.keyValueStore.Set(ctx, progressKey, *progressJSON, 10*time.Minute) + } + + categoriesKey := batchSoftwarePrefix + requestUUID + batchSoftwareCategoriesSuffix + categoriesJSON, err := svc.keyValueStore.Get(ctx, categoriesKey) + if err == nil && categoriesJSON != nil { + _ = svc.keyValueStore.Set(ctx, categoriesKey, *categoriesJSON, 10*time.Minute) + } + // The deleted key is only written once the downloads are done, and refreshed + // again as the batch completes, so it doesn't need this. } } }() @@ -3148,6 +3189,25 @@ func (svc *Service) softwareBatchUpload( installers := make([]*installerPayloadWithExtras, len(payloads)) toBeClosedTFRs := make([]*fleet.TempFileReader, len(payloads)) + setDownloadProgress := func(payloadIndex int, status fleet.SoftwarePackageDownloadStatus) { + downloadProgressMutex.Lock() + downloadProgress[payloadIndex] = fleet.SoftwarePackageDownloadProgress{ + Name: softwarePackageProgressName(payloads[payloadIndex]), + Status: status, + } + progressJSON, err := json.Marshal(downloadProgress) + downloadProgressMutex.Unlock() + + if err != nil { + svc.logger.ErrorContext(ctx, "encoding software package download progress", "request_uuid", requestUUID, "err", err) + return + } + + if err := svc.keyValueStore.Set(ctx, batchSoftwarePrefix+requestUUID+batchSoftwareDownloadedSuffix, string(progressJSON), 10*time.Minute); err != nil { + svc.logger.ErrorContext(ctx, "recording software package download progress", "request_uuid", requestUUID, "err", err) + } + } + for i, p := range payloads { i, p := i, p @@ -3329,6 +3389,8 @@ func (svc *Service) softwareBatchUpload( toBeClosedTFRs[i] = tfr installer.Filename = filename } else { + setDownloadProgress(i, fleet.SoftwarePackageDownloadStarted) + // Conditional GET (default behavior, disabled by always_download: true). // Look up existing installer by URL for its ETag, only when // we're about to download (avoids wasted DB queries). @@ -3357,6 +3419,7 @@ func (svc *Service) softwareBatchUpload( resp, tfr, err := retryDownload(ctx, p.URL, ifNoneMatch) if err != nil { + setDownloadProgress(i, fleet.SoftwarePackageDownloadFailed) return err } @@ -3372,6 +3435,7 @@ func (svc *Service) softwareBatchUpload( bytesExist, existErr := svc.softwareInstallStore.Exists(ctx, existingForCache.StorageID) if existErr == nil && bytesExist { if err := svc.fillSoftwareInstallerPayloadFromExisting(ctx, installer, existingForCache, existingForCache.StorageID); err != nil { + setDownloadProgress(i, fleet.SoftwarePackageDownloadFailed) return err } installer.HTTPETag = existingForCache.HTTPETag @@ -3386,9 +3450,11 @@ func (svc *Service) softwareBatchUpload( svc.logger.WarnContext(ctx, "304 received but installer bytes missing, re-downloading", "url", p.URL) resp, tfr, err = retryDownload(ctx, p.URL, "") if err != nil { + setDownloadProgress(i, fleet.SoftwarePackageDownloadFailed) return err } if resp != nil && resp.StatusCode == http.StatusNotModified { + setDownloadProgress(i, fleet.SoftwarePackageDownloadFailed) return fmt.Errorf("server returned 304 on unconditional re-download of %q", p.URL) } } @@ -3402,6 +3468,7 @@ func (svc *Service) softwareBatchUpload( if resp != nil { statusCode = resp.StatusCode } + setDownloadProgress(i, fleet.SoftwarePackageDownloadFailed) return fmt.Errorf("download of %q returned no body (status %d)", p.URL, statusCode) } @@ -3430,7 +3497,15 @@ func (svc *Service) softwareBatchUpload( installer.PreInstallQuery = "" } } + + if cacheHit { + setDownloadProgress(i, fleet.SoftwarePackageDownloadSkipped) + } else { + setDownloadProgress(i, fleet.SoftwarePackageDownloadFinished) + } } + } else { + setDownloadProgress(i, fleet.SoftwarePackageDownloadSkipped) } if p.Slug != nil && *p.Slug != "" { @@ -3874,19 +3949,18 @@ func validETag(etag string) bool { return true } -func (svc *Service) GetBatchSetSoftwareInstallersResult(ctx context.Context, tmName string, requestUUID string, dryRun bool) (string, string, []fleet.SoftwarePackageResponse, []fleet.DeletedSoftwarePackage, []string, error) { - // We've already authorized in the POST /api/latest/fleet/software/batch, - // but adding it here so we don't need to worry about a special case endpoint. +func (svc *Service) GetBatchSetSoftwareInstallersResult(ctx context.Context, tmName string, requestUUID string, dryRun bool) (*fleet.BatchSetSoftwareInstallersResult, error) { + // A running batch only reports download progress, so polling it takes any logged in user. if err := svc.authz.Authorize(ctx, &fleet.Team{}, fleet.ActionRead); err != nil { - return "", "", nil, nil, nil, err + return nil, ctxerr.Wrap(ctx, err, "validating authorization") } result, err := svc.keyValueStore.Get(ctx, batchSoftwarePrefix+requestUUID) if err != nil { - return "", "", nil, nil, nil, ctxerr.Wrap(ctx, err, "failed to get result") + return nil, ctxerr.Wrap(ctx, err, "failed to get result") } if result == nil { - return "", "", nil, nil, nil, ctxerr.Wrap(ctx, ¬FoundError{}, "request_uuid not found") + return nil, ctxerr.Wrap(ctx, ¬FoundError{}, "request_uuid not found") } // getDeletedPackages loads the packages the batch deleted (dry run: would @@ -3922,61 +3996,92 @@ func (svc *Service) GetBatchSetSoftwareInstallersResult(ctx context.Context, tmN return categories, nil } + // getDownloadProgress loads how far the batch got through downloading. Progress is only + // printed for the user, so an unreadable key degrades to an empty list, not an error. + getDownloadProgress := func() []fleet.SoftwarePackageDownloadProgress { + progressJSON, err := svc.keyValueStore.Get(ctx, batchSoftwarePrefix+requestUUID+batchSoftwareDownloadedSuffix) + if err != nil { + svc.logger.ErrorContext(ctx, "failed to get software package download progress", "request_uuid", requestUUID, "err", err) + return nil + } + if progressJSON == nil || *progressJSON == "" { + return nil + } + var downloadProgress []fleet.SoftwarePackageDownloadProgress + if err := json.Unmarshal([]byte(*progressJSON), &downloadProgress); err != nil { + svc.logger.ErrorContext(ctx, "unreadable software package download progress", "request_uuid", requestUUID, "err", err) + return nil + } + return downloadProgress + } + switch { case *result == batchSetCompleted: // fall through to retrieving the (deleted) software packages below. case *result == batchSetProcessing: - return fleet.BatchSetSoftwareInstallersStatusProcessing, "", nil, nil, nil, nil + return &fleet.BatchSetSoftwareInstallersResult{ + Status: fleet.BatchSetSoftwareInstallersStatusProcessing, + DownloadProgress: getDownloadProgress(), + }, nil case strings.HasPrefix(*result, batchSetFailedPrefix): - message := strings.TrimPrefix(*result, batchSetFailedPrefix) - return fleet.BatchSetSoftwareInstallersStatusFailed, message, nil, nil, nil, nil + return &fleet.BatchSetSoftwareInstallersResult{ + Status: fleet.BatchSetSoftwareInstallersStatusFailed, + Message: strings.TrimPrefix(*result, batchSetFailedPrefix), + DownloadProgress: getDownloadProgress(), + }, nil default: - return "", "", nil, nil, nil, ctxerr.New(ctx, "invalid status") + return nil, ctxerr.New(ctx, "invalid status") } - var ( - teamID uint // GetSoftwareInstallers uses 0 for "No team" - ptrTeamID *uint // Authorize uses *uint for "No team" teamID - ) + // The fleet's own packages below take the same read as its installers. Resolved here, + // not up top, to keep the lookup out of every poll. + var teamID *uint if tmName != "" { team, err := svc.ds.TeamByName(ctx, tmName) if err != nil { - return "", "", nil, nil, nil, ctxerr.Wrap(ctx, err, "load team by name") + return nil, ctxerr.Wrap(ctx, err, "load team by name") } - teamID = team.ID - ptrTeamID = &team.ID + teamID = &team.ID } - - // We've already authorized in the POST /api/latest/fleet/software/batch, - // but adding it here so we don't need to worry about a special case endpoint. - // - // We use fleet.ActionWrite because this method is the counterpart of the POST - // /api/latest/fleet/software/batch. This applies to dry runs too, since the - // deleted-packages list exposes team-scoped software data. - if err := svc.authz.Authorize(ctx, &fleet.SoftwareInstaller{TeamID: ptrTeamID}, fleet.ActionWrite); err != nil { - return "", "", nil, nil, nil, ctxerr.Wrap(ctx, err, "validating authorization") + if err := svc.authz.Authorize(ctx, &fleet.SoftwareInstaller{TeamID: teamID}, fleet.ActionRead); err != nil { + return nil, ctxerr.Wrap(ctx, err, "validating authorization") } deletedPackages, err := getDeletedPackages() if err != nil { - return "", "", nil, nil, nil, err + return nil, err } categories, err := getCategories() if err != nil { - return "", "", nil, nil, nil, err + return nil, err } + // The packages that finish last only reach the progress key as the batch ends, so a + // completed batch carries progress too. + downloadProgress := getDownloadProgress() + if dryRun { - return fleet.BatchSetSoftwareInstallersStatusCompleted, "", nil, deletedPackages, categories, nil + return &fleet.BatchSetSoftwareInstallersResult{ + Status: fleet.BatchSetSoftwareInstallersStatusCompleted, + DeletedPackages: deletedPackages, + Categories: categories, + DownloadProgress: downloadProgress, + }, nil } - softwarePackages, err := svc.ds.GetSoftwareInstallers(ctx, teamID) + softwarePackages, err := svc.ds.GetSoftwareInstallers(ctx, ptr.ValOrZero(teamID)) if err != nil { - return "", "", nil, nil, nil, ctxerr.Wrap(ctx, err, "get software installers") + return nil, ctxerr.Wrap(ctx, err, "get software installers") } - return fleet.BatchSetSoftwareInstallersStatusCompleted, "", softwarePackages, deletedPackages, categories, nil + return &fleet.BatchSetSoftwareInstallersResult{ + Status: fleet.BatchSetSoftwareInstallersStatusCompleted, + Packages: softwarePackages, + DeletedPackages: deletedPackages, + Categories: categories, + DownloadProgress: downloadProgress, + }, nil } func (svc *Service) SelfServiceInstallSoftwareTitle(ctx context.Context, host *fleet.Host, softwareTitleID uint) error { diff --git a/ee/server/service/software_installers_test.go b/ee/server/service/software_installers_test.go index 1514c4ac29..cd6f2cb699 100644 --- a/ee/server/service/software_installers_test.go +++ b/ee/server/service/software_installers_test.go @@ -2514,12 +2514,12 @@ func TestBatchSetSoftwareInstallersDryRunEmptyReportsDeletions(t *testing.T) { require.Equal(t, wouldDelete, gotDeleted) // The result endpoint returns the deleted packages on the dry-run completed branch. - status, message, packages, deletedPackages, _, err := svc.GetBatchSetSoftwareInstallersResult(ctx, "", requestUUID, true) + result, err := svc.GetBatchSetSoftwareInstallersResult(ctx, "", requestUUID, true) require.NoError(t, err) - require.Equal(t, fleet.BatchSetSoftwareInstallersStatusCompleted, status) - require.Empty(t, message) - require.Empty(t, packages) - require.Equal(t, wouldDelete, deletedPackages) + require.Equal(t, fleet.BatchSetSoftwareInstallersStatusCompleted, result.Status) + require.Empty(t, result.Message) + require.Empty(t, result.Packages) + require.Equal(t, wouldDelete, result.DeletedPackages) } func TestBatchSetSoftwareInstallersSkipsURLValidationForScriptPackages(t *testing.T) { @@ -2583,12 +2583,13 @@ func TestGetBatchSetSoftwareInstallersResultMissingDeletedKey(t *testing.T) { User: &fleet.User{GlobalRole: new(fleet.RoleAdmin)}, }) - status, message, packages, deletedPackages, _, err := svc.GetBatchSetSoftwareInstallersResult(ctx, "", "test-uuid", true) + result, err := svc.GetBatchSetSoftwareInstallersResult(ctx, "", "test-uuid", true) require.NoError(t, err) - require.Equal(t, fleet.BatchSetSoftwareInstallersStatusCompleted, status) - require.Empty(t, message) - require.Empty(t, packages) - require.Empty(t, deletedPackages) + require.Equal(t, fleet.BatchSetSoftwareInstallersStatusCompleted, result.Status) + require.Empty(t, result.Message) + require.Empty(t, result.Packages) + require.Empty(t, result.DeletedPackages) + require.Empty(t, result.DownloadProgress) } func TestVersionMatchesMajor(t *testing.T) { diff --git a/server/fleet/service.go b/server/fleet/service.go index 8fc5719f0a..1b25f547a9 100644 --- a/server/fleet/service.go +++ b/server/fleet/service.go @@ -800,13 +800,7 @@ type Service interface { // Returns a request UUID that can be used to track an ongoing batch request (with GetBatchSetSoftwareInstallersResult). BatchSetSoftwareInstallers(ctx context.Context, tmName string, payloads []*SoftwareInstallerPayload, dryRun bool) (string, error) // GetBatchSetSoftwareInstallersResult polls for the status of a batch-apply started by BatchSetSoftwareInstallers. - // Return values: - // - 'status': status of the batch-apply which can be "processing", "completed" or "failed". - // - 'message': which contains error information when the status is "failed". - // - 'packages': Contains the list of the applied software packages (when status is "completed"). This is always empty for a dry run. - // - 'deleted_packages': Contains the list of packages the batch deleted (dry run: would delete), when status is "completed". - // - 'categories': Contains the list of categories the batch uses/added, when status is "completed". - GetBatchSetSoftwareInstallersResult(ctx context.Context, tmName string, requestUUID string, dryRun bool) (status string, message string, packages []SoftwarePackageResponse, deletedPackages []DeletedSoftwarePackage, categories []string, err error) + GetBatchSetSoftwareInstallersResult(ctx context.Context, tmName string, requestUUID string, dryRun bool) (*BatchSetSoftwareInstallersResult, error) // SelfServiceInstallSoftwareTitle installs a software title // initiated by the user diff --git a/server/fleet/software_installer.go b/server/fleet/software_installer.go index 5b25b52036..9c0d7ac233 100644 --- a/server/fleet/software_installer.go +++ b/server/fleet/software_installer.go @@ -212,6 +212,38 @@ type DeletedSoftwarePackage struct { DisplayName string `json:"display_name" db:"display_name"` } +// SoftwarePackageDownloadProgress reports one software package's download in a batch. +// A package that hasn't started downloading has an empty name. Entries keep their place in +// the batch payload, which is what tells two packages with the same name apart, so nothing +// may filter or reorder them. +type SoftwarePackageDownloadProgress struct { + Name string `json:"name"` + Status SoftwarePackageDownloadStatus `json:"status"` +} + +// SoftwarePackageDownloadStatus is how far a package got through its download. +type SoftwarePackageDownloadStatus string + +const ( + SoftwarePackageDownloadStarted SoftwarePackageDownloadStatus = "downloading" + SoftwarePackageDownloadFinished SoftwarePackageDownloadStatus = "downloaded" + SoftwarePackageDownloadFailed SoftwarePackageDownloadStatus = "failed" + SoftwarePackageDownloadSkipped SoftwarePackageDownloadStatus = "skipped" +) + +// BatchSetSoftwareInstallersResult is the status of a software batch started by +// BatchSetSoftwareInstallers. +type BatchSetSoftwareInstallersResult struct { + Status string + Message string + // Packages is always empty for a dry run. + Packages []SoftwarePackageResponse + // DeletedPackages holds what the batch deleted, or would delete on a dry run. + DeletedPackages []DeletedSoftwarePackage + Categories []string + DownloadProgress []SoftwarePackageDownloadProgress +} + // VPPAppResponse is the response type used when applying app store apps by batch. type VPPAppResponse struct { // TeamID is the ID of the team. diff --git a/server/mock/service/service_mock.go b/server/mock/service/service_mock.go index edc9999619..83c23bd6ac 100644 --- a/server/mock/service/service_mock.go +++ b/server/mock/service/service_mock.go @@ -502,7 +502,7 @@ type GetSoftwareInstallResultsFunc func(ctx context.Context, installUUID string) type BatchSetSoftwareInstallersFunc func(ctx context.Context, tmName string, payloads []*fleet.SoftwareInstallerPayload, dryRun bool) (string, error) -type GetBatchSetSoftwareInstallersResultFunc func(ctx context.Context, tmName string, requestUUID string, dryRun bool) (status string, message string, packages []fleet.SoftwarePackageResponse, deletedPackages []fleet.DeletedSoftwarePackage, categories []string, err error) +type GetBatchSetSoftwareInstallersResultFunc func(ctx context.Context, tmName string, requestUUID string, dryRun bool) (*fleet.BatchSetSoftwareInstallersResult, error) type SelfServiceInstallSoftwareTitleFunc func(ctx context.Context, host *fleet.Host, softwareTitleID uint) error @@ -4121,7 +4121,7 @@ func (s *Service) BatchSetSoftwareInstallers(ctx context.Context, tmName string, return s.BatchSetSoftwareInstallersFunc(ctx, tmName, payloads, dryRun) } -func (s *Service) GetBatchSetSoftwareInstallersResult(ctx context.Context, tmName string, requestUUID string, dryRun bool) (status string, message string, packages []fleet.SoftwarePackageResponse, deletedPackages []fleet.DeletedSoftwarePackage, categories []string, err error) { +func (s *Service) GetBatchSetSoftwareInstallersResult(ctx context.Context, tmName string, requestUUID string, dryRun bool) (*fleet.BatchSetSoftwareInstallersResult, error) { s.mu.Lock() s.GetBatchSetSoftwareInstallersResultFuncInvoked = true s.mu.Unlock() diff --git a/server/service/client.go b/server/service/client.go index 309c3c66bb..b3efbf4664 100644 --- a/server/service/client.go +++ b/server/service/client.go @@ -1189,11 +1189,19 @@ func (c *Client) ApplyGroup( for tmName, software := range tmSoftwarePackagesPayloads { // For non-dry run, currentTeamName and tmName are the same currentTeamName := getTeamName(tmName) - logfn(format, numberWithPluralization(len(software), "software package", "software packages"), tmName) - installers, deletedInstallers, categories, err := c.ApplyTeamSoftwareInstallers(currentTeamName, software, opts.ApplySpecOptions) + softwareCount := numberWithPluralization(len(software), "software package", "software packages") + if !opts.DryRun { + logfn(applyingTeamFormat, softwareCount, tmName) + } + installers, deletedInstallers, categories, err := c.ApplyTeamSoftwareInstallers(currentTeamName, software, opts.ApplySpecOptions, logfn) if err != nil { return nil, nil, nil, nil, fmt.Errorf("applying software installers for fleet %q: %w", tmName, err) } + if opts.DryRun { + logfn(dryRunAppliedTeamFormat, softwareCount, tmName) + } else { + logfn(appliedTeamFormat, softwareCount, tmName) + } logSoftwareDeletions(logfn, deletedInstallers, opts.DryRun) teamsSoftwareInstallers[tmName] = installers categoriesByTeam[currentTeamName] = append(categoriesByTeam[currentTeamName], categories...) @@ -3117,11 +3125,19 @@ func (c *Client) doGitOpsNoTeamSetupAndSoftware( format = dryRunAppliedTeamFormat } - logFn(format, numberWithPluralization(len(swPkgPayload), "software package", "software packages"), "'Unassigned'") - softwareInstallers, deletedInstallers, installerCategories, err := c.ApplyNoTeamSoftwareInstallers(swPkgPayload, fleet.ApplySpecOptions{DryRun: dryRun}) + softwareCount := numberWithPluralization(len(swPkgPayload), "software package", "software packages") + if !dryRun { + logFn(applyingTeamFormat, softwareCount, "'Unassigned'") + } + softwareInstallers, deletedInstallers, installerCategories, err := c.ApplyNoTeamSoftwareInstallers(swPkgPayload, fleet.ApplySpecOptions{DryRun: dryRun}, logFn) if err != nil { return nil, nil, fmt.Errorf("applying software installers: %w", err) } + if dryRun { + logFn(dryRunAppliedTeamFormat, softwareCount, "'Unassigned'") + } else { + logFn(appliedTeamFormat, softwareCount, "'Unassigned'") + } logSoftwareDeletions(logFn, deletedInstallers, dryRun) logFn(format, numberWithPluralization(len(appsPayload), "app store app", "app store apps"), "'Unassigned'") @@ -3138,9 +3154,6 @@ func (c *Client) doGitOpsNoTeamSetupAndSoftware( } } - if !dryRun { - logFn("[+] applied software packages for unassigned hosts\n") - } return softwareInstallers, vppApps, nil } diff --git a/server/service/client_software.go b/server/service/client_software.go index ad11d08b9f..b3df8fc598 100644 --- a/server/service/client_software.go +++ b/server/service/client_software.go @@ -88,15 +88,24 @@ func (c *Client) GetSoftwareTitleIcon(titleID uint, teamID uint) ([]byte, error) return nil, nil } -func (c *Client) ApplyNoTeamSoftwareInstallers(softwareInstallers []fleet.SoftwareInstallerPayload, opts fleet.ApplySpecOptions) ([]fleet.SoftwarePackageResponse, []fleet.DeletedSoftwarePackage, []string, error) { +func (c *Client) ApplyNoTeamSoftwareInstallers( + softwareInstallers []fleet.SoftwareInstallerPayload, + opts fleet.ApplySpecOptions, + logFn func(format string, args ...any), +) ([]fleet.SoftwarePackageResponse, []fleet.DeletedSoftwarePackage, []string, error) { query, err := url.ParseQuery(opts.RawQuery()) if err != nil { return nil, nil, nil, err } - return c.applySoftwareInstallers(softwareInstallers, query, opts.DryRun) + return c.applySoftwareInstallers(softwareInstallers, query, opts.DryRun, logFn) } -func (c *Client) applySoftwareInstallers(softwareInstallers []fleet.SoftwareInstallerPayload, query url.Values, dryRun bool) ([]fleet.SoftwarePackageResponse, []fleet.DeletedSoftwarePackage, []string, error) { +func (c *Client) applySoftwareInstallers( + softwareInstallers []fleet.SoftwareInstallerPayload, + query url.Values, + dryRun bool, + logFn func(format string, args ...any), +) ([]fleet.SoftwarePackageResponse, []fleet.DeletedSoftwarePackage, []string, error) { path := "/api/latest/fleet/software/batch" var resp batchSetSoftwareInstallersResponse if err := c.authenticatedRequestWithQuery(map[string]any{"software": softwareInstallers}, "POST", path, &resp, query.Encode()); err != nil { @@ -106,12 +115,59 @@ func (c *Client) applySoftwareInstallers(softwareInstallers []fleet.SoftwareInst return nil, nil, nil, nil } + // Keyed by place in the batch, since two packages can share a name. + printedDownloading := make(map[int]struct{}) + printedResult := make(map[int]struct{}) + + // Assumes the server downloads packages one by one, so each "downloading" line prints + // right before its own "downloaded" line. Concurrent downloads would break this. + logDownloadProgress := func(downloadProgress []fleet.SoftwarePackageDownloadProgress) { + for payloadIndex, packageProgress := range downloadProgress { + // A package the batch hasn't started downloading has no name yet. + if packageProgress.Name == "" { + continue + } + + // A package Fleet doesn't download gets only this line, never a downloading one. + if packageProgress.Status == fleet.SoftwarePackageDownloadSkipped { + _, printedSkip := printedResult[payloadIndex] + if !printedSkip { + printedResult[payloadIndex] = struct{}{} + logFn("[+] skipped downloading the software package (already in storage) - %s\n", packageProgress.Name) + } + continue + } + + // A package can still turn out to be skipped after this prints, when the download returns a 304. + _, printedStart := printedDownloading[payloadIndex] + if !printedStart { + printedDownloading[payloadIndex] = struct{}{} + logFn("[+] downloading software package - %s ...\n", packageProgress.Name) + } + + _, printedFinish := printedResult[payloadIndex] + if printedFinish { + continue + } + switch packageProgress.Status { + case fleet.SoftwarePackageDownloadFailed: + printedResult[payloadIndex] = struct{}{} + logFn("Error: could not download software package %s\n", packageProgress.Name) + case fleet.SoftwarePackageDownloadFinished: + printedResult[payloadIndex] = struct{}{} + logFn("[+] downloaded software package - %s\n", packageProgress.Name) + } + } + } + requestUUID := resp.RequestUUID for { var resp batchSetSoftwareInstallersResultResponse if err := c.authenticatedRequestWithQuery(nil, "GET", path+"/"+requestUUID, &resp, query.Encode()); err != nil { return nil, nil, nil, err } + logDownloadProgress(resp.DownloadProgress) + switch { case resp.Status == fleet.BatchSetSoftwareInstallersStatusProcessing: time.Sleep(1 * time.Second) diff --git a/server/service/client_teams.go b/server/service/client_teams.go index 440e2fbbd9..7c48422cdc 100644 --- a/server/service/client_teams.go +++ b/server/service/client_teams.go @@ -128,13 +128,18 @@ func (c *Client) ApplyTeamScripts(tmName string, scripts []fleet.ScriptPayload, return resp.Scripts, err } -func (c *Client) ApplyTeamSoftwareInstallers(tmName string, softwareInstallers []fleet.SoftwareInstallerPayload, opts fleet.ApplySpecOptions) ([]fleet.SoftwarePackageResponse, []fleet.DeletedSoftwarePackage, []string, error) { +func (c *Client) ApplyTeamSoftwareInstallers( + tmName string, + softwareInstallers []fleet.SoftwareInstallerPayload, + opts fleet.ApplySpecOptions, + logFn func(format string, args ...any), +) ([]fleet.SoftwarePackageResponse, []fleet.DeletedSoftwarePackage, []string, error) { query, err := url.ParseQuery(opts.RawQuery()) if err != nil { return nil, nil, nil, err } query.Add("fleet_name", tmName) - return c.applySoftwareInstallers(softwareInstallers, query, opts.DryRun) + return c.applySoftwareInstallers(softwareInstallers, query, opts.DryRun, logFn) } func (c *Client) ApplyTeamAppStoreAppsAssociation(tmName string, vppBatchPayload []fleet.VPPBatchPayload, opts fleet.ApplySpecOptions) ([]fleet.VPPAppResponse, []string, error) { diff --git a/server/service/client_test.go b/server/service/client_test.go index 887461f960..eeeebf77b7 100644 --- a/server/service/client_test.go +++ b/server/service/client_test.go @@ -3,8 +3,14 @@ package service import ( "context" "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" "os" "path/filepath" + "strings" + "sync" "testing" "github.com/fleetdm/fleet/v4/pkg/optjson" @@ -1304,3 +1310,103 @@ func TestEnsureHistoricalDataDefaults(t *testing.T) { }) } } + +func TestApplySoftwareInstallersProgress(t *testing.T) { + pkg := func(name string, status fleet.SoftwarePackageDownloadStatus) fleet.SoftwarePackageDownloadProgress { + return fleet.SoftwarePackageDownloadProgress{Name: name, Status: status} + } + poll := func(status string, progress ...fleet.SoftwarePackageDownloadProgress) batchSetSoftwareInstallersResultResponse { + return batchSetSoftwareInstallersResultResponse{Status: status, DownloadProgress: progress} + } + // Fakes the batch endpoints, handing out one scripted response per poll. + newClient := func(t *testing.T, polls []batchSetSoftwareInstallersResultResponse) *Client { + var mu sync.Mutex + var polled int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if r.Method == "POST" { + _ = json.NewEncoder(w).Encode(batchSetSoftwareInstallersResponse{RequestUUID: "test-uuid"}) + return + } + mu.Lock() + defer mu.Unlock() + _ = json.NewEncoder(w).Encode(polls[min(polled, len(polls)-1)]) + polled++ + })) + t.Cleanup(srv.Close) + client, err := NewClient(srv.URL, true, "", "") + require.NoError(t, err) + client.SetToken("test-token") + return client + } + + processing, completed := fleet.BatchSetSoftwareInstallersStatusProcessing, fleet.BatchSetSoftwareInstallersStatusCompleted + downloading, downloaded := fleet.SoftwarePackageDownloadStarted, fleet.SoftwarePackageDownloadFinished + + testCases := []struct { + name string + polls []batchSetSoftwareInstallersResultResponse + wantLines []string + }{ + { + // A package keeps its entry for the rest of the batch, so a line that already + // printed must not print again on the next poll. + name: "prints each line once however often it polls", + polls: []batchSetSoftwareInstallersResultResponse{ + poll(processing, pkg("zoom.pkg", downloading)), + poll(processing, pkg("zoom.pkg", downloaded), pkg("slack.pkg", downloading)), + poll(completed, pkg("zoom.pkg", downloaded), pkg("slack.pkg", downloaded)), + }, + wantLines: []string{ + "[+] downloading software package - zoom.pkg ...", + "[+] downloaded software package - zoom.pkg", + "[+] downloading software package - slack.pkg ...", + "[+] downloaded software package - slack.pkg", + }, + }, + { + // Fleet already has the bytes, so there is no download to report. + name: "a package already in storage reports the skip and no download", + polls: []batchSetSoftwareInstallersResultResponse{poll(completed, pkg("zoom.pkg", fleet.SoftwarePackageDownloadSkipped))}, + wantLines: []string{ + "[+] skipped downloading the software package (already in storage) - zoom.pkg", + }, + }, + { + // The same maintained app for two platforms carries one name. + name: "two packages sharing a name each report", + polls: []batchSetSoftwareInstallersResultResponse{ + poll(processing, pkg("OneDrive", downloaded), pkg("OneDrive", downloading)), + poll(completed, pkg("OneDrive", downloaded), pkg("OneDrive", downloaded)), + }, + wantLines: []string{ + "[+] downloading software package - OneDrive ...", + "[+] downloaded software package - OneDrive", + "[+] downloading software package - OneDrive ...", + "[+] downloaded software package - OneDrive", + }, + }, + { + name: "packages the batch never downloads stay silent", + polls: []batchSetSoftwareInstallersResultResponse{poll(completed, fleet.SoftwarePackageDownloadProgress{}, pkg("zoom.pkg", downloaded), fleet.SoftwarePackageDownloadProgress{})}, + wantLines: []string{ + "[+] downloading software package - zoom.pkg ...", + "[+] downloaded software package - zoom.pkg", + }, + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + var lines []string + logFn := func(format string, args ...any) { + lines = append(lines, strings.TrimSuffix(fmt.Sprintf(format, args...), "\n")) + } + + client := newClient(t, tt.polls) + _, _, _, err := client.applySoftwareInstallers(nil, url.Values{}, false, logFn) + require.NoError(t, err) + require.Equal(t, tt.wantLines, lines) + }) + } +} diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 3fddda8682..6fbe012863 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -14433,7 +14433,8 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallers() { s.DoJSON("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: softwareToInstall}, http.StatusAccepted, &batchResponse, "team_name", tm.Name) message := waitBatchSetSoftwareInstallersFailed(t, &s.withServer, tm.Name, batchResponse.RequestUUID) require.NotEmpty(t, message) - require.Contains(t, message, fmt.Sprintf("validation failed: software.url Couldn't edit software. URL (\"%s/not_found.pkg\") returned \"Not Found\". Please make sure that URLs are reachable from your Fleet server.", srv.URL)) + expectedNotFoundMessage := fmt.Sprintf("validation failed: software.url URL (\"%s/not_found.pkg\") returned \"Not Found\". Please make sure that URLs are reachable from your Fleet server.", srv.URL) + require.Contains(t, message, expectedNotFoundMessage) // do a request with a valid URL rubyURL := srv.URL + "/ruby.deb" @@ -34921,6 +34922,79 @@ func (s *integrationEnterpriseTestSuite) TestScriptPackageFleetVariables() { require.Equal(t, updatedContents, meta.InstallScript) } +func (s *integrationEnterpriseTestSuite) TestSoftwareBatchDownloadProgressManyPackages() { + t := s.T() + teamName := "software-batch-download-progress" + const packageCount = 100 + + // One installer served under many names, instead of building an archive per package. + installer, err := os.ReadFile(filepath.Join("testdata", "software-installers", "test.tar.gz")) + require.NoError(t, err) + payloads := make([]*fleet.SoftwareInstallerPayload, 0, packageCount) + names := make([]string, 0, packageCount) + + // Serves an ETag and honors If-None-Match, so a second apply revalidates. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + name := strings.TrimPrefix(r.URL.Path, "/") + etag := fmt.Sprintf("%q", name) + w.Header().Set("ETag", etag) + if r.Header.Get("If-None-Match") == etag { + w.WriteHeader(http.StatusNotModified) + return + } + _, _ = w.Write(installer) + })) + t.Cleanup(srv.Close) + + for i := 1; i <= packageCount; i++ { + name := fmt.Sprintf("tarball-package-%03d.tar.gz", i) + names = append(names, name) + payloads = append(payloads, &fleet.SoftwareInstallerPayload{ + URL: srv.URL + "/" + name, + InstallScript: "echo installing", + UninstallScript: "echo uninstalling", + }) + } + + _, err = s.ds.NewTeam(t.Context(), &fleet.Team{Name: teamName}) + require.NoError(t, err) + + var batchResponse batchSetSoftwareInstallersResponse + s.DoJSON("POST", "/api/latest/fleet/software/batch", + batchSetSoftwareInstallersRequest{Software: payloads}, + http.StatusAccepted, &batchResponse, "fleet_name", teamName) + packages := waitBatchSetSoftwareInstallersCompleted(t, &s.withServer, teamName, batchResponse.RequestUUID) + require.Len(t, packages, packageCount) + + var batchResult batchSetSoftwareInstallersResultResponse + s.DoJSON("GET", "/api/latest/fleet/software/batch/"+batchResponse.RequestUUID, nil, http.StatusOK, + &batchResult, "fleet_name", teamName) + + // Every package reports on its own place in the payload, and none is left mid-download. + require.Len(t, batchResult.DownloadProgress, packageCount) + for i, progress := range batchResult.DownloadProgress { + require.Equal(t, names[i], progress.Name, "package at index %d", i) + require.Equal(t, fleet.SoftwarePackageDownloadFinished, progress.Status, "package at index %d", i) + } + + // The same packages again: every conditional request gets a 304, so nothing transfers. + batchResponse = batchSetSoftwareInstallersResponse{} + s.DoJSON("POST", "/api/latest/fleet/software/batch", + batchSetSoftwareInstallersRequest{Software: payloads}, + http.StatusAccepted, &batchResponse, "fleet_name", teamName) + waitBatchSetSoftwareInstallersCompleted(t, &s.withServer, teamName, batchResponse.RequestUUID) + + batchResult = batchSetSoftwareInstallersResultResponse{} + s.DoJSON("GET", "/api/latest/fleet/software/batch/"+batchResponse.RequestUUID, nil, http.StatusOK, + &batchResult, "fleet_name", teamName) + + require.Len(t, batchResult.DownloadProgress, packageCount) + for i, progress := range batchResult.DownloadProgress { + require.Equal(t, names[i], progress.Name, "package at index %d", i) + require.Equal(t, fleet.SoftwarePackageDownloadSkipped, progress.Status, "package at index %d", i) + } +} + func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallersFMARebuildSameVersion() { t := s.T() ctx := context.Background() diff --git a/server/service/software_installers.go b/server/service/software_installers.go index df2bae7db8..6f7996518f 100644 --- a/server/service/software_installers.go +++ b/server/service/software_installers.go @@ -901,6 +901,8 @@ type batchSetSoftwareInstallersResultResponse struct { DeletedPackages []fleet.DeletedSoftwarePackage `json:"deleted_packages,omitempty"` // Categories lists the self-service categories the batch's software references. Categories []string `json:"categories,omitempty"` + // DownloadProgress reports each package's download status while the batch runs. + DownloadProgress []fleet.SoftwarePackageDownloadProgress `json:"download_progress,omitempty"` Err error `json:"error,omitempty"` } @@ -909,25 +911,26 @@ func (r batchSetSoftwareInstallersResultResponse) Error() error { return r.Err } func batchSetSoftwareInstallersResultEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (fleet.Errorer, error) { req := request.(*batchSetSoftwareInstallersResultRequest) - status, message, packages, deletedPackages, categories, err := svc.GetBatchSetSoftwareInstallersResult(ctx, req.TeamName, req.RequestUUID, req.DryRun) + result, err := svc.GetBatchSetSoftwareInstallersResult(ctx, req.TeamName, req.RequestUUID, req.DryRun) if err != nil { return batchSetSoftwareInstallersResultResponse{Err: err}, nil } return batchSetSoftwareInstallersResultResponse{ - Status: status, - Message: message, - Packages: packages, - DeletedPackages: deletedPackages, - Categories: categories, + Status: result.Status, + Message: result.Message, + Packages: result.Packages, + DeletedPackages: result.DeletedPackages, + Categories: result.Categories, + DownloadProgress: result.DownloadProgress, }, nil } -func (svc *Service) GetBatchSetSoftwareInstallersResult(ctx context.Context, tmName string, requestUUID string, dryRun bool) (string, string, []fleet.SoftwarePackageResponse, []fleet.DeletedSoftwarePackage, []string, error) { +func (svc *Service) GetBatchSetSoftwareInstallersResult(ctx context.Context, tmName string, requestUUID string, dryRun bool) (*fleet.BatchSetSoftwareInstallersResult, error) { // skipauth: No authorization check needed due to implementation returning // only license error. svc.authz.SkipAuthorization(ctx) - return "", "", nil, nil, nil, fleet.ErrMissingLicense + return nil, fleet.ErrMissingLicense } ////////////////////////////////////////////////////////////////////////////// diff --git a/server/service/software_installers_test.go b/server/service/software_installers_test.go index cb666e15a6..46e5c47920 100644 --- a/server/service/software_installers_test.go +++ b/server/service/software_installers_test.go @@ -11,6 +11,7 @@ import ( "net/http/httptest" "path/filepath" "runtime" + "strings" "sync" "sync/atomic" "testing" @@ -626,6 +627,11 @@ func TestSoftwareInstallerUploadRetries(t *testing.T) { kvStore.GetFunc = func(ctx context.Context, key string) (*string, error) { statusMu.Lock() defer statusMu.Unlock() + // Only the batch status key holds a value here. The sibling keys for deleted + // packages, categories and download progress are all empty. + if strings.Contains(key, ":") { + return nil, nil + } return ptr.String(status), nil } @@ -713,12 +719,12 @@ func TestSoftwareInstallerUploadRetries(t *testing.T) { timeout := time.After(30 * time.Second) for { - status, _, packages, _, _, err := svc.GetBatchSetSoftwareInstallersResult(ctx, "foo", "requestuuid", false) + result, err := svc.GetBatchSetSoftwareInstallersResult(ctx, "foo", "requestuuid", false) require.NoError(t, err) // The status will be failed IFF // the mock installer store's Put method was called fleet.BatchUploadMaxRetries times. - if status == fleet.BatchSetSoftwareInstallersStatusFailed { - require.Empty(t, packages) + if result.Status == fleet.BatchSetSoftwareInstallersStatusFailed { + require.Empty(t, result.Packages) break } select { @@ -731,3 +737,190 @@ func TestSoftwareInstallerUploadRetries(t *testing.T) { } } + +func TestGetBatchSetSoftwareInstallersResultAuth(t *testing.T) { + ds := new(mock.Store) + license := &fleet.LicenseInfo{Tier: fleet.TierPremium, Expiration: time.Now().Add(24 * time.Hour)} + + kvStore := &mock.KVStore{} + kvStore.GetFunc = func(ctx context.Context, key string) (*string, error) { + // Completed is the only status that authorizes against the fleet. + if strings.Contains(key, ":") { + return nil, nil + } + return new(fleet.BatchSetSoftwareInstallersStatusCompleted), nil + } + + svc, ctx := newTestService(t, ds, nil, nil, &TestServerOpts{License: license, KeyValueStore: kvStore}) + + ds.TeamByNameFunc = func(ctx context.Context, name string) (*fleet.Team, error) { + return &fleet.Team{ID: 1, Name: name}, nil + } + ds.GetSoftwareInstallersFunc = func(ctx context.Context, teamID uint) ([]fleet.SoftwarePackageResponse, error) { + return nil, nil + } + + // Reading a batch result takes the same read as the fleet's installers, so observers are + // out even though they can read the fleet's software titles. + testCases := []struct { + name string + user *fleet.User + teamName string + shouldFail bool + }{ + {"global admin", test.UserAdmin, "team1", false}, + {"global maintainer", test.UserMaintainer, "team1", false}, + {"global technician", test.UserTechnician, "team1", false}, + {"global gitops", test.UserGitOps, "team1", false}, + {"global observer", test.UserObserver, "team1", true}, + {"global observer+", test.UserObserverPlus, "team1", true}, + {"no role", test.UserNoRoles, "team1", true}, + {"team admin", test.UserTeamAdminTeam1, "team1", false}, + {"team technician", test.UserTeamTechnicianTeam1, "team1", false}, + {"team gitops", test.UserTeamGitOpsTeam1, "team1", false}, + {"team observer", test.UserTeamObserverTeam1, "team1", true}, + {"team observer+", test.UserTeamObserverPlusTeam1, "team1", true}, + {"team admin other fleet", test.UserTeamAdminTeam2, "team1", true}, + {"team technician other fleet", test.UserTeamTechnicianTeam2, "team1", true}, + {"global admin unassigned", test.UserAdmin, "", false}, + {"global observer unassigned", test.UserObserver, "", true}, + {"team admin unassigned", test.UserTeamAdminTeam1, "", true}, + {"no role unassigned", test.UserNoRoles, "", true}, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + ctx := viewer.NewContext(ctx, viewer.Viewer{User: tt.user}) + + _, err := svc.GetBatchSetSoftwareInstallersResult(ctx, tt.teamName, "request-uuid", false) + checkAuthErr(t, tt.shouldFail, err) + }) + } + + // A running batch reports only progress, so anyone logged in can poll it. + t.Run("polling a running batch only takes a logged in user", func(t *testing.T) { + processingKVStore := &mock.KVStore{} + processingKVStore.GetFunc = func(ctx context.Context, key string) (*string, error) { + if strings.Contains(key, ":") { + return nil, nil + } + return new(fleet.BatchSetSoftwareInstallersStatusProcessing), nil + } + processingSvc, processingCtx := newTestService(t, ds, nil, nil, &TestServerOpts{License: license, KeyValueStore: processingKVStore}) + + ctx := viewer.NewContext(processingCtx, viewer.Viewer{User: test.UserTeamObserverTeam1}) + result, err := processingSvc.GetBatchSetSoftwareInstallersResult(ctx, "team1", "request-uuid", false) + require.NoError(t, err) + require.Equal(t, fleet.BatchSetSoftwareInstallersStatusProcessing, result.Status) + }) +} + +func TestSoftwareBatchProgressWriteFailure(t *testing.T) { + // Progress is only ever printed for the user, so losing it must not turn a batch that + // would have succeeded into a failed one. + ds := new(mock.Store) + lic := &fleet.LicenseInfo{Tier: fleet.TierPremium, Expiration: time.Now().Add(24 * time.Hour)} + + var kvMu sync.Mutex + batchStatus := fleet.BatchSetSoftwareInstallersStatusProcessing + + kvStore := &mock.KVStore{} + kvStore.SetFunc = func(ctx context.Context, key string, value string, expireTime time.Duration) error { + kvMu.Lock() + defer kvMu.Unlock() + switch { + case strings.HasSuffix(key, ":downloaded"): + return errors.New("progress write failed") + case !strings.Contains(key, ":"): + batchStatus = value + } + return nil + } + kvStore.GetFunc = func(ctx context.Context, key string) (*string, error) { + kvMu.Lock() + defer kvMu.Unlock() + if strings.Contains(key, ":") { + return nil, nil + } + return new(batchStatus), nil + } + + softwareInstallStore, err := filesystem.NewSoftwareInstallerStore(t.TempDir()) + require.NoError(t, err) + + svc, ctx := newTestService(t, ds, nil, nil, &TestServerOpts{ + License: lic, + SoftwareInstallStore: softwareInstallStore, + KeyValueStore: kvStore, + }) + + authCtx := authz_ctx.AuthorizationContext{} + ctx = authz_ctx.NewContext(ctx, &authCtx) + ctx = viewer.NewContext(ctx, viewer.Viewer{User: test.UserAdmin}) + actx, _ := authz_ctx.FromContext(ctx) + actx.SetChecked() + + ds.TeamByNameFunc = func(ctx context.Context, name string) (*fleet.Team, error) { + return &fleet.Team{ID: 1, Name: "foo"}, nil + } + ds.TeamLiteFunc = func(ctx context.Context, tid uint) (*fleet.TeamLite, error) { + return &fleet.TeamLite{ID: 1, Name: "foo"}, nil + } + ds.ValidateEmbeddedSecretsFunc = func(ctx context.Context, documents []string) error { + return nil + } + ds.ValidateReferencedCustomHostVitalsFunc = func(ctx context.Context, documents []string) error { + return nil + } + ds.GetSoftwareCategoryNameToIDMapFunc = func(ctx context.Context, teamID uint, names []string) (map[string]uint, error) { + return map[string]uint{}, nil + } + ds.GetTeamsWithInstallerByHashFunc = func(ctx context.Context, sha256 string, url string) (map[uint][]*fleet.ExistingSoftwareInstaller, error) { + return map[uint][]*fleet.ExistingSoftwareInstaller{}, nil + } + ds.GetInstallerByTeamAndURLFunc = func(ctx context.Context, teamID *uint, url string) (*fleet.ExistingSoftwareInstaller, error) { + return nil, nil + } + ds.BatchSetSoftwareInstallersFunc = func(ctx context.Context, tmID *uint, installers []*fleet.UploadSoftwareInstallerPayload) error { + return nil + } + ds.BatchSetInHouseAppsInstallersFunc = func(ctx context.Context, tmID *uint, installers []*fleet.UploadSoftwareInstallerPayload) error { + return nil + } + ds.GetSoftwareInstallersPendingDeletionFunc = func(ctx context.Context, tmID *uint, incoming []fleet.SoftwareTitleIdentifier) ([]fleet.DeletedSoftwarePackage, error) { + return nil, nil + } + ds.GetSoftwareInstallersFunc = func(ctx context.Context, tmID uint) ([]fleet.SoftwarePackageResponse, error) { + return []fleet.SoftwarePackageResponse{}, nil + } + + baseDir := getPathRelative("./testdata/software-installers/") + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.ServeFile(w, r, filepath.Join(baseDir, filepath.Base(r.URL.Path))) + })) + t.Cleanup(srv.Close) + + requestUUID, err := svc.BatchSetSoftwareInstallers(ctx, "foo", []*fleet.SoftwareInstallerPayload{{ + URL: srv.URL + "/dummy_installer.pkg", + InstallScript: "install", + UninstallScript: "uninstall", + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + Categories: optjson.SetSlice([]string{}), + }}, false) + require.NoError(t, err) + + timeout := time.After(10 * time.Second) + for { + result, err := svc.GetBatchSetSoftwareInstallersResult(ctx, "foo", requestUUID, false) + require.NoError(t, err) + if result.Status == fleet.BatchSetSoftwareInstallersStatusCompleted { + break + } + require.NotEqual(t, fleet.BatchSetSoftwareInstallersStatusFailed, result.Status, result.Message) + select { + case <-timeout: + t.Fatal("batch never completed") + case <-time.After(20 * time.Millisecond): + } + } +}