Send `DeviceConfigured` MDM command after DEP enrollment #17737

mna · 2024-03-20T17:48:45Z

Checklist for submitter

Changes file added for user-visible changes in changes/ or orbit/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)
Added/updated tests

…e changes

codecov · 2024-03-20T18:53:28Z

Codecov Report

Attention: Patch coverage is 75.60000% with 61 lines in your changes are missing coverage. Please review.

❗ No coverage uploaded for pull request base (feat-prefill-account-name@8d2deb3). Click here to learn what that means.

❗ Current head 770c9d4 differs from pull request most recent head c4da092. Consider uploading reports for the commit c4da092 to get more accurate results

Files	Patch %	Lines
server/worker/apple_mdm.go	67.96%	29 Missing and 12 partials ⚠️
...145650_UpdateDEPProfilesToAwaitDeviceConfigured.go	83.33%	6 Missing and 2 partials ⚠️
ee/server/service/mdm.go	45.45%	4 Missing and 2 partials ⚠️
ee/server/service/teams.go	40.00%	4 Missing and 2 partials ⚠️

Additional details and impacted files

@@                     Coverage Diff                      @@
##             feat-prefill-account-name   #17737   +/-   ##
============================================================
  Coverage                             ?   65.64%           
============================================================
  Files                                ?     1194           
  Lines                                ?   108292           
  Branches                             ?     2574           
============================================================
  Hits                                 ?    71093           
  Misses                               ?    31802           
  Partials                             ?     5397

Flag	Coverage Δ
backend	`66.65% <75.60%> (?)`

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.

mna · 2024-03-20T20:20:24Z

server/service/integration_mdm_dep_test.go

