diff --git a/changes/29824-delete-installs-that-has-not-reached-hosts b/changes/29824-delete-installs-that-has-not-reached-hosts new file mode 100644 index 0000000000..a88ae5c93c --- /dev/null +++ b/changes/29824-delete-installs-that-has-not-reached-hosts @@ -0,0 +1 @@ +* Fix stale pending remove apple declarations, if the host was offline while adding and removing the same declaration. \ No newline at end of file diff --git a/server/datastore/mysql/apple_mdm.go b/server/datastore/mysql/apple_mdm.go index 0ca826d790..dbeffbce6a 100644 --- a/server/datastore/mysql/apple_mdm.go +++ b/server/datastore/mysql/apple_mdm.go @@ -643,10 +643,10 @@ func cancelAppleHostInstallsForDeletedMDMDeclarations(ctx context.Context, tx sq host_mdm_apple_declarations WHERE declaration_uuid IN (?) AND - status IS NULL AND + ( status IS NULL OR status IN (?) ) AND operation_type = ?` - stmt, args, err := sqlx.In(delStmt, declUUIDs, fleet.MDMOperationTypeInstall) + stmt, args, err := sqlx.In(delStmt, declUUIDs, []fleet.MDMDeliveryStatus{fleet.MDMDeliveryPending, fleet.MDMDeliveryFailed}, fleet.MDMOperationTypeInstall) if err != nil { return ctxerr.Wrap(ctx, err, "building in statement") } @@ -5888,6 +5888,8 @@ ON DUPLICATE KEY UPDATE } } + // TODO: Do we want to take action if an update coming from the host is not found in the current "expected" declarations? + err := ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error { if len(args) != 0 { stmt := fmt.Sprintf(updateHostDeclarationsStmt, strings.TrimSuffix(insertVals.String(), ",")) diff --git a/server/datastore/mysql/apple_mdm_test.go b/server/datastore/mysql/apple_mdm_test.go index ce5d97fccb..3206e3f927 100644 --- a/server/datastore/mysql/apple_mdm_test.go +++ b/server/datastore/mysql/apple_mdm_test.go @@ -101,6 +101,7 @@ func TestMDMApple(t *testing.T) { {"SetMDMAppleProfilesWithVariables", testSetMDMAppleProfilesWithVariables}, {"GetNanoMDMEnrollmentTimes", testGetNanoMDMEnrollmentTimes}, {"GetNanoMDMUserEnrollment", testGetNanoMDMUserEnrollment}, + {"TestDeleteMDMAppleDeclarationWithPendingInstalls", testDeleteMDMAppleDeclarationWithPendingInstalls}, } for _, c := range cases { @@ -5526,6 +5527,45 @@ func testSetOrUpdateMDMAppleDDMDeclaration(t *testing.T, ds *Datastore) { require.Equal(t, d1tm1B.DeclarationUUID, d1tm1.DeclarationUUID) } +func testDeleteMDMAppleDeclarationWithPendingInstalls(t *testing.T, ds *Datastore) { + ctx := t.Context() + + decl, err := ds.NewMDMAppleDeclaration(ctx, &fleet.MDMAppleDeclaration{ + Identifier: "decl-1", + Name: "decl-1", + }) + require.NoError(t, err) + + host, err := ds.NewHost(ctx, &fleet.Host{ + Hostname: "test-host1-name", + OsqueryHostID: ptr.String("1337"), + NodeKey: ptr.String("1337"), + UUID: "test-uuid-1", + TeamID: nil, + Platform: "darwin", + }) + require.NoError(t, err) + nanoEnroll(t, ds, host, true) + + _, err = ds.BulkSetPendingMDMHostProfiles(ctx, nil, nil, []string{decl.DeclarationUUID}, nil) + require.NoError(t, err) + + // verify the correct state of the declaration for the host + profs, err := ds.GetHostMDMAppleProfiles(ctx, host.UUID) + require.NoError(t, err) + require.Len(t, profs, 1) + require.Equal(t, decl.DeclarationUUID, profs[0].ProfileUUID) + require.Equal(t, fleet.MDMDeliveryPending, *profs[0].Status) + require.Equal(t, fleet.MDMOperationTypeInstall, profs[0].OperationType) + + err = ds.DeleteMDMAppleDeclaration(ctx, decl.DeclarationUUID) + require.NoError(t, err) + + profs, err = ds.GetHostMDMAppleProfiles(ctx, host.UUID) + require.NoError(t, err) + require.Len(t, profs, 0) +} + func TestMDMAppleProfileVerification(t *testing.T) { ds := CreateMySQLDS(t) ctx := t.Context() diff --git a/server/service/integration_mdm_ddm_test.go b/server/service/integration_mdm_ddm_test.go index 44626ede13..a35f87624c 100644 --- a/server/service/integration_mdm_ddm_test.go +++ b/server/service/integration_mdm_ddm_test.go @@ -470,7 +470,8 @@ func (s *integrationMDMTestSuite) TestAppleDDMSecretVariables() { _, mdmDevice := createHostThenEnrollMDM(s.ds, s.server.URL, t) checkDeclarationItemsResp := func(t *testing.T, r fleet.MDMAppleDDMDeclarationItemsResponse, expectedDeclTok string, - expectedDeclsByToken map[string]fleet.MDMAppleDeclaration) { + expectedDeclsByToken map[string]fleet.MDMAppleDeclaration, + ) { require.Equal(t, expectedDeclTok, r.DeclarationsToken) require.NotEmpty(t, r.Declarations.Activations) require.Empty(t, r.Declarations.Assets) @@ -1065,7 +1066,6 @@ func (s *integrationMDMTestSuite) TestAppleDDMStatusReport() { require.NoError(t, err) assertHostDeclarations(mdmHost.UUID, []*fleet.MDMAppleHostDeclaration{ {Identifier: "I1", Status: &fleet.MDMDeliveryVerified, OperationType: fleet.MDMOperationTypeInstall}, - {Identifier: "I2", Status: &fleet.MDMDeliveryPending, OperationType: fleet.MDMOperationTypeRemove}, }) // host sends a report, declaration I2 is removed from the hosts_* table