Skip to content

Commit

Permalink
Remove Windows MDM feature flag
Browse files Browse the repository at this point in the history
  • Loading branch information
mna committed Nov 15, 2023
1 parent 8dc2076 commit de91b98
Show file tree
Hide file tree
Showing 10 changed files with 15 additions and 122 deletions.
4 changes: 2 additions & 2 deletions cmd/fleet/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -553,8 +553,8 @@ the way that the Fleet server works.
wstepCertManager microsoft_mdm.CertManager
)

// Configuring WSTEP certs if Windows MDM feature flag is enabled
if configpkg.IsMDMFeatureFlagEnabled() && config.MDM.IsMicrosoftWSTEPSet() {
// Configuring WSTEP certs
if config.MDM.IsMicrosoftWSTEPSet() {
_, crtPEM, keyPEM, err := config.MDM.MicrosoftWSTEP()
if err != nil {
initFatal(err, "validate Microsoft WSTEP certificate and key")
Expand Down
20 changes: 0 additions & 20 deletions cmd/fleetctl/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"path/filepath"
"sort"
"strconv"
"strings"
"sync"
"testing"
"time"
Expand Down Expand Up @@ -820,8 +819,6 @@ func mobileconfigForTest(name, identifier string) []byte {
}

func TestApplyAsGitOps(t *testing.T) {
t.Setenv("FLEET_DEV_MDM_ENABLED", "1")

enqueuer := new(nanomdm_mock.Storage)
license := &fleet.LicenseInfo{Tier: fleet.TierPremium, Expiration: time.Now().Add(24 * time.Hour)}

Expand Down Expand Up @@ -2998,17 +2995,6 @@ spec:
`, macSetupFile),
wantErr: `macOS MDM isn't turned on.`,
},
{
desc: "app config enable windows mdm without feature flag",
spec: `
apiVersion: v1
kind: config
spec:
mdm:
windows_enabled_and_configured: true
`,
wantErr: `422 Validation Failed: cannot enable Windows MDM without the feature flag explicitly enabled`,
},
{
desc: "app config enable windows mdm without WSTEP",
spec: `
Expand All @@ -3030,12 +3016,6 @@ spec:
license := &fleet.LicenseInfo{Tier: fleet.TierPremium, Expiration: time.Now().Add(24 * time.Hour)}
for _, c := range cases {
t.Run(c.desc, func(t *testing.T) {
// bit hacky, but since the env var is temporary while Windows MDM is in beta,
// didn't want to add a field to the test cases just for this.
if strings.Contains(c.desc, "WSTEP") {
t.Setenv("FLEET_DEV_MDM_ENABLED", "1")
}

_, ds := runServerWithMockedDS(t, &service.TestServerOpts{License: license})
setupDS(ds)
filename := writeTmpYml(t, c.spec)
Expand Down
10 changes: 0 additions & 10 deletions server/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -1741,13 +1741,3 @@ func SetTestMDMConfig(t testing.TB, cfg *FleetConfig, cert, key []byte, appleBMT
t.Fatal(err)
}
}

// Undocumented feature flag for Windows MDM, used to determine if the Windows
// MDM feature is visible in the UI and can be enabled. More details here:
// https://github.com/fleetdm/fleet/issues/12257
//
// TODO: remove this flag once the Windows MDM feature is ready for
// release.
func IsMDMFeatureFlagEnabled() bool {
return os.Getenv("FLEET_DEV_MDM_ENABLED") == "1"
}
5 changes: 1 addition & 4 deletions server/fleet/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,7 @@ type MDM struct {
// AtLeastOnePlatformEnabledAndConfigured returns true if at least one supported platform
// (macOS or Windows) has MDM enabled and configured.
func (m MDM) AtLeastOnePlatformEnabledAndConfigured() bool {
// explicitly check for the feature flag to account for the edge case of:
// 1. FF enabled, windows is turned on
// 2. FF disabled on server restart
return m.EnabledAndConfigured || (config.IsMDMFeatureFlagEnabled() && m.WindowsEnabledAndConfigured)
return m.EnabledAndConfigured || m.WindowsEnabledAndConfigured
}

// versionStringRegex is used to validate that a version string is in the x.y.z
Expand Down
47 changes: 4 additions & 43 deletions server/fleet/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,75 +233,36 @@ func TestAtLeastOnePlatformEnabledAndConfigured(t *testing.T) {
name string
macOSEnabledAndConfigured bool
windowsEnabledAndConfigured bool
isMDMFeatureFlagEnabled bool
expectedResult bool
}{
{
name: "None enabled, feature flag disabled",
name: "None enabled",
macOSEnabledAndConfigured: false,
windowsEnabledAndConfigured: false,
isMDMFeatureFlagEnabled: false,
expectedResult: false,
},
{
name: "MacOS enabled, feature flag disabled",
name: "MacOS enabled",
macOSEnabledAndConfigured: true,
windowsEnabledAndConfigured: false,
isMDMFeatureFlagEnabled: false,
expectedResult: true,
},
{
name: "Windows enabled, feature flag disabled",
macOSEnabledAndConfigured: false,
windowsEnabledAndConfigured: true,
isMDMFeatureFlagEnabled: false,
expectedResult: false,
},
{
name: "Both enabled, feature flag disabled",
name: "Both enabled",
macOSEnabledAndConfigured: true,
windowsEnabledAndConfigured: true,
isMDMFeatureFlagEnabled: false,
expectedResult: true,
},
{
name: "None enabled, feature flag enabled",
macOSEnabledAndConfigured: false,
windowsEnabledAndConfigured: false,
isMDMFeatureFlagEnabled: true,
expectedResult: false,
},
{
name: "MacOS enabled, feature flag enabled",
macOSEnabledAndConfigured: true,
windowsEnabledAndConfigured: false,
isMDMFeatureFlagEnabled: true,
expectedResult: true,
},
{
name: "Windows enabled, feature flag enabled",
name: "Windows enabled",
macOSEnabledAndConfigured: false,
windowsEnabledAndConfigured: true,
isMDMFeatureFlagEnabled: true,
expectedResult: true,
},
{
name: "Both enabled, feature flag enabled",
macOSEnabledAndConfigured: true,
windowsEnabledAndConfigured: true,
isMDMFeatureFlagEnabled: true,
expectedResult: true,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
if test.isMDMFeatureFlagEnabled {
t.Setenv("FLEET_DEV_MDM_ENABLED", "1")
} else {
t.Setenv("FLEET_DEV_MDM_ENABLED", "0")
}

mdm := MDM{
EnabledAndConfigured: test.macOSEnabledAndConfigured,
WindowsEnabledAndConfigured: test.windowsEnabledAndConfigured,
Expand Down
24 changes: 2 additions & 22 deletions server/service/appconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/fleetdm/fleet/v4/pkg/rawjson"
"github.com/fleetdm/fleet/v4/server"
"github.com/fleetdm/fleet/v4/server/authz"
"github.com/fleetdm/fleet/v4/server/config"
authz_ctx "github.com/fleetdm/fleet/v4/server/contexts/authz"
"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
"github.com/fleetdm/fleet/v4/server/contexts/license"
Expand Down Expand Up @@ -50,17 +49,6 @@ type appConfigResponseFields struct {
// SandboxEnabled is true if fleet serve was ran with server.sandbox_enabled=true
SandboxEnabled bool `json:"sandbox_enabled,omitempty"`
Err error `json:"error,omitempty"`

// MDMEnabled is true if fleet serve was started with
// FLEET_DEV_MDM_ENABLED=1.
//
// Undocumented feature flag for Windows MDM, used to determine if the
// Windows MDM feature is visible in the UI and can be enabled. More details
// here: https://github.com/fleetdm/fleet/issues/12257
//
// TODO: remove this flag once the Windows MDM feature is ready for
// release.
MDMEnabled bool `json:"mdm_enabled,omitempty"`
}

// UnmarshalJSON implements the json.Unmarshaler interface to make sure we serialize
Expand Down Expand Up @@ -185,7 +173,6 @@ func getAppConfigEndpoint(ctx context.Context, request interface{}, svc fleet.Se
Logging: loggingConfig,
Email: emailConfig,
SandboxEnabled: svc.SandboxEnabled(),
MDMEnabled: config.IsMDMFeatureFlagEnabled(),
},
}
return response, nil
Expand Down Expand Up @@ -242,9 +229,8 @@ func modifyAppConfigEndpoint(ctx context.Context, request interface{}, svc fleet
response := appConfigResponse{
AppConfig: *appConfig,
appConfigResponseFields: appConfigResponseFields{
License: license,
Logging: loggingConfig,
MDMEnabled: config.IsMDMFeatureFlagEnabled(),
License: license,
Logging: loggingConfig,
},
}

Expand Down Expand Up @@ -732,12 +718,6 @@ func (svc *Service) validateMDM(
}

// Windows validation
if !config.IsMDMFeatureFlagEnabled() {
if mdm.WindowsEnabledAndConfigured {
invalid.Append("mdm.windows_enabled_and_configured", "cannot enable Windows MDM without the feature flag explicitly enabled")
return
}
}
if !svc.config.MDM.IsMicrosoftWSTEPSet() {
if mdm.WindowsEnabledAndConfigured {
invalid.Append("mdm.windows_enabled_and_configured", "Couldn't turn on Windows MDM. Please configure Fleet with a certificate and key pair first.")
Expand Down
6 changes: 2 additions & 4 deletions server/service/integration_core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4977,7 +4977,6 @@ func (s *integrationTestSuite) TestAppConfig() {
assert.Equal(t, "free", acResp.License.Tier)
assert.Equal(t, "FleetTest", acResp.OrgInfo.OrgName) // set in SetupSuite
assert.False(t, acResp.MDM.AppleBMTermsExpired)
assert.False(t, acResp.MDMEnabled)

// set the apple BM terms expired flag, and the enabled and configured flags,
// we'll check again at the end of this test to make sure they weren't
Expand Down Expand Up @@ -5006,7 +5005,6 @@ func (s *integrationTestSuite) TestAppConfig() {
}`), http.StatusOK, &acResp)
assert.Equal(t, "test", acResp.OrgInfo.OrgName)
assert.True(t, acResp.MDM.AppleBMTermsExpired)
assert.False(t, acResp.MDMEnabled)

// the global agent options were not modified by the last call, so the
// corresponding activity should not have been created.
Expand Down Expand Up @@ -5198,13 +5196,13 @@ func (s *integrationTestSuite) TestAppConfig() {
"mdm": { "apple_bm_default_team": "xyz" }
}`), http.StatusUnprocessableEntity, &acResp)

// try to enable Windows MDM, impossible without the feature flag
// try to enable Windows MDM, impossible without the WSTEP certs
// (only set in mdm integrations tests)
res = s.Do("PATCH", "/api/latest/fleet/config", json.RawMessage(`{
"mdm": { "windows_enabled_and_configured": true }
}`), http.StatusUnprocessableEntity)
errMsg = extractServerErrorText(res.Body)
assert.Contains(t, errMsg, "cannot enable Windows MDM without the feature flag explicitly enabled")
assert.Contains(t, errMsg, "Please configure Fleet with a certificate and key pair first.")

// verify that the Apple BM terms expired flag was never modified
acResp = appConfigResponse{}
Expand Down
13 changes: 1 addition & 12 deletions server/service/integration_mdm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ import (
)

func TestIntegrationsMDM(t *testing.T) {
t.Setenv("FLEET_DEV_MDM_ENABLED", "1")

testingSuite := new(integrationMDMTestSuite)
testingSuite.s = &testingSuite.Suite
suite.Run(t, testingSuite)
Expand Down Expand Up @@ -2493,16 +2491,9 @@ func (s *integrationMDMTestSuite) TestDiskEncryptionSharedSetting() {
s.Do("POST", "/api/latest/fleet/spec/teams", teamSpecs, http.StatusOK)
}

// 1. disable both windows and mac mdm
// 2. turn off windows feature flag
// disable both windows and mac mdm
// we should get an error
setMDMEnabled(false, false)
t.Setenv("FLEET_DEV_MDM_ENABLED", "0")
checkConfigSetErrors()

// turn on windows feature flag
// we should get an error
t.Setenv("FLEET_DEV_MDM_ENABLED", "1")
checkConfigSetErrors()

// enable windows mdm, no errors
Expand Down Expand Up @@ -6646,7 +6637,6 @@ func (s *integrationMDMTestSuite) TestAppConfigWindowsMDM() {
// the feature flag is enabled for the MDM test suite
var acResp appConfigResponse
s.DoJSON("GET", "/api/latest/fleet/config", nil, http.StatusOK, &acResp)
assert.True(t, acResp.MDMEnabled)
assert.False(t, acResp.MDM.WindowsEnabledAndConfigured)

// create a couple teams
Expand Down Expand Up @@ -6694,7 +6684,6 @@ func (s *integrationMDMTestSuite) TestAppConfigWindowsMDM() {
"mdm": { "windows_enabled_and_configured": true }
}`), http.StatusOK, &acResp)
assert.True(t, acResp.MDM.WindowsEnabledAndConfigured)
assert.True(t, acResp.MDMEnabled)
s.lastActivityOfTypeMatches(fleet.ActivityTypeEnabledWindowsMDM{}.ActivityName(), `{}`, 0)

