BMAA: Add personal enrollment type including aggregate counts by JordanMontgomery · Pull Request #31091 · fleetdm/fleet

JordanMontgomery · 2025-07-21T17:11:38Z

Checklist for submitter

If some of the following don't apply, delete the relevant line.

Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
See Changes files for more information.
Input data is properly validated, SELECT * is avoided, SQL injection is prevented (using placeholders for values in statements)
If database migrations are included, checked table schema to confirm autoupdate
For database migrations:
- Checked schema for all modified table for columns that will auto-update timestamps during migration.
- Confirmed that updating the timestamps is acceptable, and will not cause unwanted side effects.
- Ensured the correct collation is explicitly set for character columns (COLLATE utf8mb4_unicode_ci).
Added/updated automated tests
- Where appropriate, automated tests simulate multiple hosts and test for host isolation (updates to one hosts's records do not affect another.)
Manual QA for all new/changed functionality

…Device) enrollment

coderabbitai · 2025-07-21T17:11:44Z

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing Touches

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch JM-bmaa-personal-enrollment

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.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai generate unit tests to generate unit tests for this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

codecov · 2025-07-21T18:12:24Z

Codecov Report

Attention: Patch coverage is 73.91304% with 18 lines in your changes missing coverage. Please review.

Project coverage is 64.03%. Comparing base (a87ec09) to head (4d18cde).
Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
...bles/20250718091828_AddPersonalEnrollmentStatus.go	69.23%	6 Missing and 2 partials ⚠️
server/service/microsoft_mdm.go	0.00%	4 Missing ⚠️
server/datastore/mysql/apple_mdm.go	75.00%	0 Missing and 3 partials ⚠️
server/datastore/mysql/hosts.go	77.77%	2 Missing ⚠️
server/mdm/lifecycle/lifecycle.go	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #31091   +/-   ##
=======================================
  Coverage   64.03%   64.03%           
=======================================
  Files        1905     1906    +1     
  Lines      187623   187655   +32     
  Branches     5415     5448   +33     
=======================================
+ Hits       120136   120169   +33     
+ Misses      58047    58042    -5     
- Partials     9440     9444    +4

