implement status reports for DDM commands #17831
Conversation
roperzh
force-pushed
the
17408-status
branch
2 times, most recently
from
a0d50be
to
1aa81cf
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat-macos-ddm #17831 +/- ##
=================================================
Coverage ? 65.75%
=================================================
Files ? 1194
Lines ? 109280
Branches ? 2568
=================================================
Hits ? 71858
Misses ? 31975
Partials ? 5447
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -2615,7 +2615,13 @@ func (svc *MDMAppleCheckinAndCommandService) CommandAndReportResults(r *mdm.Requ | |||
| cmdResult.Status == fleet.MDMAppleStatusCommandFormatError { | |||
| return nil, svc.ds.UpdateHostLockWipeStatusFromAppleMDMResult(r.Context, cmdResult.UDID, cmdResult.CommandUUID, requestType, cmdResult.Status == fleet.MDMAppleStatusAcknowledged) | |||
| } | |||
| case "DeclarativeManagement": | |||
| // set "pending-install" profiles to "verifying" | |||
There was a problem hiding this comment.
More a question for my own understanding than anything else, this implies that the DeclarativeManagement is always signaling a successful apply of the declarative stuff? I.e. we don't check for any command result like for standard MDM commands, it's necessarily a success and we mark the state as Verifying?
(it might be worth a comment just for future us, if only to explain the difference with the other cases)
There was a problem hiding this comment.
Actually it looks like it goes from pending to verifying and then only when a status is received in DeclarativeManagement below it knows if it succeeded/failed. (which I now realize was what the comment explained, but it didn't compute for me for some reason :D )
| // considerable time trying to make it work.) | ||
| // | ||
| // My current guess is that the documentation is implicitly referring | ||
| // to asset declarations (which deliver tangible "assets" to the host) |
| @@ -13266,6 +13266,144 @@ func (s *integrationMDMTestSuite) TestAppleDDMReconciliation() { | |||
| checkDDMSync(deviceThree) | |||
| } | |||
|
|
|||
| func (s *integrationMDMTestSuite) TestAppleDDMStatusReport() { | |||
There was a problem hiding this comment.
Suggestion, as this is probably only gonna grow: let's start a distinct file for DDM integration tests? Could still be in the integrationMDMTestSuite, just this file is already so huge.
There was a problem hiding this comment.
lol, you're right, that's a great idea. I know Sarah and Dante have PRs that modify this file, I will do this as soon as those land to avoid conflicts.
| require.NoError(t, err) | ||
| assertHostDeclarations(mdmHost.UUID, []*fleet.MDMAppleHostDeclaration{ | ||
| {Identifier: "I1", Status: &fleet.MDMDeliveryFailed, OperationType: fleet.MDMOperationTypeInstall}, | ||
| }) |
for #17408
Checklist for submitter
If some of the following don't apply, delete the relevant line.
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements)