diff --git a/cmd/fleetctl/fleetctl/generate_gitops.go b/cmd/fleetctl/fleetctl/generate_gitops.go index aa0fe9873e..aedd0c6287 100644 --- a/cmd/fleetctl/fleetctl/generate_gitops.go +++ b/cmd/fleetctl/fleetctl/generate_gitops.go @@ -94,7 +94,6 @@ type generateGitopsClient interface { ListFleetMaintainedApps(teamID uint) ([]fleet.MaintainedApp, error) GetFleetMaintainedApp(id uint) (*fleet.MaintainedApp, error) GetVPPTokens() ([]*fleet.VPPTokenDB, error) - ListSelfServiceCategories(teamID uint) ([]fleet.SoftwareCategory, error) } // Given a struct type and a field name, return the JSON field name. @@ -2276,19 +2275,6 @@ func (cmd *GenerateGitopsCommand) generateSoftware(filePath string, teamID uint, result["fleet_maintained_apps"] = fmas } - categories, err := cmd.Client.ListSelfServiceCategories(teamID) - if err != nil { - fmt.Fprintf(cmd.CLI.App.ErrWriter, "Error getting self-service categories: %s\n", err) - return nil, err - } - if len(categories) > 0 { - names := make([]string, 0, len(categories)) - for _, c := range categories { - names = append(names, c.Name) - } - result["self_service_categories"] = names - } - return result, nil } diff --git a/cmd/fleetctl/fleetctl/generate_gitops_test.go b/cmd/fleetctl/fleetctl/generate_gitops_test.go index f862865429..2a2b1e2505 100644 --- a/cmd/fleetctl/fleetctl/generate_gitops_test.go +++ b/cmd/fleetctl/fleetctl/generate_gitops_test.go @@ -34,7 +34,6 @@ type MockClient struct { TeamNameOverride string WithoutMDM bool WithoutVPP bool - Categories []fleet.SoftwareCategory } func (c *MockClient) GetAppConfig() (*fleet.EnrichedAppConfig, error) { @@ -346,13 +345,6 @@ func (MockClient) ListFleetMaintainedApps(teamID uint) ([]fleet.MaintainedApp, e }, nil } -func (c MockClient) ListSelfServiceCategories(teamID uint) ([]fleet.SoftwareCategory, error) { - if teamID == 1 { - return c.Categories, nil - } - return nil, nil -} - func (MockClient) GetPolicies(teamID *uint) ([]*fleet.Policy, error) { if teamID == nil { return []*fleet.Policy{ @@ -2589,35 +2581,6 @@ func TestGenerateGitopsExportOrgLogos(t *testing.T) { }) } -func TestGenerateGitopsEmitsSelfServiceCategories(t *testing.T) { - configureFMAManifestServer(t) - fleetClient := &MockClient{Categories: []fleet.SoftwareCategory{ - {ID: 10, Name: "🌎 Browsers", TeamID: 1}, - {ID: 11, Name: "💼 Engineering", TeamID: 1}, - }} - action := createGenerateGitopsAction(fleetClient) - - tempDir := os.TempDir() + "/" + uuid.New().String() - t.Cleanup(func() { _ = os.RemoveAll(tempDir) }) - - flagSet := flag.NewFlagSet("test", flag.ContinueOnError) - flagSet.String("dir", tempDir, "") - buf := new(bytes.Buffer) - cliContext := cli.NewContext(&cli.App{Name: "test", Usage: "test", Writer: buf, ErrWriter: buf}, flagSet, nil) - require.NoError(t, action(cliContext), buf.String()) - - teamYAML, err := os.ReadFile(tempDir + "/fleets/team-a-👍.yml") - require.NoError(t, err) - assert.Contains(t, string(teamYAML), "self_service_categories:") - assert.Contains(t, string(teamYAML), "🌎 Browsers") - assert.Contains(t, string(teamYAML), "💼 Engineering") - - // The fleet that returns no categories must NOT emit the key. - otherYAML, err := os.ReadFile(tempDir + "/fleets/unassigned.yml") - require.NoError(t, err) - assert.NotContains(t, string(otherYAML), "self_service_categories:") -} - func TestGeneratePoliciesPatchPolicyOrphanedFromFleetMaintainedApp(t *testing.T) { fleetClient := &MockClient{} appConfig, err := fleetClient.GetAppConfig() diff --git a/cmd/fleetctl/fleetctl/get.go b/cmd/fleetctl/fleetctl/get.go index 67b13b606f..4d1a2ff325 100644 --- a/cmd/fleetctl/fleetctl/get.go +++ b/cmd/fleetctl/fleetctl/get.go @@ -377,7 +377,7 @@ func getTeamSoftwareSpec(client *service.Client, teamID uint) (*fleet.SoftwareSp LabelsIncludeAny: scopeLabelNames(pkg.LabelsIncludeAny), LabelsExcludeAny: scopeLabelNames(pkg.LabelsExcludeAny), LabelsIncludeAll: scopeLabelNames(pkg.LabelsIncludeAll), - Categories: pkg.Categories, + Categories: optjson.SetSlice(pkg.Categories), InstallDuringSetup: setupExperienceValue(setupSoftwareByTitleID, title.ID), }) continue @@ -389,7 +389,7 @@ func getTeamSoftwareSpec(client *service.Client, teamID uint) (*fleet.SoftwareSp LabelsIncludeAny: scopeLabelNames(pkg.LabelsIncludeAny), LabelsExcludeAny: scopeLabelNames(pkg.LabelsExcludeAny), LabelsIncludeAll: scopeLabelNames(pkg.LabelsIncludeAll), - Categories: pkg.Categories, + Categories: optjson.SetSlice(pkg.Categories), InstallDuringSetup: setupExperienceValue(setupSoftwareByTitleID, title.ID), }) case detail.AppStoreApp != nil: diff --git a/cmd/fleetctl/fleetctl/gitops_test.go b/cmd/fleetctl/fleetctl/gitops_test.go index bf91fdd3a5..f9966848c8 100644 --- a/cmd/fleetctl/fleetctl/gitops_test.go +++ b/cmd/fleetctl/fleetctl/gitops_test.go @@ -7496,10 +7496,10 @@ func TestGitOpsSelfServiceCategoriesReconcile(t *testing.T) { listedTeamIDs = append(listedTeamIDs, teamID) return slices.Clone(existing), nil } - ds.NewSoftwareCategoryFunc = func(ctx context.Context, teamID uint, name string) (*fleet.SoftwareCategory, error) { - added = append(added, name) + ds.BatchNewSoftwareCategoriesFunc = func(ctx context.Context, teamID uint, names []string) error { + added = append(added, names...) actions = append(actions, "add") - return &fleet.SoftwareCategory{ID: uint(1000 + len(added)), Name: name, TeamID: teamID}, nil + return nil } ds.SoftwareCategoryFunc = func(ctx context.Context, id uint) (*fleet.SoftwareCategory, error) { for _, c := range existing { @@ -7515,16 +7515,15 @@ func TestGitOpsSelfServiceCategoriesReconcile(t *testing.T) { return nil } + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "script.sh"), []byte(`echo "hello"`), 0o600)) + teamYAML := func(body string) []string { - f, err := os.CreateTemp(t.TempDir(), "categories-*.yml") - require.NoError(t, err) - _, err = f.WriteString("name: Test Fleet\nteam_settings:\n secrets:\n" + body) - require.NoError(t, err) - require.NoError(t, f.Close()) - return []string{"gitops", "-f", f.Name()} + path := filepath.Join(dir, "team.yml") + require.NoError(t, os.WriteFile(path, []byte("name: Test Fleet\nteam_settings:\n secrets:\n"+body), 0o600)) + return []string{"gitops", "-f", path} } unassignedYAML := func(body string) []string { - dir := t.TempDir() globalPath := filepath.Join(dir, "global.yml") require.NoError(t, os.WriteFile(globalPath, []byte(` controls: @@ -7559,33 +7558,25 @@ software: {ID: 202, Name: "Stale No-team Category", TeamID: 0}, } - // explicit list reconciles: adds new, keeps existing, deletes stale, - // and all inserts run before any deletes (cascade safety). + // categories derived from software items reconcile: add new, keep existing, + // delete stale, and all inserts run before any deletes (cascade safety). reset(teamExisting, fleet.GitOpsExceptions{}) _, err := runAppNoChecks(teamYAML(`controls: policies: software: - self_service_categories: - - "🌎 Browsers" - - "💼 Engineering" + packages: + - path: ./script.sh + self_service: true + categories: + - "🌎 Browsers" + - "💼 Engineering" `)) require.NoError(t, err) assert.Equal(t, []string{"💼 Engineering"}, added) assert.ElementsMatch(t, []uint{101, 102}, deleted) assert.Equal(t, []string{"add", "del", "del"}, actions, "all inserts must run before any deletes") - // empty list deletes all categories. - reset(teamExisting, fleet.GitOpsExceptions{}) - _, err = runAppNoChecks(teamYAML(`controls: -policies: -software: - self_service_categories: [] -`)) - require.NoError(t, err) - assert.Empty(t, added) - assert.ElementsMatch(t, []uint{100, 101, 102}, deleted) - - // absent key leaves categories untouched. + // software present with no referenced categories deletes all categories. reset(teamExisting, fleet.GitOpsExceptions{}) _, err = runAppNoChecks(teamYAML(`controls: policies: @@ -7594,35 +7585,45 @@ software: `)) require.NoError(t, err) assert.Empty(t, added) - assert.Empty(t, deleted) + assert.ElementsMatch(t, []uint{100, 101, 102}, deleted) - // software exception + software omitted skips reconciliation. - reset(teamExisting, fleet.GitOpsExceptions{Software: true}) + // With the `software:` key absent, software is still declaratively managed + // (and cleared), so the fleet's categories are pruned too. This is distinct + // from omitting an individual item's `categories:` field. The software + // exception is covered in TestGitOpsSelfServiceCategoriesSoftwareException. + reset(teamExisting, fleet.GitOpsExceptions{}) _, err = runAppNoChecks(teamYAML(`controls: policies: `)) require.NoError(t, err) assert.Empty(t, added) - assert.Empty(t, deleted) + assert.ElementsMatch(t, []uint{100, 101, 102}, deleted) - // plain and emoji names treated as distinct. + // legacy plain names are normalized to their canonical emoji form, so a + // plain and emoji reference to the same default category collapse to one. reset(nil, fleet.GitOpsExceptions{}) _, err = runAppNoChecks(teamYAML(`controls: policies: software: - self_service_categories: - - "Security" - - "🔐 Security" + packages: + - path: ./script.sh + self_service: true + categories: + - "SECURITY" + - "🔐 Security" `)) require.NoError(t, err) - assert.ElementsMatch(t, []string{"Security", "🔐 Security"}, added) + assert.Equal(t, []string{"🔐 Security"}, added) - // no-team explicit list reconciles under team_id = 0. + // no-team categories reconcile under team_id = 0. reset(noTeamExisting, fleet.GitOpsExceptions{}) _, err = runAppNoChecks(unassignedYAML(`software: - self_service_categories: - - "🌎 Browsers" - - "💼 Engineering" + packages: + - path: ./script.sh + self_service: true + categories: + - "🌎 Browsers" + - "💼 Engineering" `)) require.NoError(t, err) assert.Equal(t, []string{"💼 Engineering"}, added) @@ -7631,16 +7632,83 @@ software: assert.EqualValues(t, 0, id, "no-team reconcile must list categories on team 0") } - // no-team empty list deletes all. + // no-team software present with no referenced categories deletes all. reset(noTeamExisting, fleet.GitOpsExceptions{}) _, err = runAppNoChecks(unassignedYAML(`software: - self_service_categories: [] + packages: [] `)) require.NoError(t, err) assert.Empty(t, added) assert.ElementsMatch(t, []uint{200, 201, 202}, deleted) } +func TestSelfServiceCategoriesPruneSkipped(t *testing.T) { + license := &fleet.LicenseInfo{Tier: fleet.TierPremium, Expiration: time.Now().Add(24 * time.Hour)} + + _, ds := testing_utils.RunServerWithMockedDS(t, &service.TestServerOpts{License: license, KeyValueStore: testing_utils.NewMemKeyValueStore()}) + setupEmptyGitOpsMocks(ds) + setupDefaultTeamConfigMocks(ds) + + var deleted []uint + existing := []fleet.SoftwareCategory{{ID: 100, Name: "🌎 Browsers", TeamID: 1}, {ID: 102, Name: "Stale Category", TeamID: 1}} + + ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) { + return &fleet.AppConfig{GitOpsConfig: fleet.GitOpsConfig{Exceptions: fleet.GitOpsExceptions{Software: true}}}, nil + } + ds.ListTeamsFunc = func(ctx context.Context, _ fleet.TeamFilter, _ fleet.ListOptions) ([]*fleet.Team, error) { + return []*fleet.Team{{ID: 1, Name: "Test Fleet"}}, nil + } + ds.TeamByNameFunc = func(ctx context.Context, name string) (*fleet.Team, error) { + if name == "Test Fleet" { + return &fleet.Team{ID: 1, Name: "Test Fleet"}, nil + } + return nil, ¬FoundError{} + } + ds.TeamExistsFunc = func(ctx context.Context, id uint) (bool, error) { return id == 1, nil } + ds.ListSoftwareCategoriesFunc = func(ctx context.Context, teamID uint) ([]fleet.SoftwareCategory, error) { + return slices.Clone(existing), nil + } + ds.SoftwareCategoryFunc = func(ctx context.Context, id uint) (*fleet.SoftwareCategory, error) { + for _, c := range existing { + if c.ID == id { + return &c, nil + } + } + return nil, ¬FoundError{} + } + ds.DeleteSoftwareCategoryFunc = func(ctx context.Context, id uint) error { + deleted = append(deleted, id) + return nil + } + + dir := t.TempDir() + + t.Run("software excepted from gitops", func(t *testing.T) { + deleted = nil + path := filepath.Join(dir, "exception.yml") + require.NoError(t, os.WriteFile(path, []byte("name: Test Fleet\nteam_settings:\n secrets:\ncontrols:\npolicies:\n"), 0o600)) + _, err := runAppNoChecks([]string{"gitops", "-f", path}) + require.NoError(t, err) + assert.Empty(t, deleted) + }) + + t.Run("non-gitops apply", func(t *testing.T) { + deleted = nil + path := filepath.Join(dir, "apply.yml") + require.NoError(t, os.WriteFile(path, []byte(`apiVersion: v1 +kind: fleet +spec: + team: + name: Test Fleet + secrets: + - secret: AAA +`), 0o600)) + _, err := runAppNoChecks([]string{"apply", "-f", path}) + require.NoError(t, err) + assert.Empty(t, deleted) + }) +} + func TestValidateGitOpsGroupEUA(t *testing.T) { t.Parallel() diff --git a/cmd/fleetctl/fleetctl/testing_utils/testing_utils.go b/cmd/fleetctl/fleetctl/testing_utils/testing_utils.go index 257ce2caf3..5e2f5a6680 100644 --- a/cmd/fleetctl/fleetctl/testing_utils/testing_utils.go +++ b/cmd/fleetctl/fleetctl/testing_utils/testing_utils.go @@ -502,6 +502,9 @@ func SetupFullGitOpsPremiumServer(t *testing.T) (*mock.Store, **fleet.AppConfig, savedTeams[team.Name] = &team return team, nil } + ds.TeamExistsFunc = func(ctx context.Context, teamID uint) (bool, error) { + return true, nil + } ds.SetOrUpdateMDMAppleDeclarationFunc = func(ctx context.Context, declaration *fleet.MDMAppleDeclaration, usesFleetVars []fleet.FleetVarName) ( *fleet.MDMAppleDeclaration, error, ) { @@ -520,6 +523,15 @@ func SetupFullGitOpsPremiumServer(t *testing.T) (*mock.Store, **fleet.AppConfig, ds.GetSoftwareInstallersPendingDeletionFunc = func(ctx context.Context, tmID *uint, incoming []fleet.SoftwareTitleIdentifier) ([]fleet.DeletedSoftwarePackage, error) { return nil, nil } + ds.ListSoftwareCategoriesFunc = func(ctx context.Context, teamID uint) ([]fleet.SoftwareCategory, error) { + return nil, nil + } + ds.BatchNewSoftwareCategoriesFunc = func(ctx context.Context, teamID uint, names []string) error { + return nil + } + ds.DeleteSoftwareCategoryFunc = func(ctx context.Context, id uint) error { + return nil + } ds.InsertVPPTokenFunc = func(ctx context.Context, tok *fleet.VPPTokenData) (*fleet.VPPTokenDB, error) { return &fleet.VPPTokenDB{}, nil diff --git a/cmd/fleetctl/fleetctl/testing_utils_test.go b/cmd/fleetctl/fleetctl/testing_utils_test.go index b3b55edc91..b484282897 100644 --- a/cmd/fleetctl/fleetctl/testing_utils_test.go +++ b/cmd/fleetctl/fleetctl/testing_utils_test.go @@ -220,6 +220,18 @@ func setupEmptyGitOpsMocks(ds *mock.Store) { ds.GetSoftwareCategoryNameToIDMapFunc = func(ctx context.Context, teamID uint, names []string) (map[string]uint, error) { return nil, nil } + ds.ListSoftwareCategoriesFunc = func(ctx context.Context, teamID uint) ([]fleet.SoftwareCategory, error) { + return nil, nil + } + ds.BatchNewSoftwareCategoriesFunc = func(ctx context.Context, teamID uint, names []string) error { + return nil + } + ds.DeleteSoftwareCategoryFunc = func(ctx context.Context, id uint) error { + return nil + } + ds.TeamExistsFunc = func(ctx context.Context, teamID uint) (bool, error) { + return true, nil + } ds.GetTeamsWithInstallerByHashFunc = func(ctx context.Context, sha256, url string) (map[uint][]*fleet.ExistingSoftwareInstaller, error) { return map[uint][]*fleet.ExistingSoftwareInstaller{}, nil } diff --git a/ee/server/service/categories.go b/ee/server/service/categories.go index 4ce0419316..71939f4c93 100644 --- a/ee/server/service/categories.go +++ b/ee/server/service/categories.go @@ -200,3 +200,14 @@ func (svc *Service) teamNameForActivity(ctx context.Context, teamID uint) (*stri } return &tm.Name, nil } + +func trimAndValidateCategories(ctx context.Context, categories []string) error { + for i, name := range categories { + categories[i] = strings.TrimSpace(name) + err := (fleet.SoftwareCategory{Name: categories[i]}).Validate() + if err != nil { + return ctxerr.Wrapf(ctx, err, "category %q", categories[i]) + } + } + return nil +} diff --git a/ee/server/service/software_installers.go b/ee/server/service/software_installers.go index 140e5ea64e..241ad2343d 100644 --- a/ee/server/service/software_installers.go +++ b/ee/server/service/software_installers.go @@ -14,11 +14,13 @@ import ( "path" "path/filepath" "regexp" + "slices" "strings" "time" "github.com/fleetdm/fleet/v4/pkg/file" "github.com/fleetdm/fleet/v4/pkg/fleethttp" + "github.com/fleetdm/fleet/v4/pkg/optjson" "github.com/fleetdm/fleet/v4/pkg/retry" "github.com/fleetdm/fleet/v4/server/authz" authz_ctx "github.com/fleetdm/fleet/v4/server/contexts/authz" @@ -2248,6 +2250,11 @@ const ( // batchSoftwareDeletedSuffix is appended to the batch status key to form the key holding // the JSON-encoded list of packages the batch will delete (or, on a dry run, would delete). batchSoftwareDeletedSuffix = ":deleted" + // batchSoftwareCategoriesSuffix is appended to the batch status key to form the key holding + // the JSON-encoded list of self-service categories this batch added. This is required because + // we can only be certain of all categories after downloading all FMA manifests and seeing + // which default categories we might need to add. + batchSoftwareCategoriesSuffix = ":categories" // keyExpireTime serves as a timeout for each step of the batch upload process (initial checks, download for // a package from source, upload for a package to object storage) for each package. This timeout is refreshed // at each step. If the timeout is reached, they key expires in Redis and the batch process is considered @@ -2303,6 +2310,7 @@ func (svc *Service) BatchSetSoftwareInstallers( } var allScripts []string + var categoryNames []string // Verify payloads first, to prevent starting the download+upload process if the data is invalid. for _, payload := range payloads { @@ -2348,6 +2356,16 @@ func (svc *Service) BatchSetSoftwareInstallers( payload.ValidatedLabels = validatedLabels } allScripts = append(allScripts, payload.InstallScript, payload.PostInstallScript, payload.UninstallScript) + + if err := trimAndValidateCategories(ctx, payload.Categories.Value); err != nil { + return "", ctxerr.Wrap(ctx, err, "validating software categories") + } + categoryNames = append(categoryNames, payload.Categories.Value...) + } + + categories, err := svc.batchAddSelfServiceCategories(ctx, teamID, categoryNames, dryRun) + if err != nil { + return "", err } if !dryRun { @@ -2364,6 +2382,14 @@ func (svc *Service) BatchSetSoftwareInstallers( return "", ctxerr.Wrapf(ctx, err, "failed to set key as %s", batchSetProcessing) } + categoriesJSON, err := json.Marshal(categories) + if err != nil { + return "", ctxerr.Wrap(ctx, err, "marshal self-service categories result") + } + if err := svc.keyValueStore.Set(ctx, batchSoftwarePrefix+requestUUID+batchSoftwareCategoriesSuffix, string(categoriesJSON), 10*time.Minute); err != nil { + return "", ctxerr.Wrap(ctx, err, "failed to set self-service categories result") + } + svc.logger.InfoContext(ctx, "software batch start", "request_uuid", requestUUID, "team_id", teamID, @@ -2475,8 +2501,8 @@ func (svc *Service) softwareInstallerPayloadFromSlug(ctx context.Context, payloa } payload.FleetMaintained = true payload.MaintainedApp = app - if len(payload.Categories) == 0 { - payload.Categories = app.Categories + if !payload.Categories.Set { + payload.Categories = optjson.SetSlice(app.Categories) } payload.MaintainedApp.PatchQuery = app.PatchQuery @@ -2721,7 +2747,7 @@ func (svc *Service) softwareBatchUpload( LabelsExcludeAny: p.LabelsExcludeAny, LabelsIncludeAll: p.LabelsIncludeAll, ValidatedLabels: p.ValidatedLabels, - Categories: p.Categories, + Categories: p.Categories.Value, DisplayName: p.DisplayName, RollbackVersion: p.RollbackVersion, AlwaysDownload: p.AlwaysDownload, @@ -2730,7 +2756,7 @@ func (svc *Service) softwareBatchUpload( var extraInstallers []*fleet.UploadSoftwareInstallerPayload - categories, catIDs, err := svc.removeDuplicateOrMissingCategories(ctx, tmID, p.Categories) + categories, catIDs, err := svc.removeDuplicateOrMissingCategories(ctx, tmID, p.Categories.Value) if err != nil { return ctxerr.Wrap(ctx, err, "filtering software installer categories") } @@ -3303,19 +3329,19 @@ 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, error) { +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. if err := svc.authz.Authorize(ctx, &fleet.Team{}, fleet.ActionRead); err != nil { - return "", "", nil, nil, err + return "", "", nil, nil, nil, err } result, err := svc.keyValueStore.Get(ctx, batchSoftwarePrefix+requestUUID) if err != nil { - return "", "", nil, nil, ctxerr.Wrap(ctx, err, "failed to get result") + return "", "", nil, nil, nil, ctxerr.Wrap(ctx, err, "failed to get result") } if result == nil { - return "", "", nil, nil, ctxerr.Wrap(ctx, ¬FoundError{}, "request_uuid not found") + return "", "", nil, nil, nil, ctxerr.Wrap(ctx, ¬FoundError{}, "request_uuid not found") } // getDeletedPackages loads the packages the batch deleted (dry run: would @@ -3335,16 +3361,32 @@ func (svc *Service) GetBatchSetSoftwareInstallersResult(ctx context.Context, tmN return deletedPackages, nil } + // getCategories loads the self-service categories the batch's software references + getCategories := func() ([]string, error) { + categoriesJSON, err := svc.keyValueStore.Get(ctx, batchSoftwarePrefix+requestUUID+batchSoftwareCategoriesSuffix) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "failed to get categories result") + } + if categoriesJSON == nil || *categoriesJSON == "" { + return nil, nil + } + var categories []string + if err := json.Unmarshal([]byte(*categoriesJSON), &categories); err != nil { + return nil, ctxerr.Wrap(ctx, err, "unmarshal categories result") + } + return categories, nil + } + switch { case *result == batchSetCompleted: // fall through to retrieving the (deleted) software packages below. case *result == batchSetProcessing: - return fleet.BatchSetSoftwareInstallersStatusProcessing, "", nil, nil, nil + return fleet.BatchSetSoftwareInstallersStatusProcessing, "", nil, nil, nil, nil case strings.HasPrefix(*result, batchSetFailedPrefix): message := strings.TrimPrefix(*result, batchSetFailedPrefix) - return fleet.BatchSetSoftwareInstallersStatusFailed, message, nil, nil, nil + return fleet.BatchSetSoftwareInstallersStatusFailed, message, nil, nil, nil, nil default: - return "", "", nil, nil, ctxerr.New(ctx, "invalid status") + return "", "", nil, nil, nil, ctxerr.New(ctx, "invalid status") } var ( @@ -3354,7 +3396,7 @@ func (svc *Service) GetBatchSetSoftwareInstallersResult(ctx context.Context, tmN if tmName != "" { team, err := svc.ds.TeamByName(ctx, tmName) if err != nil { - return "", "", nil, nil, ctxerr.Wrap(ctx, err, "load team by name") + return "", "", nil, nil, nil, ctxerr.Wrap(ctx, err, "load team by name") } teamID = team.ID ptrTeamID = &team.ID @@ -3367,24 +3409,29 @@ func (svc *Service) GetBatchSetSoftwareInstallersResult(ctx context.Context, tmN // /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, ctxerr.Wrap(ctx, err, "validating authorization") + return "", "", nil, nil, nil, ctxerr.Wrap(ctx, err, "validating authorization") } deletedPackages, err := getDeletedPackages() if err != nil { - return "", "", nil, nil, err + return "", "", nil, nil, nil, err + } + + categories, err := getCategories() + if err != nil { + return "", "", nil, nil, nil, err } if dryRun { - return fleet.BatchSetSoftwareInstallersStatusCompleted, "", nil, deletedPackages, nil + return fleet.BatchSetSoftwareInstallersStatusCompleted, "", nil, deletedPackages, categories, nil } softwarePackages, err := svc.ds.GetSoftwareInstallers(ctx, teamID) if err != nil { - return "", "", nil, nil, ctxerr.Wrap(ctx, err, "get software installers") + return "", "", nil, nil, nil, ctxerr.Wrap(ctx, err, "get software installers") } - return fleet.BatchSetSoftwareInstallersStatusCompleted, "", softwarePackages, deletedPackages, nil + return fleet.BatchSetSoftwareInstallersStatusCompleted, "", softwarePackages, deletedPackages, categories, nil } func (svc *Service) SelfServiceInstallSoftwareTitle(ctx context.Context, host *fleet.Host, softwareTitleID uint) error { @@ -3826,3 +3873,40 @@ func getInstallScript(extension string, packageIDs []string, currentScript strin } return file.GetInstallScript(extension) } + +// batchAddSelfServiceCategories only adds categories, because it is used across both the installer and vpp +// endpoints and we cannot know what categories to delete before those are both done. +func (svc *Service) batchAddSelfServiceCategories(ctx context.Context, teamID *uint, categoryNames []string, dryRun bool) ([]string, error) { + var allCategories []string + for _, name := range fleet.TranslateLegacySoftwareCategoryNames(categoryNames) { + if slices.ContainsFunc(allCategories, func(c string) bool { return strings.EqualFold(c, name) }) { + continue + } + allCategories = append(allCategories, name) + } + + if len(allCategories) == 0 { + return allCategories, nil + } + + existingCategories, err := svc.ds.ListSoftwareCategories(ctxdb.RequirePrimary(ctx, true), ptr.ValOrZero(teamID)) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "listing existing software categories") + } + + var categoriesToInsert []string + for _, name := range allCategories { + if !slices.ContainsFunc(existingCategories, func(c fleet.SoftwareCategory) bool { return strings.EqualFold(c.Name, name) }) { + categoriesToInsert = append(categoriesToInsert, name) + } + } + + if dryRun { + return allCategories, nil + } + + if err := svc.ds.BatchNewSoftwareCategories(ctx, ptr.ValOrZero(teamID), categoriesToInsert); err != nil { + return nil, ctxerr.Wrap(ctx, err, "creating self-service categories") + } + return allCategories, nil +} diff --git a/ee/server/service/software_installers_test.go b/ee/server/service/software_installers_test.go index a8390776e9..82a9f2f2d7 100644 --- a/ee/server/service/software_installers_test.go +++ b/ee/server/service/software_installers_test.go @@ -1701,7 +1701,7 @@ 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) + status, message, packages, deletedPackages, _, err := svc.GetBatchSetSoftwareInstallersResult(ctx, "", requestUUID, true) require.NoError(t, err) require.Equal(t, fleet.BatchSetSoftwareInstallersStatusCompleted, status) require.Empty(t, message) @@ -1733,7 +1733,7 @@ func TestGetBatchSetSoftwareInstallersResultMissingDeletedKey(t *testing.T) { User: &fleet.User{GlobalRole: new(fleet.RoleAdmin)}, }) - status, message, packages, deletedPackages, err := svc.GetBatchSetSoftwareInstallersResult(ctx, "", "test-uuid", true) + status, message, packages, deletedPackages, _, err := svc.GetBatchSetSoftwareInstallersResult(ctx, "", "test-uuid", true) require.NoError(t, err) require.Equal(t, fleet.BatchSetSoftwareInstallersStatusCompleted, status) require.Empty(t, message) diff --git a/ee/server/service/vpp.go b/ee/server/service/vpp.go index 09dfa0376b..2dc363bbe8 100644 --- a/ee/server/service/vpp.go +++ b/ee/server/service/vpp.go @@ -191,9 +191,9 @@ func (svc *Service) getVPPTokenInfo(ctx context.Context, teamID *uint) (vppToken var isAdamID = regexp.MustCompile(`^[0-9]+$`) -func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, payloads []fleet.VPPBatchPayload, dryRun bool) ([]fleet.VPPAppResponse, error) { +func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, payloads []fleet.VPPBatchPayload, dryRun bool) ([]fleet.VPPAppResponse, []string, error) { if err := svc.authz.Authorize(ctx, &fleet.Team{}, fleet.ActionRead); err != nil { - return nil, err + return nil, nil, err } var teamID *uint @@ -203,30 +203,36 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, if err != nil { // If this is a dry run, the team may not have been created yet if dryRun && fleet.IsNotFound(err) { - return nil, nil + return nil, nil, nil } - return nil, err + return nil, nil, err } teamID = &tm.ID manualAgentInstall = tm.Config.MDM.MacOSSetup.ManualAgentInstall.Value } else { ac, err := svc.ds.AppConfig(ctx) if err != nil { - return nil, ctxerr.Wrap(ctx, err, "getting app config") + return nil, nil, ctxerr.Wrap(ctx, err, "getting app config") } manualAgentInstall = ac.MDM.MacOSSetup.ManualAgentInstall.Value } if err := svc.authz.Authorize(ctx, &fleet.SoftwareInstaller{TeamID: teamID}, fleet.ActionWrite); err != nil { - return nil, ctxerr.Wrap(ctx, err, "validating authorization") + return nil, nil, ctxerr.Wrap(ctx, err, "validating authorization") } // Adding VPP apps will add them to all available platforms per decision: // https://github.com/fleetdm/fleet/issues/19447#issuecomment-2256598681 // The code is already here to support individual platforms, so we can easily enable it later. + var categoryNames []string payloadsWithPlatform := make([]fleet.VPPBatchPayloadWithPlatform, 0, len(payloads)) for _, payload := range payloads { + if err := trimAndValidateCategories(ctx, payload.Categories); err != nil { + return nil, nil, ctxerr.Wrap(ctx, err, "validating app store app categories") + } + categoryNames = append(categoryNames, payload.Categories...) + if payload.Platform == "" && isAdamID.MatchString(payload.AppStoreID) { // add all possible Apple platforms, we'll remove the ones that this app doesn't support later payloadsWithPlatform = append(payloadsWithPlatform, @@ -295,6 +301,12 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, } + // since we actually add categories here, it's possible they can stay orphaned until the next run if the run fails + categories, err := svc.batchAddSelfServiceCategories(ctx, teamID, categoryNames, dryRun) + if err != nil { + return nil, nil, err + } + var incomingAppleApps, incomingAndroidApps []fleet.VPPAppTeam var vppToken string var teamTokenInfo vppTokenInfo @@ -305,18 +317,18 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, payload.Platform = fleet.MacOSPlatform } if !payload.Platform.SupportsAppStoreApps() { - return nil, fleet.NewInvalidArgumentError("app_store_apps.platform", + return nil, nil, fleet.NewInvalidArgumentError("app_store_apps.platform", fmt.Sprintf("platform must be one of '%s', '%s', '%s', or '%s'", fleet.IOSPlatform, fleet.IPadOSPlatform, fleet.MacOSPlatform, fleet.AndroidPlatform)) } // Block Fleet Agent apps from being added via GitOps if payload.Platform == fleet.AndroidPlatform && strings.HasPrefix(payload.AppStoreID, fleetAgentPackagePrefix) { - return nil, fleet.NewInvalidArgumentError("app_store_id", "The Fleet agent cannot be added manually. "+ + return nil, nil, fleet.NewInvalidArgumentError("app_store_id", "The Fleet agent cannot be added manually. "+ "It is automatically managed by Fleet when Android MDM is enabled.") } if payload.Platform == fleet.MacOSPlatform && ptr.ValOrZero(payload.InstallDuringSetup) && manualAgentInstall { - return nil, fleet.NewUserMessageError( + return nil, nil, fleet.NewUserMessageError( errors.New(`Couldn't edit software. "setup_experience" cannot be used for macOS software if "macos_manual_agent_install" is enabled.`), http.StatusUnprocessableEntity) } @@ -325,7 +337,7 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, if payload.Platform.IsApplePlatform() && vppToken == "" { teamTokenInfo, err = svc.getVPPTokenInfo(ctx, teamID) if err != nil { - return nil, fleet.NewUserMessageError(ctxerr.Wrap(ctx, err, "could not retrieve vpp token"), http.StatusUnprocessableEntity) + return nil, nil, fleet.NewUserMessageError(ctxerr.Wrap(ctx, err, "could not retrieve vpp token"), http.StatusUnprocessableEntity) } vppToken = teamTokenInfo.Secret } @@ -334,13 +346,13 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, if !dryRun { validatedLabels, err = ValidateSoftwareLabels(ctx, svc, teamID, payload.LabelsIncludeAny, payload.LabelsExcludeAny, payload.LabelsIncludeAll) if err != nil { - return nil, ctxerr.Wrap(ctx, err, "validating software labels for batch adding vpp app") + return nil, nil, ctxerr.Wrap(ctx, err, "validating software labels for batch adding vpp app") } } categories, catIDs, err := svc.removeDuplicateOrMissingCategories(ctx, ptr.ValOrZero(teamID), payload.Categories) if err != nil { - return nil, ctxerr.Wrap(ctx, err, "filtering vpp app categories") + return nil, nil, ctxerr.Wrap(ctx, err, "filtering vpp app categories") } payload.Categories = categories @@ -361,7 +373,7 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, switch payload.Platform { case fleet.AndroidPlatform: if strings.HasPrefix(payload.AppStoreID, fleet.AndroidWebAppPrefix) && payload.Configuration != nil { - return nil, fleet.NewInvalidArgumentError("configuration", "Couldn't edit. Android web apps don't support configurations.") + return nil, nil, fleet.NewInvalidArgumentError("configuration", "Couldn't edit. Android web apps don't support configurations.") } appStoreApp.SelfService = true @@ -371,10 +383,10 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, if payload.Configuration != nil && payload.Platform != fleet.MacOSPlatform { var plist string if err := json.Unmarshal(payload.Configuration, &plist); err != nil { - return nil, fleet.NewInvalidArgumentError("configuration", "expected configuration as a JSON string containing the XML") + return nil, nil, fleet.NewInvalidArgumentError("configuration", "expected configuration as a JSON string containing the XML") } if err := fleet.ValidateAppleAppConfiguration([]byte(plist)); err != nil { - return nil, err + return nil, nil, err } appStoreApp.Configuration = []byte(plist) } @@ -387,14 +399,14 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, if dryRun { // If we're doing a dry run, we stop here and return no error to avoid making any changes. // That way we validate if a VPP token is available even on dry runs keeping it consistent. - return nil, nil + return nil, categories, nil } var missingAssets []string assets, err := vpp.GetAssets(ctx, vppToken, nil) if err != nil { - return nil, ctxerr.Wrap(ctx, err, "unable to retrieve assets") + return nil, nil, ctxerr.Wrap(ctx, err, "unable to retrieve assets") } assetMap := map[string]struct{}{} @@ -419,7 +431,7 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, if len(missingAssets) != 0 { sort.Strings(missingAssets) reqErr := ctxerr.Errorf(ctx, "requested app not available on vpp account: %s", strings.Join(missingAssets, ", ")) - return nil, fleet.NewUserMessageError(reqErr, http.StatusUnprocessableEntity) + return nil, nil, fleet.NewUserMessageError(reqErr, http.StatusUnprocessableEntity) } } } @@ -427,7 +439,7 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, if dryRun { // If we're doing a dry run, we stop here and return no error to avoid making any changes. // Another dry run check is inside the payload size > 0 statement. - return nil, nil + return nil, categories, nil } allPlatformApps := slices.Concat(incomingAppleApps, incomingAndroidApps) @@ -438,10 +450,10 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, if len(incomingAppleApps) > 0 { apps, reAnchors, err := svc.getAnchoredVPPAppsMetadata(ctx, incomingAppleApps, teamTokenInfo) if err != nil { - return nil, ctxerr.Wrap(ctx, err, "refreshing VPP app metadata") + return nil, nil, ctxerr.Wrap(ctx, err, "refreshing VPP app metadata") } if len(apps) == 0 { - return nil, fleet.NewInvalidArgumentError("app_store_apps", + return nil, nil, fleet.NewInvalidArgumentError("app_store_apps", "no valid apps found matching the provided app store IDs and platforms") } @@ -451,7 +463,7 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, enterprise, err := svc.ds.GetEnterprise(ctx) if err != nil && !fleet.IsNotFound(err) { - return nil, ctxerr.Wrap(ctx, err, "get android enterprise") + return nil, nil, ctxerr.Wrap(ctx, err, "get android enterprise") } androidHostPoliciesToUpdate := map[string]string{} @@ -460,21 +472,21 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, // to update their set of apps to the empty set (and remove/uninstall the apps). removedApps, err := svc.ds.GetVPPApps(ctx, teamID) if err != nil { - return nil, err + return nil, nil, err } for _, app := range removedApps { if app.Platform == fleet.AndroidPlatform { hostsInScope, err := svc.ds.GetIncludedHostUUIDMapForAppStoreApp(ctx, app.AppTeamID) if err != nil { - return nil, err + return nil, nil, err } maps.Copy(androidHostPoliciesToUpdate, hostsInScope) } } } else { if enterprise == nil { - return nil, &fleet.BadRequestError{Message: "Android MDM is not enabled", InternalErr: err} + return nil, nil, &fleet.BadRequestError{Message: "Android MDM is not enabled", InternalErr: err} } seenWebAppNames := make(map[string]bool) @@ -482,15 +494,15 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, androidApp, err := svc.androidModule.EnterprisesApplications(ctx, enterprise.Name(), a.AdamID) if err != nil { if fleet.IsNotFound(err) { - return nil, fleet.NewInvalidArgumentError("app_store_id", fmt.Sprintf("Couldn't add software. The application ID %q isn't available in Play Store. Please find ID on the Play Store and try again.", a.AdamID)) + return nil, nil, fleet.NewInvalidArgumentError("app_store_id", fmt.Sprintf("Couldn't add software. The application ID %q isn't available in Play Store. Please find ID on the Play Store and try again.", a.AdamID)) } - return nil, ctxerr.Wrap(ctx, err, "bulk add app store apps: check if android app exists") + return nil, nil, ctxerr.Wrap(ctx, err, "bulk add app store apps: check if android app exists") } if strings.HasPrefix(a.AdamID, fleet.AndroidWebAppPrefix) { lowerTitle := strings.ToLower(androidApp.Title) if seenWebAppNames[lowerTitle] { - return nil, fleet.ConflictError{ + return nil, nil, fleet.ConflictError{ Message: fmt.Sprintf("Couldn't add. Web app with this name (%q) already exists in this fleet. Please add a web app with a different name or delete the existing app and try again.", androidApp.Title), } } @@ -509,7 +521,7 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, if len(appStoreApps) > 0 { if err := svc.ds.BatchInsertVPPApps(ctx, appStoreApps); err != nil { - return nil, ctxerr.Wrap(ctx, err, "inserting vpp app metadata") + return nil, nil, ctxerr.Wrap(ctx, err, "inserting vpp app metadata") } } @@ -517,7 +529,7 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, // outpaces the metadata fetched from that country. for _, ra := range pendingReAnchors { if err := svc.ds.UpdateVPPAppCountryCode(ctx, ra.AdamID, ra.Platform, ra.CountryCode); err != nil { - return nil, ctxerr.Wrap(ctx, err, "re-anchoring vpp app country in batch") + return nil, nil, ctxerr.Wrap(ctx, err, "re-anchoring vpp app country in batch") } } @@ -538,9 +550,9 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, setupExperienceChanged, err := svc.ds.SetTeamVPPApps(ctx, teamID, allPlatformApps, appStoreIDToTitleID) if err != nil { if errors.Is(err, sql.ErrNoRows) { - return nil, fleet.NewUserMessageError(ctxerr.Wrap(ctx, err, "no vpp token to set team vpp assets"), http.StatusUnprocessableEntity) + return nil, nil, fleet.NewUserMessageError(ctxerr.Wrap(ctx, err, "no vpp token to set team vpp assets"), http.StatusUnprocessableEntity) } - return nil, ctxerr.Wrap(ctx, err, "set team vpp assets") + return nil, nil, ctxerr.Wrap(ctx, err, "set team vpp assets") } // Do cleanup here because this is API call 2 of 2 for setting software from GitOps @@ -553,11 +565,11 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, // First, get existing auto-update schedules to know which apps already have configs existingIosAppSchedules, err := svc.ds.ListSoftwareAutoUpdateSchedules(ctx, tmID, "ios_apps") if err != nil { - return nil, ctxerr.Wrap(ctx, err, "listing existing auto-update schedules for ios apps") + return nil, nil, ctxerr.Wrap(ctx, err, "listing existing auto-update schedules for ios apps") } existingIPadOsSchedules, err := svc.ds.ListSoftwareAutoUpdateSchedules(ctx, tmID, "ipados_apps") if err != nil { - return nil, ctxerr.Wrap(ctx, err, "listing existing auto-update schedules for ipados apps") + return nil, nil, ctxerr.Wrap(ctx, err, "listing existing auto-update schedules for ipados apps") } // Combine schedules from both sources existingSchedules := slices.Concat(existingIosAppSchedules, existingIPadOsSchedules) @@ -599,21 +611,21 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, if (app.AutoUpdateEnabled != nil && *app.AutoUpdateEnabled) || hasTimesSet { schedule := fleet.SoftwareAutoUpdateSchedule{SoftwareAutoUpdateConfig: cfg} if err := schedule.WindowIsValid(); err != nil { - return nil, ctxerr.Wrap(ctx, err, "invalid auto-update window for vpp app") + return nil, nil, ctxerr.Wrap(ctx, err, "invalid auto-update window for vpp app") } } if err := svc.ds.UpdateSoftwareTitleAutoUpdateConfig(ctx, titleID, tmID, cfg); err != nil { - return nil, ctxerr.Wrap(ctx, err, "updating auto-update config for vpp app") + return nil, nil, ctxerr.Wrap(ctx, err, "updating auto-update config for vpp app") } } if err := svc.ds.DeleteIconsAssociatedWithTitlesWithoutInstallers(ctx, tmID); err != nil { - return nil, err // returned error already includes context that we could include here + return nil, nil, err // returned error already includes context that we could include here } addedApps, err := svc.ds.GetVPPApps(ctx, teamID) if err != nil { - return nil, err + return nil, nil, err } var appIDs []string @@ -621,7 +633,7 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, if app.Platform == fleet.AndroidPlatform { hostsInScope, err := svc.ds.GetIncludedHostUUIDMapForAppStoreApp(ctx, app.AppTeamID) if err != nil { - return nil, err + return nil, nil, err } maps.Copy(androidHostPoliciesToUpdate, hostsInScope) @@ -633,7 +645,7 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, for hostUUID, policyID := range androidHostPoliciesToUpdate { err := worker.QueueBulkSetAndroidAppsAvailableForHost(ctx, svc.ds, svc.logger, hostUUID, policyID, appIDs, enterprise.Name()) if err != nil { - return nil, ctxerr.WrapWithData( + return nil, nil, ctxerr.WrapWithData( ctx, err, "batch associate app store apps: add apps to android MDM policy", @@ -650,14 +662,14 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, if setupExperienceChanged { err := svc.NewActivity(ctx, authz.UserFromContext(ctx), fleet.ActivityEditedSetupExperienceSoftware{TeamID: ptr.ValOrZero(teamID), TeamName: teamName}) if err != nil { - return nil, ctxerr.Wrap(ctx, err, "create edited setup experience activity") + return nil, nil, ctxerr.Wrap(ctx, err, "create edited setup experience activity") } } if len(addedApps) == 0 { - return []fleet.VPPAppResponse{}, nil + return []fleet.VPPAppResponse{}, categories, nil } - return addedApps, nil + return addedApps, categories, nil } func (svc *Service) GetAppStoreApps(ctx context.Context, teamID *uint) ([]*fleet.VPPApp, error) { diff --git a/ee/server/service/vpp_test.go b/ee/server/service/vpp_test.go index dd83b7aa43..3b946f22d8 100644 --- a/ee/server/service/vpp_test.go +++ b/ee/server/service/vpp_test.go @@ -38,7 +38,7 @@ func TestBatchAssociateVPPApps(t *testing.T) { return &fleet.AppConfig{}, nil } t.Run("dry run", func(t *testing.T) { - _, err := svc.BatchAssociateVPPApps(ctx, "", []fleet.VPPBatchPayload{ + _, _, err := svc.BatchAssociateVPPApps(ctx, "", []fleet.VPPBatchPayload{ { AppStoreID: "my-fake-app", LabelsExcludeAny: []string{}, @@ -51,7 +51,7 @@ func TestBatchAssociateVPPApps(t *testing.T) { require.ErrorContains(t, err, "could not retrieve vpp token") }) t.Run("not dry run", func(t *testing.T) { - _, err := svc.BatchAssociateVPPApps(ctx, "", []fleet.VPPBatchPayload{ + _, _, err := svc.BatchAssociateVPPApps(ctx, "", []fleet.VPPBatchPayload{ { AppStoreID: "my-fake-app", LabelsExcludeAny: []string{}, @@ -78,7 +78,7 @@ func TestBatchAssociateVPPApps(t *testing.T) { for _, pkg := range fleetAgentPackages { t.Run(pkg+" dry run", func(t *testing.T) { - _, err := svc.BatchAssociateVPPApps(ctx, "", []fleet.VPPBatchPayload{ + _, _, err := svc.BatchAssociateVPPApps(ctx, "", []fleet.VPPBatchPayload{ { AppStoreID: pkg, LabelsExcludeAny: []string{}, @@ -91,7 +91,7 @@ func TestBatchAssociateVPPApps(t *testing.T) { require.ErrorContains(t, err, "The Fleet agent cannot be added manually") }) t.Run(pkg+" not dry run", func(t *testing.T) { - _, err := svc.BatchAssociateVPPApps(ctx, "", []fleet.VPPBatchPayload{ + _, _, err := svc.BatchAssociateVPPApps(ctx, "", []fleet.VPPBatchPayload{ { AppStoreID: pkg, LabelsExcludeAny: []string{}, @@ -328,7 +328,7 @@ func TestBatchAssociateVPPAppsDedupsMissingAssetsError(t *testing.T) { ctx = viewer.NewContext(ctx, viewer.Viewer{User: &fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)}}) const adamID = "1107542306" - _, err := svc.BatchAssociateVPPApps(ctx, "", []fleet.VPPBatchPayload{ + _, _, err := svc.BatchAssociateVPPApps(ctx, "", []fleet.VPPBatchPayload{ { AppStoreID: adamID, LabelsExcludeAny: []string{}, diff --git a/pkg/spec/gitops.go b/pkg/spec/gitops.go index d4c5a88976..4933de7a06 100644 --- a/pkg/spec/gitops.go +++ b/pkg/spec/gitops.go @@ -310,10 +310,9 @@ func (spec SoftwarePackage) HydrateToPackageLevel(packageLevel fleet.SoftwarePac } type Software struct { - Packages []SoftwarePackage `json:"packages"` - AppStoreApps []fleet.TeamSpecAppStoreApp `json:"app_store_apps"` - FleetMaintainedApps []fleet.MaintainedAppSpec `json:"fleet_maintained_apps"` - SelfServiceCategories optjson.Slice[string] `json:"self_service_categories"` + Packages []SoftwarePackage `json:"packages"` + AppStoreApps []fleet.TeamSpecAppStoreApp `json:"app_store_apps"` + FleetMaintainedApps []fleet.MaintainedAppSpec `json:"fleet_maintained_apps"` } // GitOpsMDM extends fleet.MDM with gitops-only fields that are not part of the server type. @@ -378,10 +377,9 @@ type GitOps struct { } type GitOpsSoftware struct { - Packages []*fleet.SoftwarePackageSpec - AppStoreApps []*fleet.TeamSpecAppStoreApp - FleetMaintainedApps []*fleet.MaintainedAppSpec - SelfServiceCategories optjson.Slice[string] + Packages []*fleet.SoftwarePackageSpec + AppStoreApps []*fleet.TeamSpecAppStoreApp + FleetMaintainedApps []*fleet.MaintainedAppSpec } type Logf func(format string, a ...interface{}) @@ -1942,47 +1940,6 @@ func parseSoftware(top map[string]json.RawMessage, result *GitOps, baseDir strin multiError = multierror.Append(multiError, validateRawKeys(softwareRaw, reflect.TypeFor[Software](), filePath, []string{"software"})...) } - // validate self service categories - if software.SelfServiceCategories.Set { - declared := software.SelfServiceCategories.Value - var seen []string - - for i, name := range declared { - declared[i] = strings.TrimSpace(name) - - if err := (fleet.SoftwareCategory{Name: declared[i]}).Validate(); err != nil { - multiError = multierror.Append(multiError, fmt.Errorf("self_service_categories: %w", err)) - continue - } - - // Doesn't catch utf8mb4_unicode_ci collation collisions (e.g. "🔐 Security" vs "🛡 Security") in dry runs. - if slices.ContainsFunc(seen, func(s string) bool { return strings.EqualFold(s, declared[i]) }) { - multiError = multierror.Append(multiError, - fmt.Errorf("self_service_categories: duplicate category %q", declared[i])) - continue - } - - seen = append(seen, declared[i]) - } - - result.Software.SelfServiceCategories = optjson.SetSlice(declared) - } - - validateCategoryReferences := func(categories []string) { - if !result.Software.SelfServiceCategories.Set { - return - } - - for _, name := range categories { - if !slices.ContainsFunc(result.Software.SelfServiceCategories.Value, func(d string) bool { - return fleet.SoftwareCategoryReferenceMatches(name, d) - }) { - multiError = multierror.Append(multiError, - fmt.Errorf("category %q is not in software.self_service_categories", name)) - } - } - } - for _, item := range software.AppStoreApps { if item.AppStoreID == "" { multiError = multierror.Append(multiError, errors.New("software app store id required")) @@ -2008,7 +1965,6 @@ func parseSoftware(top map[string]json.RawMessage, result *GitOps, baseDir strin item = item.ResolvePaths(baseDir) - validateCategoryReferences(item.Categories) result.Software.AppStoreApps = append(result.Software.AppStoreApps, &item) } for _, maintainedAppSpec := range software.FleetMaintainedApps { @@ -2050,7 +2006,6 @@ func parseSoftware(top map[string]json.RawMessage, result *GitOps, baseDir strin } } - validateCategoryReferences(maintainedAppSpec.Categories) result.Software.FleetMaintainedApps = append(result.Software.FleetMaintainedApps, &maintainedAppSpec) } for _, teamLevelPackage := range software.Packages { @@ -2222,7 +2177,6 @@ func parseSoftware(top map[string]json.RawMessage, result *GitOps, baseDir strin continue } - validateCategoryReferences(softwarePackageSpec.Categories) result.Software.Packages = append(result.Software.Packages, softwarePackageSpec) } } diff --git a/pkg/spec/gitops_test.go b/pkg/spec/gitops_test.go index 75d7df7091..ff95a804bb 100644 --- a/pkg/spec/gitops_test.go +++ b/pkg/spec/gitops_test.go @@ -222,7 +222,7 @@ func TestValidGitOpsYaml(t *testing.T) { if strings.Contains(pkg.URL, "MicrosoftTeams") { assert.Equal(t, "testdata/lib/uninstall.sh", pkg.UninstallScript.Path) assert.Contains(t, pkg.LabelsIncludeAny, "a") - assert.Contains(t, pkg.Categories, "Communication") + assert.Contains(t, pkg.Categories.Value, "Communication") assert.Empty(t, pkg.LabelsExcludeAny) assert.Empty(t, pkg.LabelsIncludeAll) } else { @@ -236,14 +236,14 @@ func TestValidGitOpsYaml(t *testing.T) { for _, fma := range gitops.Software.FleetMaintainedApps { switch fma.Slug { case "slack/darwin": - require.ElementsMatch(t, fma.Categories, []string{"Productivity", "Communication"}) + require.ElementsMatch(t, fma.Categories.Value, []string{"Productivity", "Communication"}) require.Equal(t, "4.47.65", fma.Version) require.Empty(t, fma.PreInstallQuery) require.Empty(t, fma.PostInstallScript) require.Empty(t, fma.InstallScript) require.Empty(t, fma.UninstallScript) case "box-drive/windows": - require.ElementsMatch(t, fma.Categories, []string{"Productivity", "Developer tools"}) + require.ElementsMatch(t, fma.Categories.Value, []string{"Productivity", "Developer tools"}) require.Empty(t, fma.Version) require.NotEmpty(t, fma.PreInstallQuery) require.NotEmpty(t, fma.PostInstallScript) @@ -4067,7 +4067,7 @@ software: require.NoError(t, err) require.Len(t, result.Software.Packages, 1) assert.True(t, strings.HasSuffix(result.Software.Packages[0].InstallScript.Path, "install-app.sh")) - assert.Equal(t, []string{"Utilities"}, result.Software.Packages[0].Categories) + assert.Equal(t, []string{"Utilities"}, result.Software.Packages[0].Categories.Value) assert.True(t, result.Software.Packages[0].SelfService) assert.Empty(t, result.Software.Packages[0].URL) assert.Empty(t, result.Software.Packages[0].SHA256) @@ -4168,7 +4168,7 @@ software: require.NoError(t, err) require.Len(t, result.Software.Packages, 1) pkg := result.Software.Packages[0] - assert.Equal(t, []string{"Browsers", "Productivity"}, pkg.Categories) + assert.Equal(t, []string{"Browsers", "Productivity"}, pkg.Categories.Value) assert.True(t, pkg.SelfService) assert.True(t, pkg.InstallDuringSetup.Value) assert.Equal(t, []string{"include_label"}, pkg.LabelsIncludeAny) @@ -4470,163 +4470,46 @@ name: TestTeam }) } -func TestGitOpsSelfServiceCategoriesPresence(t *testing.T) { +func TestGitOpsFMACategoriesPresence(t *testing.T) { t.Parallel() - t.Run("key omitted leaves Present false", func(t *testing.T) { - t.Parallel() + parse := func(t *testing.T, categoriesYAML string) optjson.Slice[string] { config := getTeamConfig(nil) - config += "software:\n packages: []\n" + config += "software:\n fleet_maintained_apps:\n - slug: 1password/darwin\n" + categoriesYAML path, basePath := createTempFile(t, "", config) gitops, err := GitOpsFromFile(path, basePath, premiumAppConfig(), nopLogf) require.NoError(t, err) - assert.False(t, gitops.Software.SelfServiceCategories.Set) - assert.Empty(t, gitops.Software.SelfServiceCategories.Value) + require.Len(t, gitops.Software.FleetMaintainedApps, 1) + return gitops.Software.FleetMaintainedApps[0].Categories + } + + t.Run("omitted key is unset", func(t *testing.T) { + t.Parallel() + cats := parse(t, "") + assert.False(t, cats.Set) }) - t.Run("empty list sets Present true", func(t *testing.T) { + t.Run("categories: (null) is set but not valid", func(t *testing.T) { t.Parallel() - config := getTeamConfig(nil) - config += "software:\n self_service_categories: []\n" - path, basePath := createTempFile(t, "", config) - gitops, err := GitOpsFromFile(path, basePath, premiumAppConfig(), nopLogf) - require.NoError(t, err) - assert.True(t, gitops.Software.SelfServiceCategories.Set) - assert.Empty(t, gitops.Software.SelfServiceCategories.Value) + cats := parse(t, " categories:\n") + assert.True(t, cats.Set) + assert.False(t, cats.Valid) + assert.Empty(t, cats.Value) }) - t.Run("populated list sets Present true and preserves names verbatim", func(t *testing.T) { + t.Run("categories: [] is set and valid", func(t *testing.T) { t.Parallel() - config := getTeamConfig(nil) - config += `software: - self_service_categories: - - "🌎 Browsers" - - "Productivity" - - "💼 Engineering" -` - path, basePath := createTempFile(t, "", config) - gitops, err := GitOpsFromFile(path, basePath, premiumAppConfig(), nopLogf) - require.NoError(t, err) - assert.True(t, gitops.Software.SelfServiceCategories.Set) - assert.Equal(t, []string{"🌎 Browsers", "Productivity", "💼 Engineering"}, gitops.Software.SelfServiceCategories.Value) + cats := parse(t, " categories: []\n") + assert.True(t, cats.Set) + assert.True(t, cats.Valid) + assert.Empty(t, cats.Value) }) - t.Run("duplicate name in payload fails at parse", func(t *testing.T) { + t.Run("categories with a value is set with the value", func(t *testing.T) { t.Parallel() - config := getTeamConfig(nil) - config += `software: - self_service_categories: - - "🔐 Security" - - "🔐 Security" -` - path, basePath := createTempFile(t, "", config) - _, err := GitOpsFromFile(path, basePath, premiumAppConfig(), nopLogf) - require.Error(t, err) - assert.Contains(t, err.Error(), "duplicate") - assert.Contains(t, err.Error(), "🔐 Security") - }) - - t.Run("package referencing undeclared category fails at parse", func(t *testing.T) { - t.Parallel() - config := getTeamConfig(nil) - config += `software: - self_service_categories: - - "Allowed" - packages: - - url: https://example.com/installer.pkg - hash_sha256: "0000000000000000000000000000000000000000000000000000000000000000" - categories: - - "Forbidden" -` - path, basePath := createTempFile(t, "", config) - _, err := GitOpsFromFile(path, basePath, premiumAppConfig(), nopLogf) - require.Error(t, err) - assert.Contains(t, err.Error(), `"Forbidden"`) - assert.Contains(t, err.Error(), "self_service_categories") - }) - - t.Run("app_store_apps referencing undeclared category fails at parse", func(t *testing.T) { - t.Parallel() - config := getTeamConfig(nil) - config += `software: - self_service_categories: - - "Allowed" - app_store_apps: - - app_store_id: "12345" - categories: - - "Forbidden" -` - path, basePath := createTempFile(t, "", config) - _, err := GitOpsFromFile(path, basePath, premiumAppConfig(), nopLogf) - require.Error(t, err) - assert.Contains(t, err.Error(), `"Forbidden"`) - assert.Contains(t, err.Error(), "self_service_categories") - }) - - t.Run("fleet_maintained_apps referencing undeclared category fails at parse", func(t *testing.T) { - t.Parallel() - config := getTeamConfig(nil) - config += `software: - self_service_categories: - - "Allowed" - fleet_maintained_apps: - - slug: 1password/darwin - categories: - - "Forbidden" -` - path, basePath := createTempFile(t, "", config) - _, err := GitOpsFromFile(path, basePath, premiumAppConfig(), nopLogf) - require.Error(t, err) - assert.Contains(t, err.Error(), `"Forbidden"`) - assert.Contains(t, err.Error(), "self_service_categories") - }) - - t.Run("validation rejects empty, whitespace-only, and over-length names", func(t *testing.T) { - t.Parallel() - config := getTeamConfig(nil) - config += fmt.Sprintf(`software: - self_service_categories: - - "" - - " " - - %q -`, strings.Repeat("x", 256)) - path, basePath := createTempFile(t, "", config) - _, err := GitOpsFromFile(path, basePath, premiumAppConfig(), nopLogf) - require.Error(t, err) - assert.Contains(t, err.Error(), "name is required") - assert.Contains(t, err.Error(), "must be at most 255") - }) - - t.Run("names are trimmed and case-only duplicates are caught", func(t *testing.T) { - t.Parallel() - config := getTeamConfig(nil) - config += `software: - self_service_categories: - - " Productivity " - - "PRODUCTIVITY" -` - path, basePath := createTempFile(t, "", config) - _, err := GitOpsFromFile(path, basePath, premiumAppConfig(), nopLogf) - require.Error(t, err) - assert.Contains(t, err.Error(), "duplicate") - // The second entry collides with the trimmed first. - assert.Contains(t, err.Error(), "PRODUCTIVITY") - }) - - t.Run("legacy plain reference matches declared emoji form", func(t *testing.T) { - t.Parallel() - config := getTeamConfig(nil) - config += `software: - self_service_categories: - - "💻 Productivity" - packages: - - url: https://example.com/installer.pkg - hash_sha256: "0000000000000000000000000000000000000000000000000000000000000000" - categories: - - "Productivity" -` - path, basePath := createTempFile(t, "", config) - _, err := GitOpsFromFile(path, basePath, premiumAppConfig(), nopLogf) - require.NoError(t, err) + cats := parse(t, " categories:\n - somevalue\n") + assert.True(t, cats.Set) + assert.True(t, cats.Valid) + assert.Equal(t, []string{"somevalue"}, cats.Value) }) } diff --git a/server/fleet/scripts.go b/server/fleet/scripts.go index a4b44163b4..66705ab96b 100644 --- a/server/fleet/scripts.go +++ b/server/fleet/scripts.go @@ -10,6 +10,7 @@ import ( "time" "unicode/utf8" + "github.com/fleetdm/fleet/v4/pkg/optjson" "github.com/fleetdm/fleet/v4/pkg/scripts" "github.com/fleetdm/fleet/v4/server/mdm/android" ) @@ -617,9 +618,9 @@ type SoftwareInstallerPayload struct { // ValidatedLabels is a struct that contains the validated labels for the // software installer. It is nil if the labels have not been validated. ValidatedLabels *LabelIdentsWithScope - SHA256 string `json:"sha256"` - Categories []string `json:"categories"` - DisplayName string `json:"display_name"` + SHA256 string `json:"sha256"` + Categories optjson.Slice[string] `json:"categories,omitzero"` + DisplayName string `json:"display_name"` // This is to support FMAs Slug *string `json:"slug"` MaintainedApp *MaintainedApp `json:"-"` diff --git a/server/fleet/service.go b/server/fleet/service.go index 3f77adcb70..74787bd9b3 100644 --- a/server/fleet/service.go +++ b/server/fleet/service.go @@ -795,7 +795,8 @@ type Service interface { // - '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". - GetBatchSetSoftwareInstallersResult(ctx context.Context, tmName string, requestUUID string, dryRun bool) (status string, message string, packages []SoftwarePackageResponse, deletedPackages []DeletedSoftwarePackage, err error) + // - '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) // SelfServiceInstallSoftwareTitle installs a software title // initiated by the user @@ -912,7 +913,7 @@ type Service interface { CreateAndroidWebApp(ctx context.Context, title, startURL string, icon io.Reader) (string, error) - BatchAssociateVPPApps(ctx context.Context, teamName string, payloads []VPPBatchPayload, dryRun bool) ([]VPPAppResponse, error) + BatchAssociateVPPApps(ctx context.Context, teamName string, payloads []VPPBatchPayload, dryRun bool) ([]VPPAppResponse, []string, error) // GetHostDEPAssignment retrieves the host DEP assignment for the specified host. GetHostDEPAssignment(ctx context.Context, host *Host) (*HostDEPAssignment, error) diff --git a/server/fleet/software.go b/server/fleet/software.go index 570f266abf..c561bfec69 100644 --- a/server/fleet/software.go +++ b/server/fleet/software.go @@ -917,10 +917,12 @@ var LegacySoftwareCategoryNames = map[string]string{ func TranslateLegacySoftwareCategoryNames(names []string) []string { out := make([]string, len(names)) for i, n := range names { - if newName, ok := LegacySoftwareCategoryNames[n]; ok { - out[i] = newName - } else { - out[i] = n + out[i] = n + for legacy, newName := range LegacySoftwareCategoryNames { + if strings.EqualFold(n, legacy) { + out[i] = newName + break + } } } return out diff --git a/server/fleet/software_installer.go b/server/fleet/software_installer.go index a334fac263..e603291c22 100644 --- a/server/fleet/software_installer.go +++ b/server/fleet/software_installer.go @@ -888,10 +888,10 @@ type SoftwarePackageSpec struct { // It must be JSON-marshaled because it gets set during gitops file processing, // which is then re-marshaled to JSON from this struct and later re-unmarshaled // during ApplyGroup... - ReferencedYamlPath string `json:"referenced_yaml_path"` - SHA256 string `json:"hash_sha256"` - Categories []string `json:"categories"` - DisplayName string `json:"display_name,omitempty"` + ReferencedYamlPath string `json:"referenced_yaml_path"` + SHA256 string `json:"hash_sha256"` + Categories optjson.Slice[string] `json:"categories,omitzero"` + DisplayName string `json:"display_name,omitempty"` // AlwaysDownload disables conditional HTTP downloads using ETag headers. // When false (the default), Fleet sends If-None-Match with the stored ETag // on subsequent downloads. If the server returns 304 Not Modified, the @@ -912,7 +912,7 @@ func (spec SoftwarePackageSpec) ResolveSoftwarePackagePaths(baseDir string) Soft func (spec SoftwarePackageSpec) IncludesFieldsDisallowedInPackageFile() bool { return len(spec.LabelsExcludeAny) > 0 || len(spec.LabelsIncludeAny) > 0 || len(spec.LabelsIncludeAll) > 0 || - len(spec.Categories) > 0 || spec.SelfService || spec.InstallDuringSetup.Valid + len(spec.Categories.Value) > 0 || spec.SelfService || spec.InstallDuringSetup.Valid } func resolveApplyRelativePath(baseDir string, path string) string { @@ -934,7 +934,7 @@ type MaintainedAppSpec struct { LabelsIncludeAny []string `json:"labels_include_any"` LabelsExcludeAny []string `json:"labels_exclude_any"` LabelsIncludeAll []string `json:"labels_include_all"` - Categories []string `json:"categories"` + Categories optjson.Slice[string] `json:"categories,omitzero"` InstallDuringSetup optjson.Bool `json:"setup_experience"` Icon TeamSpecSoftwareAsset `json:"icon"` } diff --git a/server/mock/service/service_mock.go b/server/mock/service/service_mock.go index 380adf1672..d807dc4099 100644 --- a/server/mock/service/service_mock.go +++ b/server/mock/service/service_mock.go @@ -498,7 +498,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, err 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 SelfServiceInstallSoftwareTitleFunc func(ctx context.Context, host *fleet.Host, softwareTitleID uint) error @@ -570,7 +570,7 @@ type DeleteVPPTokenFunc func(ctx context.Context, tokenID uint) error type CreateAndroidWebAppFunc func(ctx context.Context, title string, startURL string, icon io.Reader) (string, error) -type BatchAssociateVPPAppsFunc func(ctx context.Context, teamName string, payloads []fleet.VPPBatchPayload, dryRun bool) ([]fleet.VPPAppResponse, error) +type BatchAssociateVPPAppsFunc func(ctx context.Context, teamName string, payloads []fleet.VPPBatchPayload, dryRun bool) ([]fleet.VPPAppResponse, []string, error) type GetHostDEPAssignmentFunc func(ctx context.Context, host *fleet.Host) (*fleet.HostDEPAssignment, error) @@ -3997,7 +3997,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, err error) { +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) { s.mu.Lock() s.GetBatchSetSoftwareInstallersResultFuncInvoked = true s.mu.Unlock() @@ -4249,7 +4249,7 @@ func (s *Service) CreateAndroidWebApp(ctx context.Context, title string, startUR return s.CreateAndroidWebAppFunc(ctx, title, startURL, icon) } -func (s *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, payloads []fleet.VPPBatchPayload, dryRun bool) ([]fleet.VPPAppResponse, error) { +func (s *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, payloads []fleet.VPPBatchPayload, dryRun bool) ([]fleet.VPPAppResponse, []string, error) { s.mu.Lock() s.BatchAssociateVPPAppsFuncInvoked = true s.mu.Unlock() diff --git a/server/service/client.go b/server/service/client.go index affad8210f..2cdbb07f24 100644 --- a/server/service/client.go +++ b/server/service/client.go @@ -1095,17 +1095,20 @@ func (c *Client) ApplyGroup( } } } + // Categories referenced across both the installer and app store batches, unused ones should be deleted. + categoriesByTeam := map[string][]string{} if len(tmSoftwarePackagesPayloads) > 0 { 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, err := c.ApplyTeamSoftwareInstallers(currentTeamName, software, opts.ApplySpecOptions) + installers, deletedInstallers, categories, err := c.ApplyTeamSoftwareInstallers(currentTeamName, software, opts.ApplySpecOptions) if err != nil { return nil, nil, nil, nil, fmt.Errorf("applying software installers for fleet %q: %w", tmName, err) } logSoftwareDeletions(logfn, deletedInstallers, opts.DryRun) teamsSoftwareInstallers[tmName] = installers + categoriesByTeam[currentTeamName] = append(categoriesByTeam[currentTeamName], categories...) } } if len(tmSoftwareAppsPayloads) > 0 { @@ -1113,13 +1116,24 @@ func (c *Client) ApplyGroup( // For non-dry run, currentTeamName and tmName are the same currentTeamName := getTeamName(tmName) logfn(format, numberWithPluralization(len(apps), "app store app", "app store apps"), tmName) - appsResponse, err := c.ApplyTeamAppStoreAppsAssociation(currentTeamName, apps, opts.ApplySpecOptions) + appsResponse, categories, err := c.ApplyTeamAppStoreAppsAssociation(currentTeamName, apps, opts.ApplySpecOptions) if err != nil { return nil, nil, nil, nil, fmt.Errorf("applying app store apps for fleet: %q: %w", tmName, err) } teamsVPPApps[tmName] = appsResponse + categoriesByTeam[currentTeamName] = append(categoriesByTeam[currentTeamName], categories...) } } + + // Delete categories no longer referenced by any of the fleet's software. + if viaGitOps && !opts.DryRun && !softwareExcepted { + for tmName, tmID := range teamIDsByName { + if err := c.deleteUnusedSelfServiceCategories(tmID, categoriesByTeam[tmName]); err != nil { + return nil, nil, nil, nil, fmt.Errorf("deleting unused self-service categories for fleet %q: %w", tmName, err) + } + } + } + if opts.DryRun { logfn(dryRunAppliedFormat, numberWithPluralization(len(specs.Teams), "fleet", "fleets")) } else { @@ -2496,9 +2510,6 @@ func (c *Client) DoGitOps( for _, teamID := range teamIDsByName { incoming.TeamID = &teamID } - if err := c.doSelfServiceCategories(incoming, dryRun); err != nil { - return err - } if incoming.Labels == nil || len(incoming.Labels) > 0 { return c.doGitOpsLabels(incoming, logFn, dryRun) } @@ -2827,28 +2838,32 @@ func (c *Client) doGitOpsNoTeamSetupAndSoftware( return nil, nil, fmt.Errorf("applying software installers: %w", err) } - if err := c.doSelfServiceCategories(config, dryRun); err != nil { - return nil, nil, err - } - format := applyingTeamFormat if dryRun { format = dryRunAppliedTeamFormat } logFn(format, numberWithPluralization(len(swPkgPayload), "software package", "software packages"), "'Unassigned'") - softwareInstallers, deletedInstallers, err := c.ApplyNoTeamSoftwareInstallers(swPkgPayload, fleet.ApplySpecOptions{DryRun: dryRun}) + softwareInstallers, deletedInstallers, installerCategories, err := c.ApplyNoTeamSoftwareInstallers(swPkgPayload, fleet.ApplySpecOptions{DryRun: dryRun}) if err != nil { return nil, nil, fmt.Errorf("applying software installers: %w", err) } logSoftwareDeletions(logFn, deletedInstallers, dryRun) logFn(format, numberWithPluralization(len(appsPayload), "app store app", "app store apps"), "'Unassigned'") - vppApps, err := c.ApplyNoTeamAppStoreAppsAssociation(appsPayload, fleet.ApplySpecOptions{DryRun: dryRun}) + vppApps, appCategories, err := c.ApplyNoTeamAppStoreAppsAssociation(appsPayload, fleet.ApplySpecOptions{DryRun: dryRun}) if err != nil { return nil, nil, fmt.Errorf("applying app store apps: %w", err) } + // Delete categories not referenced by any software. + if !dryRun && !softwareExcepted { + categories := slices.Concat(installerCategories, appCategories) + if err := c.deleteUnusedSelfServiceCategories(0, categories); err != nil { + return nil, nil, fmt.Errorf("deleting unused self-service categories: %w", err) + } + } + if !dryRun { logFn("[+] applied software packages for unassigned hosts\n") } @@ -2930,50 +2945,6 @@ func (c *Client) doGitOpsNoTeamWebhookSettings( return nil } -func (c *Client) doSelfServiceCategories(config *spec.GitOps, dryRun bool) error { - if !config.Software.SelfServiceCategories.Set { - return nil - } - var teamID uint - if config.TeamID != nil { - teamID = *config.TeamID - } - - existing, err := c.ListSelfServiceCategories(teamID) - if err != nil { - return fmt.Errorf("listing existing self-service categories: %w", err) - } - payloads := config.Software.SelfServiceCategories.Value - - var toInsert []string - for _, name := range payloads { - if !slices.ContainsFunc(existing, func(c fleet.SoftwareCategory) bool { return strings.EqualFold(c.Name, name) }) { - toInsert = append(toInsert, name) - } - } - var toDelete []fleet.SoftwareCategory - for _, cat := range existing { - if !slices.ContainsFunc(payloads, func(p string) bool { return strings.EqualFold(p, cat.Name) }) { - toDelete = append(toDelete, cat) - } - } - - if dryRun { - return nil - } - for _, name := range toInsert { - if _, err := c.AddSelfServiceCategory(teamID, name); err != nil { - return fmt.Errorf("adding self-service category %q: %w", name, err) - } - } - for _, cat := range toDelete { - if err := c.DeleteSelfServiceCategory(cat.ID); err != nil { - return fmt.Errorf("deleting self-service category %q: %w", cat.Name, err) - } - } - return nil -} - func (c *Client) doGitOpsLabels( config *spec.GitOps, logFn func(format string, args ...any), diff --git a/server/service/client_software.go b/server/service/client_software.go index 0c7ed57043..ad11d08b9f 100644 --- a/server/service/client_software.go +++ b/server/service/client_software.go @@ -9,6 +9,8 @@ import ( "mime/multipart" "net/http" "net/url" + "slices" + "strings" "time" "github.com/fleetdm/fleet/v4/server/fleet" @@ -86,43 +88,78 @@ 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, error) { +func (c *Client) ApplyNoTeamSoftwareInstallers(softwareInstallers []fleet.SoftwareInstallerPayload, opts fleet.ApplySpecOptions) ([]fleet.SoftwarePackageResponse, []fleet.DeletedSoftwarePackage, []string, error) { query, err := url.ParseQuery(opts.RawQuery()) if err != nil { - return nil, nil, err + return nil, nil, nil, err } return c.applySoftwareInstallers(softwareInstallers, query, opts.DryRun) } -func (c *Client) applySoftwareInstallers(softwareInstallers []fleet.SoftwareInstallerPayload, query url.Values, dryRun bool) ([]fleet.SoftwarePackageResponse, []fleet.DeletedSoftwarePackage, error) { +func (c *Client) applySoftwareInstallers(softwareInstallers []fleet.SoftwareInstallerPayload, query url.Values, dryRun bool) ([]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 { - return nil, nil, err + return nil, nil, nil, err } if dryRun && resp.RequestUUID == "" { - return nil, nil, nil + return nil, nil, nil, nil } requestUUID := resp.RequestUUID for { var resp batchSetSoftwareInstallersResultResponse if err := c.authenticatedRequestWithQuery(nil, "GET", path+"/"+requestUUID, &resp, query.Encode()); err != nil { - return nil, nil, err + return nil, nil, nil, err } switch { case resp.Status == fleet.BatchSetSoftwareInstallersStatusProcessing: time.Sleep(1 * time.Second) case resp.Status == fleet.BatchSetSoftwareInstallersStatusFailed: - return nil, nil, errors.New(resp.Message) + return nil, nil, nil, errors.New(resp.Message) case resp.Status == fleet.BatchSetSoftwareInstallersStatusCompleted: - return matchPackageIcons(softwareInstallers, resp.Packages), resp.DeletedPackages, nil + return matchPackageIcons(softwareInstallers, resp.Packages), resp.DeletedPackages, resp.Categories, nil default: - return nil, nil, fmt.Errorf("unknown status: %q", resp.Status) + return nil, nil, nil, fmt.Errorf("unknown status: %q", resp.Status) } } } +func (c *Client) ListSelfServiceCategories(teamID uint) ([]fleet.SoftwareCategory, error) { + verb, path := "GET", "/api/latest/fleet/software/self_service_categories" + query := fmt.Sprintf("fleet_id=%d", teamID) + var responseBody getSelfServiceCategoriesResponse + if err := c.authenticatedRequestWithQuery(nil, verb, path, &responseBody, query); err != nil { + return nil, err + } + return responseBody.SelfServiceCategories, nil +} + +func (c *Client) DeleteSelfServiceCategory(id uint) error { + verb, path := "DELETE", fmt.Sprintf("/api/latest/fleet/software/self_service_categories/%d", id) + var responseBody deleteSelfServiceCategoriesResponse + return c.authenticatedRequest(nil, verb, path, &responseBody) +} + +// deleteUnusedSelfServiceCategories deletes the team's existing self-service categories that aren't +// in keep. Categories are created server-side by the software/VPP batch endpoints; keep is the +// union of categories those batches reported, so anything not in it is no longer needed +func (c *Client) deleteUnusedSelfServiceCategories(teamID uint, keep []string) error { + existing, err := c.ListSelfServiceCategories(teamID) + if err != nil { + return fmt.Errorf("listing existing self-service categories: %w", err) + } + for _, cat := range existing { + if slices.ContainsFunc(keep, func(name string) bool { return strings.EqualFold(name, cat.Name) }) { + continue + } + if err := c.DeleteSelfServiceCategory(cat.ID); err != nil { + return fmt.Errorf("deleting self-service category %q: %w", cat.Name, err) + } + } + return nil +} + // matchPackageIcons hydrates software responses with references to icons in the request payload, so we can track // which API calls to make to add/update/delete icons func matchPackageIcons(request []fleet.SoftwareInstallerPayload, response []fleet.SoftwarePackageResponse) []fleet.SoftwarePackageResponse { @@ -288,29 +325,3 @@ func (c *Client) ListFleetMaintainedApps(teamID uint) ([]fleet.MaintainedApp, er } return responseBody.FleetMaintainedApps, nil } - -func (c *Client) ListSelfServiceCategories(teamID uint) ([]fleet.SoftwareCategory, error) { - verb, path := "GET", "/api/latest/fleet/software/self_service_categories" - query := fmt.Sprintf("fleet_id=%d", teamID) - var responseBody getSelfServiceCategoriesResponse - if err := c.authenticatedRequestWithQuery(nil, verb, path, &responseBody, query); err != nil { - return nil, err - } - return responseBody.SelfServiceCategories, nil -} - -func (c *Client) AddSelfServiceCategory(teamID uint, name string) (*fleet.SoftwareCategory, error) { - verb, path := "POST", "/api/latest/fleet/software/self_service_categories" - body := addSelfServiceCategoriesRequest{TeamID: &teamID, Name: name} - var responseBody addSelfServiceCategoriesResponse - if err := c.authenticatedRequest(body, verb, path, &responseBody); err != nil { - return nil, err - } - return responseBody.SelfServiceCategory, nil -} - -func (c *Client) DeleteSelfServiceCategory(id uint) error { - verb, path := "DELETE", fmt.Sprintf("/api/latest/fleet/software/self_service_categories/%d", id) - var responseBody deleteSelfServiceCategoriesResponse - return c.authenticatedRequest(nil, verb, path, &responseBody) -} diff --git a/server/service/client_teams.go b/server/service/client_teams.go index 8be097edbd..166d201146 100644 --- a/server/service/client_teams.go +++ b/server/service/client_teams.go @@ -114,40 +114,40 @@ 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, error) { +func (c *Client) ApplyTeamSoftwareInstallers(tmName string, softwareInstallers []fleet.SoftwareInstallerPayload, opts fleet.ApplySpecOptions) ([]fleet.SoftwarePackageResponse, []fleet.DeletedSoftwarePackage, []string, error) { query, err := url.ParseQuery(opts.RawQuery()) if err != nil { - return nil, nil, err + return nil, nil, nil, err } query.Add("fleet_name", tmName) return c.applySoftwareInstallers(softwareInstallers, query, opts.DryRun) } -func (c *Client) ApplyTeamAppStoreAppsAssociation(tmName string, vppBatchPayload []fleet.VPPBatchPayload, opts fleet.ApplySpecOptions) ([]fleet.VPPAppResponse, error) { +func (c *Client) ApplyTeamAppStoreAppsAssociation(tmName string, vppBatchPayload []fleet.VPPBatchPayload, opts fleet.ApplySpecOptions) ([]fleet.VPPAppResponse, []string, error) { query, err := url.ParseQuery(opts.RawQuery()) if err != nil { - return nil, err + return nil, nil, err } query.Add("fleet_name", tmName) return c.applyAppStoreAppsAssociation(vppBatchPayload, query) } -func (c *Client) ApplyNoTeamAppStoreAppsAssociation(vppBatchPayload []fleet.VPPBatchPayload, opts fleet.ApplySpecOptions) ([]fleet.VPPAppResponse, error) { +func (c *Client) ApplyNoTeamAppStoreAppsAssociation(vppBatchPayload []fleet.VPPBatchPayload, opts fleet.ApplySpecOptions) ([]fleet.VPPAppResponse, []string, error) { query, err := url.ParseQuery(opts.RawQuery()) if err != nil { - return nil, err + return nil, nil, err } return c.applyAppStoreAppsAssociation(vppBatchPayload, query) } -func (c *Client) applyAppStoreAppsAssociation(vppBatchPayload []fleet.VPPBatchPayload, query url.Values) ([]fleet.VPPAppResponse, error) { +func (c *Client) applyAppStoreAppsAssociation(vppBatchPayload []fleet.VPPBatchPayload, query url.Values) ([]fleet.VPPAppResponse, []string, error) { verb, path := "POST", "/api/latest/fleet/software/app_store_apps/batch" var appsResponse batchAssociateAppStoreAppsResponse err := c.authenticatedRequestWithQuery(map[string]interface{}{"app_store_apps": vppBatchPayload}, verb, path, &appsResponse, query.Encode()) if err != nil { - return nil, err + return nil, nil, err } - return matchAppStoreAppCustomIcons(vppBatchPayload, appsResponse.Apps), nil + return matchAppStoreAppCustomIcons(vppBatchPayload, appsResponse.Apps), appsResponse.Categories, nil } // matchAppStoreAppCustomIcons hydrates VPP responses with references to icons in the request payload, so we can track diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index df83f2e4eb..e8b146c0d6 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -22269,7 +22269,7 @@ func (s *integrationEnterpriseTestSuite) TestBatchSoftwareInstallerAndFMACategor // a request with an invalid category silently drops the unknown name pkgURL := installerServer.URL + "/non-fma.pkg" softwareToInstall := []*fleet.SoftwareInstallerPayload{ - {URL: pkgURL, Categories: []string{"Not Found"}, SelfService: true}, + {URL: pkgURL, Categories: optjson.SetSlice([]string{"Not Found"}), SelfService: true}, {Slug: &maintained1.Slug, SelfService: true}, } var batchResponse batchSetSoftwareInstallersResponse @@ -22277,35 +22277,46 @@ func (s *integrationEnterpriseTestSuite) TestBatchSoftwareInstallerAndFMACategor packages := waitBatchSetSoftwareInstallersCompleted(t, &s.withServer, team1.Name, batchResponse.RequestUUID) require.Len(t, packages, 2) + // an over-length category name is rejected up front + softwareToInstall[0].Categories = optjson.SetSlice([]string{strings.Repeat("x", 256)}) + res := s.Do("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: softwareToInstall}, http.StatusUnprocessableEntity, "team_name", team1.Name) + require.Contains(t, extractServerErrorText(res.Body), "name must be at most 255 characters") + testCases := []struct { desc string - categories []string + categories optjson.Slice[string] fmaDefaultCategories []string }{ { desc: "duplicate categories provided", - categories: []string{"🧰 Developer tools", "🌎 Browsers", "🌎 Browsers"}, + categories: optjson.SetSlice([]string{"🧰 Developer tools", "🌎 Browsers", "🌎 Browsers"}), }, { desc: "valid categories 1", - categories: []string{"🧰 Developer tools", "🌎 Browsers"}, + categories: optjson.SetSlice([]string{"🧰 Developer tools", "🌎 Browsers"}), }, { desc: "valid categories 2", - categories: []string{"👬 Communication", "💻 Productivity"}, + categories: optjson.SetSlice([]string{"👬 Communication", "💻 Productivity"}), }, { desc: "valid categories 3 - Security and Support", - categories: []string{"🔐 Security", "🛟 Support"}, + categories: optjson.SetSlice([]string{"🔐 Security", "🛟 Support"}), }, { desc: "valid categories 4 - mixed with new categories", - categories: []string{"🔐 Security", "🧰 Developer tools", "🛟 Support"}, + categories: optjson.SetSlice([]string{"🔐 Security", "🧰 Developer tools", "🛟 Support"}), }, { - desc: "empty categories", + // omitted categories (unset) fall back to the FMA's manifest default + desc: "omitted categories use FMA default", fmaDefaultCategories: []string{"💻 Productivity"}, }, + { + // an explicitly empty categories list sets zero categories (no manifest default) + desc: "explicit empty categories sets zero categories", + categories: optjson.SetSlice([]string{}), + }, } for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { @@ -22313,7 +22324,7 @@ func (s *integrationEnterpriseTestSuite) TestBatchSoftwareInstallerAndFMACategor softwareToInstall[1].Categories = tc.categories // remove duplicates if any - tc.categories = server.RemoveDuplicatesFromSlice(tc.categories) + wantCategories := server.RemoveDuplicatesFromSlice(tc.categories.Value) s.DoJSON("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: softwareToInstall}, http.StatusAccepted, &batchResponse, "team_name", team1.Name) packages := waitBatchSetSoftwareInstallersCompleted(t, &s.withServer, team1.Name, batchResponse.RequestUUID) require.Len(t, packages, 2) @@ -22327,13 +22338,12 @@ func (s *integrationEnterpriseTestSuite) TestBatchSoftwareInstallerAndFMACategor stResp := getSoftwareTitleResponse{} s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d", *p.TitleID), getSoftwareTitleRequest{}, http.StatusOK, &stResp, "team_id", fmt.Sprint(team1.ID)) require.NotNil(t, stResp.SoftwareTitle.SoftwarePackage) - if stResp.SoftwareTitle.SoftwarePackage.FleetMaintainedAppID != nil && len(tc.categories) == 0 { - // if no categories are set on an FMA in GitOps, we set categories to - // default values + if stResp.SoftwareTitle.SoftwarePackage.FleetMaintainedAppID != nil && !tc.categories.Set { + // categories omitted on an FMA → fall back to the manifest default require.ElementsMatch(t, tc.fmaDefaultCategories, stResp.SoftwareTitle.SoftwarePackage.Categories) continue } - require.ElementsMatch(t, tc.categories, stResp.SoftwareTitle.SoftwarePackage.Categories) + require.ElementsMatch(t, wantCategories, stResp.SoftwareTitle.SoftwarePackage.Categories) } // check that the categories come back on the My device page @@ -22343,14 +22353,13 @@ func (s *integrationEnterpriseTestSuite) TestBatchSoftwareInstallerAndFMACategor require.NoError(t, err) require.Len(t, getDeviceSw.Software, 2) for _, s := range getDeviceSw.Software { - if s.Name == maintained1.Name && len(tc.categories) == 0 { - // if no categories are set on an FMA in GitOps, we set categories to - // default values + if s.Name == maintained1.Name && !tc.categories.Set { + // categories omitted on an FMA → fall back to the manifest default require.ElementsMatch(t, tc.fmaDefaultCategories, s.SoftwarePackage.Categories) continue } - require.ElementsMatch(t, tc.categories, s.SoftwarePackage.Categories) + require.ElementsMatch(t, wantCategories, s.SoftwarePackage.Categories) } }) } diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index b879dacfa7..36684f83a6 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -21005,6 +21005,8 @@ func (s *integrationMDMTestSuite) TestSoftwareCategories() { }, }, http.StatusOK, &batchAssociateResponse, ) + // the batch response returns the categories it uses and added + require.ElementsMatch(t, []string{"🧰 Developer tools", "👬 Communication"}, batchAssociateResponse.Categories) s.DoJSON("GET", fmt.Sprintf("/api/v1/fleet/software/titles/%d", vppAppTitleID), nil, http.StatusOK, &titleResponse, "team_id", "0") require.NotNil(t, titleResponse.SoftwareTitle.AppStoreApp) diff --git a/server/service/software_installers.go b/server/service/software_installers.go index 773d5839fc..f909631fc6 100644 --- a/server/service/software_installers.go +++ b/server/service/software_installers.go @@ -856,6 +856,8 @@ type batchSetSoftwareInstallersResultResponse struct { // DeletedPackages lists the packages the batch deleted (dry run: would // delete) because their title matches no entry in the request payload. DeletedPackages []fleet.DeletedSoftwarePackage `json:"deleted_packages,omitempty"` + // Categories lists the self-service categories the batch's software references. + Categories []string `json:"categories,omitempty"` Err error `json:"error,omitempty"` } @@ -864,7 +866,7 @@ 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, err := svc.GetBatchSetSoftwareInstallersResult(ctx, req.TeamName, req.RequestUUID, req.DryRun) + status, message, packages, deletedPackages, categories, err := svc.GetBatchSetSoftwareInstallersResult(ctx, req.TeamName, req.RequestUUID, req.DryRun) if err != nil { return batchSetSoftwareInstallersResultResponse{Err: err}, nil } @@ -873,15 +875,16 @@ func batchSetSoftwareInstallersResultEndpoint(ctx context.Context, request inter Message: message, Packages: packages, DeletedPackages: deletedPackages, + Categories: categories, }, nil } -func (svc *Service) GetBatchSetSoftwareInstallersResult(ctx context.Context, tmName string, requestUUID string, dryRun bool) (string, string, []fleet.SoftwarePackageResponse, []fleet.DeletedSoftwarePackage, error) { +func (svc *Service) GetBatchSetSoftwareInstallersResult(ctx context.Context, tmName string, requestUUID string, dryRun bool) (string, string, []fleet.SoftwarePackageResponse, []fleet.DeletedSoftwarePackage, []string, error) { // skipauth: No authorization check needed due to implementation returning // only license error. svc.authz.SkipAuthorization(ctx) - return "", "", nil, nil, fleet.ErrMissingLicense + return "", "", nil, nil, nil, fleet.ErrMissingLicense } ////////////////////////////////////////////////////////////////////////////// @@ -1033,26 +1036,28 @@ func (b *batchAssociateAppStoreAppsRequest) DecodeBody(ctx context.Context, r io type batchAssociateAppStoreAppsResponse struct { Apps []fleet.VPPAppResponse `json:"app_store_apps"` - Err error `json:"error,omitempty"` + // Categories lists the self-service categories the batch's apps reference. + Categories []string `json:"categories,omitempty"` + Err error `json:"error,omitempty"` } func (r batchAssociateAppStoreAppsResponse) Error() error { return r.Err } func batchAssociateAppStoreAppsEndpoint(ctx context.Context, request any, svc fleet.Service) (fleet.Errorer, error) { req := request.(*batchAssociateAppStoreAppsRequest) - apps, err := svc.BatchAssociateVPPApps(ctx, req.TeamName, req.Apps, req.DryRun) + apps, categories, err := svc.BatchAssociateVPPApps(ctx, req.TeamName, req.Apps, req.DryRun) if err != nil { return batchAssociateAppStoreAppsResponse{Err: err}, nil } - return batchAssociateAppStoreAppsResponse{Apps: apps}, nil + return batchAssociateAppStoreAppsResponse{Apps: apps, Categories: categories}, nil } -func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, payloads []fleet.VPPBatchPayload, dryRun bool) ([]fleet.VPPAppResponse, error) { +func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string, payloads []fleet.VPPBatchPayload, dryRun bool) ([]fleet.VPPAppResponse, []string, error) { // skipauth: No authorization check needed due to implementation returning // only license error. svc.authz.SkipAuthorization(ctx) - return nil, fleet.ErrMissingLicense + return nil, nil, fleet.ErrMissingLicense } type getInHouseAppManifestRequest struct { diff --git a/server/service/software_installers_test.go b/server/service/software_installers_test.go index 8a6c77face..26a87d5374 100644 --- a/server/service/software_installers_test.go +++ b/server/service/software_installers_test.go @@ -15,6 +15,7 @@ import ( "time" eeservice "github.com/fleetdm/fleet/v4/ee/server/service" + "github.com/fleetdm/fleet/v4/pkg/optjson" authz_ctx "github.com/fleetdm/fleet/v4/server/contexts/authz" "github.com/fleetdm/fleet/v4/server/contexts/viewer" "github.com/fleetdm/fleet/v4/server/datastore/filesystem" @@ -660,14 +661,14 @@ func TestSoftwareInstallerUploadRetries(t *testing.T) { FleetMaintained: false, Filename: "foo", ValidatedLabels: &fleet.LabelIdentsWithScope{}, - Categories: []string{}, + Categories: optjson.SetSlice([]string{}), DisplayName: "foo", }}, false) require.NoError(t, err) timeout := time.After(30 * time.Second) for { - status, _, packages, _, err := svc.GetBatchSetSoftwareInstallersResult(ctx, "foo", "requestuuid", false) + status, _, packages, _, _, 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. diff --git a/tools/cloner-check/generated_files/teamconfig.txt b/tools/cloner-check/generated_files/teamconfig.txt index 6f92d8225b..a3b7e6c299 100644 --- a/tools/cloner-check/generated_files/teamconfig.txt +++ b/tools/cloner-check/generated_files/teamconfig.txt @@ -124,7 +124,7 @@ github.com/fleetdm/fleet/v4/server/fleet/SoftwarePackageSpec Slug *string github.com/fleetdm/fleet/v4/server/fleet/SoftwarePackageSpec Version string github.com/fleetdm/fleet/v4/server/fleet/SoftwarePackageSpec ReferencedYamlPath string github.com/fleetdm/fleet/v4/server/fleet/SoftwarePackageSpec SHA256 string -github.com/fleetdm/fleet/v4/server/fleet/SoftwarePackageSpec Categories []string +github.com/fleetdm/fleet/v4/server/fleet/SoftwarePackageSpec Categories optjson.Slice[string] github.com/fleetdm/fleet/v4/server/fleet/SoftwarePackageSpec DisplayName string github.com/fleetdm/fleet/v4/server/fleet/SoftwarePackageSpec AlwaysDownload bool github.com/fleetdm/fleet/v4/server/fleet/SoftwareSpec FleetMaintainedApps optjson.Slice[github.com/fleetdm/fleet/v4/server/fleet.MaintainedAppSpec] @@ -141,7 +141,7 @@ github.com/fleetdm/fleet/v4/server/fleet/MaintainedAppSpec UninstallScript fleet github.com/fleetdm/fleet/v4/server/fleet/MaintainedAppSpec LabelsIncludeAny []string github.com/fleetdm/fleet/v4/server/fleet/MaintainedAppSpec LabelsExcludeAny []string github.com/fleetdm/fleet/v4/server/fleet/MaintainedAppSpec LabelsIncludeAll []string -github.com/fleetdm/fleet/v4/server/fleet/MaintainedAppSpec Categories []string +github.com/fleetdm/fleet/v4/server/fleet/MaintainedAppSpec Categories optjson.Slice[string] github.com/fleetdm/fleet/v4/server/fleet/MaintainedAppSpec InstallDuringSetup optjson.Bool github.com/fleetdm/fleet/v4/server/fleet/MaintainedAppSpec Icon fleet.TeamSpecSoftwareAsset github.com/fleetdm/fleet/v4/server/fleet/SoftwareSpec AppStoreApps optjson.Slice[github.com/fleetdm/fleet/v4/server/fleet.TeamSpecAppStoreApp]