Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Windows MDM feature flag #15167

Merged
merged 7 commits into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changes/issue-14959-remove-feature-flag
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
* Removed the `FLEET_DEV_MDM_ENABLED` feature flag that allowed enabling Windows MDM. The feature flag is not used anymore, and Windows MDM can be enabled without it.

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 @@ -902,8 +901,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 @@ -3338,17 +3335,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 @@ -3370,12 +3356,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
4 changes: 0 additions & 4 deletions frontend/interfaces/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,6 @@ export interface IConfig {
};
};
mdm: IMdmConfig;
/** This is the flag that determines if the windwos mdm feature flag is enabled.
TODO: WINDOWS FEATURE FLAG: remove when windows MDM is released. Only used for windows MDM dev currently.
*/
mdm_enabled?: boolean;
}

export interface IWebhookSettings {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,7 @@ const DiskEncryption = ({
return "If turned on, hosts' disk encryption keys will be stored in Fleet. ";
}

const isWindowsFeatureFlagEnabled = config?.mdm_enabled ?? false;
const dynamicText = isWindowsFeatureFlagEnabled
? " and “BitLocker” on Windows"
: "";
return `Also known as “FileVault” on macOS${dynamicText}. If turned on, hosts' disk encryption keys will be stored in Fleet. `;
return `Also known as “FileVault” on macOS and “BitLocker” on Windows. If turned on, hosts' disk encryption keys will be stored in Fleet. `;
};

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ interface IDiskEncryptionTableProps {
}

const DiskEncryptionTable = ({ currentTeamId }: IDiskEncryptionTableProps) => {
const { config } = useContext(AppContext);

const {
data: diskEncryptionStatusData,
error: diskEncryptionStatusError,
Expand All @@ -34,15 +32,8 @@ const DiskEncryptionTable = ({ currentTeamId }: IDiskEncryptionTableProps) => {
}
);

// TODO: WINDOWS FEATURE FLAG: remove this when windows feature flag is removed.
// this is used to conditianlly show "View all hosts" link in table cells.
const windowsFeatureFlagEnabled = config?.mdm_enabled ?? false;
const tableHeaders = generateTableHeaders(windowsFeatureFlagEnabled);
const tableData = generateTableData(
windowsFeatureFlagEnabled,
diskEncryptionStatusData,
currentTeamId
);
const tableHeaders = generateTableHeaders();
const tableData = generateTableData(diskEncryptionStatusData, currentTeamId);

if (diskEncryptionStatusError) {
return <DataError />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ interface ICellProps {
};
row: {
original: {
includeWindows: boolean;
status: IStatusCellValue;
teamId: number;
};
Expand Down Expand Up @@ -94,18 +93,13 @@ const defaultTableHeaders: IDataColumn[] = [
return (
<div className="disk-encryption-table__aggregate-table-data">
<TextCell value={aggregateCount} formatter={(val) => <>{val}</>} />
{/* TODO: WINDOWS FEATURE FLAG: remove this conditional when windows mdm
is released. the view all UI will show in the windows column when we
release the feature. */}
{!original.includeWindows && (
<ViewAllHostsLink
className="view-hosts-link"
queryParams={{
[HOSTS_QUERY_PARAMS.DISK_ENCRYPTION]: original.status.value,
team_id: original.teamId,
}}
/>
)}
<ViewAllHostsLink
className="view-hosts-link"
queryParams={{
[HOSTS_QUERY_PARAMS.DISK_ENCRYPTION]: original.status.value,
team_id: original.teamId,
}}
/>
</div>
);
},
Expand Down Expand Up @@ -144,14 +138,8 @@ const windowsTableHeader: IDataColumn[] = [
},
];

// TODO: WINDOWS FEATURE FLAG: return all headers when windows feature flag is removed.
export const generateTableHeaders = (
includeWindows: boolean
): IDataColumn[] => {
return includeWindows
? [...defaultTableHeaders, ...windowsTableHeader]
: defaultTableHeaders;
return defaultTableHeaders;
export const generateTableHeaders = (): IDataColumn[] => {
return [...defaultTableHeaders, ...windowsTableHeader];
};

const STATUS_CELL_VALUES: Record<DiskEncryptionStatus, IStatusCellValue> = {
Expand Down Expand Up @@ -215,9 +203,6 @@ const STATUS_ORDER = [
] as const;

export const generateTableData = (
// TODO: WINDOWS FEATURE FLAG: remove includeWindows when windows feature flag is removed.
// This is used to conditionally show "View all hosts" link in table cells.
includeWindows: boolean,
data?: IDiskEncryptionSummaryResponse,
currentTeamId?: number
) => {
Expand All @@ -227,7 +212,6 @@ export const generateTableData = (
status: DiskEncryptionStatus,
statusAggregate: IDiskEncryptionStatusAggregate
) => ({
includeWindows,
status: STATUS_CELL_VALUES[status],
macosHosts: statusAggregate.macos,
windowsHosts: statusAggregate.windows,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,11 +234,9 @@ const AppleBusinessManagerSection = ({
<div className={baseClass}>
<h2>Apple Business Manager</h2>
{isLoadingMdmAppleBm ? <Spinner /> : renderAppleBMInfo()}
{config?.mdm_enabled && (
<WindowsAutomaticEnrollmentCard
viewDetails={navigateToWindowsAutomaticEnrollment}
/>
)}
<WindowsAutomaticEnrollmentCard
viewDetails={navigateToWindowsAutomaticEnrollment}
/>
{showEditTeamModal && (
<EditTeamModal
onCancel={toggleEditTeamModal}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,10 @@ const MdmSettings = ({ router }: IMdmSettingsProps) => {
turnOnMacOSMdm={navigateToMacOSMdm}
viewDetails={navigateToMacOSMdm}
/>
{/* TODO: remove conditional rendering when windows MDM is released. */}
{config?.mdm_enabled && (
<WindowsMdmCard
turnOnWindowsMdm={navigateToWindowsMdm}
editWindowsMdm={navigateToWindowsMdm}
/>
)}
<WindowsMdmCard
turnOnWindowsMdm={navigateToWindowsMdm}
editWindowsMdm={navigateToWindowsMdm}
/>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or should I have kept the {} around <WindowsMdmCard>?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without the {} looks good to me! (the {} are used to wrap expressions)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll remove them from here then.

</>
)}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,6 @@ interface IWindowsMdmPageProps {
const WindowsMdmPage = ({ router }: IWindowsMdmPageProps) => {
const { config } = useContext(AppContext);

// TODO: remove when windows MDM is fully released. This is a temporary redirect
// when the feature is not enabled.
if (!config?.mdm_enabled) {
router.replace(PATHS.ADMIN_INTEGRATIONS_MDM);
}

const isWindowsMdmEnabled =
config?.mdm?.windows_enabled_and_configured ?? false;

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 @@ -174,10 +174,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 @@ -315,75 +315,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/ctxdb"
"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
Expand Down Expand Up @@ -51,17 +50,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 @@ -186,7 +174,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 @@ -243,9 +230,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 @@ -786,12 +772,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
Loading
Loading