Add labels to update custom installer endpoint (#24857)

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")

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")
 	}

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.

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)
 	})

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