// get the orbit config for each host, verify that only the expected ones
Expand Down
7 changes: 2 additions & 5 deletions server/service/orbit.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"time"

"github.com/fleetdm/fleet/v4/server"
"github.com/fleetdm/fleet/v4/server/config"
"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
hostctx "github.com/fleetdm/fleet/v4/server/contexts/host"
"github.com/fleetdm/fleet/v4/server/contexts/license"
Expand Down Expand Up @@ -218,7 +217,7 @@ func (svc *Service) GetOrbitConfig(ctx context.Context) (fleet.OrbitConfig, erro
notifs.NeedsProgrammaticWindowsMDMEnrollment = true
}
}
if config.IsMDMFeatureFlagEnabled() && !appConfig.MDM.WindowsEnabledAndConfigured {
if !appConfig.MDM.WindowsEnabledAndConfigured {
if host.IsEligibleForWindowsMDMUnenrollment() {
notifs.NeedsProgrammaticWindowsMDMUnenrollment = true
}
Expand Down Expand Up @@ -271,8 +270,7 @@ func (svc *Service) GetOrbitConfig(ctx context.Context) (fleet.OrbitConfig, erro
}
}

if config.IsMDMFeatureFlagEnabled() &&
mdmConfig.EnableDiskEncryption &&
if mdmConfig.EnableDiskEncryption &&
host.IsEligibleForBitLockerEncryption() {
notifs.EnforceBitLockerEncryption = true
}
Expand Down Expand Up @@ -308,7 +306,6 @@ func (svc *Service) GetOrbitConfig(ctx context.Context) (fleet.OrbitConfig, erro
}

if appConfig.MDM.WindowsEnabledAndConfigured &&
config.IsMDMFeatureFlagEnabled() &&
appConfig.MDM.EnableDiskEncryption.Value &&
host.IsEligibleForBitLockerEncryption() {
notifs.EnforceBitLockerEncryption = true
Expand Down
1 change: 1 addition & 0 deletions terraform/addons/mdm/outputs.tf
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// TODO: the feature flag here should also be removed, maybe in a distinct PR (reviewed by infra)
output "extra_environment_variables" {
value = merge(var.enable_apple_mdm == false ? {} : {
FLEET_MDM_APPLE_SERVER_ADDRESS = var.public_domain_name
Expand Down

0 comments on commit de91b98

Please sign in to comment.