diff --git a/ee/server/service/software_installers.go b/ee/server/service/software_installers.go index 380dc14eab..cd0ada79f9 100644 --- a/ee/server/service/software_installers.go +++ b/ee/server/service/software_installers.go @@ -176,7 +176,7 @@ func (svc *Service) UpdateSoftwareInstaller(ctx context.Context, payload *fleet. var teamName *string if *payload.TeamID != 0 { - t, err := svc.ds.Team(ctx, *payload.TeamID) + t, err := svc.ds.TeamWithoutExtras(ctx, *payload.TeamID) if err != nil { return nil, err } @@ -205,7 +205,8 @@ func (svc *Service) UpdateSoftwareInstaller(ctx context.Context, payload *fleet. } if payload.SelfService == nil && payload.InstallerFile == nil && payload.PreInstallQuery == nil && - payload.InstallScript == nil && payload.PostInstallScript == nil && payload.UninstallScript == nil { + payload.InstallScript == nil && payload.PostInstallScript == nil && payload.UninstallScript == nil && + payload.LabelsIncludeAny == nil && payload.LabelsExcludeAny == nil { return existingInstaller, nil // no payload, noop } @@ -216,6 +217,15 @@ func (svc *Service) UpdateSoftwareInstaller(ctx context.Context, payload *fleet. dirty["SelfService"] = true } + shouldUpdateLabels, validatedLabels, err := svc.validateSoftwareLabelsForUpdate(ctx, existingInstaller, payload.LabelsIncludeAny, payload.LabelsExcludeAny) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "validating software labels for update") + } + if shouldUpdateLabels { + dirty["Labels"] = true + } + payload.ValidatedLabels = validatedLabels + // activity team ID must be null if no team, not zero var actTeamID *uint if payload.TeamID != nil && *payload.TeamID != 0 { @@ -396,6 +406,63 @@ func (svc *Service) UpdateSoftwareInstaller(ctx context.Context, payload *fleet. return updatedInstaller, nil } +func (svc *Service) validateSoftwareLabelsForUpdate(ctx context.Context, existingInstaller *fleet.SoftwareInstaller, includeAny, excludeAny []string) (shouldUpdate bool, validatedLabels *fleet.LabelIdentsWithScope, err error) { + if existingInstaller == nil { + return false, nil, errors.New("existing installer must be provided") + } + + if len(existingInstaller.LabelsIncludeAny) > 0 && len(existingInstaller.LabelsExcludeAny) > 0 { + return false, nil, errors.New("existing installer must have only one label scope") + } + + if includeAny == nil && excludeAny == nil { + // nothing to do + return false, nil, nil + } + + incoming, err := svc.validateSoftwareLabels(ctx, includeAny, excludeAny) + if err != nil { + return false, nil, err + } + + var prevScope fleet.LabelScope + var prevLabels []fleet.SoftwareScopeLabel + switch { + case len(existingInstaller.LabelsIncludeAny) > 0: + prevScope = fleet.LabelScopeIncludeAny + prevLabels = existingInstaller.LabelsIncludeAny + case len(existingInstaller.LabelsExcludeAny) > 0: + prevScope = fleet.LabelScopeExcludeAny + prevLabels = existingInstaller.LabelsExcludeAny + } + + prevByName := make(map[string]fleet.LabelIdent, len(prevLabels)) + for _, pl := range prevLabels { + prevByName[pl.LabelName] = fleet.LabelIdent{ + LabelID: pl.LabelID, + LabelName: pl.LabelName, + } + } + + if prevScope != incoming.LabelScope { + return true, incoming, nil + } + + if len(prevByName) != len(incoming.ByName) { + return true, incoming, nil + } + + // compare labels by name + for n, il := range incoming.ByName { + pl, ok := prevByName[n] + if !ok || pl != il { + return true, incoming, nil + } + } + + return false, nil, nil +} + func (svc *Service) DeleteSoftwareInstaller(ctx context.Context, titleID uint, teamID *uint) error { if teamID == nil { return fleet.NewInvalidArgumentError("team_id", "is required") diff --git a/server/datastore/mysql/software_installers.go b/server/datastore/mysql/software_installers.go index a51ea0c735..8733619064 100644 --- a/server/datastore/mysql/software_installers.go +++ b/server/datastore/mysql/software_installers.go @@ -330,7 +330,8 @@ func (ds *Datastore) SaveInstallerUpdates(ctx context.Context, payload *fleet.Up touchUploaded = ", uploaded_at = NOW()" } - stmt := fmt.Sprintf(`UPDATE software_installers SET + err = ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error { + stmt := fmt.Sprintf(`UPDATE software_installers SET storage_id = ?, filename = ?, version = ?, @@ -345,23 +346,34 @@ func (ds *Datastore) SaveInstallerUpdates(ctx context.Context, payload *fleet.Up user_email = (SELECT email FROM users WHERE id = ?) %s WHERE id = ?`, touchUploaded) - args := []interface{}{ - payload.StorageID, - payload.Filename, - payload.Version, - strings.Join(payload.PackageIDs, ","), - installScriptID, - *payload.PreInstallQuery, - postInstallScriptID, - uninstallScriptID, - *payload.SelfService, - payload.UserID, - payload.UserID, - payload.UserID, - payload.InstallerID, - } + args := []interface{}{ + payload.StorageID, + payload.Filename, + payload.Version, + strings.Join(payload.PackageIDs, ","), + installScriptID, + *payload.PreInstallQuery, + postInstallScriptID, + uninstallScriptID, + *payload.SelfService, + payload.UserID, + payload.UserID, + payload.UserID, + payload.InstallerID, + } - _, err = ds.writer(ctx).ExecContext(ctx, stmt, args...) + if _, err := tx.ExecContext(ctx, stmt, args...); err != nil { + return ctxerr.Wrap(ctx, err, "update software installer") + } + + if payload.ValidatedLabels != nil { + if err := setOrUpdateSoftwareInstallerLabelsDB(ctx, tx, payload.InstallerID, *payload.ValidatedLabels); err != nil { + return ctxerr.Wrap(ctx, err, "upsert software installer labels") + } + } + + return nil + }) if err != nil { return ctxerr.Wrap(ctx, err, "update software installer") } diff --git a/server/fleet/software_installer.go b/server/fleet/software_installer.go index 3d1f6057b2..e033efe28a 100644 --- a/server/fleet/software_installer.go +++ b/server/fleet/software_installer.go @@ -367,6 +367,9 @@ type UpdateSoftwareInstallerPayload struct { PackageIDs []string LabelsIncludeAny []string // names of "include any" labels LabelsExcludeAny []string // names of "exclude any" labels + // ValidatedLabels is a struct that contains the validated labels for the software installer. It + // can be nil if the labels have not been validated or if the labels are not being updated. + ValidatedLabels *LabelIdentsWithScope } // DownloadSoftwareInstallerPayload is the payload for downloading a software installer. diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 4a097f15ab..5433638573 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -11,6 +11,7 @@ import ( "errors" "fmt" "io" + "mime/multipart" "net/http" "net/http/httptest" "os" @@ -10655,7 +10656,7 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareInstallerUploadDownloadAndD // check labels exclude any require.Len(t, meta2.LabelsExcludeAny, len(payload.LabelsExcludeAny)) byName = make(map[string]struct{}, len(meta2.LabelsExcludeAny)) - for _, l := range meta.LabelsExcludeAny { + for _, l := range meta2.LabelsExcludeAny { byName[l.LabelName] = struct{}{} require.Equal(t, *meta2.TitleID, l.TitleID) require.True(t, l.Exclude) @@ -10707,13 +10708,47 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareInstallerUploadDownloadAndD // upload again fails s.uploadSoftwareInstaller(t, payload, http.StatusConflict, "already exists") + // patch the software installer to change the labels + var b bytes.Buffer + w := multipart.NewWriter(&b) + require.NoError(t, w.WriteField("team_id", "0")) + require.NoError(t, w.WriteField("labels_exclude_any", t.Name())) + w.Close() + headers := map[string]string{ + "Content-Type": w.FormDataContentType(), + "Accept": "application/json", + "Authorization": fmt.Sprintf("Bearer %s", s.token), + } + s.DoRawWithHeaders("PATCH", fmt.Sprintf("/api/latest/fleet/software/titles/%d/package", titleID), b.Bytes(), http.StatusOK, headers) + expectedPayload := *payload + expectedPayload.LabelsIncludeAny = nil + expectedPayload.LabelsExcludeAny = []string{t.Name()} + checkSoftwareInstaller(t, &expectedPayload) + + // patch the software installer again but this time change the pre install query and leave the labels as is + var b2 bytes.Buffer + w2 := multipart.NewWriter(&b2) + require.NoError(t, w2.WriteField("team_id", "0")) + require.NoError(t, w2.WriteField("pre_install_query", "some other pre install query")) + w2.Close() + headers = map[string]string{ + "Content-Type": w2.FormDataContentType(), + "Accept": "application/json", + "Authorization": fmt.Sprintf("Bearer %s", s.token), + } + s.DoRawWithHeaders("PATCH", fmt.Sprintf("/api/latest/fleet/software/titles/%d/package", titleID), b2.Bytes(), http.StatusOK, headers) + expectedPayload.PreInstallQuery = "some other pre install query" + expectedPayload.LabelsIncludeAny = nil // no change + expectedPayload.LabelsExcludeAny = []string{t.Name()} // no change + checkSoftwareInstaller(t, &expectedPayload) + // update the installer succeeds body, headers := generateMultipartRequest(t, "software", "", []byte{}, s.token, map[string][]string{"self_service": {"true"}, "team_id": {"0"}}) s.DoRawWithHeaders("PATCH", fmt.Sprintf("/api/latest/fleet/software/titles/%d/package", titleID), body.Bytes(), http.StatusOK, headers) activityData = fmt.Sprintf(`{"software_title": "ruby", "software_package": "ruby.deb", "team_name": null, - "team_id": null, "self_service": true, "labels_include_any": [{"id": %d, "name": %q}]}`, + "team_id": null, "self_service": true, "labels_exclude_any": [{"id": %d, "name": %q}]}`, labelResp.Label.ID, t.Name()) s.lastActivityMatches(fleet.ActivityTypeEditedSoftware{}.ActivityName(), activityData, 0) @@ -10732,7 +10767,7 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareInstallerUploadDownloadAndD // delete from team 0 succeeds s.Do("DELETE", fmt.Sprintf("/api/latest/fleet/software/titles/%d/available_for_install", titleID), nil, http.StatusNoContent, "team_id", "0") activityData = fmt.Sprintf(`{"software_title": "ruby", "software_package": "ruby.deb", "team_name": null, - "team_id": null, "self_service": true, "labels_include_any": [{"id": %d, "name": %q}]}`, + "team_id": null, "self_service": true, "labels_exclude_any": [{"id": %d, "name": %q}]}`, labelResp.Label.ID, t.Name()) s.lastActivityMatches(fleet.ActivityTypeDeletedSoftware{}.ActivityName(), activityData, 0) }) diff --git a/server/service/software_installers.go b/server/service/software_installers.go index a72b64c21e..6711063ecd 100644 --- a/server/service/software_installers.go +++ b/server/service/software_installers.go @@ -138,11 +138,12 @@ func (updateSoftwareInstallerRequest) DecodeRequest(ctx context.Context, r *http // decode labels var existsInclAny, existsExclAny bool decoded.LabelsIncludeAny, existsInclAny = r.MultipartForm.Value[string(fleet.LabelsIncludeAny)] + if !existsInclAny { + decoded.LabelsIncludeAny = nil + } decoded.LabelsExcludeAny, existsExclAny = r.MultipartForm.Value[string(fleet.LabelsExcludeAny)] - - // validate that only one of the labels type is provided - if existsInclAny && existsExclAny { - return nil, &fleet.BadRequestError{Message: `Only one of "labels_include_any" or "labels_exclude_any" can be included.`} + if !existsExclAny { + decoded.LabelsExcludeAny = nil } return &decoded, nil