From b8acdfacdf00a0f6d2dc796caedf9912481db417 Mon Sep 17 00:00:00 2001 From: Sarah Gillespie <73313222+gillespi314@users.noreply.github.com> Date: Tue, 20 May 2025 14:22:40 -0500 Subject: [PATCH] Fix MDM lifecycle bug when deleting multiple hosts (#29278) --- changes/29119-restore-dep-hosts | 2 + server/service/hosts.go | 14 ++- server/service/integration_mdm_dep_test.go | 117 +++++++++++++++++++++ 3 files changed, 130 insertions(+), 3 deletions(-) create mode 100644 changes/29119-restore-dep-hosts diff --git a/changes/29119-restore-dep-hosts b/changes/29119-restore-dep-hosts new file mode 100644 index 0000000000..5371e8924d --- /dev/null +++ b/changes/29119-restore-dep-hosts @@ -0,0 +1,2 @@ +- Fixed bug where Fleet failed to restore some "pending" hosts (i.e. hosts that remained assigned to + Fleet in Apple Business Manager) when multiple hosts are deleted from Fleet. diff --git a/server/service/hosts.go b/server/service/hosts.go index 09bf680dc3..adb73cd491 100644 --- a/server/service/hosts.go +++ b/server/service/hosts.go @@ -332,16 +332,24 @@ func (svc *Service) DeleteHosts(ctx context.Context, ids []uint, filter *map[str } mdmLifecycle := mdmlifecycle.New(svc.ds, svc.logger) + lifecycleErrs := []error{} + serialsWithErrs := []string{} for _, host := range hosts { if fleet.MDMSupported(host.Platform) { - err := mdmLifecycle.Do(ctx, mdmlifecycle.HostOptions{ + if err := mdmLifecycle.Do(ctx, mdmlifecycle.HostOptions{ Action: mdmlifecycle.HostActionDelete, Host: host, Platform: host.Platform, - }) - return err + }); err != nil { + lifecycleErrs = append(lifecycleErrs, err) + serialsWithErrs = append(serialsWithErrs, host.HardwareSerial) + } } } + if len(lifecycleErrs) > 0 { + msg := fmt.Sprintf("failed to recreate pending host records for one or more MDM devices: %+v", serialsWithErrs) + return ctxerr.Wrap(ctx, errors.Join(lifecycleErrs...), msg) + } return nil } diff --git a/server/service/integration_mdm_dep_test.go b/server/service/integration_mdm_dep_test.go index 08690da062..a4afeff389 100644 --- a/server/service/integration_mdm_dep_test.go +++ b/server/service/integration_mdm_dep_test.go @@ -3726,3 +3726,120 @@ func (s *integrationMDMTestSuite) TestSetupExperienceFlowCancelScript() { require.Len(t, cmds, 1) require.Equal(t, "DeviceConfigured", cmds[0].Command.RequestType) } + +func (s *integrationMDMTestSuite) TestDeleteMultipleHostsPendingDEP() { + t := s.T() + ctx := context.Background() + + devices := []godep.Device{ + {SerialNumber: uuid.New().String(), Model: "MacBook Pro", OS: "osx", OpType: "added"}, + {SerialNumber: uuid.New().String(), Model: "MacBook Mini", OS: "osx", OpType: "added"}, + {SerialNumber: uuid.New().String(), Model: "MacBook Mini", OS: "osx", OpType: "added"}, + {SerialNumber: uuid.New().String(), Model: "MacBook Mini", OS: "osx", OpType: "added"}, + } + profileAssignmentReqs := []profileAssignmentReq{} + + s.setSkipWorkerJobs(t) + s.enableABM(t.Name()) + s.mockDEPResponse(t.Name(), http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + encoder := json.NewEncoder(w) + switch r.URL.Path { + case "/session": + err := encoder.Encode(map[string]string{"auth_session_token": "xyz"}) + require.NoError(t, err) + case "/profile": + err := encoder.Encode(godep.ProfileResponse{ProfileUUID: uuid.New().String()}) + require.NoError(t, err) + case "/server/devices": + // This endpoint is used to get an initial list of + // devices, return a single device + err := encoder.Encode(godep.DeviceResponse{Devices: devices[:1]}) + require.NoError(t, err) + case "/devices/sync": + // This endpoint is polled over time to sync devices from + // ABM, send a repeated serial and a new one + err := encoder.Encode(godep.DeviceResponse{Devices: devices, Cursor: "foo"}) + require.NoError(t, err) + case "/profile/devices": + b, err := io.ReadAll(r.Body) + require.NoError(t, err) + var prof profileAssignmentReq + require.NoError(t, json.Unmarshal(b, &prof)) + profileAssignmentReqs = append(profileAssignmentReqs, prof) + var resp godep.ProfileResponse + resp.ProfileUUID = prof.ProfileUUID + resp.Devices = make(map[string]string, len(prof.Devices)) + for _, device := range prof.Devices { + resp.Devices[device] = string(fleet.DEPAssignProfileResponseSuccess) + } + err = encoder.Encode(resp) + require.NoError(t, err) + default: + _, _ = w.Write([]byte(`{}`)) + } + })) + + // query all hosts + listHostsRes := listHostsResponse{} + s.DoJSON("GET", "/api/latest/fleet/hosts", nil, http.StatusOK, &listHostsRes) + require.Empty(t, listHostsRes.Hosts) // no hosts yet + + // trigger a profile sync + s.runDEPSchedule() + + // all devices should be returned from the hosts endpoint + listHostsRes = listHostsResponse{} + s.DoJSON("GET", "/api/latest/fleet/hosts", nil, http.StatusOK, &listHostsRes) + require.Len(t, listHostsRes.Hosts, len(devices)) + + bySerial := make(map[string]*fleet.HostResponse, len(devices)) + for _, host := range listHostsRes.Hosts { + bySerial[host.HardwareSerial] = &host + } + + for _, device := range devices { + h, ok := bySerial[device.SerialNumber] + require.True(t, ok) + // entries for all hosts get created in the host_dep_assignments table + hdep, err := s.ds.GetHostDEPAssignment(ctx, h.ID) + require.NoError(t, err) + require.NotNil(t, hdep) + require.Nil(t, hdep.DeletedAt) + } + + // to confirm that hosts actually get recreated, we'll manually set the hosts.created_at + // time to 48 hours in the past (hopefully this interval is adequate avoid any flakiness in + // timestamp comparisons) + target := listHostsRes.Hosts[3] + then := target.CreatedAt.Add(-48 * time.Hour) + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(ctx, `UPDATE hosts SET created_at = ? WHERE hardware_serial = ?`, then, target.HardwareSerial) + return err + }) + hostResp := getHostResponse{} + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d", target.ID), nil, http.StatusOK, &hostResp) + require.Equal(t, then, hostResp.Host.CreatedAt) + + // delete all hosts + deleteIds := []uint{} + for _, host := range listHostsRes.Hosts { + deleteIds = append(deleteIds, host.ID) + } + s.Do("POST", "/api/latest/fleet/hosts/delete", deleteHostsRequest{IDs: deleteIds}, http.StatusOK) + + // all devices should be restored as pending DEP hosts + gotHosts := listHostsResponse{} + s.DoJSON("GET", "/api/latest/fleet/hosts", nil, http.StatusOK, &gotHosts) + require.Len(t, gotHosts.Hosts, len(listHostsRes.Hosts)) + for _, host := range gotHosts.Hosts { + h, ok := bySerial[host.HardwareSerial] + require.True(t, ok) + require.Equal(t, h.ID, host.ID) // host restored with the same id + + if host.HardwareSerial == target.HardwareSerial { + require.NotEqual(t, then, host.CreatedAt) + } + + } +}