From 55423f67e2c39a3d4dde96958cd79719768df52f Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky Date: Thu, 6 Feb 2025 16:39:15 -0600 Subject: [PATCH] Fixed parsing of relative paths for MDM profiles in gitops no-team.yml (#26046) For #25770 We already unmarshal macOS/Windows settings (added by Martin), so we replace the path with an absolute file path and keep them unmarshalled so they don't have to be re-unmarshalled later. Note: the custom UnmarshalJSON method on these structs checks for (and handles) legacy format (before labels were added). Also some refactorings: - extracted `extractControlsForNoTeam` - reorganized `TestGitOpsBasicGlobalAndNoTeam` with subtests -- I did not actually change functionality of this test # Checklist for submitter - [x] 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] Added/updated automated tests - [x] A detailed QA plan exists on the associated ticket (if it isn't there, work with the product group's QA engineer to add it) - [x] Manual QA for all new/changed functionality --- .../25770-relative-profile-path-in-no-team | 1 + cmd/fleetctl/gitops.go | 53 ++- cmd/fleetctl/gitops_test.go | 433 +++++++++++------- pkg/spec/gitops.go | 51 ++- pkg/spec/gitops_test.go | 4 +- server/fleet/app.go | 18 + server/service/client.go | 22 +- server/service/client_test.go | 23 +- 8 files changed, 394 insertions(+), 211 deletions(-) create mode 100644 changes/25770-relative-profile-path-in-no-team diff --git a/changes/25770-relative-profile-path-in-no-team b/changes/25770-relative-profile-path-in-no-team new file mode 100644 index 0000000000..8a5043f609 --- /dev/null +++ b/changes/25770-relative-profile-path-in-no-team @@ -0,0 +1 @@ +Fixed parsing of relative paths for MDM profiles in gitops no-team.yml diff --git a/cmd/fleetctl/gitops.go b/cmd/fleetctl/gitops.go index 0ac25d3e4c..5d11af95e4 100644 --- a/cmd/fleetctl/gitops.go +++ b/cmd/fleetctl/gitops.go @@ -81,27 +81,10 @@ func gitopsCommand() *cli.Command { _, _ = fmt.Fprintf(c.App.Writer, format, a...) } - // We need to extract the controls from no-team.yml to be able to apply them when applying the global app config. - var ( - noTeamControls spec.Controls - noTeamPresent bool - ) - isPremium := appConfig.License.IsPremium() - for _, flFilename := range flFilenames.Value() { - if filepath.Base(flFilename) == "no-team.yml" { - if !isPremium { - // Message is printed in the next flFilenames loop to avoid printing it multiple times - break - } - baseDir := filepath.Dir(flFilename) - config, err := spec.GitOpsFromFile(flFilename, baseDir, appConfig, func(format string, a ...interface{}) {}) - if err != nil { - return err - } - noTeamControls = config.Controls - noTeamPresent = true - break - } + // We need the controls from no-team.yml to apply them when applying the global app config. + noTeamControls, noTeamPresent, err := extractControlsForNoTeam(flFilenames, appConfig) + if err != nil { + return fmt.Errorf("extracting controls from no-team.yml: %w", err) } var originalABMConfig []any @@ -156,7 +139,7 @@ func gitopsCommand() *cli.Command { if !config.Controls.Set() { config.Controls = noTeamControls } - } else if !isPremium { + } else if !appConfig.License.IsPremium() { logf("[!] skipping team config %s since teams are only supported for premium Fleet users\n", flFilename) continue } @@ -166,7 +149,7 @@ func gitopsCommand() *cli.Command { // grab some information to help us determine allowed/restricted actions and // when to perform the associations. - if isGlobalConfig && totalFilenames > 1 && !(totalFilenames == 2 && noTeamPresent) && isPremium { + if isGlobalConfig && totalFilenames > 1 && !(totalFilenames == 2 && noTeamPresent) && appConfig.License.IsPremium() { abmTeams, hasMissingABMTeam, usesLegacyABMConfig, err = checkABMTeamAssignments(config, fleetClient) if err != nil { return err @@ -228,7 +211,8 @@ func gitopsCommand() *cli.Command { if err != nil { return err } - assumptions, err := fleetClient.DoGitOps(c.Context, config, flFilename, logf, flDryRun, teamDryRunAssumptions, appConfig, teamsSoftwareInstallers, teamsVPPApps, teamsScripts) + assumptions, err := fleetClient.DoGitOps(c.Context, config, flFilename, logf, flDryRun, teamDryRunAssumptions, appConfig, + teamsSoftwareInstallers, teamsVPPApps, teamsScripts) if err != nil { return err } @@ -241,7 +225,8 @@ func gitopsCommand() *cli.Command { // if there were assignments to tokens, and some of the teams were missing at that time, submit a separate patch request to set them now. if len(abmTeams) > 0 && hasMissingABMTeam { - if err = applyABMTokenAssignmentIfNeeded(c, teamNames, abmTeams, originalABMConfig, usesLegacyABMConfig, flDryRun, fleetClient); err != nil { + if err = applyABMTokenAssignmentIfNeeded(c, teamNames, abmTeams, originalABMConfig, usesLegacyABMConfig, flDryRun, + fleetClient); err != nil { return err } } @@ -288,6 +273,24 @@ func gitopsCommand() *cli.Command { } } +func extractControlsForNoTeam(flFilenames cli.StringSlice, appConfig *fleet.EnrichedAppConfig) (spec.Controls, bool, error) { + for _, flFilename := range flFilenames.Value() { + if filepath.Base(flFilename) == "no-team.yml" { + if !appConfig.License.IsPremium() { + // Message is printed in the next flFilenames loop to avoid printing it multiple times + break + } + baseDir := filepath.Dir(flFilename) + config, err := spec.GitOpsFromFile(flFilename, baseDir, appConfig, func(format string, a ...interface{}) {}) + if err != nil { + return spec.Controls{}, false, err + } + return config.Controls, true, nil + } + } + return spec.Controls{}, false, nil +} + // checkABMTeamAssignments validates the spec, and finds if: // // 1. The user is using the legacy apple_bm_default_team config. diff --git a/cmd/fleetctl/gitops_test.go b/cmd/fleetctl/gitops_test.go index d10e06bad7..9bb4887623 100644 --- a/cmd/fleetctl/gitops_test.go +++ b/cmd/fleetctl/gitops_test.go @@ -1569,9 +1569,208 @@ func TestGitOpsBasicGlobalAndNoTeam(t *testing.T) { return nil } - globalFileBasic, err := os.CreateTemp(t.TempDir(), "*.yml") + globalFileBasic := createGlobalFileBasic(t, fleetServerURL, orgName) + + teamFileBasic := createTeamFileBasic(t, secret) + + // We cannot use os.CreateTemp because the filename must be exactly "no-team.yml" + noTeamFilePath := filepath.Join(t.TempDir(), "no-team.yml") + noTeamFileBasic, err := os.Create(noTeamFilePath) + require.NoError(t, err) + _, err = noTeamFileBasic.WriteString(` +controls: +policies: +name: No team +software: +`) require.NoError(t, err) + t.Run("global defines software -- should fail", func(t *testing.T) { + globalFileWithSoftware, err := os.CreateTemp(t.TempDir(), "*.yml") + require.NoError(t, err) + _, err = globalFileWithSoftware.WriteString(fmt.Sprintf( + ` +controls: +queries: +policies: +agent_options: +org_settings: + server_settings: + server_url: %s + org_info: + contact_url: https://example.com/contact + org_logo_url: "" + org_logo_url_light_background: "" + org_name: %s + secrets: [{"secret":"globalSecret"}] +software: + packages: + - url: https://example.com +`, fleetServerURL, orgName), + ) + require.NoError(t, err) + + // Dry run, global defines software, should fail. + _, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithSoftware.Name(), "-f", teamFileBasic.Name(), "-f", + noTeamFileBasic.Name(), + "--dry-run"}) + require.Error(t, err) + assert.ErrorContains(t, err, "'software' cannot be set on global file") + // Real run, global defines software, should fail. + _, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithSoftware.Name(), "-f", teamFileBasic.Name(), "-f", + noTeamFileBasic.Name()}) + require.Error(t, err) + assert.ErrorContains(t, err, "'software' cannot be set on global file") + }) + + t.Run("both global and no-team.yml define controls -- should fail", func(t *testing.T) { + globalFileWithControls := createGlobalFileWithControls(t, fleetServerURL, orgName) + + noTeamFilePathWithControls := filepath.Join(t.TempDir(), "no-team.yml") + noTeamFileWithControls, err := os.Create(noTeamFilePathWithControls) + require.NoError(t, err) + _, err = noTeamFileWithControls.WriteString(` +controls: + ipados_updates: + deadline: "2023-03-03" + minimum_version: "18.0" +policies: +name: No team +software: +`) + require.NoError(t, err) + + // Dry run, both global and no-team.yml define controls. + _, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithControls.Name(), "-f", teamFileBasic.Name(), "-f", + noTeamFileWithControls.Name(), "--dry-run"}) + require.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "'controls' cannot be set on both global config and on no-team.yml")) + // Real run, both global and no-team.yml define controls. + _, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithControls.Name(), "-f", teamFileBasic.Name(), "-f", + noTeamFileWithControls.Name()}) + require.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "'controls' cannot be set on both global config and on no-team.yml")) + }) + + t.Run("no-team.yml defines policy with calendar events enabled -- should fail", func(t *testing.T) { + globalFileWithControls := createGlobalFileWithControls(t, fleetServerURL, orgName) + + noTeamFilePathPoliciesCalendarPath := filepath.Join(t.TempDir(), "no-team.yml") + noTeamFilePathPoliciesCalendar, err := os.Create(noTeamFilePathPoliciesCalendarPath) + require.NoError(t, err) + _, err = noTeamFilePathPoliciesCalendar.WriteString(` +controls: +policies: + - name: Foobar + query: SELECT 1 FROM osquery_info WHERE start_time < 0; + calendar_events_enabled: true +name: No team +software: +`) + require.NoError(t, err) + + // Dry run, both global and no-team.yml defines policy with calendar events enabled. + _, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithControls.Name(), "-f", teamFileBasic.Name(), "-f", + noTeamFilePathPoliciesCalendar.Name(), "--dry-run"}) + require.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "calendar events are not supported on \"No team\" policies: \"Foobar\""), err.Error()) + // Real run, both global and no-team.yml define controls. + _, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithControls.Name(), "-f", teamFileBasic.Name(), "-f", + noTeamFilePathPoliciesCalendar.Name()}) + require.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "calendar events are not supported on \"No team\" policies: \"Foobar\""), err.Error()) + }) + + t.Run("global and no-team.yml DO NOT define controls -- should fail", func(t *testing.T) { + globalFileWithoutControlsAndSoftwareKeys := createGlobalFileWithoutControlsAndSoftwareKeys(t, fleetServerURL, orgName) + + noTeamFilePathWithoutControls := filepath.Join(t.TempDir(), "no-team.yml") + noTeamFileWithoutControls, err := os.Create(noTeamFilePathWithoutControls) + require.NoError(t, err) + _, err = noTeamFileWithoutControls.WriteString(` +policies: +name: No team +software: +`) + require.NoError(t, err) + + // Dry run, controls should be defined somewhere, either in no-team.yml or global. + _, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithoutControlsAndSoftwareKeys.Name(), "-f", teamFileBasic.Name(), "-f", + noTeamFileWithoutControls.Name(), "--dry-run"}) + require.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "'controls' must be set on global config or no-team.yml")) + // Real run + _, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithoutControlsAndSoftwareKeys.Name(), "-f", teamFileBasic.Name(), "-f", + noTeamFileWithoutControls.Name()}) + require.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "'controls' must be set on global config or no-team.yml")) + }) + + t.Run("controls only defined in no-team.yml", func(t *testing.T) { + savedAppConfig = &fleet.AppConfig{} + + globalFileWithoutControlsAndSoftwareKeys := createGlobalFileWithoutControlsAndSoftwareKeys(t, fleetServerURL, orgName) + + // Dry run, global file without controls and software keys. + _ = runAppForTest(t, + []string{"gitops", "-f", globalFileWithoutControlsAndSoftwareKeys.Name(), "-f", teamFileBasic.Name(), "-f", + noTeamFileBasic.Name(), + "--dry-run"}) + assert.Equal(t, fleet.AppConfig{}, *savedAppConfig, "AppConfig should be empty") + + // Real run, global file without controls and software keys. + _ = runAppForTest(t, + []string{"gitops", "-f", globalFileWithoutControlsAndSoftwareKeys.Name(), "-f", teamFileBasic.Name(), "-f", + noTeamFileBasic.Name()}) + assert.Equal(t, orgName, savedAppConfig.OrgInfo.OrgName) + assert.Equal(t, fleetServerURL, savedAppConfig.ServerSettings.ServerURL) + assert.Len(t, enrolledSecrets, 1) + require.NotNil(t, savedTeam) + assert.Equal(t, teamName, savedTeam.Name) + require.Len(t, enrolledTeamSecrets, 1) + assert.Equal(t, secret, enrolledTeamSecrets[0].Secret) + + }) + + t.Run("basic global and no-team.yml", func(t *testing.T) { + savedAppConfig = &fleet.AppConfig{} + // Dry run + _ = runAppForTest(t, + []string{"gitops", "-f", globalFileBasic.Name(), "-f", teamFileBasic.Name(), "-f", noTeamFileBasic.Name(), "--dry-run"}) + assert.Equal(t, fleet.AppConfig{}, *savedAppConfig, "AppConfig should be empty") + // Real run + _ = runAppForTest(t, []string{"gitops", "-f", globalFileBasic.Name(), "-f", teamFileBasic.Name(), "-f", noTeamFileBasic.Name()}) + assert.Equal(t, orgName, savedAppConfig.OrgInfo.OrgName) + assert.Equal(t, fleetServerURL, savedAppConfig.ServerSettings.ServerURL) + assert.Len(t, enrolledSecrets, 1) + require.NotNil(t, savedTeam) + assert.Equal(t, teamName, savedTeam.Name) + require.Len(t, enrolledTeamSecrets, 1) + assert.Equal(t, secret, enrolledTeamSecrets[0].Secret) + }) +} + +func createTeamFileBasic(t *testing.T, secret string) *os.File { + teamFileBasic, err := os.CreateTemp(t.TempDir(), "*.yml") + require.NoError(t, err) + _, err = teamFileBasic.WriteString(fmt.Sprintf(` +controls: +queries: +policies: +agent_options: +name: %s +team_settings: + secrets: [{"secret":"%s"}] +software: +`, teamName, secret), + ) + require.NoError(t, err) + return teamFileBasic +} + +func createGlobalFileBasic(t *testing.T, fleetServerURL string, orgName string) *os.File { + globalFileBasic, err := os.CreateTemp(t.TempDir(), "*.yml") + require.NoError(t, err) _, err = globalFileBasic.WriteString(fmt.Sprintf( ` controls: @@ -1591,12 +1790,14 @@ software: `, fleetServerURL, orgName), ) require.NoError(t, err) + return globalFileBasic +} - globalFileWithSoftware, err := os.CreateTemp(t.TempDir(), "*.yml") +func createGlobalFileWithoutControlsAndSoftwareKeys(t *testing.T, fleetServerURL string, orgName string) *os.File { + globalFileWithoutControlsAndSoftwareKeys, err := os.CreateTemp(t.TempDir(), "*.yml") require.NoError(t, err) - _, err = globalFileWithSoftware.WriteString(fmt.Sprintf( + _, err = globalFileWithoutControlsAndSoftwareKeys.WriteString(fmt.Sprintf( ` -controls: queries: policies: agent_options: @@ -1609,13 +1810,13 @@ org_settings: org_logo_url_light_background: "" org_name: %s secrets: [{"secret":"globalSecret"}] -software: - packages: - - url: https://example.com `, fleetServerURL, orgName), ) require.NoError(t, err) + return globalFileWithoutControlsAndSoftwareKeys +} +func createGlobalFileWithControls(t *testing.T, fleetServerURL string, orgName string) *os.File { globalFileWithControls, err := os.CreateTemp(t.TempDir(), "*.yml") require.NoError(t, err) _, err = globalFileWithControls.WriteString(fmt.Sprintf( @@ -1640,156 +1841,7 @@ software: `, fleetServerURL, orgName), ) require.NoError(t, err) - - globalFileWithoutControlsAndSoftwareKeys, err := os.CreateTemp(t.TempDir(), "*.yml") - require.NoError(t, err) - _, err = globalFileWithoutControlsAndSoftwareKeys.WriteString(fmt.Sprintf( - ` -queries: -policies: -agent_options: -org_settings: - server_settings: - server_url: %s - org_info: - contact_url: https://example.com/contact - org_logo_url: "" - org_logo_url_light_background: "" - org_name: %s - secrets: [{"secret":"globalSecret"}] -`, fleetServerURL, orgName), - ) - require.NoError(t, err) - - teamFile, err := os.CreateTemp(t.TempDir(), "*.yml") - require.NoError(t, err) - _, err = teamFile.WriteString(fmt.Sprintf(` -controls: -queries: -policies: -agent_options: -name: %s -team_settings: - secrets: [{"secret":"%s"}] -software: -`, teamName, secret), - ) - require.NoError(t, err) - - noTeamFilePath := filepath.Join(t.TempDir(), "no-team.yml") - noTeamFile, err := os.Create(noTeamFilePath) - require.NoError(t, err) - _, err = noTeamFile.WriteString(` -controls: -policies: -name: No team -software: -`) - require.NoError(t, err) - - noTeamFilePathPoliciesCalendarPath := filepath.Join(t.TempDir(), "no-team.yml") - noTeamFilePathPoliciesCalendar, err := os.Create(noTeamFilePathPoliciesCalendarPath) - require.NoError(t, err) - _, err = noTeamFilePathPoliciesCalendar.WriteString(` -controls: -policies: - - name: Foobar - query: SELECT 1 FROM osquery_info WHERE start_time < 0; - calendar_events_enabled: true -name: No team -software: -`) - require.NoError(t, err) - - noTeamFilePathWithControls := filepath.Join(t.TempDir(), "no-team.yml") - noTeamFileWithControls, err := os.Create(noTeamFilePathWithControls) - require.NoError(t, err) - _, err = noTeamFileWithControls.WriteString(` -controls: - ipados_updates: - deadline: "2023-03-03" - minimum_version: "18.0" -policies: -name: No team -software: -`) - require.NoError(t, err) - - noTeamFilePathWithoutControls := filepath.Join(t.TempDir(), "no-team.yml") - noTeamFileWithoutControls, err := os.Create(noTeamFilePathWithoutControls) - require.NoError(t, err) - _, err = noTeamFileWithoutControls.WriteString(` -policies: -name: No team -software: -`) - require.NoError(t, err) - - // Dry run, global defines software, should fail. - _, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithSoftware.Name(), "-f", teamFile.Name(), "-f", noTeamFile.Name(), "--dry-run"}) - require.Error(t, err) - assert.True(t, strings.Contains(err.Error(), "'software' cannot be set on global file")) - // Real run, global defines software, should fail. - _, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithSoftware.Name(), "-f", teamFile.Name(), "-f", noTeamFile.Name()}) - require.Error(t, err) - assert.True(t, strings.Contains(err.Error(), "'software' cannot be set on global file")) - - // Dry run, both global and no-team.yml define controls. - _, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithControls.Name(), "-f", teamFile.Name(), "-f", noTeamFileWithControls.Name(), "--dry-run"}) - require.Error(t, err) - assert.True(t, strings.Contains(err.Error(), "'controls' cannot be set on both global config and on no-team.yml")) - // Real run, both global and no-team.yml define controls. - _, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithControls.Name(), "-f", teamFile.Name(), "-f", noTeamFileWithControls.Name()}) - require.Error(t, err) - assert.True(t, strings.Contains(err.Error(), "'controls' cannot be set on both global config and on no-team.yml")) - - // Dry run, both global and no-team.yml defines policy with calendar events enabled. - _, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithControls.Name(), "-f", teamFile.Name(), "-f", noTeamFilePathPoliciesCalendar.Name(), "--dry-run"}) - require.Error(t, err) - assert.True(t, strings.Contains(err.Error(), "calendar events are not supported on \"No team\" policies: \"Foobar\""), err.Error()) - // Real run, both global and no-team.yml define controls. - _, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithControls.Name(), "-f", teamFile.Name(), "-f", noTeamFilePathPoliciesCalendar.Name()}) - require.Error(t, err) - assert.True(t, strings.Contains(err.Error(), "calendar events are not supported on \"No team\" policies: \"Foobar\""), err.Error()) - - // Dry run, controls should be defined somewhere, either in no-team.yml or global. - _, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithoutControlsAndSoftwareKeys.Name(), "-f", teamFile.Name(), "-f", noTeamFileWithoutControls.Name(), "--dry-run"}) - require.Error(t, err) - assert.True(t, strings.Contains(err.Error(), "'controls' must be set on global config or no-team.yml")) - // Real run, both global and no-team.yml define controls. - _, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithoutControlsAndSoftwareKeys.Name(), "-f", teamFile.Name(), "-f", noTeamFileWithoutControls.Name()}) - require.Error(t, err) - assert.True(t, strings.Contains(err.Error(), "'controls' must be set on global config or no-team.yml")) - - // Dry run, global file without controls and software keys. - _ = runAppForTest(t, []string{"gitops", "-f", globalFileWithoutControlsAndSoftwareKeys.Name(), "-f", teamFile.Name(), "-f", noTeamFile.Name(), "--dry-run"}) - assert.Equal(t, fleet.AppConfig{}, *savedAppConfig, "AppConfig should be empty") - - // Real run, global file without controls and software keys. - _ = runAppForTest(t, []string{"gitops", "-f", globalFileWithoutControlsAndSoftwareKeys.Name(), "-f", teamFile.Name(), "-f", noTeamFile.Name()}) - assert.Equal(t, orgName, savedAppConfig.OrgInfo.OrgName) - assert.Equal(t, fleetServerURL, savedAppConfig.ServerSettings.ServerURL) - assert.Len(t, enrolledSecrets, 1) - require.NotNil(t, savedTeam) - assert.Equal(t, teamName, savedTeam.Name) - require.Len(t, enrolledTeamSecrets, 1) - assert.Equal(t, secret, enrolledTeamSecrets[0].Secret) - - // Restore to test below. - savedAppConfig = &fleet.AppConfig{} - - // Dry run - _ = runAppForTest(t, []string{"gitops", "-f", globalFileBasic.Name(), "-f", teamFile.Name(), "-f", noTeamFile.Name(), "--dry-run"}) - assert.Equal(t, fleet.AppConfig{}, *savedAppConfig, "AppConfig should be empty") - // Real run - _ = runAppForTest(t, []string{"gitops", "-f", globalFileBasic.Name(), "-f", teamFile.Name(), "-f", noTeamFile.Name()}) - assert.Equal(t, orgName, savedAppConfig.OrgInfo.OrgName) - assert.Equal(t, fleetServerURL, savedAppConfig.ServerSettings.ServerURL) - assert.Len(t, enrolledSecrets, 1) - require.NotNil(t, savedTeam) - assert.Equal(t, teamName, savedTeam.Name) - require.Len(t, enrolledTeamSecrets, 1) - assert.Equal(t, secret, enrolledTeamSecrets[0].Secret) + return globalFileWithControls } func TestGitOpsFullGlobalAndTeam(t *testing.T) { @@ -1887,6 +1939,73 @@ func TestGitOpsFullGlobalAndTeam(t *testing.T) { require.NotNil(t, *savedTeams[teamName]) assert.Equal(t, teamName, (*savedTeams[teamName]).Name) require.Len(t, enrolledTeamSecrets, 2) + + t.Run("no-team.yml using relative paths", func(t *testing.T) { + globalFileBasic := createGlobalFileBasic(t, fleetServerURL, orgName) + teamFileBasic := createTeamFileBasic(t, teamName) + + noTeamDir := t.TempDir() + noTeamFile, err := os.Create(filepath.Join(noTeamDir, "no-team.yml")) + require.NoError(t, err) + _, err = noTeamFile.WriteString(` +controls: + scripts: + - path: ./script.sh + windows_enabled_and_configured: true + macos_settings: + custom_settings: + - path: ./config.json + windows_settings: + custom_settings: + - path: ./config2.xml +policies: +name: No team +software: +`) + require.NoError(t, err) + + ddmFile, err := os.Create(filepath.Join(noTeamDir, "config.json")) + require.NoError(t, err) + _, err = ddmFile.WriteString(` +{ + "Type": "com.apple.configuration.passcode.settings", + "Identifier": "com.fleetdm.config.passcode.settings", + "Payload": { + "RequireAlphanumericPasscode": true + } +} + `) + require.NoError(t, err) + + cspFile, err := os.Create(filepath.Join(noTeamDir, "config2.xml")) + require.NoError(t, err) + _, err = cspFile.WriteString(`bozo`) + require.NoError(t, err) + + scriptFile, err := os.Create(filepath.Join(noTeamDir, "script.sh")) + require.NoError(t, err) + _, err = scriptFile.WriteString(`echo "Hello, world!"`) + require.NoError(t, err) + + // Dry run + ds.SaveAppConfigFuncInvoked = false + ds.BatchSetScriptsFuncInvoked = false + _ = runAppForTest(t, + []string{"gitops", "-f", globalFileBasic.Name(), "-f", teamFileBasic.Name(), "-f", noTeamFile.Name(), "--dry-run"}) + assert.False(t, ds.SaveAppConfigFuncInvoked) + assert.False(t, ds.BatchSetScriptsFuncInvoked) + + // Real run + _ = runAppForTest(t, []string{"gitops", "-f", globalFileBasic.Name(), "-f", teamFileBasic.Name(), "-f", noTeamFile.Name()}) + assert.Equal(t, orgName, (*savedAppConfigPtr).OrgInfo.OrgName) + assert.Equal(t, fleetServerURL, (*savedAppConfigPtr).ServerSettings.ServerURL) + require.Len(t, (*savedAppConfigPtr).MDM.MacOSSettings.CustomSettings, 1) + assert.Equal(t, filepath.Base(ddmFile.Name()), filepath.Base((*savedAppConfigPtr).MDM.MacOSSettings.CustomSettings[0].Path)) + require.Len(t, (*savedAppConfigPtr).MDM.WindowsSettings.CustomSettings.Value, 1) + assert.Equal(t, filepath.Base(cspFile.Name()), filepath.Base((*savedAppConfigPtr).MDM.WindowsSettings.CustomSettings.Value[0].Path)) + assert.True(t, ds.BatchSetScriptsFuncInvoked) + }) + } func TestGitOpsTeamSofwareInstallers(t *testing.T) { diff --git a/pkg/spec/gitops.go b/pkg/spec/gitops.go index deda14007b..1b759313af 100644 --- a/pkg/spec/gitops.go +++ b/pkg/spec/gitops.go @@ -492,7 +492,6 @@ func parseControls(top map[string]json.RawMessage, result *GitOps, baseDir strin } // Find Fleet secrets in profiles - var profiles []fleet.MDMProfileSpec if result.Controls.MacOSSettings != nil { // We are marshalling/unmarshalling to get the data into the fleet.MacOSSettings struct. // This is inefficient, but it is more robust and less error-prone. @@ -505,7 +504,15 @@ func parseControls(top map[string]json.RawMessage, result *GitOps, baseDir strin if err != nil { return multierror.Append(multiError, fmt.Errorf("failed to process controls.macos_settings: %v", err)) } - profiles = append(profiles, macOSSettings.CustomSettings...) + + for i := range macOSSettings.CustomSettings { + err := resolveAndUpdateProfilePathToAbsolute(controlsDir, &macOSSettings.CustomSettings[i], result) + if err != nil { + return multierror.Append(multiError, err) + } + } + // Since we already unmarshalled and updated the path, we need to update the result struct. + result.Controls.MacOSSettings = macOSSettings } if result.Controls.WindowsSettings != nil { // We are marshalling/unmarshalling to get the data into the fleet.WindowsSettings struct. @@ -520,24 +527,40 @@ func parseControls(top map[string]json.RawMessage, result *GitOps, baseDir strin return multierror.Append(multiError, fmt.Errorf("failed to process controls.windows_settings: %v", err)) } if windowsSettings.CustomSettings.Valid { - profiles = append(profiles, windowsSettings.CustomSettings.Value...) - } - } - for _, profile := range profiles { - resolvedPath := resolveApplyRelativePath(controlsDir, profile.Path) - fileBytes, err := os.ReadFile(resolvedPath) - if err != nil { - return multierror.Append(multiError, fmt.Errorf("failed to read profile file %s: %v", resolvedPath, err)) - } - err = LookupEnvSecrets(string(fileBytes), result.FleetSecrets) - if err != nil { - return multierror.Append(multiError, err) + for i := range windowsSettings.CustomSettings.Value { + err := resolveAndUpdateProfilePathToAbsolute(controlsDir, &windowsSettings.CustomSettings.Value[i], result) + if err != nil { + return multierror.Append(multiError, err) + } + } } + // Since we already unmarshalled and updated the path, we need to update the result struct. + result.Controls.WindowsSettings = windowsSettings } return multiError } +func resolveAndUpdateProfilePathToAbsolute(controlsDir string, profile *fleet.MDMProfileSpec, result *GitOps) error { + resolvedPath := resolveApplyRelativePath(controlsDir, profile.Path) + // We switch to absolute path so that we don't have to keep track of the base directory. + // This is useful because controls section can come from either the global config file or the no-team file. + var err error + profile.Path, err = filepath.Abs(resolvedPath) + if err != nil { + return fmt.Errorf("failed to resolve profile path %s: %v", resolvedPath, err) + } + fileBytes, err := os.ReadFile(resolvedPath) + if err != nil { + return fmt.Errorf("failed to read profile file %s: %v", resolvedPath, err) + } + err = LookupEnvSecrets(string(fileBytes), result.FleetSecrets) + if err != nil { + return err + } + return nil +} + func resolveScriptPaths(input []BaseItem, baseDir string) ([]BaseItem, error) { var resolved []BaseItem for _, item := range input { diff --git a/pkg/spec/gitops_test.go b/pkg/spec/gitops_test.go index a835d52874..943d36f4a3 100644 --- a/pkg/spec/gitops_test.go +++ b/pkg/spec/gitops_test.go @@ -218,9 +218,9 @@ func TestValidGitOpsYaml(t *testing.T) { } // Check controls - _, ok := gitops.Controls.MacOSSettings.(map[string]interface{}) + _, ok := gitops.Controls.MacOSSettings.(fleet.MacOSSettings) assert.True(t, ok, "macos_settings not found") - _, ok = gitops.Controls.WindowsSettings.(map[string]interface{}) + _, ok = gitops.Controls.WindowsSettings.(fleet.WindowsSettings) assert.True(t, ok, "windows_settings not found") _, ok = gitops.Controls.EnableDiskEncryption.(bool) assert.True(t, ok, "enable_disk_encryption not found") diff --git a/server/fleet/app.go b/server/fleet/app.go index d2674deae9..e4d1f6e9f2 100644 --- a/server/fleet/app.go +++ b/server/fleet/app.go @@ -342,6 +342,10 @@ type MacOSSettings struct { // NOTE: make sure to update the ToMap/FromMap methods when adding/updating fields. } +func (s MacOSSettings) GetMDMProfileSpecs() []MDMProfileSpec { + return s.CustomSettings +} + func (s MacOSSettings) ToMap() map[string]interface{} { return map[string]interface{}{ "custom_settings": s.CustomSettings, @@ -349,6 +353,13 @@ func (s MacOSSettings) ToMap() map[string]interface{} { } } +type WithMDMProfileSpecs interface { + GetMDMProfileSpecs() []MDMProfileSpec +} + +// Compile-time interface check +var _ WithMDMProfileSpecs = MacOSSettings{} + // FromMap sets the macOS settings from the provided map, which is the map type // from the ApplyTeams spec struct. It returns a map of fields that were set in // the map (ie. the key was present even if empty) or an error. If the @@ -1391,6 +1402,13 @@ type WindowsSettings struct { CustomSettings optjson.Slice[MDMProfileSpec] `json:"custom_settings"` } +func (ws WindowsSettings) GetMDMProfileSpecs() []MDMProfileSpec { + return ws.CustomSettings.Value +} + +// Compile-time interface check +var _ WithMDMProfileSpecs = WindowsSettings{} + type YaraRuleSpec struct { Path string `json:"path"` } diff --git a/server/service/client.go b/server/service/client.go index 6f4b8edff1..52fe612f79 100644 --- a/server/service/client.go +++ b/server/service/client.go @@ -1073,11 +1073,19 @@ func extractAppCfgCustomSettings(appCfg interface{}, platformKey string) []fleet if !ok { return nil } + mos, ok := mmdm[platformKey].(fleet.WithMDMProfileSpecs) + if !ok || mos == nil { + return legacyExtractAppCfgCustomSettings(mmdm, platformKey) + } + return mos.GetMDMProfileSpecs() +} + +// legacyExtractAppCfgCustomSettings is used to extract custom settings for legacy fleetctl apply use case +func legacyExtractAppCfgCustomSettings(mmdm map[string]interface{}, platformKey string) []fleet.MDMProfileSpec { mos, ok := mmdm[platformKey].(map[string]interface{}) if !ok || mos == nil { return nil } - cs, ok := mos["custom_settings"] if !ok { // custom settings is not present @@ -1666,11 +1674,7 @@ func (c *Client) DoGitOps( if config.Controls.MacOSSettings != nil { mdmAppConfig["macos_settings"] = config.Controls.MacOSSettings } else { - mdmAppConfig["macos_settings"] = map[string]interface{}{} - } - macOSSettings := mdmAppConfig["macos_settings"].(map[string]interface{}) - if customSettings, ok := macOSSettings["custom_settings"]; !ok || customSettings == nil { - macOSSettings["custom_settings"] = []interface{}{} + mdmAppConfig["macos_settings"] = fleet.MacOSSettings{} } // Put in default values for macos_updates if config.Controls.MacOSUpdates != nil { @@ -1731,11 +1735,7 @@ func (c *Client) DoGitOps( if config.Controls.WindowsSettings != nil { mdmAppConfig["windows_settings"] = config.Controls.WindowsSettings } else { - mdmAppConfig["windows_settings"] = map[string]interface{}{} - } - windowsSettings := mdmAppConfig["windows_settings"].(map[string]interface{}) - if customSettings, ok := windowsSettings["custom_settings"]; !ok || customSettings == nil { - windowsSettings["custom_settings"] = []interface{}{} + mdmAppConfig["windows_settings"] = fleet.WindowsSettings{} } // Put in default values for windows_updates if config.Controls.WindowsUpdates != nil { diff --git a/server/service/client_test.go b/server/service/client_test.go index 0a6d50ace2..352084e0d2 100644 --- a/server/service/client_test.go +++ b/server/service/client_test.go @@ -7,6 +7,7 @@ import ( "path/filepath" "testing" + "github.com/fleetdm/fleet/v4/pkg/optjson" "github.com/fleetdm/fleet/v4/pkg/spec" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/stretchr/testify/assert" @@ -140,8 +141,16 @@ spec: specs, err := spec.GroupFromBytes([]byte(c.yaml)) require.NoError(t, err) if specs.AppConfig != nil { + // Legacy fleetctl apply got := extractAppCfgMacOSCustomSettings(specs.AppConfig) - require.Equal(t, c.want, got) + assert.Equal(t, c.want, got) + + // GitOps + mdm, ok := specs.AppConfig.(map[string]interface{})["mdm"].(map[string]interface{}) + require.True(t, ok) + mdm["macos_settings"] = fleet.MacOSSettings{CustomSettings: c.want} + got = extractAppCfgMacOSCustomSettings(specs.AppConfig) + assert.Equal(t, c.want, got) } }) } @@ -274,8 +283,18 @@ spec: specs, err := spec.GroupFromBytes([]byte(c.yaml)) require.NoError(t, err) if specs.AppConfig != nil { + // Legacy fleetctl apply got := extractAppCfgWindowsCustomSettings(specs.AppConfig) - require.Equal(t, c.want, got) + assert.Equal(t, c.want, got) + + // GitOps + mdm, ok := specs.AppConfig.(map[string]interface{})["mdm"].(map[string]interface{}) + require.True(t, ok) + windowsSettings := fleet.WindowsSettings{} + windowsSettings.CustomSettings = optjson.SetSlice(c.want) + mdm["windows_settings"] = windowsSettings + got = extractAppCfgWindowsCustomSettings(specs.AppConfig) + assert.Equal(t, c.want, got) } }) }