Fixed a race where a host could silently revert to its previous team#44074
Fixed a race where a host could silently revert to its previous team#44074
Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
/agentic_review |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughUpdateHost was changed to stop writing the host's team_id during refreshes to avoid a lost-update race that could revert admin-initiated team transfers. Tests across mysql packages were updated to persist host-team membership via AddHostsToTeam instead of setting Host.TeamID + UpdateHost. A regression test was added to ensure updateHostFunc (covering ds.UpdateHost and ds.SerialUpdateHost) does not overwrite hosts.team_id. A single release-note line documents the fix. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Fixes a lost-update race where UpdateHost could overwrite a concurrent admin team transfer by writing a stale team_id back to the hosts row (Resolves #44071).
Changes:
- Stop
Datastore.UpdateHostfrom updatinghosts.team_id(team membership is now exclusively managed via team transfer/enrollment paths). - Update affected tests to use
AddHostsToTeamfor team assignment instead of relying onUpdateHostto persistTeamID. - Add a regression test ensuring
UpdateHostcannot clobber (or resurrect)team_idduring concurrent transfers.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| server/datastore/mysql/hosts.go | Removes team_id from the UpdateHost UPDATE statement and documents the rationale. |
| server/datastore/mysql/hosts_test.go | Adds regression coverage to ensure UpdateHost does not write team_id. |
| server/service/hosts_test.go | Adjusts test setup to assign teams via AddHostsToTeam. |
| server/datastore/mysql/mdm_test.go | Adjusts Windows MDM test hosts’ team assignment to use AddHostsToTeam. |
| server/datastore/mysql/host_certificate_templates_test.go | Updates “team transfer” simulation to use AddHostsToTeam (not UpdateHost). |
| server/datastore/mysql/certificate_templates_test.go | Uses AddHostsToTeam for team assignment before certificate template assertions. |
| server/datastore/mysql/android_test.go | Uses AddHostsToTeam for team assignment in Android MDM profile send tests. |
| changes/44071-team-transfer-race-with-update-host | Adds release note entry for the bug fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #44074 +/- ##
==========================================
+ Coverage 66.75% 66.78% +0.02%
==========================================
Files 2623 2621 -2
Lines 211148 211121 -27
Branches 9386 9386
==========================================
+ Hits 140959 140992 +33
+ Misses 57357 57314 -43
+ Partials 12832 12815 -17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Related issue: Resolves #44071
Verified fix in loadtest.
Checklist for submitter
If some of the following don't apply, delete the relevant line.
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing
Summary by CodeRabbit
Bug Fixes
Tests