Use webview in MacOS setup experience#33884
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #33884 +/- ##
==========================================
+ Coverage 64.13% 64.21% +0.07%
==========================================
Files 2052 2054 +2
Lines 206102 206553 +451
Branches 6788 6788
==========================================
+ Hits 132183 132628 +445
+ Misses 63541 63510 -31
- Partials 10378 10415 +37
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:
|
There was a problem hiding this comment.
Now that we're using the "My Device" page as the content for the MacOS setup experience, we need to ensure that we have a valid token for the duration of the setup experience. Previously, we only initialized the token and started rotating it if we detected that fleet desktop was enabled. Now, we'll always initialize it (just a single API call) and start rotating if either fleet desktop is enabled OR setup experience is starting (or both). To facilitate this, the token rotation code has been moved into ReadWriter itself, and callers can initiate it with a call to StartRotation(). This is an idempotent function that will start rotation once and return a function to request that the rotation stops; it will only actually stop when every caller has called StopRotation().
| UpdateRunner: updateRunner, RootDir: c.String("root-dir"), Interval: nudgeLaunchInterval, | ||
| })) | ||
| setupExperiencer := setupexperience.NewSetupExperiencer(orbitClient, c.String("root-dir")) | ||
| setupExperiencer := setupexperience.NewSetupExperiencer(orbitClient, deviceClient, c.String("root-dir"), trw) |
There was a problem hiding this comment.
deviceClient is needed in order to call BrowserDeviceURL.
| // Get the My Device URL. | ||
| browserURL := s.DeviceClient.BrowserDeviceURL(token) | ||
| // log out the url | ||
| log.Info().Msgf("setup experience: opening web content URL: %s", browserURL) | ||
| // Set the web content URL. | ||
| if err := s.sd.SetWebContent(browserURL + "?setup_only=1"); err != nil { | ||
| log.Info().Err(err).Msg("setting web content URL in setup experience UI") | ||
| return nil | ||
| } |
There was a problem hiding this comment.
If the token rotates, we'll automatically refresh the page here since the browserURL will have changed. So no extra code is needed to detect token rotation and manually refresh the page.
| func (s *SwiftDialog) HideIcon() error { | ||
| return s.sendCommand("icon", "hide") | ||
| return s.sendCommand("icon", "none") | ||
| } |
There was a problem hiding this comment.
This was wrong, but unused before so we didn't know it.
| return os.Chmod(rw.Path, constant.DefaultWorldReadableFileMode) | ||
| } | ||
|
|
||
| func (rw *ReadWriter) StartRotation() func() { |
There was a problem hiding this comment.
Mostly copied from the former implementation in orbit.go, except for the logic around stopping the rotation and making the durations configurable (mainly for testing purposes).
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughIntegrates token read/write and automated rotation into Orbit; updates the macOS setup experience to use a web UI via a device URL; extends the token package with rotation lifecycle and remote checks; tweaks SwiftDialog commands; and adds a CLI tool to prepare/enqueue macOS VM setup experience data via MySQL. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as macOS User
participant Orbit as orbit (agent)
participant TRW as Token ReadWriter
participant DevClient as DeviceClient
participant SwiftDlg as SwiftDialog
participant WebUI as Setup Web UI
rect rgba(230,240,255,0.5)
note over Orbit,TRW: Init & token wiring
Orbit->>TRW: NewReadWriter(path, checkTokenFunc)
Orbit->>TRW: SetRemoteUpdateFunc(update via orbit client)
Orbit->>DevClient: Init with Fleet URL/certs
Orbit->>TRW: Read/validate token (rotate if needed)
end
rect rgba(240,255,240,0.5)
note over Orbit,TRW: Rotation lifecycle
Orbit->>TRW: StartRotation()
note right of TRW: returns stop() to end rotation
end
rect rgba(255,250,230,0.5)
note over Orbit,WebUI: Setup experience flow
Orbit->>SwiftDlg: Prepare dialog (hide title/icon/message)
Orbit->>TRW: Read token
Orbit->>DevClient: BrowserDeviceURL(token)
DevClient-->>Orbit: https://.../device?token=...&setup_only=1
Orbit->>SwiftDlg: Load web content (device URL)
User-->>WebUI: Complete browser UI steps
end
Orbit->>TRW: stop() (on completion/exit)
sequenceDiagram
autonumber
participant TRW as Token ReadWriter
participant FS as Token File
participant Server as Fleet Server
rect rgba(245,245,255,0.6)
note over TRW: Local rotation checks
loop local check interval
TRW->>FS: Stat/Read (mtime, contents)
alt changed or expired
TRW->>TRW: Rotate()
TRW->>FS: Write(new token)
TRW-->>Server: remoteUpdate(new token)
else
TRW-->>TRW: adjust next local check
end
end
end
rect rgba(255,245,240,0.6)
note over TRW,Server: Remote validity checks
loop remote check interval
TRW->>Server: checkTokenFunc(token)
alt invalid/unreachable
TRW->>TRW: Rotate()
TRW->>FS: Write(new token)
TRW-->>Server: remoteUpdate(new token)
else
TRW-->>TRW: no-op
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@CodeRabbit review again |
|
@sgress454: I'll review the changes again. ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
orbit/pkg/setup_experience/setup_experience.go(7 hunks)tools/mdm/apple/setupexperience/README.md(1 hunks)tools/mdm/apple/setupexperience/main.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/mdm/apple/setupexperience/README.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
⚙️ CodeRabbit configuration file
When reviewing SQL queries that are added or modified, ensure that appropriate filtering criteria are applied—especially when a query is intended to return data for a specific entity (e.g., a single host). Check for missing WHERE clauses or incorrect filtering that could lead to incorrect or non-deterministic results (e.g., returning the first row instead of the correct one). Flag any queries that may return unintended results due to lack of precise scoping.
Files:
tools/mdm/apple/setupexperience/main.goorbit/pkg/setup_experience/setup_experience.go
🧠 Learnings (1)
📚 Learning: 2025-10-03T18:16:11.429Z
Learnt from: MagnusHJensen
PR: fleetdm/fleet#33805
File: server/service/integration_mdm_test.go:1248-1251
Timestamp: 2025-10-03T18:16:11.429Z
Learning: In server/service/integration_mdm_test.go, the helper createAppleMobileHostThenEnrollMDM(platform string) is exclusively for iOS/iPadOS hosts (mobile). Do not flag macOS model/behavior issues based on changes within this helper; macOS provisioning uses different helpers such as createHostThenEnrollMDM.
Applied to files:
tools/mdm/apple/setupexperience/main.go
🔇 Additional comments (1)
orbit/pkg/setup_experience/setup_experience.go (1)
178-181: ...
lucasmrod
left a comment
lucasmrod
left a comment
There was a problem hiding this comment.
LGTM!
Left some nit comments.
| uiSteps map[string]swiftdialog.ListItem | ||
| started bool | ||
| sd *swiftdialog.SwiftDialog | ||
| uiSteps map[string]swiftdialog.ListItem |
| if err != nil { | ||
| log.Error().Err(err).Msg("marshalling setup experience payload for logging") | ||
| } else { | ||
| log.Info().Msgf("setup experience payload: %s", string(payloadBytes)) |
There was a problem hiding this comment.
Should probably be log.Debug()?
| // Get the My Device URL. | ||
| browserURL := s.DeviceClient.BrowserDeviceURL(token) | ||
| // log out the url | ||
| log.Info().Msgf("setup experience: opening web content URL: %s", browserURL) |
There was a problem hiding this comment.
Same here, maybe just log.Debug().
| log.Info().Msgf("setup experience: opening web content URL: %s", browserURL) | ||
| // Set the web content URL. | ||
| if err := s.sd.SetWebContent(browserURL + "?setup_only=1"); err != nil { | ||
| log.Info().Err(err).Msg("setting web content URL in setup experience UI") |
|
|
||
| // Clear the dialog message. | ||
| if err := s.sd.HideMessage(); err != nil { | ||
| log.Info().Err(err).Msg("clearing message in setup experience UI") |
There was a problem hiding this comment.
log.Error() same for the ones below.
| 7. Run this tool with the appropriate flags to set up the necessary database records, e.g.: | ||
|
|
||
| ```bash | ||
| go run main.go -server-private-key=$(cat ~/path/to/private/key) -host-uuid="your-enrolled-host-uuid" |
There was a problem hiding this comment.
Maybe instruct how to get the UUID of the VM?
|
@lucasmrod comments addressed, thanks! |
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** For #33111 Fixes a possible race condition introduced in #33884 which saw a test failure [here](https://github.com/fleetdm/fleet/actions/runs/18454713591/job/52574112376#step:14:3571). # Checklist for submitter ## Testing - [ ] Added/updated automated tests Existing test is sufficient, will circle back if it fails again. - [X] QA'd all new/changed functionality manually Verified that Fleet Desktop still opens My Device page correctly and token rotation logs are still seen. For unreleased bug fixes in a release candidate, one of: - [X] Confirmed that the fix is not expected to adversely impact load test results
2 participants
Related issue: For #33111
Details
This PR updates the setup experience for MacOS to use a web view pointed at the device's "Setting up your device" page rather than using native MacOS UI elements, bringing it more in line with Linux and Windows setup experiences.
This covers only the new web UI for the setup experience progress, not the UI for the new case of blocking the device when a piece of software fails to install. I'll add that in a separate PR.
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
Added/updated automated tests
Added tests for the updates to the token rotation code.
QA'd all new/changed functionality manually
A new tool is provided to allow testing this code against a virtual machine if a separate host that you can wipe and run setup on is not available. See https://github.com/fleetdm/fleet/blob/sgress454/new-setup-experience/tools/mdm/apple/setupexperience/README.md for details.
Summary by CodeRabbit