diff --git a/changes/22122-mdm-apple-status-queries b/changes/22122-mdm-apple-status-queries new file mode 100644 index 000000000000..2ea893d31ff5 --- /dev/null +++ b/changes/22122-mdm-apple-status-queries @@ -0,0 +1 @@ +- Improved performance of SQL queries used to determine MDM profile status for Apple hosts. \ No newline at end of file diff --git a/server/datastore/mysql/apple_mdm.go b/server/datastore/mysql/apple_mdm.go index b4ae8c5ac0d1..610a25db52fa 100644 --- a/server/datastore/mysql/apple_mdm.go +++ b/server/datastore/mysql/apple_mdm.go @@ -2535,377 +2535,150 @@ func (ds *Datastore) UpdateOrDeleteHostMDMAppleProfile(ctx context.Context, prof return err } -const ( - appleMDMFailedProfilesStmt = ` - h.uuid = hmap.host_uuid AND - hmap.status = :failed` - - appleMDMPendingProfilesStmt = ` - h.uuid = hmap.host_uuid AND - ( - hmap.status IS NULL OR - hmap.status = :pending OR +// sqlCaseMDMAppleStatus returns a SQL snippet that can be used to determine the status of a host +// based on the status of its profiles and declarations and filevault status. It should be used in +// conjunction with sqlJoinMDMAppleProfilesStatus and sqlJoinMDMAppleDeclarationsStatus. It assumes the +// hosts table to be aliased as 'h' and the host_disk_encryption_keys table to be aliased as 'hdek'. +func sqlCaseMDMAppleStatus() string { + // NOTE: To make this snippet reusable, we're not using sqlx.Named here because it would + // complicate usage in other queries (e.g., list hosts). + var ( + failed = fmt.Sprintf("'%s'", string(fleet.MDMDeliveryFailed)) + pending = fmt.Sprintf("'%s'", string(fleet.MDMDeliveryPending)) + verifying = fmt.Sprintf("'%s'", string(fleet.MDMDeliveryVerifying)) + verified = fmt.Sprintf("'%s'", string(fleet.MDMDeliveryVerified)) + ) + return ` + CASE WHEN (prof_failed + OR decl_failed + OR fv_failed) THEN + ` + failed + ` + WHEN (prof_pending + OR decl_pending -- special case for filevault, it's pending if the profile is -- pending OR the profile is verified or verifying but we still -- don't have an encryption key. - ( - hmap.profile_identifier = :filevault AND - hmap.status IN (:verifying, :verified) AND - hmap.operation_type = :install AND - NOT EXISTS ( - SELECT 1 - FROM host_disk_encryption_keys hdek - WHERE h.id = hdek.host_id AND - (hdek.decryptable = 1 OR hdek.decryptable IS NULL) - ) - ) - )` - - appleMDMVerifyingProfilesStmt = ` - h.uuid = hmap.host_uuid AND - hmap.operation_type = :install AND - ( - -- all profiles except filevault that are 'verifying' - ( - hmap.profile_identifier != :filevault AND - hmap.status = :verifying - ) - OR - -- special cases for filevault - ( - hmap.profile_identifier = :filevault AND - ( - -- filevault profile is verified, but we didn't verify the encryption key - ( - hmap.status = :verified AND - EXISTS ( - SELECT 1 - FROM host_disk_encryption_keys AS hdek - WHERE h.id = hdek.host_id AND - hdek.decryptable IS NULL - ) - ) - OR - -- filevault profile is verifying, and we already have an encryption key, in any state - ( - hmap.status = :verifying AND - EXISTS ( - SELECT 1 - FROM host_disk_encryption_keys AS hdek - WHERE h.id = hdek.host_id AND - hdek.decryptable = 1 OR hdek.decryptable IS NULL - ) - ) - ) - ) - )` - - appleVerifiedProfilesStmt = ` - h.uuid = hmap.host_uuid AND - hmap.operation_type = :install AND - hmap.status = :verified AND - ( - hmap.profile_identifier != :filevault OR - EXISTS ( - SELECT 1 - FROM host_disk_encryption_keys hdek - WHERE h.id = hdek.host_id AND - hdek.decryptable = 1 - ) - )` -) - -// subqueryAppleProfileStatus builds the right subquery that can be used to -// filter hosts based on their profile status. -// -// The subquery mechanism works by finding profiles for hosts that: -// - match with the provided status -// - match any status that supercedes the provided status (eg: failed supercedes verifying) -// -// Hosts will be considered to be in the given status only if the profiles -// match the given status and zero profiles match any superceding status. -func subqueryAppleProfileStatus(status fleet.MDMDeliveryStatus) (string, []any, error) { - var condition string - var excludeConditions string - switch status { - case fleet.MDMDeliveryFailed: - condition = appleMDMFailedProfilesStmt - excludeConditions = "FALSE" - case fleet.MDMDeliveryPending: - condition = appleMDMPendingProfilesStmt - excludeConditions = appleMDMFailedProfilesStmt - case fleet.MDMDeliveryVerifying: - condition = appleMDMVerifyingProfilesStmt - excludeConditions = fmt.Sprintf("(%s) OR (%s)", appleMDMPendingProfilesStmt, appleMDMFailedProfilesStmt) - case fleet.MDMDeliveryVerified: - condition = appleVerifiedProfilesStmt - excludeConditions = fmt.Sprintf("(%s) OR (%s) OR (%s)", appleMDMPendingProfilesStmt, appleMDMFailedProfilesStmt, appleMDMVerifyingProfilesStmt) - default: - return "", nil, fmt.Errorf("invalid status: %s", status) - } - - sql := fmt.Sprintf(` - SELECT 1 - FROM host_mdm_apple_profiles hmap - WHERE %s AND - NOT EXISTS ( - SELECT 1 - FROM host_mdm_apple_profiles hmap - WHERE %s - )`, condition, excludeConditions) - - arg := map[string]any{ - "install": fleet.MDMOperationTypeInstall, - "verifying": fleet.MDMDeliveryVerifying, - "failed": fleet.MDMDeliveryFailed, - "verified": fleet.MDMDeliveryVerified, - "pending": fleet.MDMDeliveryPending, - "filevault": mobileconfig.FleetFileVaultPayloadIdentifier, - } - query, args, err := sqlx.Named(sql, arg) - if err != nil { - return "", nil, fmt.Errorf("subqueryAppleProfileStatus %s: %w", status, err) - } - - return query, args, nil + OR(fv_pending + OR((fv_verifying + OR fv_verified) + AND (hdek.base64_encrypted IS NULL OR (hdek.decryptable IS NOT NULL AND hdek.decryptable != 1))))) THEN + ` + pending + ` + WHEN (prof_verifying + OR decl_verifying + -- special case when fv profile is verifying, and we already have an encryption key, in any state, we treat as verifying + OR(fv_verifying + AND hdek.base64_encrypted IS NOT NULL AND (hdek.decryptable IS NULL OR hdek.decryptable = 1)) + -- special case when fv profile is verified, but we didn't verify the encryption key, we treat as verifying + OR(fv_verified + AND hdek.base64_encrypted IS NOT NULL AND hdek.decryptable IS NULL)) THEN + ` + verifying + ` + WHEN (prof_verified + OR decl_verified + OR(fv_verified + AND hdek.base64_encrypted IS NOT NULL AND hdek.decryptable = 1)) THEN + ` + verified + ` + END +` } -// subqueryAppleDeclarationStatus builds out the subquery for declaration status -func subqueryAppleDeclarationStatus() (string, []any, error) { - const declNamedStmt = ` - CASE WHEN EXISTS ( - SELECT - 1 - FROM - host_mdm_apple_declarations d1 - WHERE - h.uuid = d1.host_uuid - AND d1.operation_type = :install - AND d1.status = :failed - AND d1.declaration_name NOT IN (:reserved_names)) THEN - 'declarations_failed' - WHEN EXISTS ( - SELECT - 1 - FROM - host_mdm_apple_declarations d2 - WHERE - h.uuid = d2.host_uuid - AND d2.operation_type = :install - AND(d2.status IS NULL - OR d2.status = :pending) - AND d2.declaration_name NOT IN (:reserved_names) - AND NOT EXISTS ( - SELECT - 1 - FROM - host_mdm_apple_declarations d3 - WHERE - h.uuid = d3.host_uuid - AND d3.operation_type = :install - AND d3.status = :failed - AND d3.declaration_name NOT IN (:reserved_names))) THEN - 'declarations_pending' - WHEN EXISTS ( - SELECT - 1 - FROM - host_mdm_apple_declarations d4 - WHERE - h.uuid = d4.host_uuid - AND d4.operation_type = :install - AND d4.status = :verifying - AND d4.declaration_name NOT IN (:reserved_names) - AND NOT EXISTS ( - SELECT - 1 - FROM - host_mdm_apple_declarations d5 - WHERE (h.uuid = d5.host_uuid - AND d5.operation_type = :install - AND d5.declaration_name NOT IN (:reserved_names) - AND(d5.status IS NULL - OR d5.status IN(:pending, :failed))))) THEN - 'declarations_verifying' - WHEN EXISTS ( - SELECT - 1 - FROM - host_mdm_apple_declarations d6 - WHERE - h.uuid = d6.host_uuid - AND d6.operation_type = :install - AND d6.status = :verified - AND d6.declaration_name NOT IN (:reserved_names) - AND NOT EXISTS ( - SELECT - 1 - FROM - host_mdm_apple_declarations d7 - WHERE (h.uuid = d7.host_uuid - AND d7.operation_type = :install - AND d7.declaration_name NOT IN (:reserved_names) - AND(d7.status IS NULL - OR d7.status IN(:pending, :failed, :verifying))))) THEN - 'declarations_verified' - ELSE - '' - END` - - arg := map[string]any{ - "install": fleet.MDMOperationTypeInstall, - "verifying": fleet.MDMDeliveryVerifying, - "failed": fleet.MDMDeliveryFailed, - "verified": fleet.MDMDeliveryVerified, - "pending": fleet.MDMDeliveryPending, - "reserved_names": fleetmdm.ListFleetReservedMacOSDeclarationNames(), - } - query, args, err := sqlx.Named(declNamedStmt, arg) - if err != nil { - return "", nil, fmt.Errorf("subqueryAppleDeclarationStatus: %w", err) - } - query, args, err = sqlx.In(query, args...) - if err != nil { - return "", nil, fmt.Errorf("subqueryAppleDeclarationStatus resolve IN: %w", err) - } - - return query, args, nil +// sqlJoinMDMAppleProfilesStatus returns a SQL snippet that can be used to join a table derived from +// host_mdm_apple_profiles (grouped by host_uuid and status) and the hosts table. For each host_uuid, +// it derives a boolean value for each status category. The value will be 1 if the host has any +// profile in the given status category. Separate columns are used for status of the filevault profile +// vs. all other profiles. The snippet assumes the hosts table to be aliased as 'h'. +func sqlJoinMDMAppleProfilesStatus() string { + // NOTE: To make this snippet reusable, we're not using sqlx.Named here because it would + // complicate usage in other queries (e.g., list hosts). + var ( + failed = fmt.Sprintf("'%s'", string(fleet.MDMDeliveryFailed)) + pending = fmt.Sprintf("'%s'", string(fleet.MDMDeliveryPending)) + verifying = fmt.Sprintf("'%s'", string(fleet.MDMDeliveryVerifying)) + verified = fmt.Sprintf("'%s'", string(fleet.MDMDeliveryVerified)) + install = fmt.Sprintf("'%s'", string(fleet.MDMOperationTypeInstall)) + filevault = fmt.Sprintf("'%s'", mobileconfig.FleetFileVaultPayloadIdentifier) + ) + return ` + LEFT JOIN ( + -- profile statuses grouped by host uuid, boolean value will be 1 if host has any profile with the given status + -- filevault profiles are treated separately + SELECT + host_uuid, + MAX( IF((status IS NULL OR status = ` + pending + `) AND profile_identifier != ` + filevault + `, 1, 0)) AS prof_pending, + MAX( IF(status = ` + failed + ` AND profile_identifier != ` + filevault + `, 1, 0)) AS prof_failed, + MAX( IF(status = ` + verifying + ` AND profile_identifier != ` + filevault + ` AND operation_type = ` + install + `, 1, 0)) AS prof_verifying, + MAX( IF(status = ` + verified + ` AND profile_identifier != ` + filevault + ` AND operation_type = ` + install + `, 1, 0)) AS prof_verified, + MAX( IF((status IS NULL OR status = ` + pending + `) AND profile_identifier = ` + filevault + `, 1, 0)) AS fv_pending, + MAX( IF(status = ` + failed + ` AND profile_identifier = ` + filevault + `, 1, 0)) AS fv_failed, + MAX( IF(status = ` + verifying + ` AND profile_identifier = ` + filevault + ` AND operation_type = ` + install + `, 1, 0)) AS fv_verifying, + MAX( IF(status = ` + verified + ` AND profile_identifier = ` + filevault + ` AND operation_type = ` + install + `, 1, 0)) AS fv_verified + FROM + host_mdm_apple_profiles + GROUP BY + host_uuid) hmap ON h.uuid = hmap.host_uuid +` } -func subqueryOSSettingsStatusMac() (string, []any, error) { - var profArgs []any - profFailed, profFailedArgs, err := subqueryAppleProfileStatus(fleet.MDMDeliveryFailed) - if err != nil { - return "", nil, err - } - profArgs = append(profArgs, profFailedArgs...) - - profPending, profPendingArgs, err := subqueryAppleProfileStatus(fleet.MDMDeliveryPending) - if err != nil { - return "", nil, err - } - profArgs = append(profArgs, profPendingArgs...) - - profVerifying, profVerifyingArgs, err := subqueryAppleProfileStatus(fleet.MDMDeliveryVerifying) - if err != nil { - return "", nil, err - } - profArgs = append(profArgs, profVerifyingArgs...) - - profVerified, profVerifiedArgs, err := subqueryAppleProfileStatus(fleet.MDMDeliveryVerified) - if err != nil { - return "", nil, err - } - profArgs = append(profArgs, profVerifiedArgs...) - - profStmt := fmt.Sprintf(` - CASE WHEN EXISTS (%s) THEN - 'profiles_failed' - WHEN EXISTS (%s) THEN - 'profiles_pending' - WHEN EXISTS (%s) THEN - 'profiles_verifying' - WHEN EXISTS (%s) THEN - 'profiles_verified' - ELSE - '' - END`, - profFailed, - profPending, - profVerifying, - profVerified, +// sqlJoinMDMAppleDeclarationsStatus returns a SQL snippet that can be used to join a table derived from +// host_mdm_apple_declarations (grouped by host_uuid and status) and the hosts table. For each host_uuid, +// it derives a boolean value for each status category. The value will be 1 if the host has any +// declaration in the given status category. The snippet assumes the hosts table to be aliased as 'h'. +func sqlJoinMDMAppleDeclarationsStatus() string { + // NOTE: To make this snippet reusable, we're not using sqlx.Named here because it would + // complicate usage in other queries (e.g., list hosts). + var ( + failed = fmt.Sprintf("'%s'", string(fleet.MDMDeliveryFailed)) + pending = fmt.Sprintf("'%s'", string(fleet.MDMDeliveryPending)) + verifying = fmt.Sprintf("'%s'", string(fleet.MDMDeliveryVerifying)) + verified = fmt.Sprintf("'%s'", string(fleet.MDMDeliveryVerified)) + install = fmt.Sprintf("'%s'", string(fleet.MDMOperationTypeInstall)) + reservedDeclNames = fmt.Sprintf("'%s', '%s', '%s'", fleetmdm.FleetMacOSUpdatesProfileName, fleetmdm.FleetIOSUpdatesProfileName, fleetmdm.FleetIPadOSUpdatesProfileName) ) - - declStmt, declArgs, err := subqueryAppleDeclarationStatus() - if err != nil { - return "", nil, err - } - - stmt := fmt.Sprintf(` - CASE (%s) - WHEN 'profiles_failed' THEN - 'failed' - WHEN 'profiles_pending' THEN ( - CASE (%s) - WHEN 'declarations_failed' THEN - 'failed' - ELSE - 'pending' - END) - WHEN 'profiles_verifying' THEN ( - CASE (%s) - WHEN 'declarations_failed' THEN - 'failed' - WHEN 'declarations_pending' THEN - 'pending' - ELSE - 'verifying' - END) - WHEN 'profiles_verified' THEN ( - CASE (%s) - WHEN 'declarations_failed' THEN - 'failed' - WHEN 'declarations_pending' THEN - 'pending' - WHEN 'declarations_verifying' THEN - 'verifying' - ELSE - 'verified' - END) - ELSE - REPLACE((%s), 'declarations_', '') - END`, profStmt, declStmt, declStmt, declStmt, declStmt) - - args := append(profArgs, declArgs...) - args = append(args, declArgs...) - args = append(args, declArgs...) - args = append(args, declArgs...) - - // FIXME(roberto): we found issues in MySQL 5.7.17 (only that version, - // which we must support for now) with prepared statements on this - // query. The results returned by the DB were always different what - // expected unless the arguments are inlined in the query. - // - // We decided to do this given: - // - // - The time constraints we were given to develop DDM - // - The fact that all the variables in this query are really strings managed by us - // - The imminent deprecation of MySQL 5.7 - return fmt.Sprintf(strings.Replace(stmt, "?", "'%s'", -1), args...), []any{}, nil + return ` + LEFT JOIN ( + -- declaration statuses grouped by host uuid, boolean value will be 1 if host has any declaration with the given status + SELECT + host_uuid, + MAX( IF((status IS NULL OR status = ` + pending + `), 1, 0)) AS decl_pending, + MAX( IF(status = ` + failed + `, 1, 0)) AS decl_failed, + MAX( IF(status = ` + verifying + ` , 1, 0)) AS decl_verifying, + MAX( IF(status = ` + verified + ` , 1, 0)) AS decl_verified + FROM + host_mdm_apple_declarations + WHERE + operation_type = ` + install + ` AND declaration_name NOT IN(` + reservedDeclNames + `) + GROUP BY + host_uuid) hmad ON h.uuid = hmad.host_uuid +` } func (ds *Datastore) GetMDMAppleProfilesSummary(ctx context.Context, teamID *uint) (*fleet.MDMProfilesSummary, error) { - subquery, args, err := subqueryOSSettingsStatusMac() - if err != nil { - return nil, ctxerr.Wrap(ctx, err, "building os settings subquery") - } - - sqlFmt := ` + stmt := ` SELECT - %s as status, - COUNT(id) as count + COUNT(id) AS count, + %s AS status FROM - hosts h -WHERE platform = 'darwin' OR platform = 'ios' OR platform = 'ipados' -GROUP BY status, team_id HAVING status IN (?, ?, ?, ?) AND %s` - - args = append(args, fleet.MDMDeliveryFailed, fleet.MDMDeliveryPending, fleet.MDMDeliveryVerifying, fleet.MDMDeliveryVerified) + hosts h + %s + %s + LEFT JOIN host_disk_encryption_keys hdek ON h.id = hdek.host_id +WHERE + platform IN('darwin', 'ios', 'ipad_os') AND %s +GROUP BY + status HAVING status IS NOT NULL` teamFilter := "team_id IS NULL" if teamID != nil && *teamID > 0 { - teamFilter = "team_id = ?" - args = append(args, *teamID) + teamFilter = fmt.Sprintf("team_id = %d", *teamID) } - stmt := fmt.Sprintf(sqlFmt, subquery, teamFilter) + stmt = fmt.Sprintf(stmt, sqlCaseMDMAppleStatus(), sqlJoinMDMAppleProfilesStatus(), sqlJoinMDMAppleDeclarationsStatus(), teamFilter) var dest []struct { Count uint `db:"count"` Status string `db:"status"` } - err = sqlx.SelectContext(ctx, ds.reader(ctx), &dest, stmt, args...) - if err != nil { + if err := sqlx.SelectContext(ctx, ds.reader(ctx), &dest, stmt); err != nil { return nil, err } diff --git a/server/datastore/mysql/hosts.go b/server/datastore/mysql/hosts.go index 0b3a0e498362..ee23749de80a 100644 --- a/server/datastore/mysql/hosts.go +++ b/server/datastore/mysql/hosts.go @@ -1114,6 +1114,14 @@ func (ds *Datastore) applyHostFilters( whereParams = append(whereParams, microsoft_mdm.MDMDeviceStateEnrolled) } + mdmAppleProfilesStatusJoin := "" + mdmAppleDeclarationsStatusJoin := "" + if opt.OSSettingsFilter.IsValid() || + opt.MacOSSettingsFilter.IsValid() { + mdmAppleProfilesStatusJoin = sqlJoinMDMAppleProfilesStatus() + mdmAppleDeclarationsStatusJoin = sqlJoinMDMAppleDeclarationsStatus() + } + sqlStmt += fmt.Sprintf( `FROM hosts h LEFT JOIN host_seen_times hst ON (h.id = hst.host_id) @@ -1128,6 +1136,8 @@ func (ds *Datastore) applyHostFilters( %s %s %s + %s + %s %s WHERE TRUE AND %s AND %s AND %s AND %s `, @@ -1142,6 +1152,8 @@ func (ds *Datastore) applyHostFilters( munkiJoin, displayNameJoin, connectedToFleetJoin, + mdmAppleProfilesStatusJoin, + mdmAppleDeclarationsStatusJoin, // Conditions ds.whereFilterHostsByTeams(filter, "h"), @@ -1304,15 +1316,9 @@ func filterHostsByMacOSSettingsStatus(sql string, opt fleet.HostListOptions, par whereStatus += ` AND h.team_id IS NULL` } - subqueryStatus, paramsStatus, err := subqueryOSSettingsStatusMac() - if err != nil { - return "", nil, err - } - - whereStatus += fmt.Sprintf(` AND %s = ?`, subqueryStatus) - paramsStatus = append(paramsStatus, opt.MacOSSettingsFilter) + whereStatus += fmt.Sprintf(` AND %s = ?`, sqlCaseMDMAppleStatus()) - return sql + whereStatus, append(params, paramsStatus...), nil + return sql + whereStatus, append(params, opt.MacOSSettingsFilter), nil } func filterHostsByMacOSDiskEncryptionStatus(sql string, opt fleet.HostListOptions, params []interface{}) (string, []interface{}) { @@ -1364,13 +1370,9 @@ func (ds *Datastore) filterHostsByOSSettingsStatus(sql string, opt fleet.HostLis AND ((h.platform = 'windows' AND (%s)) OR ((h.platform = 'darwin' OR h.platform = 'ios' OR h.platform = 'ipados') AND (%s)))` - whereMacOS, paramsMacOS, err := subqueryOSSettingsStatusMac() - if err != nil { - return "", nil, err - } - whereMacOS += ` = ?` - // ensure the host has MDM turned on - paramsMacOS = append(paramsMacOS, opt.OSSettingsFilter) + // construct the WHERE for macOS + whereMacOS = fmt.Sprintf(`(%s) = ?`, sqlCaseMDMAppleStatus()) + paramsMacOS := []any{opt.OSSettingsFilter} // construct the WHERE for windows whereWindows = `hmdm.is_server = 0` diff --git a/server/datastore/mysql/labels.go b/server/datastore/mysql/labels.go index d604b286a4aa..5ac777be3939 100644 --- a/server/datastore/mysql/labels.go +++ b/server/datastore/mysql/labels.go @@ -638,6 +638,12 @@ func (ds *Datastore) applyHostLabelFilters(ctx context.Context, filter fleet.Tea joinParams = append(joinParams, microsoft_mdm.MDMDeviceStateEnrolled) } + if opt.OSSettingsFilter.IsValid() || + opt.MacOSSettingsFilter.IsValid() { + query += sqlJoinMDMAppleProfilesStatus() + query += sqlJoinMDMAppleDeclarationsStatus() + } + query += fmt.Sprintf(` WHERE lm.label_id = ? AND %s `, ds.whereFilterHostsByTeams(filter, "h")) whereParams = append(whereParams, lid)