MDM Windows push perf fixes (#46917)

<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #46567 

Loadtest feedback: reducing the number of `UPDATE
mdm_windows_enrollments e SET e.has_pending_commands` writes.

# Checklist for submitter

## Testing

- [x] Added/updated automated tests
- [x] Where appropriate, [automated tests simulate multiple hosts and
test for host
isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing)
(updates to one hosts's records do not affect another)

- [x] QA'd all new/changed functionality manually

## Database migrations

- [x] Checked schema for all modified table for columns that will
auto-update timestamps during migration.
- [x] Confirmed that updating the timestamps is acceptable, and will not
cause unwanted side effects.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Fixed Windows MDM enrolled devices to correctly track pending
commands. Acknowledged commands are now properly removed from the
pending list, and the system accurately reflects command status after
device acknowledgment. Command cleanup for processed requests is now
more efficient.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Victor Lyuboslavsky
2026-06-08 15:34:35 +01:00
committed by GitHub
parent 684d87d9f9
commit cd5c44db72
11 changed files with 280 additions and 64 deletions
+1 -4
View File
@@ -400,10 +400,7 @@ WHERE
WHERE
mwe.host_uuid IN (?)
AND NOT EXISTS (
SELECT 1 FROM windows_mdm_command_results r
WHERE r.enrollment_id = wq.enrollment_id AND r.command_uuid = wq.command_uuid
)
AND wq.acked_at IS NULL
%[1]s
+54 -41
View File
@@ -89,6 +89,7 @@ func (ds *Datastore) MDMWindowsGetEnrolledDeviceWithDeviceID(ctx context.Context
credentials_acknowledged,
poll_schedule_relaxed,
fleetd_sync_capable,
has_pending_commands,
created_at,
updated_at,
host_uuid
@@ -303,16 +304,16 @@ func (ds *Datastore) GetMDMWindowsHostConfigState(ctx context.Context, hostUUID
// windowsMDMHasPendingCommandsExpr computes whether an enrollment (aliased e) has queued, unacknowledged commands other than internal
// poll-schedule Replaces. It backs the denormalized mdm_windows_enrollments.has_pending_commands flag. The single ? placeholder is the
// poll-schedule LocURI to exclude.
//
// Pending is "queued with acked_at still NULL": the ack transaction stamps acked_at on the rows it records results for
// (soft dequeue), so this is an index probe on (enrollment_id, acked_at) over only the actually-pending rows.
const windowsMDMHasPendingCommandsExpr = `EXISTS (
SELECT 1
FROM windows_mdm_command_queue q
JOIN windows_mdm_commands c ON c.command_uuid = q.command_uuid
WHERE q.enrollment_id = e.id
AND q.acked_at IS NULL
AND c.target_loc_uri <> ?
AND NOT EXISTS (
SELECT 1 FROM windows_mdm_command_results r
WHERE r.enrollment_id = q.enrollment_id AND r.command_uuid = q.command_uuid
)
)`
// recomputeMDMWindowsHasPendingCommandsByEnrollmentIDs refreshes the has_pending_commands flag for the given enrollments by evaluating the
@@ -325,8 +326,14 @@ func (ds *Datastore) recomputeMDMWindowsHasPendingCommandsByEnrollmentIDs(ctx co
}
// Batch to match the bounded queue-insert path; a single IN (...) over a large fan-out could exceed MySQL's placeholder limit.
return common_mysql.BatchProcessSimple(enrollmentIDs, windowsMDMCommandQueueBatchSize, func(batch []uint) error {
// The has_pending_commands = 1 guard is defense-in-depth, not the primary idle-path optimization: the management
// session already skips the refresh entirely (no statement at all) when the flag loaded at session start is 0,
// so this clause only matters for callers that do not pre-gate, or when the loaded flag was stale. It is safe
// because the refresh only exists for the 1 -> 0 transition - the enqueue paths own 0 -> 1 by setting the flag
// directly.
stmt, args, err := sqlx.In(
`UPDATE mdm_windows_enrollments e SET e.has_pending_commands = `+windowsMDMHasPendingCommandsExpr+` WHERE e.id IN (?)`,
`UPDATE mdm_windows_enrollments e SET e.has_pending_commands = `+windowsMDMHasPendingCommandsExpr+
` WHERE e.id IN (?) AND e.has_pending_commands = 1`,
syncml.DMClientPollIntervalLocURI, batch,
)
if err != nil {
@@ -339,6 +346,16 @@ func (ds *Datastore) recomputeMDMWindowsHasPendingCommandsByEnrollmentIDs(ctx co
})
}
// MDMWindowsRefreshHasPendingCommands recomputes the denormalized has_pending_commands flag for one enrollment on the
// writer. The management session flow calls it at most once per OMA-DM session: only when the pending-commands fetch for
// the reply comes back empty (the session has drained the queue, so the flag may flip to 0). Mid-session messages skip it
// entirely. The flag provably stays 1 while commands remain queued, having been set by the enqueue paths. Running it
// outside the ack transaction is safe: the EXISTS recompute is authoritative against the writer, so a concurrent enqueue
// between the fetch and this refresh still lands on has_pending_commands = 1.
func (ds *Datastore) MDMWindowsRefreshHasPendingCommands(ctx context.Context, enrollmentID uint) error {
return ds.recomputeMDMWindowsHasPendingCommandsByEnrollmentIDs(ctx, ds.writer(ctx), []uint{enrollmentID})
}
// markMDMWindowsHasPendingCommandsByEnrollmentIDs sets has_pending_commands = 1 for the given enrollments without recomputing the EXISTS.
// The enqueue paths use it because inserting a non-poll command means a pending command now exists by construction; only the acknowledgment
// path, which can clear the last pending command, needs the full recompute.
@@ -890,21 +907,12 @@ func (ds *Datastore) getEnrollmentIDsByHostUUIDOrDeviceIDDB(ctx context.Context,
// MDMWindowsGetPendingCommands retrieves all commands awaiting execution for the given enrollment.
func (ds *Datastore) MDMWindowsGetPendingCommands(ctx context.Context, enrollmentID uint) ([]*fleet.MDMWindowsCommand, error) {
// Fast path: probe the queue. An MDM management session runs this query on every
// check-in, and the overwhelming majority of devices have nothing queued, so short-circuit
// before paying for the full scan + anti-join. SELECT EXISTS always returns a row, so the
// idle path does not go through a sql.ErrNoRows branch.
// Queue rows now persist after ACK (cleaned by periodic GC), so the probe
// must also exclude rows that already have a result in
// windows_mdm_command_results.
// Fast path: probe the queue. An MDM management session runs this query on every check-in, and the overwhelming majority of
// devices have nothing queued, so short-circuit before paying for the fetch JOIN. SELECT EXISTS always returns a row, so the idle
// path does not go through a sql.ErrNoRows branch. This is an index probe on (enrollment_id, acked_at).
const probe = `SELECT EXISTS(
SELECT 1 FROM windows_mdm_command_queue wmcq
WHERE wmcq.enrollment_id = ?
AND NOT EXISTS (
SELECT 1 FROM windows_mdm_command_results wmcr
WHERE wmcr.enrollment_id = wmcq.enrollment_id
AND wmcr.command_uuid = wmcq.command_uuid
)
WHERE wmcq.enrollment_id = ? AND wmcq.acked_at IS NULL
)`
var hasPending bool
if err := sqlx.GetContext(ctx, ds.reader(ctx), &hasPending, probe, enrollmentID); err != nil {
@@ -929,14 +937,7 @@ ON
wmc.command_uuid = wmcq.command_uuid
WHERE
wmcq.enrollment_id = ? AND
NOT EXISTS (
SELECT 1
FROM
windows_mdm_command_results wmcr
WHERE
wmcr.enrollment_id = wmcq.enrollment_id AND
wmcr.command_uuid = wmcq.command_uuid
)
wmcq.acked_at IS NULL
ORDER BY
wmc.created_at ASC
`
@@ -1132,10 +1133,25 @@ ON DUPLICATE KEY UPDATE
}
}
// Acknowledgments may have drained this enrollment's pending commands; refresh the denormalized flag in
// this same transaction so the orbit-config hot path reflects it on the next check-in.
if err := ds.recomputeMDMWindowsHasPendingCommandsByEnrollmentIDs(ctx, tx, []uint{enrolledDevice.ID}); err != nil {
return err
// Soft-dequeue the commands we just recorded results for: stamp acked_at on exactly those queue rows, in the
// same transaction as the results insert so "has a result row" and "acked_at set" can never disagree. This is
// the ONLY path that inserts windows_mdm_command_results; any new results-insert path must stamp acked_at too,
// or the rows will look pending forever under the acked_at IS NULL predicates. The periodic GC range-deletes
// stamped rows after an hour. COALESCE preserves the first ack time on duplicate acks so the GC age floor is
// measured from the original acknowledgment.
//
// The has_pending_commands recompute deliberately does NOT happen here: it runs at most once per OMA-DM session (in
// getManagementResponse, when the reply-building pending-commands fetch finds no non-poll commands remaining and the flag was
// loaded as 1 at session start), not once per message.
markStmt, markArgs, err := sqlx.In(
`UPDATE windows_mdm_command_queue SET acked_at = COALESCE(acked_at, NOW(6)) WHERE enrollment_id = ? AND command_uuid IN (?)`,
enrolledDevice.ID, matchingCmdUUIDs,
)
if err != nil {
return ctxerr.Wrap(ctx, err, "build acked_at stamp for queue rows")
}
if _, err := tx.ExecContext(ctx, markStmt, markArgs...); err != nil {
return ctxerr.Wrap(ctx, err, "stamp acked_at on queue rows")
}
return nil
@@ -3951,18 +3967,15 @@ func (ds *Datastore) MDMWindowsAcknowledgeEnrolledDeviceCredentials(ctx context.
func (ds *Datastore) CleanupWindowsMDMCommandQueue(ctx context.Context) error {
const batchSize = 1000
// Multi-table DELETE does not support LIMIT directly, so we use a
// subquery to select the rows to delete in batches.
// Acknowledged rows carry acked_at (stamped in the ack transaction), so GC is a single-table index range delete on
// (enrollment_id, acked_at)'s acked_at part. The 1-hour age floor preserves the resend/debugging window the join-based predicate
// had via r.created_at. ORDER BY makes the LIMIT deterministic (oldest acknowledged rows first) and keeps the optimizer on the
// acked_at index for a bounded range delete.
const stmt = `
DELETE q FROM windows_mdm_command_queue q
INNER JOIN (
SELECT q2.enrollment_id, q2.command_uuid
FROM windows_mdm_command_queue q2
INNER JOIN windows_mdm_command_results r
ON r.enrollment_id = q2.enrollment_id AND r.command_uuid = q2.command_uuid
WHERE r.created_at < NOW() - INTERVAL 1 HOUR
LIMIT ?
) batch ON batch.enrollment_id = q.enrollment_id AND batch.command_uuid = q.command_uuid`
DELETE FROM windows_mdm_command_queue
WHERE acked_at IS NOT NULL AND acked_at < NOW() - INTERVAL 1 HOUR
ORDER BY acked_at
LIMIT ?`
const maxBatches = 500 // cap total work per cron tick (500k rows)
var totalDeleted int64
exhausted := true
+28 -6
View File
@@ -2115,7 +2115,7 @@ func testMDMWindowsGetHostConfigState(t *testing.T, ds *Datastore) {
state, err = ds.GetMDMWindowsHostConfigState(ctx, d.HostUUID)
require.NoError(t, err)
require.True(t, state.HasPendingCommands)
// (The ack -> not-pending transition, including the recompute-on-ack wiring inside MDMWindowsSaveResponse, is covered by testSaveResponse.)
// The ack -> not-pending transition (SaveResponse soft-dequeue plus the per-session MDMWindowsRefreshHasPendingCommands) is covered by testSaveResponse.
// awaiting_configuration is reflected in the combined read
_, err = ds.SetMDMWindowsAwaitingConfiguration(ctx, d.MDMDeviceID, fleet.WindowsMDMAwaitingConfigurationNone, fleet.WindowsMDMAwaitingConfigurationPending)
@@ -3910,8 +3910,10 @@ func testSaveResponse(t *testing.T, ds *Datastore) {
[]string{enrolledDevice1.MDMDeviceID, enrolledDevice2.MDMDeviceID, enrolledDevice3.MDMDeviceID}, cmd)
require.NoError(t, err)
// has_pending_commands is a denormalized flag: queuing a non-poll command marks it, and MDMWindowsSaveResponse must recompute it to false
// when the command is acked. hasPending reads it through the same getter the orbit-config check-in uses.
// has_pending_commands is a denormalized flag: queuing a non-poll command marks it; MDMWindowsSaveResponse soft-dequeues
// the acked rows (stamps acked_at); and MDMWindowsRefreshHasPendingCommands - called by the service once per session,
// when the pending fetch comes back empty - recomputes it to false. hasPending reads the flag through the same getter
// the orbit-config check-in uses.
hasPending := func(d *fleet.MDMWindowsEnrolledDevice) bool {
st, err := ds.GetMDMWindowsHostConfigState(context.Background(), d.HostUUID)
require.NoError(t, err)
@@ -3932,7 +3934,16 @@ VALUES (?, 'pending', 'install', ?, 'disable-onedrive', ?)`, enrolledDevice1.Hos
// Do test
_, err = ds.MDMWindowsSaveResponse(context.Background(), enrolledDevice1, enrichedSyncML, []string{})
require.NoError(t, err)
require.False(t, hasPending(enrolledDevice1), "MDMWindowsSaveResponse must recompute has_pending_commands to false after the ack")
// SaveResponse soft-dequeues the acked rows (acked_at stamped), so the pending fetch is empty
pendingCmds, err := ds.MDMWindowsGetPendingCommands(context.Background(), enrolledDevice1.ID)
require.NoError(t, err)
require.Empty(t, pendingCmds, "acked commands must be soft-dequeued from the pending fetch")
require.True(t, hasPending(enrolledDevice1), "MDMWindowsSaveResponse must not recompute the flag mid-session")
// The service calls the refresh when the pending fetch comes back empty (session drained); the flag flips to false.
require.NoError(t, ds.MDMWindowsRefreshHasPendingCommands(context.Background(), enrolledDevice1.ID))
require.False(t, hasPending(enrolledDevice1), "refresh must recompute has_pending_commands to false after the ack")
// Verify results
results, err := ds.GetMDMWindowsCommandResults(context.Background(), cmd.CommandUUID, "")
@@ -5929,21 +5940,32 @@ func testCleanupWindowsMDMCommandQueue(t *testing.T, ds *Datastore) {
return nil
})
// Insert a result for cmd1 with a timestamp >1 hour ago (eligible for GC).
// Insert a result for cmd1 acked >1 hour ago (eligible for GC). The ack transaction stamps acked_at alongside the
// results insert (soft dequeue), so the direct-SQL setup mirrors both writes.
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
_, err := q.ExecContext(ctx, `
INSERT INTO windows_mdm_command_results (enrollment_id, command_uuid, raw_result, status_code, response_id, created_at)
VALUES (?, ?, '<Status/>', '200', ?, NOW() - INTERVAL 2 HOUR)`,
dev.ID, cmd1.CommandUUID, responseID)
if err != nil {
return err
}
_, err = q.ExecContext(ctx, `UPDATE windows_mdm_command_queue SET acked_at = NOW() - INTERVAL 2 HOUR WHERE enrollment_id = ? AND command_uuid = ?`,
dev.ID, cmd1.CommandUUID)
return err
})
// Insert a result for cmd2 with a recent timestamp (not yet eligible for GC).
// Insert a result for cmd2 acked just now (not yet eligible for GC).
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
_, err := q.ExecContext(ctx, `
INSERT INTO windows_mdm_command_results (enrollment_id, command_uuid, raw_result, status_code, response_id, created_at)
VALUES (?, ?, '<Status/>', '200', ?, NOW())`,
dev.ID, cmd2.CommandUUID, responseID)
if err != nil {
return err
}
_, err = q.ExecContext(ctx, `UPDATE windows_mdm_command_queue SET acked_at = NOW() WHERE enrollment_id = ? AND command_uuid = ?`,
dev.ID, cmd2.CommandUUID)
return err
})
@@ -0,0 +1,44 @@
package tables
import (
"database/sql"
"fmt"
)
func init() {
MigrationClient.AddMigration(Up_20260606051849, Down_20260606051849)
}
func Up_20260606051849(tx *sql.Tx) error {
// acked_at is the soft-dequeue marker for the Windows MDM command queue. Queue rows persist after acknowledgment until the
// periodic GC, so every predicate that needs "pending = queued and unacknowledged" had to anti-join windows_mdm_command_results
// per row. With the wake model recomputing the denormalized has_pending_commands flag during ack processing, that anti-join walk
// over acked-but-not-yet-GC'd rows became the top writer statement under bulk profile waves. The ack transaction now stamps
// acked_at on exactly the rows it records results for, turning every pending predicate into an index probe on (enrollment_id,
// acked_at) and the GC into an index range delete. Two indexes: the composite serves the hot-path pending probes (WHERE
// enrollment_id = ? AND acked_at IS NULL, index-only), and the single-column index serves the GC's range delete (WHERE acked_at <
// ?), which cannot use the second column of the composite.
if _, err := tx.Exec(`ALTER TABLE windows_mdm_command_queue
ADD COLUMN acked_at DATETIME(6) NULL DEFAULT NULL,
ADD INDEX idx_win_mdm_cmd_queue_enrollment_acked (enrollment_id, acked_at),
ADD INDEX idx_win_mdm_cmd_queue_acked (acked_at)`); err != nil {
return fmt.Errorf("add acked_at to windows_mdm_command_queue: %w", err)
}
// Backfill: rows acknowledged before this migration (result row exists) must not become visible as pending under the
// new acked_at IS NULL predicate, or every previously delivered command would be re-sent on the next session. Use the
// result row's created_at so the GC's 1-hour age floor keeps its meaning. The join is PK-to-PK; the work is bounded
// by the acked-but-not-yet-GC'd backlog (at most the GC interval's worth of traffic).
if _, err := tx.Exec(`UPDATE windows_mdm_command_queue q
JOIN windows_mdm_command_results r
ON r.enrollment_id = q.enrollment_id AND r.command_uuid = q.command_uuid
SET q.acked_at = r.created_at
WHERE q.acked_at IS NULL`); err != nil {
return fmt.Errorf("backfill acked_at from windows_mdm_command_results: %w", err)
}
return nil
}
func Down_20260606051849(tx *sql.Tx) error {
return nil
}
@@ -0,0 +1,67 @@
package tables
import (
"testing"
"github.com/stretchr/testify/require"
)
func TestUp_20260606051849(t *testing.T) {
db := applyUpToPrev(t)
insertEnrollment := func(deviceID string) int64 {
res, err := db.Exec(`INSERT INTO mdm_windows_enrollments
(mdm_device_id, mdm_hardware_id, device_state, device_type, device_name, enroll_type, enroll_user_id, enroll_proto_version, enroll_client_version)
VALUES (?, ?, '', '', '', '', '', '', '')`, deviceID, deviceID+"-hw")
require.NoError(t, err)
id, err := res.LastInsertId()
require.NoError(t, err)
return id
}
insertCommand := func(uuid string) {
_, err := db.Exec(`INSERT INTO windows_mdm_commands (command_uuid, raw_command, target_loc_uri) VALUES (?, '<Get></Get>', './Some/Node')`, uuid)
require.NoError(t, err)
}
queue := func(enrollID int64, uuid string) {
_, err := db.Exec(`INSERT INTO windows_mdm_command_queue (enrollment_id, command_uuid) VALUES (?, ?)`, enrollID, uuid)
require.NoError(t, err)
}
// A: queued, never acknowledged -> acked_at must stay NULL (still pending).
enrollA := insertEnrollment("deviceA")
insertCommand("cmd-a")
queue(enrollA, "cmd-a")
// B: queued and acknowledged before the migration (result row exists) -> acked_at must be backfilled from the
// result's created_at so the row does not reappear as pending.
enrollB := insertEnrollment("deviceB")
insertCommand("cmd-b")
queue(enrollB, "cmd-b")
respRes, err := db.Exec(`INSERT INTO windows_mdm_responses (enrollment_id, raw_response) VALUES (?, '<resp/>')`, enrollB)
require.NoError(t, err)
respID, err := respRes.LastInsertId()
require.NoError(t, err)
_, err = db.Exec(`INSERT INTO windows_mdm_command_results (enrollment_id, command_uuid, raw_result, response_id, status_code)
VALUES (?, 'cmd-b', '<r/>', ?, '200')`, enrollB, respID)
require.NoError(t, err)
applyNext(t, db)
ackedAt := func(enrollID int64, uuid string) *string {
var v *string
require.NoError(t, db.Get(&v, `SELECT acked_at FROM windows_mdm_command_queue WHERE enrollment_id = ? AND command_uuid = ?`, enrollID, uuid))
return v
}
require.Nil(t, ackedAt(enrollA, "cmd-a"), "unacknowledged queue row must keep acked_at NULL")
ackedB := ackedAt(enrollB, "cmd-b")
require.NotNil(t, ackedB, "acknowledged queue row must be backfilled with acked_at")
// Compare in SQL rather than as strings: created_at is a second-resolution timestamp while acked_at is DATETIME(6),
// so the driver renders them with different fractional-second suffixes.
var backfillMatches bool
require.NoError(t, db.Get(&backfillMatches, `SELECT q.acked_at = r.created_at
FROM windows_mdm_command_queue q
JOIN windows_mdm_command_results r ON r.enrollment_id = q.enrollment_id AND r.command_uuid = q.command_uuid
WHERE q.enrollment_id = ? AND q.command_uuid = 'cmd-b'`, enrollB))
require.True(t, backfillMatches, "backfilled acked_at must equal the result row's created_at")
}
File diff suppressed because one or more lines are too long
+5
View File
@@ -2148,6 +2148,11 @@ type Datastore interface {
// MDMWindowsGetPendingCommands returns all pending commands for the given enrollment.
MDMWindowsGetPendingCommands(ctx context.Context, enrollmentID uint) ([]*MDMWindowsCommand, error)
// MDMWindowsRefreshHasPendingCommands recomputes the denormalized has_pending_commands flag for the enrollment.
// Called at most once per OMA-DM session, when the pending-commands fetch comes back empty (the session has drained
// the queue and the flag may flip to 0); mid-session messages skip it since the flag provably stays 1.
MDMWindowsRefreshHasPendingCommands(ctx context.Context, enrollmentID uint) error
// MDMWindowsSaveResponse saves a full response for the given enrollment.
MDMWindowsSaveResponse(ctx context.Context, enrolledDevice *MDMWindowsEnrolledDevice, enrichedSyncML EnrichedSyncML, commandIDsBeingResent []string) (*MDMWindowsSaveResponseResult, error)
+7 -3
View File
@@ -880,9 +880,13 @@ type MDMWindowsEnrolledDevice struct {
PollScheduleRelaxed bool `db:"poll_schedule_relaxed"`
// FleetdSyncCapable is the last-observed CapabilityWindowsMDMSync value for this enrollment, persisted by the orbit-config endpoint. The
// management session has no capability header, so it gates poll relaxation on this persisted flag rather than re-deriving the capability.
FleetdSyncCapable bool `db:"fleetd_sync_capable"`
CreatedAt time.Time `db:"created_at"`
UpdatedAt time.Time `db:"updated_at"`
FleetdSyncCapable bool `db:"fleetd_sync_capable"`
// HasPendingCommands is the denormalized pending-commands flag as loaded at session start. The management session uses it to gate the
// per-session refresh: when it is already false and the pending fetch is empty, the refresh is skipped so idle check-ins do zero
// writer-side statements.
HasPendingCommands bool `db:"has_pending_commands"`
CreatedAt time.Time `db:"created_at"`
UpdatedAt time.Time `db:"updated_at"`
}
func (e MDMWindowsEnrolledDevice) AuthzType() string {
+12
View File
@@ -1338,6 +1338,8 @@ type MDMWindowsEnqueueCommandAndUpsertHostProfilesFunc func(ctx context.Context,
type MDMWindowsGetPendingCommandsFunc func(ctx context.Context, enrollmentID uint) ([]*fleet.MDMWindowsCommand, error)
type MDMWindowsRefreshHasPendingCommandsFunc func(ctx context.Context, enrollmentID uint) error
type MDMWindowsSaveResponseFunc func(ctx context.Context, enrolledDevice *fleet.MDMWindowsEnrolledDevice, enrichedSyncML fleet.EnrichedSyncML, commandIDsBeingResent []string) (*fleet.MDMWindowsSaveResponseResult, error)
type GetMDMWindowsCommandResultsFunc func(ctx context.Context, commandUUID string, hostUUID string) ([]*fleet.MDMCommandResult, error)
@@ -4040,6 +4042,9 @@ type DataStore struct {
MDMWindowsGetPendingCommandsFunc MDMWindowsGetPendingCommandsFunc
MDMWindowsGetPendingCommandsFuncInvoked bool
MDMWindowsRefreshHasPendingCommandsFunc MDMWindowsRefreshHasPendingCommandsFunc
MDMWindowsRefreshHasPendingCommandsFuncInvoked bool
MDMWindowsSaveResponseFunc MDMWindowsSaveResponseFunc
MDMWindowsSaveResponseFuncInvoked bool
@@ -9737,6 +9742,13 @@ func (s *DataStore) MDMWindowsGetPendingCommands(ctx context.Context, enrollment
return s.MDMWindowsGetPendingCommandsFunc(ctx, enrollmentID)
}
func (s *DataStore) MDMWindowsRefreshHasPendingCommands(ctx context.Context, enrollmentID uint) error {
s.mu.Lock()
s.MDMWindowsRefreshHasPendingCommandsFuncInvoked = true
s.mu.Unlock()
return s.MDMWindowsRefreshHasPendingCommandsFunc(ctx, enrollmentID)
}
func (s *DataStore) MDMWindowsSaveResponse(ctx context.Context, enrolledDevice *fleet.MDMWindowsEnrolledDevice, enrichedSyncML fleet.EnrichedSyncML, commandIDsBeingResent []string) (*fleet.MDMWindowsSaveResponseResult, error) {
s.mu.Lock()
s.MDMWindowsSaveResponseFuncInvoked = true
+31 -6
View File
@@ -2012,21 +2012,27 @@ func handleResendingAlreadyExistsCommands(ctx context.Context, svc *Service, alr
return topLevelExists, nil
}
// getPendingMDMCmds returns the list of pending MDM commands for the given enrollment.
func (svc *Service) getPendingMDMCmds(ctx context.Context, enrollmentID uint) ([]*mdm_types.SyncMLCmd, error) {
// getPendingMDMCmds returns the list of pending MDM commands for the given enrollment, plus onlyPollCmdsPending: true
// when everything still pending (if anything) is an internal poll-schedule Replace.
func (svc *Service) getPendingMDMCmds(ctx context.Context, enrollmentID uint) ([]*mdm_types.SyncMLCmd, bool, error) {
pendingCmds, err := svc.ds.MDMWindowsGetPendingCommands(ctx, enrollmentID)
if err != nil {
return nil, fmt.Errorf("getting incoming cmds %w", err)
return nil, false, fmt.Errorf("getting incoming cmds %w", err)
}
// Converting the pending commands to its target SyncML types
var cmds []*mdm_types.SyncMLCmd
onlyPollCmdsPending := true
for _, pendingCmd := range pendingCmds {
isPollCmd := pendingCmd.TargetLocURI == syncml.DMClientPollIntervalLocURI
if !isPollCmd {
onlyPollCmdsPending = false
}
// The raw MDM command may contain a $FLEET_SECRET_XXX, the value of which should never be exposed or stored unencrypted.
rawCommandWithSecret, err := svc.ds.ExpandEmbeddedSecrets(ctx, string(pendingCmd.RawCommand))
if err != nil {
// This error should never happen since we validate the presence of needed secrets on profile upload.
return nil, ctxerr.Wrap(ctx, err, "expanding embedded secrets for Windows pending commands")
return nil, false, ctxerr.Wrap(ctx, err, "expanding embedded secrets for Windows pending commands")
}
parsedCmds, err := fleet.UnmarshallMultiTopLevelXMLProfile([]byte(rawCommandWithSecret))
if err != nil {
@@ -2038,7 +2044,7 @@ func (svc *Service) getPendingMDMCmds(ctx context.Context, enrollmentID uint) ([
}
}
return cmds, nil
return cmds, onlyPollCmdsPending, nil
}
// createResponseSyncML returns a valid SyncML message
@@ -2114,12 +2120,31 @@ func (svc *Service) getManagementResponse(ctx context.Context, reqMsg *fleet.Syn
}
// Process the pending operations and get the MDM response protocol commands
pendingCmds, err := svc.getPendingMDMCmds(ctx, enrolledDevice.ID)
pendingCmds, onlyPollCmdsPending, err := svc.getPendingMDMCmds(ctx, enrolledDevice.ID)
if err != nil {
return nil, fmt.Errorf("message processing error %w", err)
}
resPendingCmds = pendingCmds
// Per-session has_pending_commands maintenance: refresh the denormalized flag only when everything still
// pending (if anything) is an internal poll-schedule Replace, which the flag's definition excludes. While
// non-poll commands remain queued the flag provably stays 1 (set by the enqueue paths), so mid-session
// messages skip the recompute entirely.
//
// The HasPendingCommands gate (as loaded at session start) keeps idle check-ins at zero writer-side statements:
// when the flag was already 0 and nothing is pending, there is no 1 -> 0 transition to record. A flag stranded
// at 1 by an aborted session still self-heals - it loads as 1 on the next session and the refresh runs. A
// mid-session enqueue that flips 0 -> 1 after this row was loaded needs no refresh either: its commands are
// genuinely pending, so 1 is already correct. Best-effort: a failed refresh only delays the flag flip until
// the next session, so log and continue rather than failing the device's response.
if onlyPollCmdsPending && enrolledDevice.HasPendingCommands {
if err := svc.ds.MDMWindowsRefreshHasPendingCommands(ctx, enrolledDevice.ID); err != nil {
svc.logger.ErrorContext(ctx, "refresh windows mdm has_pending_commands", "err", err,
"enrollment_id", enrolledDevice.ID)
ctxerr.Handle(ctx, err)
}
}
// Build ESP (Enrollment Status Page) commands for Windows Autopilot devices. Only run for trusted requests
// so we don't leak ESP state to unauthenticated devices.
if enrolledDevice.AwaitingConfiguration != fleet.WindowsMDMAwaitingConfigurationNone {
+25 -1
View File
@@ -1114,12 +1114,17 @@ func TestRekeyWindowsDevice(t *testing.T) {
var credsHash *[]byte
const testEnrollmentID uint = 123
// Captured before the local `syncml` string variable below shadows the syncml package.
pollScheduleLocURI := syncml.DMClientPollIntervalLocURI
ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFunc = func(ctx context.Context, mdmDeviceID string) (*fleet.MDMWindowsEnrolledDevice, error) {
return &fleet.MDMWindowsEnrolledDevice{
ID: testEnrollmentID,
MDMDeviceID: "device",
HostUUID: "host-uuid-123",
CredentialsHash: credsHash,
// Loaded as 1 so the per-session refresh fires when the pending fetch returns empty (asserted at the end of
// the test); a device loaded with the flag at 0 skips the refresh entirely.
HasPendingCommands: true,
}, nil
}
@@ -1233,7 +1238,24 @@ func TestRekeyWindowsDevice(t *testing.T) {
// WE only need to mock this as we short-circuit when challenging or invalid creds
ds.MDMWindowsGetPendingCommandsFunc = func(ctx context.Context, enrollmentID uint) ([]*fleet.MDMWindowsCommand, error) {
require.Equal(t, testEnrollmentID, enrollmentID)
return []*fleet.MDMWindowsCommand{}, nil
// A still-pending internal poll-schedule Replace must NOT block the per-session refresh: the
// has_pending_commands flag excludes poll commands by definition, so the refresh gate must too.
return []*fleet.MDMWindowsCommand{
{
CommandUUID: "poll-schedule-cmd-uuid",
RawCommand: []byte(`<Replace><CmdID>poll-schedule-cmd-uuid</CmdID></Replace>`),
TargetLocURI: pollScheduleLocURI,
},
}, nil
}
ds.ExpandEmbeddedSecretsFunc = func(ctx context.Context, document string) (string, error) {
return document, nil
}
// No NON-POLL pending commands means the session has drained the flag-relevant queue, so the service refreshes the
// denormalized has_pending_commands flag (at most once per session).
ds.MDMWindowsRefreshHasPendingCommandsFunc = func(ctx context.Context, enrollmentID uint) error {
require.Equal(t, testEnrollmentID, enrollmentID)
return nil
}
ds.GetWindowsMDMCommandsForResendingFunc = func(ctx context.Context, deviceID string, failedCommandIds []string) ([]*fleet.MDMWindowsCommand, error) {
return []*fleet.MDMWindowsCommand{}, nil
@@ -1276,6 +1298,8 @@ func TestRekeyWindowsDevice(t *testing.T) {
require.NotNil(t, res)
require.Equal(t, 1, ackCalled, "acknowledge should have been called once")
require.True(t, ds.MDMWindowsRefreshHasPendingCommandsFuncInvoked,
"refresh should run when no non-poll commands are pending, even with a poll-schedule command still queued")
}
func hashMDMCredentials(username, password, nonce string) []byte {