From 0a8a3966433fdbd0212137eb6c158e9e65592e45 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Thu, 10 Oct 2024 06:12:24 -0500 Subject: [PATCH] Ensure scripts set in no-team.yml can be used in run-script actions for No Team (#22809) For #22787 Also revises the spec check to explain that scripts have to be defined "controls" when used in policies for the same team, with an explicit call-out for no-team.yml since this fix doesn't support pulling scripts from the global file. This is because parsing and script-matching happens early enough that we can't throw an error in the part of the code where we bail when controls are defined in both no-team and default files. To minimize diff size, we're both "passing-by-ref" and returning the maps-by-team of scripts and installers, though the former would be sufficient on its own. # Checklist for submitter If some of the following don't apply, delete the relevant line. - N/A Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) - [x] Added/updated tests (sorta) - [x] Manual QA for all new/changed functionality --- cmd/fleetctl/apply.go | 6 +++++- cmd/fleetctl/gitops.go | 7 ++++++- cmd/fleetctl/preview.go | 4 +++- pkg/spec/gitops.go | 5 ++++- pkg/spec/gitops_test.go | 2 +- server/service/client.go | 17 ++++++++++------- 6 files changed, 29 insertions(+), 12 deletions(-) diff --git a/cmd/fleetctl/apply.go b/cmd/fleetctl/apply.go index b5ac4fc22f..4e29f15b18 100644 --- a/cmd/fleetctl/apply.go +++ b/cmd/fleetctl/apply.go @@ -90,7 +90,11 @@ func applyCommand() *cli.Command { opts.TeamForPolicies = policiesTeamName } baseDir := filepath.Dir(flFilename) - _, _, _, err = fleetClient.ApplyGroup(c.Context, specs, baseDir, logf, nil, opts) + + teamsSoftwareInstallers := make(map[string][]fleet.SoftwarePackageResponse) + teamsScripts := make(map[string][]fleet.ScriptResponse) + + _, _, _, err = fleetClient.ApplyGroup(c.Context, specs, baseDir, logf, nil, opts, teamsSoftwareInstallers, teamsScripts) if err != nil { return err } diff --git a/cmd/fleetctl/gitops.go b/cmd/fleetctl/gitops.go index a7db6be73d..1c64e3f607 100644 --- a/cmd/fleetctl/gitops.go +++ b/cmd/fleetctl/gitops.go @@ -109,6 +109,11 @@ func gitopsCommand() *cli.Command { if totalFilenames > 1 { firstFileMustBeGlobal = ptr.Bool(true) } + + // we keep track of team software installers and scripts for correct policy application + teamsSoftwareInstallers := make(map[string][]fleet.SoftwarePackageResponse) + teamsScripts := make(map[string][]fleet.ScriptResponse) + // We keep track of the secrets to check if duplicates exist during dry run secrets := make(map[string]struct{}) for _, flFilename := range flFilenames.Value() { @@ -207,7 +212,7 @@ func gitopsCommand() *cli.Command { } } - assumptions, err := fleetClient.DoGitOps(c.Context, config, flFilename, logf, flDryRun, teamDryRunAssumptions, appConfig) + assumptions, err := fleetClient.DoGitOps(c.Context, config, flFilename, logf, flDryRun, teamDryRunAssumptions, appConfig, teamsSoftwareInstallers, teamsScripts) if err != nil { return err } diff --git a/cmd/fleetctl/preview.go b/cmd/fleetctl/preview.go index 820e9dcc1d..cddf401309 100644 --- a/cmd/fleetctl/preview.go +++ b/cmd/fleetctl/preview.go @@ -387,7 +387,9 @@ Use the stop and reset subcommands to manage the server and dependencies once st } // this only applies standard queries, the base directory is not used, // so pass in the current working directory. - _, _, _, err = client.ApplyGroup(c.Context, specs, ".", logf, nil, fleet.ApplyClientSpecOptions{}) + teamsSoftwareInstallers := make(map[string][]fleet.SoftwarePackageResponse) + teamsScripts := make(map[string][]fleet.ScriptResponse) + _, _, _, err = client.ApplyGroup(c.Context, specs, ".", logf, nil, fleet.ApplyClientSpecOptions{}, teamsSoftwareInstallers, teamsScripts) if err != nil { return err } diff --git a/pkg/spec/gitops.go b/pkg/spec/gitops.go index e124f7efa7..2706271e4b 100644 --- a/pkg/spec/gitops.go +++ b/pkg/spec/gitops.go @@ -596,7 +596,10 @@ func parsePolicyRunScript(baseDir string, teamName *string, policy *Policy, scri } } if !scriptOnTeamFound { - return fmt.Errorf("policy script not found on team: %v vs. %v", foundScriptPaths, scriptPath) + if *teamName == noTeam { + return fmt.Errorf("policy script %s was not defined in controls in no-team.yml", scriptPath) + } + return fmt.Errorf("policy script %s was not defined in controls for %s", scriptPath, *teamName) } scriptName := filepath.Base(policy.RunScript.Path) diff --git a/pkg/spec/gitops_test.go b/pkg/spec/gitops_test.go index 067e6c0bf5..8ca334ce36 100644 --- a/pkg/spec/gitops_test.go +++ b/pkg/spec/gitops_test.go @@ -1016,7 +1016,7 @@ controls: } _, err = GitOpsFromFile(path, basePath, &appConfig, nopLogf) assert.ErrorContains(t, err, - "policy script not found on team", + "was not defined in controls for TeamName", ) } diff --git a/server/service/client.go b/server/service/client.go index 0d568ba270..4eb260818d 100644 --- a/server/service/client.go +++ b/server/service/client.go @@ -397,9 +397,9 @@ func (c *Client) ApplyGroup( logf func(format string, args ...interface{}), appconfig *fleet.EnrichedAppConfig, opts fleet.ApplyClientSpecOptions, + teamsSoftwareInstallers map[string][]fleet.SoftwarePackageResponse, + teamsScripts map[string][]fleet.ScriptResponse, ) (map[string]uint, map[string][]fleet.SoftwarePackageResponse, map[string][]fleet.ScriptResponse, error) { - teamSoftwareInstallers := make(map[string][]fleet.SoftwarePackageResponse) - teamScripts := make(map[string][]fleet.ScriptResponse) logfn := func(format string, args ...interface{}) { if logf != nil { @@ -513,7 +513,7 @@ func (c *Client) ApplyGroup( if err != nil { return nil, nil, nil, fmt.Errorf("applying no-team scripts: %w", err) } - teamScripts["No team"] = noTeamScripts + teamsScripts["No team"] = noTeamScripts } if err := c.ApplyAppConfig(specs.AppConfig, opts.ApplySpecOptions); err != nil { return nil, nil, nil, fmt.Errorf("applying fleet config: %w", err) @@ -683,7 +683,7 @@ func (c *Client) ApplyGroup( if err != nil { return nil, nil, nil, fmt.Errorf("applying scripts for team %q: %w", tmName, err) } - teamScripts[tmName] = scriptResponses + teamsScripts[tmName] = scriptResponses } } if len(tmSoftwarePackagesPayloads) > 0 { @@ -695,7 +695,7 @@ func (c *Client) ApplyGroup( if err != nil { return nil, nil, nil, fmt.Errorf("applying software installers for team %q: %w", tmName, err) } - teamSoftwareInstallers[tmName] = installers + teamsSoftwareInstallers[tmName] = installers } } if len(tmSoftwareAppsPayloads) > 0 { @@ -749,7 +749,7 @@ func (c *Client) ApplyGroup( } } - return teamIDsByName, teamSoftwareInstallers, teamScripts, nil + return teamIDsByName, teamsSoftwareInstallers, teamsScripts, nil } func buildSoftwarePackagesPayload(baseDir string, specs []fleet.SoftwarePackageSpec) ([]fleet.SoftwareInstallerPayload, error) { @@ -1223,6 +1223,9 @@ func (c *Client) DoGitOps( dryRun bool, teamDryRunAssumptions *fleet.TeamSpecsDryRunAssumptions, appConfig *fleet.EnrichedAppConfig, + // pass-by-ref to build lists + teamsSoftwareInstallers map[string][]fleet.SoftwarePackageResponse, + teamsScripts map[string][]fleet.ScriptResponse, ) (*fleet.TeamSpecsDryRunAssumptions, error) { baseDir := filepath.Dir(fullFilename) filename := filepath.Base(fullFilename) @@ -1468,7 +1471,7 @@ func (c *Client) DoGitOps( DryRun: dryRun, }, ExpandEnvConfigProfiles: true, - }) + }, teamsSoftwareInstallers, teamsScripts) if err != nil { return nil, err }