**Related issue:** Resolves #44629 This folds the connected-to-Fleet check into `GetHostMDM` via a `connected_to_fleet` column that mirrors the existing `IsHostConnectedToFleetMDM` and `hostMDMSelect` conditions, and derives the value in `GetOrbitConfig` from the `host_mdm` data it already fetches. Result: **2 queries → 1** on the orbit check-in hot path, with no semantic change. # Checklist for submitter - [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), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. ## 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 * **Performance Improvements** * Orbit check-ins now determine MDM connection status from existing host MDM data, reducing database work and improving response time. * **Bug Fixes** * Added platform-aware connection detection so Windows, Apple, and Android devices report MDM connectivity more accurately. * Updated related checks and tests to keep connection status consistent across enrollment and unenrollment changes. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -0,0 +1 @@
|
||||
* Improved orbit check-in performance by deriving the Fleet MDM connection state from existing host MDM data instead of running a separate 3-table JOIN query on every check-in for every host.
|
||||
@@ -5118,6 +5118,9 @@ func (ds *Datastore) GetHostMunkiVersion(ctx context.Context, hostID uint) (stri
|
||||
|
||||
func (ds *Datastore) GetHostMDM(ctx context.Context, hostID uint) (*fleet.HostMDM, error) {
|
||||
var hmdm fleet.HostMDM
|
||||
// connected_to_fleet field mirrors IsHostConnectedToFleetMDM (and the connected_to_fleet condition in hostMDMSelect): the
|
||||
// host_mdm row must be enrolled and the platform-specific enrollment record must be active. NOTE: if you change any of these
|
||||
// conditions, also update IsHostConnectedToFleetMDM and the hostMDMSelect constant.
|
||||
err := sqlx.GetContext(ctx, ds.reader(ctx), &hmdm, `
|
||||
SELECT
|
||||
hm.host_id,
|
||||
@@ -5129,9 +5132,27 @@ func (ds *Datastore) GetHostMDM(ctx context.Context, hostID uint) (*fleet.HostMD
|
||||
hm.managed_apple_id,
|
||||
COALESCE(hm.is_server, false) AS is_server,
|
||||
COALESCE(mdms.name, ?) AS name,
|
||||
hdep.assign_profile_response AS dep_profile_assign_status
|
||||
hdep.assign_profile_response AS dep_profile_assign_status,
|
||||
CASE
|
||||
WHEN hm.enrolled = 1 AND h.platform = 'windows' THEN EXISTS (
|
||||
SELECT 1 FROM mdm_windows_enrollments mwe
|
||||
WHERE mwe.host_uuid = h.uuid
|
||||
AND mwe.device_state = '`+microsoft_mdm.MDMDeviceStateEnrolled+`'
|
||||
)
|
||||
WHEN hm.enrolled = 1 AND h.platform IN ('ios', 'ipados', 'darwin') THEN EXISTS (
|
||||
SELECT 1 FROM nano_enrollments ne
|
||||
WHERE ne.id = h.uuid
|
||||
AND ne.enabled = 1
|
||||
AND ne.type IN ('Device', 'User Enrollment (Device)')
|
||||
)
|
||||
WHEN hm.enrolled = 1 AND h.platform = 'android' THEN 1
|
||||
ELSE 0
|
||||
END AS connected_to_fleet
|
||||
FROM
|
||||
host_mdm hm
|
||||
LEFT OUTER JOIN
|
||||
hosts h
|
||||
ON h.id = hm.host_id
|
||||
LEFT OUTER JOIN
|
||||
mobile_device_management_solutions mdms
|
||||
ON hm.mdm_id = mdms.id
|
||||
|
||||
@@ -4023,6 +4023,26 @@ func testAreHostsConnectedToFleetMDM(t *testing.T, ds *Datastore) {
|
||||
|
||||
func testIsHostConnectedToFleetMDM(t *testing.T, ds *Datastore) {
|
||||
ctx := context.Background()
|
||||
|
||||
// requireConnected asserts that IsHostConnectedToFleetMDM and the connected_to_fleet flag computed by GetHostMDM agree with the
|
||||
// expected value. GetOrbitConfig derives the connection state from GetHostMDM instead of a separate IsHostConnectedToFleetMDM
|
||||
// query, so the two must stay in lockstep across every enrollment state. When the host has no host_mdm row, GetHostMDM returns
|
||||
// NotFound and the host cannot be connected.
|
||||
requireConnected := func(t *testing.T, h *fleet.Host, want bool) {
|
||||
t.Helper()
|
||||
connected, err := ds.IsHostConnectedToFleetMDM(ctx, h)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, want, connected)
|
||||
|
||||
mdmInfo, err := ds.GetHostMDM(ctx, h.ID)
|
||||
if err != nil {
|
||||
require.True(t, fleet.IsNotFound(err))
|
||||
require.False(t, want, "host without a host_mdm row cannot be connected to Fleet MDM")
|
||||
return
|
||||
}
|
||||
require.Equal(t, want, mdmInfo.ConnectedToFleet)
|
||||
}
|
||||
|
||||
macH, err := ds.NewHost(ctx, &fleet.Host{
|
||||
Hostname: "macos-test",
|
||||
OsqueryHostID: ptr.String("osquery-macos"),
|
||||
@@ -4032,17 +4052,13 @@ func testIsHostConnectedToFleetMDM(t *testing.T, ds *Datastore) {
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
connected, err := ds.IsHostConnectedToFleetMDM(ctx, macH)
|
||||
require.NoError(t, err)
|
||||
require.False(t, connected)
|
||||
requireConnected(t, macH, false)
|
||||
|
||||
nanoEnroll(t, ds, macH, false)
|
||||
err = ds.SetOrUpdateMDMData(ctx, macH.ID, false, true, "http://foo.com", false, "foo", "", false)
|
||||
require.NoError(t, err)
|
||||
|
||||
connected, err = ds.IsHostConnectedToFleetMDM(ctx, macH)
|
||||
require.NoError(t, err)
|
||||
require.True(t, connected)
|
||||
requireConnected(t, macH, true)
|
||||
|
||||
byodIpadH, err := ds.NewHost(ctx, &fleet.Host{
|
||||
Hostname: "ipados-test",
|
||||
@@ -4057,9 +4073,7 @@ func testIsHostConnectedToFleetMDM(t *testing.T, ds *Datastore) {
|
||||
err = ds.SetOrUpdateMDMData(ctx, byodIpadH.ID, false, true, "http://foo.com", false, "foo", "", false)
|
||||
require.NoError(t, err)
|
||||
|
||||
connected, err = ds.IsHostConnectedToFleetMDM(ctx, byodIpadH)
|
||||
require.NoError(t, err)
|
||||
require.True(t, connected)
|
||||
requireConnected(t, byodIpadH, true)
|
||||
|
||||
windowsH, err := ds.NewHost(ctx, &fleet.Host{
|
||||
Hostname: "windows-test",
|
||||
@@ -4069,9 +4083,7 @@ func testIsHostConnectedToFleetMDM(t *testing.T, ds *Datastore) {
|
||||
Platform: "windows",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
connected, err = ds.IsHostConnectedToFleetMDM(ctx, windowsH)
|
||||
require.NoError(t, err)
|
||||
require.False(t, connected)
|
||||
requireConnected(t, windowsH, false)
|
||||
|
||||
windowsEnrollment := &fleet.MDMWindowsEnrolledDevice{
|
||||
MDMDeviceID: uuid.New().String(),
|
||||
@@ -4091,9 +4103,7 @@ func testIsHostConnectedToFleetMDM(t *testing.T, ds *Datastore) {
|
||||
err = ds.SetOrUpdateMDMData(ctx, windowsH.ID, false, true, "http://foo.com", false, "foo", "", false)
|
||||
require.NoError(t, err)
|
||||
|
||||
connected, err = ds.IsHostConnectedToFleetMDM(ctx, windowsH)
|
||||
require.NoError(t, err)
|
||||
require.True(t, connected)
|
||||
requireConnected(t, windowsH, true)
|
||||
|
||||
// now simulate an un-enrollment without checkout, in this case, osquery reports the host as not-enrolled
|
||||
err = ds.SetOrUpdateMDMData(ctx, macH.ID, false, false, "", false, "", "", false)
|
||||
@@ -4101,13 +4111,8 @@ func testIsHostConnectedToFleetMDM(t *testing.T, ds *Datastore) {
|
||||
err = ds.SetOrUpdateMDMData(ctx, windowsH.ID, false, false, "", false, "", "", false)
|
||||
require.NoError(t, err)
|
||||
|
||||
connected, err = ds.IsHostConnectedToFleetMDM(ctx, macH)
|
||||
require.NoError(t, err)
|
||||
require.False(t, connected)
|
||||
|
||||
connected, err = ds.IsHostConnectedToFleetMDM(ctx, windowsH)
|
||||
require.NoError(t, err)
|
||||
require.False(t, connected)
|
||||
requireConnected(t, macH, false)
|
||||
requireConnected(t, windowsH, false)
|
||||
|
||||
// Simulate the ipad checking out(user removing work account)
|
||||
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
|
||||
@@ -4115,9 +4120,30 @@ func testIsHostConnectedToFleetMDM(t *testing.T, ds *Datastore) {
|
||||
return err
|
||||
})
|
||||
|
||||
connected, err = ds.IsHostConnectedToFleetMDM(ctx, byodIpadH)
|
||||
requireConnected(t, byodIpadH, false)
|
||||
|
||||
// Android: connection is determined solely by host_mdm.enrolled, so the connected_to_fleet column must track enrollment without
|
||||
// any separate enrollment record.
|
||||
androidH, err := ds.NewHost(ctx, &fleet.Host{
|
||||
Hostname: "android-test",
|
||||
OsqueryHostID: new("osquery-android"),
|
||||
NodeKey: new("node-key-android"),
|
||||
UUID: uuid.NewString(),
|
||||
Platform: "android",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.False(t, connected)
|
||||
|
||||
requireConnected(t, androidH, false)
|
||||
|
||||
err = ds.SetOrUpdateMDMData(ctx, androidH.ID, false, true, "http://foo.com", false, fleet.WellKnownMDMFleet, "", false)
|
||||
require.NoError(t, err)
|
||||
|
||||
requireConnected(t, androidH, true)
|
||||
|
||||
err = ds.SetOrUpdateMDMData(ctx, androidH.ID, false, false, "", false, "", "", false)
|
||||
require.NoError(t, err)
|
||||
|
||||
requireConnected(t, androidH, false)
|
||||
}
|
||||
|
||||
// This test now only covers android, as the other platforms no longer rely on the BulkSetPendingMDMHostProfiles,
|
||||
|
||||
@@ -1342,6 +1342,8 @@ type HostMDM struct {
|
||||
// OAuth Bearer token at TokenUpdate time. Apple does not reliably populate
|
||||
// UserLongName on User Enrollment so we don't fall back to it.
|
||||
ManagedAppleID *string `db:"managed_apple_id" json:"-" csv:"-"`
|
||||
// ConnectedToFleet reports whether the host is currently connected to Fleet's MDM.
|
||||
ConnectedToFleet bool `db:"connected_to_fleet" json:"-" csv:"-"`
|
||||
}
|
||||
|
||||
// HasJSONProfileAssigned returns true if Fleet has assigned an ADE/DEP JSON
|
||||
|
||||
@@ -491,16 +491,15 @@ func (svc *Service) GetOrbitConfig(ctx context.Context) (fleet.OrbitConfig, erro
|
||||
return fleet.OrbitConfig{}, err
|
||||
}
|
||||
|
||||
isConnectedToFleetMDM, err := svc.ds.IsHostConnectedToFleetMDM(ctx, host)
|
||||
if err != nil {
|
||||
return fleet.OrbitConfig{}, ctxerr.Wrap(ctx, err, "checking if host is connected to Fleet")
|
||||
}
|
||||
|
||||
mdmInfo, err := svc.ds.GetHostMDM(ctx, host.ID)
|
||||
if err != nil && !errors.Is(err, sql.ErrNoRows) {
|
||||
if err != nil && !fleet.IsNotFound(err) {
|
||||
return fleet.OrbitConfig{}, ctxerr.Wrap(ctx, err, "retrieving host mdm info")
|
||||
}
|
||||
|
||||
// Derive the Fleet-MDM connection state from the host_mdm data fetched above rather than issuing a separate
|
||||
// IsHostConnectedToFleetMDM query.
|
||||
isConnectedToFleetMDM := mdmInfo != nil && mdmInfo.ConnectedToFleet
|
||||
|
||||
// set the host's orbit notifications for macOS MDM
|
||||
var notifs fleet.OrbitConfigNotifications
|
||||
if appConfig.MDM.EnabledAndConfigured && host.IsOsqueryEnrolled() && host.Platform == "darwin" {
|
||||
|
||||
@@ -2,7 +2,6 @@ package service
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"log/slog"
|
||||
@@ -346,6 +345,7 @@ func TestGetOrbitConfigNudge(t *testing.T) {
|
||||
InstalledFromDep: true,
|
||||
Enrolled: true,
|
||||
Name: fleet.WellKnownMDMFleet,
|
||||
ConnectedToFleet: true,
|
||||
}, nil
|
||||
}
|
||||
|
||||
@@ -424,6 +424,7 @@ func TestGetOrbitConfigNudge(t *testing.T) {
|
||||
InstalledFromDep: true,
|
||||
Enrolled: true,
|
||||
Name: fleet.WellKnownMDMFleet,
|
||||
ConnectedToFleet: true,
|
||||
}, nil
|
||||
}
|
||||
|
||||
@@ -500,7 +501,7 @@ func TestGetOrbitConfigNudge(t *testing.T) {
|
||||
return nil, nil
|
||||
}
|
||||
ds.GetHostMDMFunc = func(ctx context.Context, hostID uint) (*fleet.HostMDM, error) {
|
||||
return nil, sql.ErrNoRows
|
||||
return nil, newNotFoundError()
|
||||
}
|
||||
var isHostConnectedToFleet bool
|
||||
ds.IsHostConnectedToFleetMDMFunc = func(ctx context.Context, h *fleet.Host) (bool, error) {
|
||||
@@ -587,6 +588,7 @@ func TestGetOrbitConfigNudge(t *testing.T) {
|
||||
InstalledFromDep: true,
|
||||
Enrolled: true,
|
||||
Name: fleet.WellKnownMDMFleet,
|
||||
ConnectedToFleet: true,
|
||||
}, nil
|
||||
}
|
||||
ds.IsHostPendingEscrowFunc = func(ctx context.Context, hostID uint) bool {
|
||||
@@ -686,7 +688,7 @@ func TestGetOrbitConfigScriptTimeoutFallback(t *testing.T) {
|
||||
return false, nil
|
||||
}
|
||||
ds.GetHostMDMFunc = func(ctx context.Context, hostID uint) (*fleet.HostMDM, error) {
|
||||
return nil, sql.ErrNoRows
|
||||
return nil, newNotFoundError()
|
||||
}
|
||||
ds.IsHostPendingEscrowFunc = func(ctx context.Context, hostID uint) bool {
|
||||
return false
|
||||
@@ -776,6 +778,7 @@ func TestGetSoftwareInstallDetails(t *testing.T) {
|
||||
InstalledFromDep: true,
|
||||
Enrolled: true,
|
||||
Name: fleet.WellKnownMDMFleet,
|
||||
ConnectedToFleet: true,
|
||||
}, nil
|
||||
}
|
||||
|
||||
@@ -1130,7 +1133,7 @@ func TestGetOrbitConfigWindowsSetupExperience(t *testing.T) {
|
||||
return false
|
||||
}
|
||||
ds.GetHostMDMFunc = func(ctx context.Context, hostID uint) (*fleet.HostMDM, error) {
|
||||
return &fleet.HostMDM{Enrolled: true, Name: fleet.WellKnownMDMFleet}, nil
|
||||
return &fleet.HostMDM{Enrolled: true, Name: fleet.WellKnownMDMFleet, ConnectedToFleet: true}, nil
|
||||
}
|
||||
ds.GetHostAwaitingConfigurationFunc = func(ctx context.Context, hostUUID string) (bool, error) {
|
||||
return false, nil
|
||||
|
||||
Reference in New Issue
Block a user