+	}
+}
+
+func (s *integrationMDMTestSuite) TestDEPProfileAssignment() {


This test was not modified, just moved here from the integration_mdm_test.go as it is DEP-specific.

mna · 2024-03-20T20:21:40Z

server/service/integration_mdm_test.go

@@ -1981,745 +1999,6 @@ func createWindowsHostThenEnrollMDM(ds fleet.Datastore, fleetServerURL string, t
 	return host, mdmDevice
 }

-func (s *integrationMDMTestSuite) TestDEPProfileAssignment() {


Since this file is already huuuuuge, I created a new integration_mdm_dep_test.go file for the DEP-specific MDM integration tests, and moved this one over there.

mna · 2024-03-20T20:23:22Z

server/worker/apple_mdm.go

+	// On the next try, new commands will be enqueued for the same task (e.g. if
+	// it failed at the bootstrap package, install fleetd will be enqeueued
+	// again), will Apple MDM process both commands successfully or would the
+	// second one fail? Relevant for the device release.


@roperzh maybe you know this? My hunch is that those are idempotent so if e.g. fleetd is already installed and the command is sent again, it would succeed again.

it would! they're not idempotent, but I believe the commands we send won't produce errors either.

All InstallProfile commands won't have any effect (because it'll be considered an 'edit' by the OS and nothing changed)

All InstallEnterpriseApplication will trigger a second install. This might be problematic for the bootstrap package, but I believe Noah's plan is to document that the bootstrap package must be "idempotent" (as much as an application can be)

I don't know if I'm missing other commands.

I believe the commands we send won't produce errors either.
This might be problematic for the bootstrap package, but I believe Noah's plan is to document that the bootstrap package must be "idempotent" (as much as an application can be)

Awesome, thanks for the clarifications!

I don't know if I'm missing other commands.

The other potential one is AccountConfiguration, which I assume would be like an edit to the previous settings and would not result in an error.

agreed, IIRC that's fine as well.

mna · 2024-03-20T20:24:55Z

server/worker/apple_mdm.go

+	// the release device task is always added with a delay
+	var delay time.Duration
+	if task == AppleMDMPostDEPReleaseDeviceTask {
+		delay = 30 * time.Second


This task is always delayed, so that the device has the time to process the initial enrollment commands and a Reconcile profiles job has a chance to run.

mna · 2024-03-20T20:25:57Z

server/worker/worker.go

@@ -191,6 +209,7 @@ func (w *Worker) processJob(ctx context.Context, job *fleet.Job) error {
 		args = *job.Args
 	}

+	ctx = context.WithValue(ctx, retryNumberCtxKey, job.Retries)


A bit hacky, but that was a quick and easy way to make that info available to the job processor.

mna · 2024-03-20T20:28:34Z

...datastore/mysql/migrations/tables/20240320145650_UpdateDEPProfilesToAwaitDeviceConfigured.go

+	//
+	// but without calling that function, in case the code changes in the future,
+	// breaking this migration. Instead we insert directly the job in the
+	// database, and the worker will process it shortly after Fleet restarts.


@roperzh do you know if that means that existing DEP-enrolled devices will now need to be explicitly released? The Apple docs mention it blocks in Setup Assistant so presumably devices that are already past this assistant will be ok, but worth double-checking in tests? Because re-generating the profiles also re-assigns them to existing hosts here: https://github.com/fleetdm/fleet/blob/main/server/worker/macos_setup_assistant.go#L113-L137

Yes, I believe we're fine here! those settings only affect the next time the device goes through the setup assistant 👍

…ll-device-release

roperzh

very elegant solution for a hard problem ✨ . LGTM

server/worker/apple_mdm.go

mna · 2024-03-25T17:25:18Z

@roperzh I'll merge as it's only going to the feature branch and Gabe is waiting on this to implement some fixes on top of it, but I did merge a fix and some minor changes after your approval, if you want to take another look (if there's anything to address, I'll do so in a subsequent PR). The Go mysql8 tests are also failing, but looks like that might be something we inherited from merging main into our branch, I'll take a look for a fix in the next PR too.

mna · 2024-03-25T18:24:06Z

It's a test time-out on mysql 8:

2024-03-25T17:22:15.7627631Z panic: test timed out after 15m0s
2024-03-25T17:22:15.7627722Z running tests:
2024-03-25T17:22:15.7627848Z 	TestIntegrationsMDM (11m11s)
2024-03-25T17:22:15.7628055Z 	TestIntegrationsMDM/TestDEPProfileAssignment (10m42s)

I'll increase the test timeout limit for now, in a future PR.

mna added 8 commits March 19, 2024 16:52

Ensure the DEP enrollment profiles get regenerated when enable releas…

0b3134e

…e changes

Fix some tests

324b5bc

Fix test that failed when running the suite, but not in isolation

49f6bb1

Fix-ish for idp/macos setup settings

e642835

Add plan for release implementation

f946401

Fix worker tests

17cf645

Queue delayed job for post-dep release, adjust tests

463a947

Finish integration test, make it pass

62e321b

mna temporarily deployed to Docker Hub March 20, 2024 17:48 — with GitHub Actions Inactive

Fix dep release test with fleetd config profile also sent

cbd3377

mna temporarily deployed to Docker Hub March 20, 2024 18:08 — with GitHub Actions Inactive

Add integration test for global manual release

f289693

mna temporarily deployed to Docker Hub March 20, 2024 18:20 — with GitHub Actions Inactive

Add team-enrolled release device test

a7e068b

mna temporarily deployed to Docker Hub March 20, 2024 18:35 — with GitHub Actions Inactive

Refactor integration tests

51aa4e3

mna temporarily deployed to Docker Hub March 20, 2024 18:53 — with GitHub Actions Inactive

Add database migration to re-generate all DEP profiles

938983d

mna temporarily deployed to Docker Hub March 20, 2024 19:27 — with GitHub Actions Inactive

Add DB migration, adjust tests

b9095ea

mna had a problem deploying to Docker Hub March 20, 2024 19:48 — with GitHub Actions Error

Add schema changes

6041f0e

mna temporarily deployed to Docker Hub March 20, 2024 19:50 — with GitHub Actions Inactive

Use hard-coded timestamps so schema.sql is stable

3d55339

mna temporarily deployed to Docker Hub March 20, 2024 20:04 — with GitHub Actions Inactive

Add changes file

a53a8c4

mna commented Mar 20, 2024

View reviewed changes

Fix erroneous comments

8c8803e

mna temporarily deployed to Docker Hub March 20, 2024 20:30 — with GitHub Actions Inactive

mna marked this pull request as ready for review March 20, 2024 20:31

mna requested a review from a team as a code owner March 20, 2024 20:31

mna mentioned this pull request Mar 20, 2024

Backend: Prefill local account oobe macos setup #17401

Closed

3 tasks

Remove obsolete todo

c09f3e8

mna temporarily deployed to Docker Hub March 20, 2024 20:42 — with GitHub Actions Inactive

Merge branch 'feat-prefill-account-name' into mna-17401-post-dep-enro…

b976ee9

…ll-device-release

ghernandez345 temporarily deployed to Docker Hub March 25, 2024 11:40 — with GitHub Actions Inactive

roperzh approved these changes Mar 25, 2024

View reviewed changes

server/worker/apple_mdm.go Show resolved Hide resolved

Add debugging information to release device job

770c9d4

mna temporarily deployed to Docker Hub March 25, 2024 16:01 — with GitHub Actions Inactive

Fix awaiting for empty command uuid

c4da092

mna temporarily deployed to Docker Hub March 25, 2024 16:28 — with GitHub Actions Inactive

mna merged commit 994040b into feat-prefill-account-name Mar 25, 2024
11 of 12 checks passed

mna deleted the mna-17401-post-dep-enroll-device-release branch March 25, 2024 17:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Send `DeviceConfigured` MDM command after DEP enrollment #17737

Send `DeviceConfigured` MDM command after DEP enrollment #17737

mna commented Mar 20, 2024 •

edited

codecov bot commented Mar 20, 2024 •

edited

mna Mar 20, 2024

mna Mar 20, 2024

mna Mar 20, 2024

roperzh Mar 20, 2024

mna Mar 20, 2024

mna Mar 20, 2024

roperzh Mar 20, 2024

mna Mar 20, 2024

mna Mar 20, 2024

mna Mar 20, 2024

roperzh Mar 20, 2024

roperzh left a comment

mna commented Mar 25, 2024

mna commented Mar 25, 2024

Send DeviceConfigured MDM command after DEP enrollment #17737

Send DeviceConfigured MDM command after DEP enrollment #17737

Conversation

mna commented Mar 20, 2024 • edited

Checklist for submitter

codecov bot commented Mar 20, 2024 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roperzh left a comment

Choose a reason for hiding this comment

mna commented Mar 25, 2024

mna commented Mar 25, 2024

Send `DeviceConfigured` MDM command after DEP enrollment #17737

Send `DeviceConfigured` MDM command after DEP enrollment #17737

mna commented Mar 20, 2024 •

edited

codecov bot commented Mar 20, 2024 •

edited