Update host expiry logic to account for Apple MDM checkin times (#34698)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #32499 # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved host expiry logic to correctly identify and preserve Apple MDM-enrolled hosts that don't check in through Orbit, preventing unintended host deletions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -0,0 +1 @@
|
||||
* Update host expiry logic to not delete macOS hosts that checkin via MDM protocol but not via orbit
|
||||
@@ -3545,18 +3545,23 @@ func (ds *Datastore) CleanupExpiredHosts(ctx context.Context) ([]uint, error) {
|
||||
// so instead, we get the ids one by one and delete things one by one
|
||||
// it might take longer, but it should lock only the row we need.
|
||||
//
|
||||
// host_seen_time entries are not available for ios/ipados devices, since they're updated on
|
||||
// osquery check-in. Instead we fall back to detail_updated_at, which is updated every time a
|
||||
// full detail refetch happens. For the detail_updated_at value, we consider server.NeverTimestamp
|
||||
// to be nullish because this value is set as the default in some scenarios, in which
|
||||
// case we will fall back to the created_at timestamp.
|
||||
// host_seen_time entries are not available for ios/ipados/android devices, since they're updated on
|
||||
// osquery check-in. Instead we fall back to the MDM protocol last_seen_at, then detail_updated_at,
|
||||
// which is updated every time a full detail refetch happens. For the detail_updated_at value, we
|
||||
// consider server.NeverTimestamp to be nullish because this value is set as the default in some scenarios,
|
||||
// in which case we will fall back to the created_at timestamp.
|
||||
// Additionally, COALESCE(GREATEST(COALESCE...)) with seen_time and last_seen at ensures that we get the greater
|
||||
// value if both are set(GREATEST normally returning NULL if either operand is NULL) but still treat as NULL if
|
||||
// neither is set. This ensures that we cover hosts that for some reason stop checking in via OSQuery but keep
|
||||
// checking in via MDM
|
||||
//
|
||||
// To avoid prematurely deleting hosts that are ingested from Apple DEP, we cross-reference the
|
||||
// host_dep_assignments table.
|
||||
findHostsSql := `SELECT h.id FROM hosts h
|
||||
LEFT JOIN host_seen_times hst ON h.id = hst.host_id
|
||||
LEFT JOIN host_dep_assignments hda ON h.id = hda.host_id
|
||||
WHERE COALESCE(hst.seen_time, NULLIF(h.detail_updated_at, '` + server.NeverTimestamp + `'), h.created_at) < DATE_SUB(NOW(), INTERVAL ? DAY)
|
||||
LEFT JOIN nano_enrollments ne ON ne.id=h.uuid AND ne.type IN ('Device', 'User Enrollment (Device)')
|
||||
WHERE COALESCE(GREATEST(COALESCE(hst.seen_time, ne.last_seen_at), COALESCE(ne.last_seen_at, hst.seen_time)), NULLIF(h.detail_updated_at, '` + server.NeverTimestamp + `'), h.created_at) < DATE_SUB(NOW(), INTERVAL ? DAY)
|
||||
AND (hda.host_id IS NULL OR hda.deleted_at IS NOT NULL)`
|
||||
|
||||
var allIdsToDelete []uint
|
||||
|
||||
@@ -127,6 +127,7 @@ func TestHosts(t *testing.T) {
|
||||
{"HostsExpiration", testHostsExpiration},
|
||||
{"IOSHostExpiration", testIOSHostsExpiration},
|
||||
{"DEPHostExpiration", testDEPHostsExpiration},
|
||||
{"AppleMDMHostWithoutOrbitExpiration", testAppleMDMHostsWithoutOrbitExpiration},
|
||||
{"TeamHostsExpiration", testTeamHostsExpiration},
|
||||
{"HostsIncludesScheduledQueriesInPackStats", testHostsIncludesScheduledQueriesInPackStats},
|
||||
{"HostsAllPackStats", testHostsAllPackStats},
|
||||
@@ -5217,6 +5218,91 @@ func testIOSHostsExpiration(t *testing.T, ds *Datastore) {
|
||||
require.Len(t, hosts, 5)
|
||||
}
|
||||
|
||||
func testAppleMDMHostsWithoutOrbitExpiration(t *testing.T, ds *Datastore) {
|
||||
// Apple MDM enrolled hosts(macOS devices specifically) which never get orbit
|
||||
// installed and also don't have our usual REFETCH commands run(which only run
|
||||
// on iOS/iPadOS devices)
|
||||
ctx := context.Background()
|
||||
hostExpiryWindow := 70
|
||||
|
||||
ac, err := ds.AppConfig(ctx)
|
||||
require.NoError(t, err)
|
||||
|
||||
ac.HostExpirySettings.HostExpiryEnabled = false
|
||||
ac.HostExpirySettings.HostExpiryWindow = hostExpiryWindow
|
||||
|
||||
err = ds.SaveAppConfig(ctx, ac)
|
||||
require.NoError(t, err)
|
||||
|
||||
never, err := time.Parse("2006-01-02 15:04:05", server.NeverTimestamp)
|
||||
require.NoError(t, err)
|
||||
|
||||
for i := 0; i < 10; i++ {
|
||||
platform := "darwin"
|
||||
nanoLastSeen := time.Now()
|
||||
if i >= 5 {
|
||||
nanoLastSeen = nanoLastSeen.Add(time.Duration(-1*(hostExpiryWindow+1)*24) * time.Hour)
|
||||
}
|
||||
|
||||
host, err := ds.NewHost(ctx, &fleet.Host{
|
||||
LabelUpdatedAt: time.Now(),
|
||||
PolicyUpdatedAt: time.Now(),
|
||||
DetailUpdatedAt: never, // Hosts will get this timestamp when enrolling only via MDM
|
||||
UUID: fmt.Sprintf("%d", i),
|
||||
Hostname: fmt.Sprintf("foo.local%d", i),
|
||||
Platform: platform,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
nanoEnroll(t, ds, host, platform == "darwin")
|
||||
|
||||
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
|
||||
// Hosts that only enroll via MDM get no host_seen_times
|
||||
_, err := q.ExecContext(ctx, `DELETE FROM host_seen_times WHERE host_id = ?`, host.ID)
|
||||
require.NoError(t, err)
|
||||
r, err := q.ExecContext(ctx,
|
||||
`UPDATE nano_enrollments SET last_seen_at = ? WHERE device_id = ?`,
|
||||
nanoLastSeen, host.UUID)
|
||||
require.NoError(t, err)
|
||||
rowsAffected, _ := r.RowsAffected()
|
||||
require.GreaterOrEqual(t, rowsAffected, int64(1))
|
||||
return err
|
||||
})
|
||||
}
|
||||
|
||||
filter := fleet.TeamFilter{User: test.UserAdmin}
|
||||
|
||||
hosts := listHostsCheckCount(t, ds, filter, fleet.HostListOptions{}, 10)
|
||||
require.Len(t, hosts, 10)
|
||||
|
||||
deleted, err := ds.CleanupExpiredHosts(ctx)
|
||||
require.NoError(t, err)
|
||||
|
||||
// host expiration is still disabled so nothing should have been deleted
|
||||
require.Len(t, deleted, 0)
|
||||
listHostsCheckCount(t, ds, filter, fleet.HostListOptions{}, 10)
|
||||
|
||||
// once enabled, it works
|
||||
ac.HostExpirySettings.HostExpiryEnabled = true
|
||||
err = ds.SaveAppConfig(context.Background(), ac)
|
||||
require.NoError(t, err)
|
||||
|
||||
deleted, err = ds.CleanupExpiredHosts(ctx)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, deleted, 5)
|
||||
|
||||
hosts = listHostsCheckCount(t, ds, filter, fleet.HostListOptions{}, 5)
|
||||
require.Len(t, hosts, 5)
|
||||
|
||||
// Calling it again deletes nothing
|
||||
deleted, err = ds.CleanupExpiredHosts(ctx)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, deleted, 0)
|
||||
|
||||
hosts = listHostsCheckCount(t, ds, filter, fleet.HostListOptions{}, 5)
|
||||
require.Len(t, hosts, 5)
|
||||
}
|
||||
|
||||
func testDEPHostsExpiration(t *testing.T, ds *Datastore) {
|
||||
ctx := context.Background()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user