Flag	Coverage Δ
backend	`64.89% <73.91%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

gillespi314

Looks good! Feel free to address comments in a follow up PR as you see fit :)

Does this also cover filtering hosts by the new status? I didn't see any changes to transport so I'm assuming that's still on our list as something that @ghernandez345 can work on adding that and supplementing related tests?

gillespi314 · 2025-07-21T21:38:37Z

+- "host_serial": Serial number of the host (Apple enrollments only, always empty for Microsoft).  Only for company-owned hosts.
+- "enrollment_id": Enrollment identifier of the personal (BYOD) host (iPhone, iPad and Android enrollments only, null for other platforms).
 - "host_display_name": Display name of the host.
+- "personal_host": Whether the host is personal or company-owned.


Do we need this personal_host boolean? It seems like we can infer that this is a personal device based on the presence or absence of enrollment_id. Also, we may want to disambiguate enrollment_id (to avoid collisions with other activity types). Maybe byod_identifier is more distinctive?

if we do switch this lets remember to upate the global activities UI as it currently relies on personal_host to render correctly

This is what Marko specified so I can ask in design review. Enrollment ID is technically the term Apple uses here though

I agree with @gillespi314 about inferring this from the presence of an enrollment_id rather than adding an extra boolean, but am not super opposed to personal_host if it's too late to change. (Would prefer we called it byod_host since that's more in line with our usual terminology.)

Sorry to ask for changes this late in the game! I was never requested for review on #30186 so this is my first time seeing this

No worries. What I am gonna do is split these activity/doc changes out so someone else can tighten up the spec and get them merged in later this week

gillespi314 · 2025-07-21T21:46:04Z

+			assert.True(t, entry.IsPersonalEnrollment, "is_personal_enrollment should be true for host_id 7")
+			assert.Equal(t, "On (personal)", *entry.EnrollmentStatus)
+		case 8:
+			assert.True(t, entry.IsPersonalEnrollment, "is_personal_enrollment should be true for host_id 6")


Nit: Seems like the host_id is wrong in the message. Consider using a format string based on entry.HostID?

You're right. I came back and updated this test after wanting to add more cases and messed it up. Will fix

gillespi314 · 2025-07-21T21:53:26Z

+	// Fetch the hosts we enrolled and check their details. Specifically we want to verify that MDM
+	// shows a personal enrollment via all 3 endpoints(list hosts, host details and host MDM
+	// details) since they query for the information slightly differently


Quick double-check: There are a few other endpoints that also return similar data that you may want to spot check. For example:

https://fleetdm.com/docs/rest-api/rest-api#get-host-by-identifier
https://fleetdm.com/docs/rest-api/rest-api#list-hosts-in-a-label
https://fleetdm.com/docs/rest-api/rest-api#targets

Potentially others too that I don't remember off the top of my head (but I think they are documented in the code comments on the other host endpoints).

gillespi314 · 2025-07-21T21:59:12Z

+	// Note I am explicitly not re-enrolling the iPhone here as I don't believe that particular
+	// device and enrollment type actually supports re-enrollment like this


Did this comment come back as a result of merge conflicts?

Good catch. It must have. The actual command to re-enroll was there even though this comment said otherwise. Thanks for catching that

JordanMontgomery added 13 commits July 16, 2025 14:36

Update PR Template

d712987

Implement SCEP renwal and host mdm idp account association for User (…

9cc35b3

…Device) enrollment

Merge branch 'main' into JM-bmaa-scep-renewal

3245cd9

Add unit tests

60489e5

Add integration tests for SCEP renewal and assertions on hmia links

e607599

Add one more unit test for host isolation checks

acf91ad

Personal enrollment WIP

b6774d0

Add personal enrollment migration

3dec926

Test updates

b930d0e

Set personal enrollment status

02644c1

Add assertions around host MDM enrollment_Status to integration tests

3568a66

Update tests in missed file

9c59f21

Update tests for counts

e5b0777

Merge branch 'main' into JM-bmaa-personal-enrollment

29b6e3a

JordanMontgomery temporarily deployed to Docker Hub July 21, 2025 17:13 — with GitHub Actions Inactive

Add activities updates

1c0c1f8

JordanMontgomery temporarily deployed to Docker Hub July 21, 2025 18:00 — with GitHub Actions Inactive

Update integration tests around MDM enabled activity

cf762be

JordanMontgomery temporarily deployed to Docker Hub July 21, 2025 18:15 — with GitHub Actions Inactive

JordanMontgomery had a problem deploying to Docker Hub July 21, 2025 18:15 — with GitHub Actions Error

Update comments

accd737

JordanMontgomery temporarily deployed to Docker Hub July 21, 2025 18:18 — with GitHub Actions Inactive

JordanMontgomery had a problem deploying to Docker Hub July 21, 2025 18:18 — with GitHub Actions Error

Update docs

ef00aea

JordanMontgomery temporarily deployed to Docker Hub July 21, 2025 18:21 — with GitHub Actions Inactive

JordanMontgomery temporarily deployed to Docker Hub July 21, 2025 18:45 — with GitHub Actions Inactive

Add missed DEP test

699f272

JordanMontgomery temporarily deployed to Docker Hub July 21, 2025 19:26 — with GitHub Actions Inactive

gillespi314 previously approved these changes Jul 21, 2025

View reviewed changes

Fixes from review comments

875a303

JordanMontgomery dismissed gillespi314’s stale review via 875a303 July 22, 2025 13:43

JordanMontgomery temporarily deployed to Docker Hub July 22, 2025 13:43 — with GitHub Actions Inactive

Merge branch 'main' into JM-bmaa-personal-enrollment

e8d1ea9

JordanMontgomery temporarily deployed to Docker Hub July 22, 2025 13:58 — with GitHub Actions Inactive

gillespi314 previously approved these changes Jul 22, 2025

View reviewed changes

MagnusHJensen reviewed Jul 22, 2025

View reviewed changes

Comment thread server/fleet/activities.go Outdated

MagnusHJensen reviewed Jul 22, 2025

View reviewed changes

Comment thread server/service/integration_mdm_dep_test.go Outdated

Remove activity changes

e695a3b

JordanMontgomery dismissed gillespi314’s stale review via e695a3b July 22, 2025 20:57

JordanMontgomery temporarily deployed to Docker Hub July 22, 2025 20:57 — with GitHub Actions Inactive

JordanMontgomery requested a review from gillespi314 July 22, 2025 20:58

gillespi314 previously approved these changes Jul 22, 2025

View reviewed changes

Fix missed test

4d18cde

JordanMontgomery dismissed gillespi314’s stale review via 4d18cde July 22, 2025 21:10

JordanMontgomery temporarily deployed to Docker Hub July 22, 2025 21:10 — with GitHub Actions Inactive

gillespi314 approved these changes Jul 22, 2025

View reviewed changes

JordanMontgomery merged commit fea2b40 into main Jul 22, 2025
42 checks passed

JordanMontgomery deleted the JM-bmaa-personal-enrollment branch July 22, 2025 21:24

MagnusHJensen mentioned this pull request Jul 23, 2025

BMAA: Follow up tasks #31058

Closed

7 tasks

		// Note I am explicitly not re-enrolling the iPhone here as I don't believe that particular
		// device and enrollment type actually supports re-enrollment like this

Conversation

JordanMontgomery commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist for submitter

Uh oh!

coderabbitai Bot commented Jul 21, 2025

Review skipped

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

codecov Bot commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gillespi314 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghernandez345 Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JordanMontgomery commented Jul 21, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

codecov Bot commented Jul 21, 2025 •

edited

Loading

ghernandez345 Jul 22, 2025 •

edited

Loading