don't filter DEP hosts by OS before ingesting and improve logs (#9815)

Related to https://github.com/fleetdm/fleet/issues/9653 I couldn't find
any documentation to back this up, but I have a strong suspicion that
the `os` field in the device sync response might come empty in some
scenarios (particularly, when a laptop is brand new, which is hard to
reproduce 😅)

My thoughts are:

1. For the recently purchased MacBooks,
`IngestMDMAppleDevicesFromDEPSync` didn't create an entry in the
database, BUT `nanodep.Assigner.ProcessDeviceResponse` correctly
assigned a DEP profile (the devices were able to enroll). Both methods
filter by `op_type` but only ours filters by `os`.
2. I think this is safe-ish to do, as you will normally assign a MDM
server per device type in ABM

![image](https://user-images.githubusercontent.com/4419992/218732609-0936e3a9-cadf-4485-9aa4-af2c9398cff9.png)
3. I have added extra logs to try to prove this hypothesis next time a
brand new device comes in, let's keep an eye on and re-evaluate this
approach.
This commit is contained in:
Roberto Dip 2023-02-14 10:23:19 -03:00 committed by GitHub
parent 35e513adf7
commit 0f5a35061e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 19 additions and 11 deletions

View file

@ -380,10 +380,12 @@ func insertMDMAppleHostDB(
func (ds *Datastore) IngestMDMAppleDevicesFromDEPSync(ctx context.Context, devices []godep.Device) (int64, error) {
if len(devices) < 1 {
level.Debug(ds.logger).Log("msg", "ingesting devices from DEP received < 1 device, skipping", "len(devices)", len(devices))
return 0, nil
}
filteredDevices := filterMDMAppleDevices(devices)
filteredDevices := filterMDMAppleDevices(devices, ds.logger)
if len(filteredDevices) < 1 {
level.Debug(ds.logger).Log("msg", "ingesting devices from DEP filtered all devices, skipping", "len(devices)", len(devices))
return 0, nil
}
@ -397,6 +399,12 @@ func (ds *Datastore) IngestMDMAppleDevicesFromDEPSync(ctx context.Context, devic
team, err := ds.TeamByName(ctx, name)
switch {
case errors.Is(err, sql.ErrNoRows):
level.Debug(ds.logger).Log(
"msg",
"ingesting devices from DEP: unable to find default team assigned in config, the devices won't be assigned to a team",
"team_name",
name,
)
// If the team doesn't exist, we still ingest the device, but it won't
// belong to any team.
case err != nil:
@ -590,14 +598,9 @@ func (ds *Datastore) UpdateHostTablesOnMDMUnenroll(ctx context.Context, uuid str
})
}
func filterMDMAppleDevices(devices []godep.Device) []godep.Device {
func filterMDMAppleDevices(devices []godep.Device, logger log.Logger) []godep.Device {
var filtered []godep.Device
for _, device := range devices {
// We currently only support macOS devices so we screen out iOS
// and tvOS.
if strings.ToLower(device.OS) != "osx" {
continue
}
// We currently only listen for an op_type of "added", the
// other op_types are ambiguous and it would be needless to
// ingest the device every single time we get an update.
@ -606,8 +609,11 @@ func filterMDMAppleDevices(devices []godep.Device) []godep.Device {
// API call, Empty op_type come from the first call to
// FetchDevices without a cursor.
strings.ToLower(device.OpType) == "" {
level.Debug(logger).Log("msg", "filterMDMAppleDevices: adding device", "serial", device.SerialNumber, "op_type", device.OpType, "os", device.OS)
filtered = append(filtered, device)
continue
}
level.Debug(logger).Log("msg", "filterMDMAppleDevices: skipping device", "serial", device.SerialNumber, "op_type", device.OpType, "os", device.OS)
}
return filtered
}

View file

@ -45,17 +45,17 @@ func TestIngestMDMAppleDevicesFromDEPSync(t *testing.T) {
{SerialNumber: "abc", Model: "MacBook Pro", OS: "OSX", OpType: "added"}, // ingested; new serial, macOS, "added" op type
{SerialNumber: "abc", Model: "MacBook Pro", OS: "OSX", OpType: "added"}, // not ingested; duplicate serial
{SerialNumber: hosts[0].HardwareSerial, Model: "MacBook Pro", OS: "OSX", OpType: "added"}, // not ingested; existing serial
{SerialNumber: "ijk", Model: "IPad Pro", OS: "iOS", OpType: "added"}, // not ingested; iOS
{SerialNumber: "tuv", Model: "Apple TV", OS: "tvOS", OpType: "added"}, // not ingested; tvOS
{SerialNumber: "ijk", Model: "MacBook Pro", OS: "", OpType: "added"}, // ingested; empty OS
{SerialNumber: "tuv", Model: "MacBook Pro", OS: "OSX", OpType: "modified"}, // not ingested; op type "modified"
{SerialNumber: "xyz", Model: "MacBook Pro", OS: "OSX", OpType: "updated"}, // not ingested; op type "updated"
{SerialNumber: "xyz", Model: "MacBook Pro", OS: "OSX", OpType: "deleted"}, // not ingested; op type "deleted"
{SerialNumber: "xyz", Model: "MacBook Pro", OS: "OSX", OpType: "added"}, // ingested; new serial, macOS, "added" op type
}
wantSerials = append(wantSerials, "abc", "xyz")
wantSerials = append(wantSerials, "abc", "xyz", "ijk")
n, err := ds.IngestMDMAppleDevicesFromDEPSync(ctx, depDevices)
require.NoError(t, err)
require.Equal(t, int64(2), n) // 2 new hosts ("abc", "xyz")
require.Equal(t, int64(3), n) // 3 new hosts ("abc", "xyz", "ijk")
hosts = listHostsCheckCount(t, ds, fleet.TeamFilter{User: test.UserAdmin}, fleet.HostListOptions{}, len(wantSerials))
gotSerials := []string{}

View file

@ -124,6 +124,8 @@ func NewDEPSyncer(
sentry.CaptureException(err)
case n > 0:
level.Info(kitlog.With(logger)).Log("msg", fmt.Sprintf("added %d new mdm device(s) to pending hosts", n))
case n == 0:
level.Info(kitlog.With(logger)).Log("msg", "no DEP hosts to add")
}
return assigner.ProcessDeviceResponse(ctx, resp)