Fixed a bug where fleetd could not start on-demand Windows MDM session#48765
Conversation
|
@coderabbitai full review |
|
/agentic_review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
This PR fixes Windows on-demand MDM sync startup failures on newer Windows builds by broadening how orbit identifies Fleet’s MDM enrollment in the registry (accepting non-zero EnrollmentState values like 3, instead of only 1).
Changes:
- Replace the hard-coded
EnrollmentState == 1check with a helper that treats any non-zero enrollment state as enrolled, while still scoping byProviderID == Fleetand validating the subkey GUID format. - Centralize the enrollment GUID validation regex + matching logic in a shared helper (
isActiveFleetEnrollment). - Add unit tests covering accepted/rejected enrollment states and malformed registry subkey names.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| orbit/pkg/update/execwinapi.go | Adds shared helper (isActiveFleetEnrollment) and GUID validation used to identify Fleet’s Windows MDM enrollment. |
| orbit/pkg/update/execwinapi_windows.go | Updates registry scanning logic to use the new helper, allowing non-zero EnrollmentState values. |
| orbit/pkg/update/execwinapi_test.go | Adds unit tests validating the new enrollment-selection rules (including state 3). |
| changes/48760-windows-mdm-enrollment-state | User-visible change entry (content excluded from review per policy). |
Files excluded by content exclusion policy (1)
- changes/48760-windows-mdm-enrollment-state
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
WalkthroughChangesThis change fixes Windows MDM on-demand enrollment detection in fleetd. A new Related issues: Suggested labels: bug, orbit, mdm, windows Suggested reviewers: (based on ownership of orbit/pkg/update package; none specified in provided context) 🐰 A registry key with state of three, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #48765 +/- ##
==========================================
+ Coverage 68.02% 68.03% +0.01%
==========================================
Files 3680 3681 +1
Lines 233959 233970 +11
Branches 12456 12456
==========================================
+ Hits 159145 159178 +33
+ Misses 60496 60481 -15
+ Partials 14318 14311 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
| import ( | ||
| "regexp" | ||
|
|
||
| "github.com/fleetdm/fleet/v4/server/mdm/microsoft/syncml" |
There was a problem hiding this comment.
This import used to be a dependency of windows only (//go:build windows). This makes syncml a dependency of the orbit update package on all platforms.
There was a problem hiding this comment.
@ksykulev nice catch.
I checked the dependencies and we are already pulling it in via: server/fleet → server/mdm/microsoft/syncml
So, there is no new dependency or binary-size impact.
Related issue: Resolves #48760
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
fleetd/orbit/Fleet Desktop
Note: changes are Windows only; only verified/tested these items for Linux and Windows
runtime.GOOSis used as needed to isolate changesSummary by CodeRabbit