From 6a16995af1b5bf85d9706459314dd09e79d4da0b Mon Sep 17 00:00:00 2001 From: Martin Angers Date: Wed, 11 Dec 2024 11:43:35 -0500 Subject: [PATCH 1/5] Add labels_include_any and labels_exclude_any to gitops software --- pkg/spec/gitops.go | 4 ++++ server/fleet/scripts.go | 20 +++++++++++--------- server/fleet/software_installer.go | 2 ++ server/service/client.go | 2 ++ 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/pkg/spec/gitops.go b/pkg/spec/gitops.go index bdfdf1ceec3c..d0332c297a06 100644 --- a/pkg/spec/gitops.go +++ b/pkg/spec/gitops.go @@ -785,6 +785,10 @@ func parseSoftware(top map[string]json.RawMessage, result *GitOps, baseDir strin multiError = multierror.Append(multiError, fmt.Errorf("software URL %q is too long, must be less than 256 characters", softwarePackageSpec.URL)) continue } + if len(softwarePackageSpec.LabelsExcludeAny) > 0 && len(softwarePackageSpec.LabelsIncludeAny) > 0 { + multiError = multierror.Append(multiError, fmt.Errorf(`only one of "labels_exclude_any" or "labels_include_any" can be specified for software URL %q`, softwarePackageSpec.URL)) + continue + } result.Software.Packages = append(result.Software.Packages, &softwarePackageSpec) } diff --git a/server/fleet/scripts.go b/server/fleet/scripts.go index 8b011642b10b..16295b886217 100644 --- a/server/fleet/scripts.go +++ b/server/fleet/scripts.go @@ -380,15 +380,17 @@ type ScriptPayload struct { } type SoftwareInstallerPayload struct { - URL string `json:"url"` - PreInstallQuery string `json:"pre_install_query"` - InstallScript string `json:"install_script"` - UninstallScript string `json:"uninstall_script"` - PostInstallScript string `json:"post_install_script"` - SelfService bool `json:"self_service"` - FleetMaintained bool `json:"-"` - Filename string `json:"-"` - InstallDuringSetup *bool `json:"install_during_setup"` // if nil, do not change saved value, otherwise set it + URL string `json:"url"` + PreInstallQuery string `json:"pre_install_query"` + InstallScript string `json:"install_script"` + UninstallScript string `json:"uninstall_script"` + PostInstallScript string `json:"post_install_script"` + SelfService bool `json:"self_service"` + FleetMaintained bool `json:"-"` + Filename string `json:"-"` + InstallDuringSetup *bool `json:"install_during_setup"` // if nil, do not change saved value, otherwise set it + LabelsIncludeAny []string `json:"labels_include_any"` + LabelsExcludeAny []string `json:"labels_exclude_any"` } type HostLockWipeStatus struct { diff --git a/server/fleet/software_installer.go b/server/fleet/software_installer.go index 73fee6431db2..1b71784c8aef 100644 --- a/server/fleet/software_installer.go +++ b/server/fleet/software_installer.go @@ -460,6 +460,8 @@ type SoftwarePackageSpec struct { InstallScript TeamSpecSoftwareAsset `json:"install_script"` PostInstallScript TeamSpecSoftwareAsset `json:"post_install_script"` UninstallScript TeamSpecSoftwareAsset `json:"uninstall_script"` + LabelsIncludeAny []string `json:"labels_include_any"` + LabelsExcludeAny []string `json:"labels_exclude_any"` // ReferencedYamlPath is the resolved path of the file used to fill the // software package. Only present after parsing a GitOps file on the fleetctl diff --git a/server/service/client.go b/server/service/client.go index 935ca0743433..d9e12c136f2d 100644 --- a/server/service/client.go +++ b/server/service/client.go @@ -1006,6 +1006,8 @@ func buildSoftwarePackagesPayload(specs []fleet.SoftwarePackageSpec, installDuri PostInstallScript: string(pc), UninstallScript: string(us), InstallDuringSetup: installDuringSetup, + LabelsIncludeAny: si.LabelsIncludeAny, + LabelsExcludeAny: si.LabelsExcludeAny, } } From 109e3b150123f6489f95c1231e91c6ad0aa3b38f Mon Sep 17 00:00:00 2001 From: Martin Angers Date: Wed, 11 Dec 2024 13:56:08 -0500 Subject: [PATCH 2/5] Add todos for gitops tests --- cmd/fleetctl/gitops_test.go | 2 ++ ee/server/service/software_installers.go | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/cmd/fleetctl/gitops_test.go b/cmd/fleetctl/gitops_test.go index ad11066b2aa5..732df7a2e0be 100644 --- a/cmd/fleetctl/gitops_test.go +++ b/cmd/fleetctl/gitops_test.go @@ -1872,6 +1872,7 @@ func TestGitOpsTeamSofwareInstallers(t *testing.T) { startSoftwareInstallerServer(t) startAndServeVPPServer(t) + // TODO(mna): add cases for error include/exclude and valid only one cases := []struct { file string wantErr string @@ -1956,6 +1957,7 @@ func TestGitOpsNoTeamSoftwareInstallers(t *testing.T) { startSoftwareInstallerServer(t) startAndServeVPPServer(t) + // TODO(mna): add cases for error include/exclude and valid only one cases := []struct { noTeamFile string wantErr string diff --git a/ee/server/service/software_installers.go b/ee/server/service/software_installers.go index b5e1e2a94351..f0801665a7ea 100644 --- a/ee/server/service/software_installers.go +++ b/ee/server/service/software_installers.go @@ -1164,6 +1164,8 @@ func (svc *Service) BatchSetSoftwareInstallers( } } + // TODO(mna): validate only one of include/exclude labels are provided, and that they exist + // keyExpireTime is the current maximum time supported for retrieving // the result of a software by batch operation. const keyExpireTime = 24 * time.Hour @@ -1313,6 +1315,8 @@ func (svc *Service) softwareBatchUpload( UserID: userID, URL: p.URL, InstallDuringSetup: p.InstallDuringSetup, + LabelsIncludeAny: p.LabelsIncludeAny, + LabelsExcludeAny: p.LabelsExcludeAny, } // set the filename before adding metadata, as it is used as fallback From 8642aaeb77bb654938046b4d0a4722aa7dcefff7 Mon Sep 17 00:00:00 2001 From: Martin Angers Date: Wed, 11 Dec 2024 14:43:31 -0500 Subject: [PATCH 3/5] Add gitops test scenarios --- cmd/fleetctl/gitops_test.go | 40 +++++++++++++++++++ ...installer_invalid_both_include_exclude.yml | 20 ++++++++++ ...ftware_installer_invalid_unknown_label.yml | 18 +++++++++ ..._team_software_installer_valid_exclude.yml | 19 +++++++++ ..._team_software_installer_valid_include.yml | 19 +++++++++ ...installer_invalid_both_include_exclude.yml | 29 ++++++++++++++ ...ftware_installer_invalid_unknown_label.yml | 27 +++++++++++++ .../team_software_installer_valid_exclude.yml | 27 +++++++++++++ .../team_software_installer_valid_include.yml | 27 +++++++++++++ ee/server/service/software_installers.go | 8 +++- 10 files changed, 233 insertions(+), 1 deletion(-) create mode 100644 cmd/fleetctl/testdata/gitops/no_team_software_installer_invalid_both_include_exclude.yml create mode 100644 cmd/fleetctl/testdata/gitops/no_team_software_installer_invalid_unknown_label.yml create mode 100644 cmd/fleetctl/testdata/gitops/no_team_software_installer_valid_exclude.yml create mode 100644 cmd/fleetctl/testdata/gitops/no_team_software_installer_valid_include.yml create mode 100644 cmd/fleetctl/testdata/gitops/team_software_installer_invalid_both_include_exclude.yml create mode 100644 cmd/fleetctl/testdata/gitops/team_software_installer_invalid_unknown_label.yml create mode 100644 cmd/fleetctl/testdata/gitops/team_software_installer_valid_exclude.yml create mode 100644 cmd/fleetctl/testdata/gitops/team_software_installer_valid_include.yml diff --git a/cmd/fleetctl/gitops_test.go b/cmd/fleetctl/gitops_test.go index 732df7a2e0be..4d6f3767d6c5 100644 --- a/cmd/fleetctl/gitops_test.go +++ b/cmd/fleetctl/gitops_test.go @@ -1893,6 +1893,10 @@ func TestGitOpsTeamSofwareInstallers(t *testing.T) { {"testdata/gitops/team_software_installer_post_install_not_found.yml", "no such file or directory"}, {"testdata/gitops/team_software_installer_no_url.yml", "software URL is required"}, {"testdata/gitops/team_software_installer_invalid_self_service_value.yml", "\"packages.self_service\" must be a bool, found string"}, + {"testdata/gitops/team_software_installer_invalid_both_include_exclude.yml", `only one of "labels_exclude_any" or "labels_include_any" can be specified`}, + {"testdata/gitops/team_software_installer_valid_include.yml", ""}, + {"testdata/gitops/team_software_installer_valid_exclude.yml", ""}, + {"testdata/gitops/team_software_installer_invalid_unknown_label.yml", "zzzzz"}, // team tests for setup experience software/script {"testdata/gitops/team_setup_software_valid.yml", ""}, {"testdata/gitops/team_setup_software_invalid_script.yml", "no_such_script.sh: no such file"}, @@ -1922,6 +1926,22 @@ func TestGitOpsTeamSofwareInstallers(t *testing.T) { Teams: nil, }, nil } + labelToIDs := map[string]uint{ + fleet.BuiltinLabelMacOS14Plus: 1, + "a": 2, + "b": 3, + } + ds.LabelIDsByNameFunc = func(ctx context.Context, labels []string) (map[string]uint, error) { + // for this test, recognize labels a and b (as well as the built-in macos 14+ one) + ret := make(map[string]uint) + for _, lbl := range labels { + id, ok := labelToIDs[lbl] + if ok { + ret[lbl] = id + } + } + return ret, nil + } _, err = runAppNoChecks([]string{"gitops", "-f", c.file}) if c.wantErr == "" { @@ -1976,6 +1996,10 @@ func TestGitOpsNoTeamSoftwareInstallers(t *testing.T) { {"testdata/gitops/no_team_software_installer_post_install_not_found.yml", "no such file or directory"}, {"testdata/gitops/no_team_software_installer_no_url.yml", "software URL is required"}, {"testdata/gitops/no_team_software_installer_invalid_self_service_value.yml", "\"packages.self_service\" must be a bool, found string"}, + {"testdata/gitops/no_team_software_installer_invalid_both_include_exclude.yml", `only one of "labels_exclude_any" or "labels_include_any" can be specified`}, + {"testdata/gitops/no_team_software_installer_valid_include.yml", ""}, + {"testdata/gitops/no_team_software_installer_valid_exclude.yml", ""}, + {"testdata/gitops/no_team_software_installer_invalid_unknown_label.yml", "zzzzz"}, // No team tests for setup experience software/script {"testdata/gitops/no_team_setup_software_valid.yml", ""}, {"testdata/gitops/no_team_setup_software_invalid_script.yml", "no_such_script.sh: no such file"}, @@ -2005,6 +2029,22 @@ func TestGitOpsNoTeamSoftwareInstallers(t *testing.T) { Teams: nil, }, nil } + labelToIDs := map[string]uint{ + fleet.BuiltinLabelMacOS14Plus: 1, + "a": 2, + "b": 3, + } + ds.LabelIDsByNameFunc = func(ctx context.Context, labels []string) (map[string]uint, error) { + // for this test, recognize labels a and b (as well as the built-in macos 14+ one) + ret := make(map[string]uint) + for _, lbl := range labels { + id, ok := labelToIDs[lbl] + if ok { + ret[lbl] = id + } + } + return ret, nil + } t.Setenv("APPLE_BM_DEFAULT_TEAM", "") globalFile := "./testdata/gitops/global_config_no_paths.yml" diff --git a/cmd/fleetctl/testdata/gitops/no_team_software_installer_invalid_both_include_exclude.yml b/cmd/fleetctl/testdata/gitops/no_team_software_installer_invalid_both_include_exclude.yml new file mode 100644 index 000000000000..a114afa6e45b --- /dev/null +++ b/cmd/fleetctl/testdata/gitops/no_team_software_installer_invalid_both_include_exclude.yml @@ -0,0 +1,20 @@ +name: No team +controls: +policies: +software: + packages: + - url: ${SOFTWARE_INSTALLER_URL}/ruby.deb + install_script: + path: lib/install_ruby.sh + pre_install_query: + path: lib/query_ruby.yml + post_install_script: + path: lib/post_install_ruby.sh + uninstall_script: + path: lib/uninstall_ruby.sh + labels_include_any: + - a + labels_exclude_any: + - b + - url: ${SOFTWARE_INSTALLER_URL}/other.deb + self_service: true diff --git a/cmd/fleetctl/testdata/gitops/no_team_software_installer_invalid_unknown_label.yml b/cmd/fleetctl/testdata/gitops/no_team_software_installer_invalid_unknown_label.yml new file mode 100644 index 000000000000..73ebf1310ae5 --- /dev/null +++ b/cmd/fleetctl/testdata/gitops/no_team_software_installer_invalid_unknown_label.yml @@ -0,0 +1,18 @@ +name: No team +controls: +policies: +software: + packages: + - url: ${SOFTWARE_INSTALLER_URL}/ruby.deb + install_script: + path: lib/install_ruby.sh + pre_install_query: + path: lib/query_ruby.yml + post_install_script: + path: lib/post_install_ruby.sh + uninstall_script: + path: lib/uninstall_ruby.sh + labels_exclude_any: + - zzz + - url: ${SOFTWARE_INSTALLER_URL}/other.deb + self_service: true diff --git a/cmd/fleetctl/testdata/gitops/no_team_software_installer_valid_exclude.yml b/cmd/fleetctl/testdata/gitops/no_team_software_installer_valid_exclude.yml new file mode 100644 index 000000000000..bbe779982d2a --- /dev/null +++ b/cmd/fleetctl/testdata/gitops/no_team_software_installer_valid_exclude.yml @@ -0,0 +1,19 @@ +name: No team +controls: +policies: +software: + packages: + - url: ${SOFTWARE_INSTALLER_URL}/ruby.deb + install_script: + path: lib/install_ruby.sh + pre_install_query: + path: lib/query_ruby.yml + post_install_script: + path: lib/post_install_ruby.sh + uninstall_script: + path: lib/uninstall_ruby.sh + labels_exclude_any: + - a + - b + - url: ${SOFTWARE_INSTALLER_URL}/other.deb + self_service: true diff --git a/cmd/fleetctl/testdata/gitops/no_team_software_installer_valid_include.yml b/cmd/fleetctl/testdata/gitops/no_team_software_installer_valid_include.yml new file mode 100644 index 000000000000..225e4d6b2730 --- /dev/null +++ b/cmd/fleetctl/testdata/gitops/no_team_software_installer_valid_include.yml @@ -0,0 +1,19 @@ +name: No team +controls: +policies: +software: + packages: + - url: ${SOFTWARE_INSTALLER_URL}/ruby.deb + install_script: + path: lib/install_ruby.sh + pre_install_query: + path: lib/query_ruby.yml + post_install_script: + path: lib/post_install_ruby.sh + uninstall_script: + path: lib/uninstall_ruby.sh + labels_include_any: + - a + - b + - url: ${SOFTWARE_INSTALLER_URL}/other.deb + self_service: true diff --git a/cmd/fleetctl/testdata/gitops/team_software_installer_invalid_both_include_exclude.yml b/cmd/fleetctl/testdata/gitops/team_software_installer_invalid_both_include_exclude.yml new file mode 100644 index 000000000000..63911bd11227 --- /dev/null +++ b/cmd/fleetctl/testdata/gitops/team_software_installer_invalid_both_include_exclude.yml @@ -0,0 +1,29 @@ +name: "${TEST_TEAM_NAME}" +team_settings: + secrets: + - secret: "ABC" + features: + enable_host_users: true + enable_software_inventory: true + host_expiry_settings: + host_expiry_enabled: true + host_expiry_window: 30 +agent_options: +controls: +policies: +queries: +software: + packages: + - url: ${SOFTWARE_INSTALLER_URL}/ruby.deb + install_script: + path: lib/install_ruby.sh + pre_install_query: + path: lib/query_ruby_apply.yml + post_install_script: + path: lib/post_install_ruby.sh + labels_include_any: + - a + labels_exclude_any: + - b + - url: ${SOFTWARE_INSTALLER_URL}/other.deb + self_service: true diff --git a/cmd/fleetctl/testdata/gitops/team_software_installer_invalid_unknown_label.yml b/cmd/fleetctl/testdata/gitops/team_software_installer_invalid_unknown_label.yml new file mode 100644 index 000000000000..090f14b2f981 --- /dev/null +++ b/cmd/fleetctl/testdata/gitops/team_software_installer_invalid_unknown_label.yml @@ -0,0 +1,27 @@ +name: "${TEST_TEAM_NAME}" +team_settings: + secrets: + - secret: "ABC" + features: + enable_host_users: true + enable_software_inventory: true + host_expiry_settings: + host_expiry_enabled: true + host_expiry_window: 30 +agent_options: +controls: +policies: +queries: +software: + packages: + - url: ${SOFTWARE_INSTALLER_URL}/ruby.deb + install_script: + path: lib/install_ruby.sh + pre_install_query: + path: lib/query_ruby_apply.yml + post_install_script: + path: lib/post_install_ruby.sh + labels_include_any: + - zzz + - url: ${SOFTWARE_INSTALLER_URL}/other.deb + self_service: true diff --git a/cmd/fleetctl/testdata/gitops/team_software_installer_valid_exclude.yml b/cmd/fleetctl/testdata/gitops/team_software_installer_valid_exclude.yml new file mode 100644 index 000000000000..2bf6d967a280 --- /dev/null +++ b/cmd/fleetctl/testdata/gitops/team_software_installer_valid_exclude.yml @@ -0,0 +1,27 @@ +name: "${TEST_TEAM_NAME}" +team_settings: + secrets: + - secret: "ABC" + features: + enable_host_users: true + enable_software_inventory: true + host_expiry_settings: + host_expiry_enabled: true + host_expiry_window: 30 +agent_options: +controls: +policies: +queries: +software: + packages: + - url: ${SOFTWARE_INSTALLER_URL}/ruby.deb + install_script: + path: lib/install_ruby.sh + pre_install_query: + path: lib/query_ruby_apply.yml + post_install_script: + path: lib/post_install_ruby.sh + labels_exclude_any: + - b + - url: ${SOFTWARE_INSTALLER_URL}/other.deb + self_service: true diff --git a/cmd/fleetctl/testdata/gitops/team_software_installer_valid_include.yml b/cmd/fleetctl/testdata/gitops/team_software_installer_valid_include.yml new file mode 100644 index 000000000000..8a6ebf5821e0 --- /dev/null +++ b/cmd/fleetctl/testdata/gitops/team_software_installer_valid_include.yml @@ -0,0 +1,27 @@ +name: "${TEST_TEAM_NAME}" +team_settings: + secrets: + - secret: "ABC" + features: + enable_host_users: true + enable_software_inventory: true + host_expiry_settings: + host_expiry_enabled: true + host_expiry_window: 30 +agent_options: +controls: +policies: +queries: +software: + packages: + - url: ${SOFTWARE_INSTALLER_URL}/ruby.deb + install_script: + path: lib/install_ruby.sh + pre_install_query: + path: lib/query_ruby_apply.yml + post_install_script: + path: lib/post_install_ruby.sh + labels_include_any: + - a + - url: ${SOFTWARE_INSTALLER_URL}/other.deb + self_service: true diff --git a/ee/server/service/software_installers.go b/ee/server/service/software_installers.go index f0801665a7ea..9eb3409c3fdb 100644 --- a/ee/server/service/software_installers.go +++ b/ee/server/service/software_installers.go @@ -1162,9 +1162,15 @@ func (svc *Service) BatchSetSoftwareInstallers( fmt.Sprintf("Couldn't edit software. URL (%q) is invalid", payload.URL), ) } + if len(payload.LabelsExcludeAny) > 0 && len(payload.LabelsIncludeAny) > 0 { + return "", fleet.NewInvalidArgumentError( + "software.labels_exclude_any", + "Only one of `labels_include_any` or `labels_exclude_any` can be specified.", + ) + } } - // TODO(mna): validate only one of include/exclude labels are provided, and that they exist + // TODO(mna): validate that labels exist if provided // keyExpireTime is the current maximum time supported for retrieving // the result of a software by batch operation. From a8114b89ea0d09775ba75e27c4ebb4c91e0ac4a8 Mon Sep 17 00:00:00 2001 From: Martin Angers Date: Wed, 11 Dec 2024 15:23:24 -0500 Subject: [PATCH 4/5] Pass the labels down to the batch endpoint --- ee/server/service/software_installers.go | 90 +++++++++++++++---- server/datastore/mysql/software_installers.go | 2 + server/fleet/scripts.go | 5 ++ server/fleet/service.go | 2 +- server/fleet/software_installer.go | 5 ++ server/service/software_installers.go | 8 +- 6 files changed, 90 insertions(+), 22 deletions(-) diff --git a/ee/server/service/software_installers.go b/ee/server/service/software_installers.go index 9eb3409c3fdb..16a171749f71 100644 --- a/ee/server/service/software_installers.go +++ b/ee/server/service/software_installers.go @@ -16,6 +16,7 @@ import ( "github.com/fleetdm/fleet/v4/pkg/file" "github.com/fleetdm/fleet/v4/pkg/fleethttp" + "github.com/fleetdm/fleet/v4/server" "github.com/fleetdm/fleet/v4/server/authz" "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" hostctx "github.com/fleetdm/fleet/v4/server/contexts/host" @@ -1120,7 +1121,7 @@ const ( ) func (svc *Service) BatchSetSoftwareInstallers( - ctx context.Context, tmName string, payloads []fleet.SoftwareInstallerPayload, dryRun bool, + ctx context.Context, tmName string, payloads []*fleet.SoftwareInstallerPayload, dryRun bool, ) (string, error) { if err := svc.authz.Authorize(ctx, &fleet.Team{}, fleet.ActionRead); err != nil { return "", err @@ -1162,16 +1163,28 @@ func (svc *Service) BatchSetSoftwareInstallers( fmt.Sprintf("Couldn't edit software. URL (%q) is invalid", payload.URL), ) } - if len(payload.LabelsExcludeAny) > 0 && len(payload.LabelsIncludeAny) > 0 { + switch { + case len(payload.LabelsExcludeAny) > 0 && len(payload.LabelsIncludeAny) > 0: return "", fleet.NewInvalidArgumentError( "software.labels_exclude_any", "Only one of `labels_include_any` or `labels_exclude_any` can be specified.", ) + case len(payload.LabelsExcludeAny) > 0: + scopeLabels, err := svc.validateSoftwareInstallerLabels(ctx, payload.LabelsExcludeAny) + if err != nil { + return "", err + } + // TODO(mna): I don't technically need the Exclude field in SoftwareScopeLabel, is it necessary? + payload.ResolvedLabelsExcludeAny = scopeLabels + case len(payload.LabelsIncludeAny) > 0: + scopeLabels, err := svc.validateSoftwareInstallerLabels(ctx, payload.LabelsIncludeAny) + if err != nil { + return "", err + } + payload.ResolvedLabelsIncludeAny = scopeLabels } } - // TODO(mna): validate that labels exist if provided - // keyExpireTime is the current maximum time supported for retrieving // the result of a software by batch operation. const keyExpireTime = 24 * time.Hour @@ -1199,6 +1212,47 @@ func (svc *Service) BatchSetSoftwareInstallers( return requestUUID, nil } +func (svc *Service) validateSoftwareInstallerLabels(ctx context.Context, labelNames []string) ([]*fleet.SoftwareScopeLabel, error) { + labelMap, err := svc.batchValidateSoftwareInstallerLabels(ctx, labelNames) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "validating software installer labels") + } + + var swLabels []*fleet.SoftwareScopeLabel + for _, label := range labelMap { + swLabels = append(swLabels, label) + } + return swLabels, nil +} + +func (svc *Service) batchValidateSoftwareInstallerLabels(ctx context.Context, labelNames []string) (map[string]*fleet.SoftwareScopeLabel, error) { + if len(labelNames) == 0 { + return nil, nil + } + + labelNames = server.RemoveDuplicatesFromSlice(labelNames) + labels, err := svc.ds.LabelIDsByName(ctx, labelNames) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "getting label IDs by name") + } + + if len(labels) != len(labelNames) { + return nil, &fleet.BadRequestError{ + Message: "some or all the labels provided don't exist", + InternalErr: fmt.Errorf("names provided: %v", labelNames), + } + } + + swLabels := make(map[string]*fleet.SoftwareScopeLabel) + for labelName, labelID := range labels { + swLabels[labelName] = &fleet.SoftwareScopeLabel{ + LabelName: labelName, + LabelID: labelID, + } + } + return swLabels, nil +} + const ( batchSetProcessing = "processing" batchSetCompleted = "completed" @@ -1209,7 +1263,7 @@ func (svc *Service) softwareBatchUpload( requestUUID string, teamID *uint, userID uint, - payloads []fleet.SoftwareInstallerPayload, + payloads []*fleet.SoftwareInstallerPayload, dryRun bool, ) { var batchErr error @@ -1311,18 +1365,20 @@ func (svc *Service) softwareBatchUpload( // readers will have their Close deferred after the join/wait of // goroutines. installer := &fleet.UploadSoftwareInstallerPayload{ - TeamID: teamID, - InstallScript: p.InstallScript, - PreInstallQuery: p.PreInstallQuery, - PostInstallScript: p.PostInstallScript, - UninstallScript: p.UninstallScript, - InstallerFile: tfr, - SelfService: p.SelfService, - UserID: userID, - URL: p.URL, - InstallDuringSetup: p.InstallDuringSetup, - LabelsIncludeAny: p.LabelsIncludeAny, - LabelsExcludeAny: p.LabelsExcludeAny, + TeamID: teamID, + InstallScript: p.InstallScript, + PreInstallQuery: p.PreInstallQuery, + PostInstallScript: p.PostInstallScript, + UninstallScript: p.UninstallScript, + InstallerFile: tfr, + SelfService: p.SelfService, + UserID: userID, + URL: p.URL, + InstallDuringSetup: p.InstallDuringSetup, + LabelsIncludeAny: p.LabelsIncludeAny, + LabelsExcludeAny: p.LabelsExcludeAny, + ResolvedLabelsIncludeAny: p.ResolvedLabelsIncludeAny, + ResolvedLabelsExcludeAny: p.ResolvedLabelsExcludeAny, } // set the filename before adding metadata, as it is used as fallback diff --git a/server/datastore/mysql/software_installers.go b/server/datastore/mysql/software_installers.go index 90d0b732af6c..73f26b47a96a 100644 --- a/server/datastore/mysql/software_installers.go +++ b/server/datastore/mysql/software_installers.go @@ -832,6 +832,8 @@ func (ds *Datastore) CleanupUnusedSoftwareInstallers(ctx context.Context, softwa } func (ds *Datastore) BatchSetSoftwareInstallers(ctx context.Context, tmID *uint, installers []*fleet.UploadSoftwareInstallerPayload) error { + // TODO(mna): handle the include/exclude labels... + const upsertSoftwareTitles = ` INSERT INTO software_titles (name, source, browser) diff --git a/server/fleet/scripts.go b/server/fleet/scripts.go index 16295b886217..96286b0eb145 100644 --- a/server/fleet/scripts.go +++ b/server/fleet/scripts.go @@ -391,6 +391,11 @@ type SoftwareInstallerPayload struct { InstallDuringSetup *bool `json:"install_during_setup"` // if nil, do not change saved value, otherwise set it LabelsIncludeAny []string `json:"labels_include_any"` LabelsExcludeAny []string `json:"labels_exclude_any"` + + // Those resolved fields are only filled once the string-version of the + // labels have been validated in the batch-set endpoint and known to exist. + ResolvedLabelsIncludeAny []*SoftwareScopeLabel `json:"-"` + ResolvedLabelsExcludeAny []*SoftwareScopeLabel `json:"-"` } type HostLockWipeStatus struct { diff --git a/server/fleet/service.go b/server/fleet/service.go index d8f9db8e9881..7add8e10e6b6 100644 --- a/server/fleet/service.go +++ b/server/fleet/service.go @@ -649,7 +649,7 @@ type Service interface { // BatchSetSoftwareInstallers asynchronously replaces the software installers for a specified team. // Returns a request UUID that can be used to track an ongoing batch request (with GetBatchSetSoftwareInstallersResult). - BatchSetSoftwareInstallers(ctx context.Context, tmName string, payloads []SoftwareInstallerPayload, dryRun bool) (string, error) + BatchSetSoftwareInstallers(ctx context.Context, tmName string, payloads []*SoftwareInstallerPayload, dryRun bool) (string, error) // GetBatchSetSoftwareInstallersResult polls for the status of a batch-apply started by BatchSetSoftwareInstallers. // Return values: // - 'status': status of the batch-apply which can be "processing", "completed" or "failed". diff --git a/server/fleet/software_installer.go b/server/fleet/software_installer.go index 1b71784c8aef..f23af1d6c45c 100644 --- a/server/fleet/software_installer.go +++ b/server/fleet/software_installer.go @@ -338,6 +338,11 @@ type UploadSoftwareInstallerPayload struct { InstallDuringSetup *bool // keep saved value if nil, otherwise set as indicated LabelsIncludeAny []string // names of "include any" labels LabelsExcludeAny []string // names of "exclude any" labels + + // Those resolved fields are only filled once the string-version of the + // labels have been validated in the batch-set endpoint and known to exist. + ResolvedLabelsIncludeAny []*SoftwareScopeLabel `json:"-"` + ResolvedLabelsExcludeAny []*SoftwareScopeLabel `json:"-"` } type UpdateSoftwareInstallerPayload struct { diff --git a/server/service/software_installers.go b/server/service/software_installers.go index ba6bf84bb213..80ead7f1d8e9 100644 --- a/server/service/software_installers.go +++ b/server/service/software_installers.go @@ -549,9 +549,9 @@ func (svc *Service) GetSoftwareInstallResults(ctx context.Context, resultUUID st //////////////////////////////////////////////////////////////////////////////// type batchSetSoftwareInstallersRequest struct { - TeamName string `json:"-" query:"team_name,optional"` - DryRun bool `json:"-" query:"dry_run,optional"` // if true, apply validation but do not save changes - Software []fleet.SoftwareInstallerPayload `json:"software"` + TeamName string `json:"-" query:"team_name,optional"` + DryRun bool `json:"-" query:"dry_run,optional"` // if true, apply validation but do not save changes + Software []*fleet.SoftwareInstallerPayload `json:"software"` } type batchSetSoftwareInstallersResponse struct { @@ -571,7 +571,7 @@ func batchSetSoftwareInstallersEndpoint(ctx context.Context, request interface{} return batchSetSoftwareInstallersResponse{RequestUUID: requestUUID}, nil } -func (svc *Service) BatchSetSoftwareInstallers(ctx context.Context, tmName string, payloads []fleet.SoftwareInstallerPayload, dryRun bool) (string, error) { +func (svc *Service) BatchSetSoftwareInstallers(ctx context.Context, tmName string, payloads []*fleet.SoftwareInstallerPayload, dryRun bool) (string, error) { // skipauth: No authorization check needed due to implementation returning // only license error. svc.authz.SkipAuthorization(ctx) From 0e535e39fa365d4795314c1ea0e0de364536790b Mon Sep 17 00:00:00 2001 From: Martin Angers Date: Wed, 11 Dec 2024 15:35:05 -0500 Subject: [PATCH 5/5] Fix test --- server/service/integration_enterprise_test.go | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index b6362351039e..6f9cc093ba17 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -11156,13 +11156,13 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallers() { require.NoError(t, err) // software with a bad URL - softwareToInstall := []fleet.SoftwareInstallerPayload{ + softwareToInstall := []*fleet.SoftwareInstallerPayload{ {URL: "."}, } s.Do("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: softwareToInstall}, http.StatusUnprocessableEntity, "team_name", tm.Name) // software with a too big URL - softwareToInstall = []fleet.SoftwareInstallerPayload{ + softwareToInstall = []*fleet.SoftwareInstallerPayload{ {URL: "https://ftp.mozilla.org/" + strings.Repeat("a", 233)}, } s.Do("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: softwareToInstall}, http.StatusUnprocessableEntity, "team_name", tm.Name) @@ -11185,7 +11185,7 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallers() { t.Cleanup(srv.Close) // do a request with a URL that returns a 404. - softwareToInstall = []fleet.SoftwareInstallerPayload{ + softwareToInstall = []*fleet.SoftwareInstallerPayload{ {URL: srv.URL + "/not_found.pkg"}, } var batchResponse batchSetSoftwareInstallersResponse @@ -11196,7 +11196,7 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallers() { // do a request with a valid URL rubyURL := srv.URL + "/ruby.deb" - softwareToInstall = []fleet.SoftwareInstallerPayload{ + softwareToInstall = []*fleet.SoftwareInstallerPayload{ {URL: rubyURL}, } s.DoJSON("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: softwareToInstall}, http.StatusAccepted, &batchResponse, "team_name", tm.Name) @@ -11258,7 +11258,7 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallers() { require.Equal(t, titlesResp, newTitlesResp) // empty payload cleans the software items - softwareToInstall = []fleet.SoftwareInstallerPayload{} + softwareToInstall = []*fleet.SoftwareInstallerPayload{} s.DoJSON("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: softwareToInstall}, http.StatusAccepted, &batchResponse, "team_name", tm.Name) packages = waitBatchSetSoftwareInstallersCompleted(t, s, tm.Name, batchResponse.RequestUUID) require.Empty(t, packages) @@ -11271,7 +11271,7 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallers() { ////////////////////////// // Do a request with a valid URL with no team ////////////////////////// - softwareToInstall = []fleet.SoftwareInstallerPayload{ + softwareToInstall = []*fleet.SoftwareInstallerPayload{ {URL: rubyURL}, } s.DoJSON("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: softwareToInstall}, http.StatusAccepted, &batchResponse) @@ -11312,7 +11312,7 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallers() { require.Equal(t, titlesResp, newTitlesResp) // empty payload cleans the software items - softwareToInstall = []fleet.SoftwareInstallerPayload{} + softwareToInstall = []*fleet.SoftwareInstallerPayload{} s.DoJSON("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: softwareToInstall}, http.StatusAccepted, &batchResponse) packages = waitBatchSetSoftwareInstallersCompleted(t, s, "", batchResponse.RequestUUID) require.Empty(t, packages) @@ -11384,7 +11384,7 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallersSideEffec t.Cleanup(srv.Close) // set up software to install - softwareToInstall := []fleet.SoftwareInstallerPayload{ + softwareToInstall := []*fleet.SoftwareInstallerPayload{ {URL: srv.URL}, } var batchResponse batchSetSoftwareInstallersResponse @@ -11472,7 +11472,7 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallersSideEffec require.Equal(t, fleet.SoftwareInstallPending, *getHostSoftwareResp.Software[0].Status) // update pre-install query - withUpdatedPreinstallQuery := []fleet.SoftwareInstallerPayload{ + withUpdatedPreinstallQuery := []*fleet.SoftwareInstallerPayload{ {URL: srv.URL, PreInstallQuery: "SELECT * FROM os_version"}, } s.DoJSON("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: withUpdatedPreinstallQuery}, http.StatusAccepted, &batchResponse, "team_name", tm.Name) @@ -11517,7 +11517,7 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallersSideEffec require.Equal(t, fleet.SoftwareInstalled, *hostResp.Software[0].Status) // update install script - withUpdatedInstallScript := []fleet.SoftwareInstallerPayload{ + withUpdatedInstallScript := []*fleet.SoftwareInstallerPayload{ {URL: srv.URL, InstallScript: "apt install ruby"}, } s.DoJSON("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: withUpdatedInstallScript}, http.StatusAccepted, &batchResponse, "team_name", tm.Name) @@ -11585,7 +11585,7 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallersSideEffec require.Equal(t, fleet.SoftwareUninstallPending, *afterPreinstallHostResp.Software[0].Status) // delete all installers - s.DoJSON("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: []fleet.SoftwareInstallerPayload{}}, http.StatusAccepted, &batchResponse, "team_name", tm.Name) + s.DoJSON("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: []*fleet.SoftwareInstallerPayload{}}, http.StatusAccepted, &batchResponse, "team_name", tm.Name) packages = waitBatchSetSoftwareInstallersCompleted(t, s, tm.Name, batchResponse.RequestUUID) require.Len(t, packages, 0) @@ -11645,7 +11645,7 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallersWithPolic t.Cleanup(srv.Close) // team1 has ruby.deb - softwareToInstall := []fleet.SoftwareInstallerPayload{ + softwareToInstall := []*fleet.SoftwareInstallerPayload{ { URL: srv.URL + "/ruby.deb", }, @@ -11660,7 +11660,7 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallersWithPolic require.Equal(t, srv.URL+"/ruby.deb", packages[0].URL) // team2 has dummy_installer.pkg and ruby.deb. - softwareToInstall = []fleet.SoftwareInstallerPayload{ + softwareToInstall = []*fleet.SoftwareInstallerPayload{ { URL: srv.URL + "/dummy_installer.pkg", }, @@ -11710,7 +11710,7 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallersWithPolic }, http.StatusOK, &mtplr) // Get rid of all installers in team1. - softwareToInstall = []fleet.SoftwareInstallerPayload{} + softwareToInstall = []*fleet.SoftwareInstallerPayload{} s.DoJSON("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: softwareToInstall}, http.StatusAccepted, &batchResponse, "team_name", team1.Name) packages = waitBatchSetSoftwareInstallersCompleted(t, s, team1.Name, batchResponse.RequestUUID) require.Len(t, packages, 0)