For fleetctl gitops, when MDM configs are not explicitly defined in gitops yml file, they are now set to default values. (#17223)

For fleetctl gitops, when MDM configs are not explicitly defined in
gitops yml file, they are now set to default values.
#17209

Gitops role can now read org config/settings. This is used to determine
whether license is Premium.
Doc changes for permission access:
https://github.com/fleetdm/fleet/pull/17238

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

<!-- Note that API documentation changes are now addressed by the
product design team. -->

- [x] Changes file added for user-visible changes in `changes/` or
`orbit/changes/`.
See [Changes
files](https://fleetdm.com/docs/contributing/committing-changes#changes-files)
for more information.
- [x] Documented any permissions changes (docs/Using
Fleet/manage-access.md)
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality

changes/17209-fleetctl-gitops-mdm-configs

@@ -0,0 +1,2 @@
+For fleetctl gitops, when MDM configs are not explicitly defined in gitops yml file, they are now set to default values.
+- GitOps user can now read fleet config, which is needed to determine if Fleet Premium is being used.

cmd/fleetctl/gitops.go

@@ -56,7 +56,14 @@ func gitopsCommand() *cli.Command {
 			logf := func(format string, a ...interface{}) {
 				_, _ = fmt.Fprintf(c.App.Writer, format, a...)
 			}
-			err = fleetClient.DoGitOps(c.Context, config, baseDir, logf, flDryRun)
+			appConfig, err := fleetClient.GetAppConfig()
+			if err != nil {
+				return err
+			}
+			if appConfig.License == nil {
+				return errors.New("no license struct found in app config")
+			}
+			err = fleetClient.DoGitOps(c.Context, config, baseDir, logf, flDryRun, appConfig)
 			if err != nil {
 				return err
 			}

cmd/fleetctl/gitops_enterprise_integration_test.go

@@ -34,12 +34,11 @@ type enterpriseIntegrationGitopsTestSuite struct {
 }
 
 func (s *enterpriseIntegrationGitopsTestSuite) SetupSuite() {
-	s.withDS.SetupSuite("integrationGitopsTestSuite")
+	s.withDS.SetupSuite("enterpriseIntegrationGitopsTestSuite")
 
 	appConf, err := s.ds.AppConfig(context.Background())
 	require.NoError(s.T(), err)
 	appConf.MDM.EnabledAndConfigured = true
-	appConf.MDM.WindowsEnabledAndConfigured = true
 	appConf.MDM.AppleBMEnabledAndConfigured = true
 	err = s.ds.SaveAppConfig(context.Background(), appConf)
 	require.NoError(s.T(), err)

cmd/fleetctl/gitops_enterprise_no_mdm_integration_test.go

@@ -0,0 +1,155 @@
+package main
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"github.com/ghodss/yaml"
+	"os"
+	"path"
+	"path/filepath"
+	"testing"
+
+	"github.com/fleetdm/fleet/v4/server/config"
+	"github.com/fleetdm/fleet/v4/server/datastore/redis/redistest"
+	"github.com/fleetdm/fleet/v4/server/fleet"
+	"github.com/fleetdm/fleet/v4/server/ptr"
+	"github.com/fleetdm/fleet/v4/server/service"
+	"github.com/fleetdm/fleet/v4/server/test"
+	"github.com/go-git/go-git/v5"
+	"github.com/stretchr/testify/require"
+	"github.com/stretchr/testify/suite"
+)
+
+func TestEnterpriseNoMdmIntegrationsGitops(t *testing.T) {
+	testingSuite := new(enterpriseNoMdmIntegrationGitopsTestSuite)
+	testingSuite.suite = &testingSuite.Suite
+	suite.Run(t, testingSuite)
+}
+
+type enterpriseNoMdmIntegrationGitopsTestSuite struct {
+	suite.Suite
+	withServer
+	fleetCfg config.FleetConfig
+}
+
+func (s *enterpriseNoMdmIntegrationGitopsTestSuite) SetupSuite() {
+	s.withDS.SetupSuite("enterpriseNoMdmIntegrationGitopsTestSuite")
+
+	fleetCfg := config.TestConfig()
+	fleetCfg.Osquery.EnrollCooldown = 0
+
+	redisPool := redistest.SetupRedis(s.T(), "zz", false, false, false)
+
+	serverConfig := service.TestServerOpts{
+		License: &fleet.LicenseInfo{
+			Tier: fleet.TierPremium,
+		},
+		FleetConfig: &fleetCfg,
+		Pool:        redisPool,
+	}
+	users, server := service.RunServerForTestsWithDS(s.T(), s.ds, &serverConfig)
+	s.T().Setenv("FLEET_SERVER_ADDRESS", server.URL) // fleetctl always uses this env var in tests
+	s.server = server
+	s.users = users
+	s.fleetCfg = fleetCfg
+
+	appConf, err := s.ds.AppConfig(context.Background())
+	require.NoError(s.T(), err)
+	appConf.ServerSettings.ServerURL = server.URL
+	err = s.ds.SaveAppConfig(context.Background(), appConf)
+	require.NoError(s.T(), err)
+}
+
+// TestFleetGitops runs `fleetctl gitops` command on configs in https://github.com/fleetdm/fleet-gitops repo.
+// Changes to that repo may cause this test to fail.
+func (s *enterpriseNoMdmIntegrationGitopsTestSuite) TestFleetGitops() {
+	t := s.T()
+	const fleetGitopsRepo = "https://github.com/fleetdm/fleet-gitops"
+
+	// Create GitOps user
+	user := fleet.User{
+		Name:       "GitOps User",
+		Email:      "fleetctl-gitops@example.com",
+		GlobalRole: ptr.String(fleet.RoleGitOps),
+	}
+	require.NoError(t, user.SetPassword(test.GoodPassword, 10, 10))
+	_, err := s.ds.NewUser(context.Background(), &user)
+	require.NoError(t, err)
+
+	// Create a temporary fleetctl config file
+	fleetctlConfig, err := os.CreateTemp(t.TempDir(), "*.yml")
+	require.NoError(t, err)
+	token := s.getTestToken(user.Email, test.GoodPassword)
+	configStr := fmt.Sprintf(
+		`
+contexts:
+  default:
+    address: %s
+    tls-skip-verify: true
+    token: %s
+`, s.server.URL, token,
+	)
+	_, err = fleetctlConfig.WriteString(configStr)
+	require.NoError(t, err)
+
+	// Clone git repo
+	repoDir := t.TempDir()
+	_, err = git.PlainClone(
+		repoDir, false, &git.CloneOptions{
+			ReferenceName: "main",
+			SingleBranch:  true,
+			Depth:         1,
+			URL:           fleetGitopsRepo,
+			Progress:      os.Stdout,
+		},
+	)
+	require.NoError(t, err)
+
+	// Set the required environment variables
+	t.Setenv("FLEET_SSO_METADATA", "sso_metadata")
+	t.Setenv("FLEET_GLOBAL_ENROLL_SECRET", "global_enroll_secret")
+	t.Setenv("FLEET_WORKSTATIONS_ENROLL_SECRET", "workstations_enroll_secret")
+	t.Setenv("FLEET_WORKSTATIONS_CANARY_ENROLL_SECRET", "workstations_canary_enroll_secret")
+	// Read the global file
+	globalFile := path.Join(repoDir, "default.yml")
+	s.removeControls(globalFile)
+	teamsDir := path.Join(repoDir, "teams")
+	teamFiles, err := os.ReadDir(teamsDir)
+	require.NoError(t, err)
+
+	// Dry run
+	_ = runAppForTest(t, []string{"gitops", "--config", fleetctlConfig.Name(), "-f", globalFile, "--dry-run"})
+	for _, file := range teamFiles {
+		if filepath.Ext(file.Name()) == ".yml" {
+			teamsFile := path.Join(teamsDir, file.Name())
+			s.removeControls(teamsFile)
+			_ = runAppForTest(t, []string{"gitops", "--config", fleetctlConfig.Name(), "-f", teamsFile, "--dry-run"})
+		}
+	}
+
+	// Real run
+	_ = runAppForTest(t, []string{"gitops", "--config", fleetctlConfig.Name(), "-f", globalFile})
+	for _, file := range teamFiles {
+		if filepath.Ext(file.Name()) == ".yml" {
+			_ = runAppForTest(t, []string{"gitops", "--config", fleetctlConfig.Name(), "-f", path.Join(teamsDir, file.Name())})
+		}
+	}
+
+}
+
+// removeControls removes MDM settings (controls) from the gitops YAML file
+func (s *enterpriseNoMdmIntegrationGitopsTestSuite) removeControls(file string) {
+	t := s.T()
+	b, err := os.ReadFile(file)
+	require.NoError(t, err)
+	var top map[string]json.RawMessage
+	err = yaml.Unmarshal(b, &top)
+	require.NoError(t, err)
+	// Remove MDM settings (controls)
+	top["controls"] = []byte(`null`)
+	dataToWrite, err := yaml.Marshal(top)
+	require.NoError(t, err)
+	err = os.WriteFile(file, dataToWrite, os.ModePerm)
+	require.NoError(t, err)
+}

cmd/fleetctl/gitops_integration_test.go

@@ -37,7 +37,6 @@ func (s *integrationGitopsTestSuite) SetupSuite() {
 	appConf, err := s.ds.AppConfig(context.Background())
 	require.NoError(s.T(), err)
 	appConf.MDM.EnabledAndConfigured = true
-	appConf.MDM.WindowsEnabledAndConfigured = true
 	appConf.MDM.AppleBMEnabledAndConfigured = true
 	err = s.ds.SaveAppConfig(context.Background(), appConf)
 	require.NoError(s.T(), err)

cmd/fleetctl/gitops_test.go

@@ -23,6 +23,16 @@ const teamName = "Team Test"
 func TestBasicGlobalGitOps(t *testing.T) {
 	_, ds := runServerWithMockedDS(t)
 
+	ds.BatchSetMDMProfilesFunc = func(
+		ctx context.Context, tmID *uint, macProfiles []*fleet.MDMAppleConfigProfile, winProfiles []*fleet.MDMWindowsConfigProfile,
+	) error {
+		return nil
+	}
+	ds.BulkSetPendingMDMHostProfilesFunc = func(
+		ctx context.Context, hostIDs []uint, teamIDs []uint, profileUUIDs []string, hostUUIDs []string,
+	) error {
+		return nil
+	}
 	ds.BatchSetScriptsFunc = func(ctx context.Context, tmID *uint, scripts []*fleet.Script) error { return nil }
 	ds.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error {
 		return nil
@@ -122,6 +132,16 @@ func TestBasicTeamGitOps(t *testing.T) {
 	const secret = "TestSecret"
 
 	ds.BatchSetScriptsFunc = func(ctx context.Context, tmID *uint, scripts []*fleet.Script) error { return nil }
+	ds.BatchSetMDMProfilesFunc = func(
+		ctx context.Context, tmID *uint, macProfiles []*fleet.MDMAppleConfigProfile, winProfiles []*fleet.MDMWindowsConfigProfile,
+	) error {
+		return nil
+	}
+	ds.BulkSetPendingMDMHostProfilesFunc = func(
+		ctx context.Context, hostIDs []uint, teamIDs []uint, profileUUIDs []string, hostUUIDs []string,
+	) error {
+		return nil
+	}
 	ds.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error {
 		return nil
 	}
@@ -549,4 +569,10 @@ team_settings:
 	assert.Equal(t, secret, enrolledSecrets[0].Secret)
 	assert.False(t, savedTeam.Config.WebhookSettings.HostStatusWebhook.Enable)
 	assert.Equal(t, "", savedTeam.Config.WebhookSettings.HostStatusWebhook.DestinationURL)
+	assert.Empty(t, savedTeam.Config.MDM.MacOSSettings.CustomSettings)
+	assert.Empty(t, savedTeam.Config.MDM.WindowsSettings.CustomSettings.Value)
+	assert.Empty(t, savedTeam.Config.MDM.MacOSUpdates.Deadline.Value)
+	assert.Empty(t, savedTeam.Config.MDM.MacOSUpdates.MinimumVersion.Value)
+	assert.Empty(t, savedTeam.Config.MDM.MacOSSetup.BootstrapPackage.Value)
+	assert.False(t, savedTeam.Config.MDM.EnableDiskEncryption)
 }

server/authz/policy.rego

@@ -50,10 +50,10 @@ team_role(subject, team_id) = role {
 # Global config
 ##
 
-# Global admin, maintainer, observer_plus and observer can read global config.
+# Global admin, gitops, maintainer, observer_plus and observer can read global config.
 allow {
   object.type == "app_config"
-  subject.global_role == [admin, maintainer, observer_plus, observer][_]
+  subject.global_role == [admin, gitops, maintainer, observer_plus, observer][_]
   action == read
 }
 

server/authz/policy_test.go

@@ -65,7 +65,7 @@ func TestAuthorizeAppConfig(t *testing.T) {
 		{user: test.UserObserverPlus, object: config, action: read, allow: true},
 		{user: test.UserObserverPlus, object: config, action: write, allow: false},
 
-		{user: test.UserGitOps, object: config, action: read, allow: false},
+		{user: test.UserGitOps, object: config, action: read, allow: true},
 		{user: test.UserGitOps, object: config, action: write, allow: true},
 
 		{user: test.UserTeamAdminTeam1, object: config, action: read, allow: true},

server/service/appconfig_test.go

@@ -84,7 +84,7 @@ func TestAppConfigAuth(t *testing.T) {
 			"global gitops",
 			&fleet.User{GlobalRole: ptr.String(fleet.RoleGitOps)},
 			false,
-			true,
+			false,
 		},
 		{
 			"team admin",
@@ -521,7 +521,7 @@ func TestAppConfigSecretsObfuscated(t *testing.T) {
 		{
 			"global gitops",
 			&fleet.User{GlobalRole: ptr.String(fleet.RoleGitOps)},
-			true,
+			false,
 		},
 		{
 			"team admin",

server/service/client.go

@@ -865,6 +865,7 @@ func (c *Client) DoGitOps(
 	baseDir string,
 	logf func(format string, args ...interface{}),
 	dryRun bool,
+	appConfig *fleet.EnrichedAppConfig,
 ) error {
 	var err error
 	logFn := func(format string, args ...interface{}) {
@@ -898,8 +899,23 @@ func (c *Client) DoGitOps(
 			return errors.New("org_settings.mdm config is not a map")
 		}
 
-		mdmAppConfig["macos_migration"] = config.Controls.MacOSMigration
+		// Put in default values for macos_migration
+		if config.Controls.MacOSMigration != nil {
+			mdmAppConfig["macos_migration"] = config.Controls.MacOSMigration
+		} else {
+			mdmAppConfig["macos_migration"] = map[string]interface{}{}
+		}
+		macOSMigration := mdmAppConfig["macos_migration"].(map[string]interface{})
+		if enable, ok := macOSMigration["enable"]; !ok || enable == nil {
+			macOSMigration["enable"] = false
+		}
+		// Put in default values for windows_enabled_and_configured
 		mdmAppConfig["windows_enabled_and_configured"] = config.Controls.WindowsEnabledAndConfigured
+		if config.Controls.WindowsEnabledAndConfigured != nil {
+			mdmAppConfig["windows_enabled_and_configured"] = config.Controls.WindowsEnabledAndConfigured
+		} else {
+			mdmAppConfig["windows_enabled_and_configured"] = false
+		}
 		group.AppConfig.(map[string]interface{})["scripts"] = scripts
 	} else {
 		team = make(map[string]interface{})
@@ -929,12 +945,76 @@ func (c *Client) DoGitOps(
 		mdmAppConfig = team["mdm"].(map[string]interface{})
 	}
 	// Common controls settings between org and team settings
-	mdmAppConfig["macos_settings"] = config.Controls.MacOSSettings
-	mdmAppConfig["macos_updates"] = config.Controls.MacOSUpdates
-	mdmAppConfig["macos_setup"] = config.Controls.MacOSSetup
-	mdmAppConfig["windows_updates"] = config.Controls.WindowsUpdates
-	mdmAppConfig["windows_settings"] = config.Controls.WindowsSettings
-	mdmAppConfig["enable_disk_encryption"] = config.Controls.EnableDiskEncryption
+	// Put in default values for macos_settings
+	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{}{}
+	}
+	// Put in default values for macos_updates
+	if config.Controls.MacOSUpdates != nil {
+		mdmAppConfig["macos_updates"] = config.Controls.MacOSUpdates
+	} else {
+		mdmAppConfig["macos_updates"] = map[string]interface{}{}
+	}
+	macOSUpdates := mdmAppConfig["macos_updates"].(map[string]interface{})
+	if minimumVersion, ok := macOSUpdates["minimum_version"]; !ok || minimumVersion == nil {
+		macOSUpdates["minimum_version"] = ""
+	}
+	if deadline, ok := macOSUpdates["deadline"]; !ok || deadline == nil {
+		macOSUpdates["deadline"] = ""
+	}
+	// Put in default values for macos_setup
+	if config.Controls.MacOSSetup != nil {
+		mdmAppConfig["macos_setup"] = config.Controls.MacOSSetup
+	} else {
+		mdmAppConfig["macos_setup"] = map[string]interface{}{}
+	}
+	macOSSetup := mdmAppConfig["macos_setup"].(map[string]interface{})
+	if bootstrapPackage, ok := macOSSetup["bootstrap_package"]; !ok || bootstrapPackage == nil {
+		macOSSetup["bootstrap_package"] = ""
+	}
+	if enableEndUserAuthentication, ok := macOSSetup["enable_end_user_authentication"]; !ok || enableEndUserAuthentication == nil {
+		macOSSetup["enable_end_user_authentication"] = false
+	}
+	if macOSSetupAssistant, ok := macOSSetup["macos_setup_assistant"]; !ok || macOSSetupAssistant == nil {
+		macOSSetup["macos_setup_assistant"] = ""
+	}
+	// Put in default values for windows_settings
+	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{}{}
+	}
+	// Put in default values for windows_updates
+	if config.Controls.WindowsUpdates != nil {
+		mdmAppConfig["windows_updates"] = config.Controls.WindowsUpdates
+	} else {
+		mdmAppConfig["windows_updates"] = map[string]interface{}{}
+	}
+	if appConfig.License.IsPremium() {
+		windowsUpdates := mdmAppConfig["windows_updates"].(map[string]interface{})
+		if deadlineDays, ok := windowsUpdates["deadline_days"]; !ok || deadlineDays == nil {
+			windowsUpdates["deadline_days"] = 0
+		}
+		if gracePeriodDays, ok := windowsUpdates["grace_period_days"]; !ok || gracePeriodDays == nil {
+			windowsUpdates["grace_period_days"] = 0
+		}
+	}
+	// Put in default value for enable_disk_encryption
+	if config.Controls.EnableDiskEncryption != nil {
+		mdmAppConfig["enable_disk_encryption"] = config.Controls.EnableDiskEncryption
+	} else {
+		mdmAppConfig["enable_disk_encryption"] = false
+	}
 	if config.TeamName != nil {
 		rawTeam, err := json.Marshal(team)
 		if err != nil {

server/service/integration_enterprise_test.go

@@ -8,6 +8,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"github.com/fleetdm/fleet/v4/server/pubsub"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -60,6 +61,7 @@ func (s *integrationEnterpriseTestSuite) SetupSuite() {
 			Tier: fleet.TierPremium,
 		},
 		Pool:           s.redisPool,
+		Rs:             pubsub.NewInmemQueryResults(),
 		Lq:             s.lq,
 		Logger:         log.NewLogfmtLogger(os.Stdout),
 		EnableCachedDS: true,
@@ -4094,8 +4096,8 @@ func (s *integrationEnterpriseTestSuite) TestGitOpsUserActions() {
 	s.DoJSON("GET", "/api/latest/fleet/software/1", getSoftwareRequest{}, http.StatusForbidden, &getSoftwareResponse{})
 	s.DoJSON("GET", "/api/latest/fleet/software/versions/1", getSoftwareRequest{}, http.StatusForbidden, &getSoftwareResponse{})
 
-	// Attempt to read app config, should fail.
-	s.DoJSON("GET", "/api/latest/fleet/config", nil, http.StatusForbidden, &appConfigResponse{})
+	// Attempt to read app config, should pass.
+	s.DoJSON("GET", "/api/latest/fleet/config", nil, http.StatusOK, &appConfigResponse{})
 
 	// Attempt to write app config, should allow.
 	acr = appConfigResponse{}